117616: Ported unique-decorators rule

.eslintrc.json

@@ -267,7 +267,8 @@
         "dspace-angular-ts/themed-component-classes": "error",
         "dspace-angular-ts/themed-component-selectors": "error",
         "dspace-angular-ts/themed-component-usages": "error",
-        "dspace-angular-ts/themed-wrapper-no-input-defaults": "error"
+        "dspace-angular-ts/themed-wrapper-no-input-defaults": "error",
+        "dspace-angular-ts/unique-decorators": "error"
       }
     },
     {

docs/lint/ts/index.md

@@ -6,3 +6,4 @@ _______
 - [`dspace-angular-ts/themed-component-selectors`](./rules/themed-component-selectors.md): Themeable component selectors should follow the DSpace convention
 - [`dspace-angular-ts/themed-component-usages`](./rules/themed-component-usages.md): Themeable components should be used via their `ThemedComponent` wrapper class
 - [`dspace-angular-ts/themed-wrapper-no-input-defaults`](./rules/themed-wrapper-no-input-defaults.md): ThemedComponent wrappers should not declare input defaults (see [DSpace Angular #2164](https://github.com/DSpace/dspace-angular/pull/2164))
+- [`dspace-angular-ts/unique-decorators`](./rules/unique-decorators.md): Some decorators must be called with unique arguments (e.g. when they construct a mapping based on the argument values)

docs/lint/ts/rules/unique-decorators.md

@@ -0,0 +1,73 @@
+[DSpace ESLint plugins](../../../../lint/README.md) > [TypeScript rules](../index.md) > `dspace-angular-ts/unique-decorators`
+_______
+
+Some decorators must be called with unique arguments (e.g. when they construct a mapping based on the argument values)
+
+_______
+
+[Source code](../../../../lint/src/rules/ts/unique-decorators.ts)
+
+### Examples
+
+
+#### Valid code
+    
+##### checked decorator, no repetitions
+        
+```typescript
+@listableObjectComponent(a)
+export class A {
+}
+
+@listableObjectComponent(a, 'b')
+export class B {
+}
+
+@listableObjectComponent(a, 'b', 3)
+export class C {
+}
+
+@listableObjectComponent(a, 'b', 3, Enum.TEST1)
+export class C {
+}
+
+@listableObjectComponent(a, 'b', 3, Enum.TEST2)
+export class C {
+}
+```
+    
+##### unchecked decorator, some repetitions
+        
+```typescript
+@something(a)
+export class A {
+}
+
+@something(a)
+export class B {
+}
+```
+    
+
+
+
+#### Invalid code 
+    
+##### checked decorator, some repetitions
+        
+```typescript
+@listableObjectComponent(a)
+export class A {
+}
+
+@listableObjectComponent(a)
+export class B {
+}
+```
+Will produce the following error(s):
+```
+Duplicate decorator call
+```
+        
+    
+

lint/src/rules/ts/index.ts

@@ -15,6 +15,7 @@ import * as themedComponentClasses from './themed-component-classes';
 import * as themedComponentSelectors from './themed-component-selectors';
 import * as themedComponentUsages from './themed-component-usages';
 import * as themedWrapperNoInputDefaults from './themed-wrapper-no-input-defaults';
+import * as uniqueDecorators from './unique-decorators';
 
 const index = [
   aliasImports,
@@ -22,6 +23,7 @@ const index = [
   themedComponentSelectors,
   themedComponentUsages,
   themedWrapperNoInputDefaults,
+  uniqueDecorators,
 ] as unknown as RuleExports[];
 
 export = {

lint/src/rules/ts/unique-decorators.ts

@@ -0,0 +1,185 @@
+import {
+  AST_NODE_TYPES,
+  TSESLint,
+  TSESTree,
+} from '@typescript-eslint/utils';
+
+import { isTestFile } from '../../util/filter';
+import {
+  DSpaceESLintRuleInfo,
+  NamedTests,
+} from '../../util/structure';
+
+export enum Message {
+  DUPLICATE_DECORATOR_CALL = 'duplicateDecoratorCall',
+}
+
+export const info: DSpaceESLintRuleInfo = {
+  name: 'unique-decorators',
+  meta: {
+    docs: {
+      description: 'Some decorators must be called with unique arguments (e.g. when they construct a mapping based on the argument values)',
+    },
+    messages: {
+      [Message.DUPLICATE_DECORATOR_CALL]: 'Duplicate decorator call',
+    },
+    type: 'problem',
+    schema: [
+      {
+        type: 'object',
+        properties: {
+          decorators: {
+            type: 'array',
+          },
+        },
+      },
+    ],
+  },
+  defaultOptions: [
+    {
+      decorators: [
+        'listableObjectComponent',  // todo: must take default arguments into account!
+      ],
+    },
+  ],
+};
+
+export const rule = {
+  ...info,
+  create(context: TSESLint.RuleContext<Message, unknown[]>, options: any) {
+    const decoratorCalls: Map<string, Set<string>> = new Map();
+
+    return {
+      'ClassDeclaration > Decorator > CallExpression': (node: TSESTree.CallExpression) => {  // todo: limit to class decorators
+        if (isTestFile(context)) {
+          return;
+        }
+
+        if (node.callee.type !== AST_NODE_TYPES.Identifier) {
+          // We only support regular method identifiers
+          return;
+        }
+
+        // todo: can we fold this into the selector actually?
+        if (!(options[0].decorators as string[]).includes(node.callee.name)) {
+          // We don't care about this decorator
+          return;
+        }
+
+        if (!isUnique(node, decoratorCalls)) {
+          context.report({
+            messageId: Message.DUPLICATE_DECORATOR_CALL,
+            node: node,
+          });
+        }
+      },
+    };
+  },
+};
+
+export const tests: NamedTests = {
+  plugin: info.name,
+  valid: [
+    {
+      name: 'checked decorator, no repetitions',
+      code: `
+@listableObjectComponent(a)
+export class A {
+}
+
+@listableObjectComponent(a, 'b')
+export class B {
+}
+
+@listableObjectComponent(a, 'b', 3)
+export class C {
+}
+
+@listableObjectComponent(a, 'b', 3, Enum.TEST1)
+export class C {
+}
+
+@listableObjectComponent(a, 'b', 3, Enum.TEST2)
+export class C {
+}
+      `,
+    },
+    {
+      name: 'unchecked decorator, some repetitions',
+      code: `
+@something(a)
+export class A {
+}
+
+@something(a)
+export class B {
+}
+      `,
+    },
+  ],
+  invalid: [
+    {
+      name: 'checked decorator, some repetitions',
+      code: `
+@listableObjectComponent(a)
+export class A {
+}
+
+@listableObjectComponent(a)
+export class B {
+}
+      `,
+      errors: [
+        {
+          messageId: 'duplicateDecoratorCall',
+        },
+      ],
+    },
+  ],
+};
+
+function callKey(node: TSESTree.CallExpression): string {
+  let key = '';
+
+  for (const arg of node.arguments) {
+    switch ((arg as TSESTree.Node).type) {
+      // todo: can we make this more generic somehow?
+      case AST_NODE_TYPES.Identifier:
+        key += (arg as TSESTree.Identifier).name;
+        break;
+      case AST_NODE_TYPES.Literal:
+        // eslint-disable-next-line @typescript-eslint/no-base-to-string
+        key += (arg as TSESTree.Literal).value;
+        break;
+      case AST_NODE_TYPES.MemberExpression:
+        key += (arg as any).object.name + '.' + (arg as any).property.name;
+        break;
+      default:
+        throw new Error(`Unrecognized decorator argument type: ${arg.type}`);
+    }
+
+    key += ', ';
+  }
+
+  return key;
+}
+
+function isUnique(node: TSESTree.CallExpression, decoratorCalls: Map<string, Set<string>>): boolean {
+  const decorator = (node.callee as TSESTree.Identifier).name;
+
+  if (!decoratorCalls.has(decorator)) {
+    decoratorCalls.set(decorator, new Set());
+  }
+
+  const key = callKey(node);
+
+  let unique = true;
+
+  if (decoratorCalls.get(decorator)?.has(key)) {
+    unique = !unique;
+  }
+
+  decoratorCalls.get(decorator)?.add(key);
+
+  return unique;
+}

lint/src/util/filter.ts

@@ -0,0 +1,10 @@
+import { RuleContext } from '@typescript-eslint/utils/ts-eslint';
+
+/**
+ * Determine whether the current file is a test file
+ * @param context the current ESLint rule context
+ */
+export function isTestFile(context: RuleContext<any, any>): boolean {
+  // note: shouldn't use plain .filename (doesn't work in DSpace Angular 7.4)
+  return context.getFilename()?.endsWith('.spec.ts')  ;
+}