From eb0572640b82fa0d50849b033ef6be3b34ca962d Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Mon, 23 Sep 2024 11:51:50 +0200 Subject: [PATCH] 117616: Ported themed-decorators rule --- .eslintrc.json | 1 + docs/lint/ts/index.md | 1 + docs/lint/ts/rules/themed-decorators.md | 158 +++++++++++ lint/src/rules/ts/index.ts | 2 + lint/src/rules/ts/themed-decorators.ts | 267 ++++++++++++++++++ lint/src/util/theme-support.ts | 16 ++ .../dynamic-component.spec.ts | 0 .../dynamic-component/dynamic-component.ts | 0 .../dynamic-component/dynamic-component.ts | 0 .../dynamic-component/dynamic-component.ts | 0 10 files changed, 445 insertions(+) create mode 100644 docs/lint/ts/rules/themed-decorators.md create mode 100644 lint/src/rules/ts/themed-decorators.ts create mode 100644 lint/test/fixture/src/app/dynamic-component/dynamic-component.spec.ts create mode 100644 lint/test/fixture/src/app/dynamic-component/dynamic-component.ts create mode 100644 lint/test/fixture/src/themes/test-2/app/dynamic-component/dynamic-component.ts create mode 100644 lint/test/fixture/src/themes/test/app/dynamic-component/dynamic-component.ts diff --git a/.eslintrc.json b/.eslintrc.json index 2dfecc0bd4..4d1f73b22a 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -267,6 +267,7 @@ "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-decorators": "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 48c6e07bf7..432b435a05 100644 --- a/docs/lint/ts/index.md +++ b/docs/lint/ts/index.md @@ -5,5 +5,6 @@ _______ - [`dspace-angular-ts/themed-component-classes`](./rules/themed-component-classes.md): Formatting rules for themeable component classes - [`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-decorators`](./rules/themed-decorators.md): Entry components with theme support should declare the correct theme - [`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/themed-decorators.md b/docs/lint/ts/rules/themed-decorators.md new file mode 100644 index 0000000000..72c2d206b9 --- /dev/null +++ b/docs/lint/ts/rules/themed-decorators.md @@ -0,0 +1,158 @@ +[DSpace ESLint plugins](../../../../lint/README.md) > [TypeScript rules](../index.md) > `dspace-angular-ts/themed-decorators` +_______ + +Entry components with theme support should declare the correct theme + +_______ + +[Source code](../../../../lint/src/rules/ts/themed-decorators.ts) + +### Examples + + +#### Valid code + +##### theme file declares the correct theme in @listableObjectComponent + +Filename: `lint/test/fixture/src/themes/test/app/dynamic-component/dynamic-component.ts` + +```typescript +@listableObjectComponent(something, somethingElse, undefined, 'test') +export class Something extends SomethingElse { +} +``` + +##### plain file declares no theme in @listableObjectComponent + +Filename: `lint/test/fixture/src/app/dynamic-component/dynamic-component.ts` + +```typescript +@listableObjectComponent(something, somethingElse, undefined) +export class Something extends SomethingElse { +} +``` + +##### plain file declares explicit undefined theme in @listableObjectComponent + +Filename: `lint/test/fixture/src/app/dynamic-component/dynamic-component.ts` + +```typescript +@listableObjectComponent(something, somethingElse, undefined, undefined) +export class Something extends SomethingElse { +} +``` + +##### test file declares theme outside of theme directory + +Filename: `lint/test/fixture/src/app/dynamic-component/dynamic-component.spec.ts` + +```typescript +@listableObjectComponent(something, somethingElse, undefined, 'test') +export class Something extends SomethingElse { +} +``` + +##### only track configured decorators + +Filename: `lint/test/fixture/src/app/dynamic-component/dynamic-component.ts` + +```typescript +@something('test') +export class Something extends SomethingElse { +} +``` + + + + +#### Invalid code & automatic fixes + +##### theme file declares the wrong theme in @listableObjectComponent + +Filename: `lint/test/fixture/src/themes/test/app/dynamic-component/dynamic-component.ts` + +```typescript +@listableObjectComponent(something, somethingElse, undefined, 'test-2') +export class Something extends SomethingElse { +} +``` +Will produce the following error(s): +``` +Wrong theme declaration in decorator +``` + +Result of `yarn lint --fix`: +```typescript +@listableObjectComponent(something, somethingElse, undefined, 'test') +export class Something extends SomethingElse { +} +``` + + +##### plain file declares a theme in @listableObjectComponent + +Filename: `lint/test/fixture/src/app/dynamic-component/dynamic-component.ts` + +```typescript +@listableObjectComponent(something, somethingElse, undefined, 'test-2') +export class Something extends SomethingElse { +} +``` +Will produce the following error(s): +``` +There is a theme declaration in decorator, but this file is not part of a theme +``` + +Result of `yarn lint --fix`: +```typescript +@listableObjectComponent(something, somethingElse, undefined) +export class Something extends SomethingElse { +} +``` + + +##### theme file declares no theme in @listableObjectComponent + +Filename: `lint/test/fixture/src/themes/test-2/app/dynamic-component/dynamic-component.ts` + +```typescript +@listableObjectComponent(something, somethingElse, undefined) +export class Something extends SomethingElse { +} +``` +Will produce the following error(s): +``` +No theme declaration in decorator +``` + +Result of `yarn lint --fix`: +```typescript +@listableObjectComponent(something, somethingElse, undefined, 'test-2') +export class Something extends SomethingElse { +} +``` + + +##### theme file declares explicit undefined theme in @listableObjectComponent + +Filename: `lint/test/fixture/src/themes/test-2/app/dynamic-component/dynamic-component.ts` + +```typescript +@listableObjectComponent(something, somethingElse, undefined, undefined) +export class Something extends SomethingElse { +} +``` +Will produce the following error(s): +``` +No theme declaration in decorator +``` + +Result of `yarn lint --fix`: +```typescript +@listableObjectComponent(something, somethingElse, undefined, 'test-2') +export class Something extends SomethingElse { +} +``` + + + diff --git a/lint/src/rules/ts/index.ts b/lint/src/rules/ts/index.ts index e26da64f2a..639afbb067 100644 --- a/lint/src/rules/ts/index.ts +++ b/lint/src/rules/ts/index.ts @@ -14,6 +14,7 @@ import * as aliasImports from './alias-imports'; import * as themedComponentClasses from './themed-component-classes'; import * as themedComponentSelectors from './themed-component-selectors'; import * as themedComponentUsages from './themed-component-usages'; +import * as themedDecorators from './themed-decorators'; import * as themedWrapperNoInputDefaults from './themed-wrapper-no-input-defaults'; import * as uniqueDecorators from './unique-decorators'; @@ -22,6 +23,7 @@ const index = [ themedComponentClasses, themedComponentSelectors, themedComponentUsages, + themedDecorators, themedWrapperNoInputDefaults, uniqueDecorators, ] as unknown as RuleExports[]; diff --git a/lint/src/rules/ts/themed-decorators.ts b/lint/src/rules/ts/themed-decorators.ts new file mode 100644 index 0000000000..58ed21949b --- /dev/null +++ b/lint/src/rules/ts/themed-decorators.ts @@ -0,0 +1,267 @@ +import { + AST_NODE_TYPES, + ESLintUtils, + TSESLint, + TSESTree, +} from '@typescript-eslint/utils'; + +import { fixture } from '../../../test/fixture'; +import { isTestFile } from '../../util/filter'; +import { + DSpaceESLintRuleInfo, + NamedTests, +} from '../../util/structure'; +import { getFileTheme } from '../../util/theme-support'; + +export enum Message { + NO_THEME_DECLARED_IN_THEME_FILE = 'noThemeDeclaredInThemeFile', + THEME_DECLARED_IN_NON_THEME_FILE = 'themeDeclaredInNonThemeFile', + WRONG_THEME_DECLARED_IN_THEME_FILE = 'wrongThemeDeclaredInThemeFile', +} + +interface ThemedDecoratorsOption { + decorators: { [name: string]: number }; +} + +export const info: DSpaceESLintRuleInfo<[ThemedDecoratorsOption]> = { + name: 'themed-decorators', + meta: { + docs: { + description: 'Entry components with theme support should declare the correct theme', + }, + fixable: 'code', + messages: { + [Message.NO_THEME_DECLARED_IN_THEME_FILE]: 'No theme declaration in decorator', + [Message.THEME_DECLARED_IN_NON_THEME_FILE]: 'There is a theme declaration in decorator, but this file is not part of a theme', + [Message.WRONG_THEME_DECLARED_IN_THEME_FILE]: 'Wrong theme declaration in decorator', + }, + type: 'problem', + schema: [ + { + type: 'object', + properties: { + decorators: { + type: 'object', + }, + }, + }, + ], + }, + defaultOptions: [ + { + decorators: { + listableObjectComponent: 3, + rendersSectionForMenu: 2, + }, + }, + ], +}; + +export const rule = ESLintUtils.RuleCreator.withoutDocs({ + ...info, + create(context: TSESLint.RuleContext, options: any) { + return { + [`ClassDeclaration > Decorator > CallExpression[callee.name=/^(${Object.keys(options[0].decorators).join('|')})$/]`]: (node: TSESTree.CallExpression) => { + if (isTestFile(context)) { + return; + } + + if (node.callee.type !== AST_NODE_TYPES.Identifier) { + // We only support regular method identifiers + return; + } + + const fileTheme = getFileTheme(context); + const themeDeclaration = getDeclaredTheme(options, node as TSESTree.CallExpression); + + if (themeDeclaration === undefined) { + if (fileTheme !== undefined) { + context.report({ + messageId: Message.NO_THEME_DECLARED_IN_THEME_FILE, + node: node, + fix(fixer) { + return fixer.insertTextAfter(node.arguments[node.arguments.length - 1], `, '${fileTheme as string}'`); + }, + }); + } + } else if (themeDeclaration?.type === AST_NODE_TYPES.Literal) { + if (fileTheme === undefined) { + context.report({ + messageId: Message.THEME_DECLARED_IN_NON_THEME_FILE, + node: themeDeclaration, + fix(fixer) { + const idx = node.arguments.findIndex((v) => v.range === themeDeclaration.range); + + if (idx === 0) { + return fixer.remove(themeDeclaration); + } else { + const previousArgument = node.arguments[idx - 1]; + return fixer.removeRange([previousArgument.range[1], themeDeclaration.range[1]]); // todo: comma? + } + }, + }); + } else if (fileTheme !== themeDeclaration?.value) { + context.report({ + messageId: Message.WRONG_THEME_DECLARED_IN_THEME_FILE, + node: themeDeclaration, + fix(fixer) { + return fixer.replaceText(themeDeclaration, `'${fileTheme as string}'`); + }, + }); + } + } else if (themeDeclaration?.type === AST_NODE_TYPES.Identifier && themeDeclaration.name === 'undefined') { + if (fileTheme !== undefined) { + context.report({ + messageId: Message.NO_THEME_DECLARED_IN_THEME_FILE, + node: node, + fix(fixer) { + return fixer.replaceText(node.arguments[node.arguments.length - 1], `'${fileTheme as string}'`); + }, + }); + } + } else { + throw new Error('Unexpected theme declaration'); + } + }, + }; + }, +}); + +export const tests: NamedTests = { + plugin: info.name, + valid: [ + { + name: 'theme file declares the correct theme in @listableObjectComponent', + code: ` +@listableObjectComponent(something, somethingElse, undefined, 'test') +export class Something extends SomethingElse { +} + `, + filename: fixture('src/themes/test/app/dynamic-component/dynamic-component.ts'), + }, + { + name: 'plain file declares no theme in @listableObjectComponent', + code: ` +@listableObjectComponent(something, somethingElse, undefined) +export class Something extends SomethingElse { +} + `, + filename: fixture('src/app/dynamic-component/dynamic-component.ts'), + }, + { + name: 'plain file declares explicit undefined theme in @listableObjectComponent', + code: ` +@listableObjectComponent(something, somethingElse, undefined, undefined) +export class Something extends SomethingElse { +} + `, + filename: fixture('src/app/dynamic-component/dynamic-component.ts'), + }, + { + name: 'test file declares theme outside of theme directory', + code: ` +@listableObjectComponent(something, somethingElse, undefined, 'test') +export class Something extends SomethingElse { +} + `, + filename: fixture('src/app/dynamic-component/dynamic-component.spec.ts'), + }, + { + name: 'only track configured decorators', + code: ` +@something('test') +export class Something extends SomethingElse { +} + `, + filename: fixture('src/app/dynamic-component/dynamic-component.ts'), + }, + ], + invalid: [ + { + name: 'theme file declares the wrong theme in @listableObjectComponent', + code: ` +@listableObjectComponent(something, somethingElse, undefined, 'test-2') +export class Something extends SomethingElse { +} + `, + filename: fixture('src/themes/test/app/dynamic-component/dynamic-component.ts'), + errors: [ + { + messageId: 'wrongThemeDeclaredInThemeFile', + }, + ], + output: ` +@listableObjectComponent(something, somethingElse, undefined, 'test') +export class Something extends SomethingElse { +} + `, + }, + { + name: 'plain file declares a theme in @listableObjectComponent', + code: ` +@listableObjectComponent(something, somethingElse, undefined, 'test-2') +export class Something extends SomethingElse { +} + `, + filename: fixture('src/app/dynamic-component/dynamic-component.ts'), + errors: [ + { + messageId: 'themeDeclaredInNonThemeFile', + }, + ], + output: ` +@listableObjectComponent(something, somethingElse, undefined) +export class Something extends SomethingElse { +} + `, + }, + { + name: 'theme file declares no theme in @listableObjectComponent', + code: ` +@listableObjectComponent(something, somethingElse, undefined) +export class Something extends SomethingElse { +} + `, + filename: fixture('src/themes/test-2/app/dynamic-component/dynamic-component.ts'), + errors: [ + { + messageId: 'noThemeDeclaredInThemeFile', + }, + ], + output: ` +@listableObjectComponent(something, somethingElse, undefined, 'test-2') +export class Something extends SomethingElse { +} + `, + }, + { + name: 'theme file declares explicit undefined theme in @listableObjectComponent', + code: ` +@listableObjectComponent(something, somethingElse, undefined, undefined) +export class Something extends SomethingElse { +} + `, + filename: fixture('src/themes/test-2/app/dynamic-component/dynamic-component.ts'), + errors: [ + { + messageId: 'noThemeDeclaredInThemeFile', + }, + ], + output: ` +@listableObjectComponent(something, somethingElse, undefined, 'test-2') +export class Something extends SomethingElse { +} + `, + }, + ], +}; + +function getDeclaredTheme(options: [ThemedDecoratorsOption], decoratorCall: TSESTree.CallExpression): TSESTree.Node | undefined { + const index: number = options[0].decorators[(decoratorCall.callee as TSESTree.Identifier).name]; + + if (decoratorCall.arguments.length >= index + 1) { + return decoratorCall.arguments[index]; + } + + return undefined; +} diff --git a/lint/src/util/theme-support.ts b/lint/src/util/theme-support.ts index 64644145fa..7bc680b930 100644 --- a/lint/src/util/theme-support.ts +++ b/lint/src/util/theme-support.ts @@ -7,6 +7,7 @@ */ import { TSESTree } from '@typescript-eslint/utils'; +import { RuleContext } from '@typescript-eslint/utils/ts-eslint'; import { readFileSync } from 'fs'; import { basename } from 'path'; import ts, { Identifier } from 'typescript'; @@ -263,3 +264,18 @@ export const DISALLOWED_THEME_SELECTORS = 'ds-(base|themed)-'; export function fixSelectors(text: string): string { return text.replaceAll(/ds-(base|themed)-/g, 'ds-'); } + +/** + * Determine the theme of the current file based on its path in the project. + * @param context the current ESLint rule context + */ +export function getFileTheme(context: RuleContext): string | undefined { + // note: shouldn't use plain .filename (doesn't work in DSpace Angular 7.4) + const m = context.getFilename()?.match(/\/src\/themes\/([^/]+)\//); + + if (m?.length === 2) { + return m[1]; + } + + return undefined; +} diff --git a/lint/test/fixture/src/app/dynamic-component/dynamic-component.spec.ts b/lint/test/fixture/src/app/dynamic-component/dynamic-component.spec.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lint/test/fixture/src/app/dynamic-component/dynamic-component.ts b/lint/test/fixture/src/app/dynamic-component/dynamic-component.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lint/test/fixture/src/themes/test-2/app/dynamic-component/dynamic-component.ts b/lint/test/fixture/src/themes/test-2/app/dynamic-component/dynamic-component.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lint/test/fixture/src/themes/test/app/dynamic-component/dynamic-component.ts b/lint/test/fixture/src/themes/test/app/dynamic-component/dynamic-component.ts new file mode 100644 index 0000000000..e69de29bb2