From 01c8c60624aa89995d4a115875bc3a0777654255 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 29 Aug 2024 17:23:35 +0200 Subject: [PATCH] 117616: Ported unique-decorators rule --- .eslintrc.json | 3 +- docs/lint/ts/index.md | 1 + docs/lint/ts/rules/unique-decorators.md | 73 ++++++++++ lint/src/rules/ts/index.ts | 2 + lint/src/rules/ts/unique-decorators.ts | 185 ++++++++++++++++++++++++ lint/src/util/filter.ts | 10 ++ 6 files changed, 273 insertions(+), 1 deletion(-) create mode 100644 docs/lint/ts/rules/unique-decorators.md create mode 100644 lint/src/rules/ts/unique-decorators.ts create mode 100644 lint/src/util/filter.ts diff --git a/.eslintrc.json b/.eslintrc.json index 6f258de5a8..2dfecc0bd4 100644 --- a/.eslintrc.json +++ b/.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" } }, { diff --git a/docs/lint/ts/index.md b/docs/lint/ts/index.md index 1ce57cde16..48c6e07bf7 100644 --- a/docs/lint/ts/index.md +++ b/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) diff --git a/docs/lint/ts/rules/unique-decorators.md b/docs/lint/ts/rules/unique-decorators.md new file mode 100644 index 0000000000..a0d3ceedc1 --- /dev/null +++ b/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 +``` + + + diff --git a/lint/src/rules/ts/index.ts b/lint/src/rules/ts/index.ts index 19fa38d6ac..e26da64f2a 100644 --- a/lint/src/rules/ts/index.ts +++ b/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 = { diff --git a/lint/src/rules/ts/unique-decorators.ts b/lint/src/rules/ts/unique-decorators.ts new file mode 100644 index 0000000000..acbc3ab12b --- /dev/null +++ b/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, options: any) { + const decoratorCalls: Map> = 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>): 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; +} diff --git a/lint/src/util/filter.ts b/lint/src/util/filter.ts new file mode 100644 index 0000000000..5bc24250e4 --- /dev/null +++ b/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): boolean { + // note: shouldn't use plain .filename (doesn't work in DSpace Angular 7.4) + return context.getFilename()?.endsWith('.spec.ts') ; +}