Enforce plugin structure and generate documentation

2025-10-12 20:43:08 +00:00 · 2024-03-14 18:14:30 +01:00
parent e83a0cd741
commit b0758c23e5
19 changed files with 833 additions and 446 deletions
--- a/lint/src/rules/ts/index.ts
+++ b/lint/src/rules/ts/index.ts
@@ -1,9 +1,15 @@
-import themedComponentSelectors from './themed-component-selectors';
-import themedComponentUsages from './themed-component-usages';
+import {
+  bundle,
+  RuleExports,
+} from '../../util/structure';
+import * as themedComponentUsages from './themed-component-usages';
+import * as themedComponentSelectors from './themed-component-selectors';
+
+const index = [
+  themedComponentUsages,
+  themedComponentSelectors,
+] as unknown as RuleExports[];

 export = {
-  rules: {
-    'themed-component-selectors': themedComponentSelectors,
-    'themed-component-usages': themedComponentUsages,
-  },
+  ...bundle('dspace-angular-ts', 'TypeScript',  index),
 };
--- a/lint/src/rules/ts/themed-component-selectors.ts
+++ b/lint/src/rules/ts/themed-component-selectors.ts
@@ -6,27 +6,53 @@
 * http://www.dspace.org/license/
 */
 import { ESLintUtils } from '@typescript-eslint/utils';
+import { fixture } from '../../../test/fixture';

 import { getComponentSelectorNode } from '../../util/angular';
 import { stringLiteral } from '../../util/misc';
+import { DSpaceESLintRuleInfo } from '../../util/structure';
 import {
  inThemedComponentOverrideFile,
  isThemeableComponent,
  isThemedComponentWrapper,
 } from '../../util/theme-support';

-export default ESLintUtils.RuleCreator.withoutDocs({
+export enum Message {
+  BASE = 'wrongSelectorUnthemedComponent',
+  WRAPPER = 'wrongSelectorThemedComponentWrapper',
+  THEMED = 'wrongSelectorThemedComponentOverride',
+}
+
+export const info = {
+  name: 'themed-component-selectors',
  meta: {
+    docs: {
+      description: `Themeable component selectors should follow the DSpace convention
+
+Each themeable component is comprised of a base component, a wrapper component and any number of themed components
+- Base components should have a selector starting with \`ds-base-\`
+- Themed components should have a selector starting with \`ds-themed-\`
+- Wrapper components should have a selector starting with \`ds-\`, but not \`ds-base-\` or \`ds-themed-\`
+  - This is the regular DSpace selector prefix
+  - **When making a regular component themeable, its selector prefix should be changed to \`ds-base-\`, and the new wrapper's component should reuse the previous selector**
+
+Unit tests are exempt from this rule, because they may redefine components using the same class name as other themeable components elsewhere in the source.
+      `,
+    },
    type: 'problem',
    schema: [],
    fixable: 'code',
    messages: {
-      wrongSelectorUnthemedComponent: 'Unthemed version of themeable components should have a selector starting with \'ds-base-\'',
-      wrongSelectorThemedComponentWrapper: 'Themed component wrapper of themeable components shouldn\'t have a selector starting with \'ds-themed-\'',
-      wrongSelectorThemedComponentOverride: 'Theme override of themeable component should have a selector starting with \'ds-themed-\'',
+      [Message.BASE]: 'Unthemed version of themeable components should have a selector starting with \'ds-base-\'',
+      [Message.WRAPPER]: 'Themed component wrapper of themeable components shouldn\'t have a selector starting with \'ds-themed-\'',
+      [Message.THEMED]: 'Theme override of themeable component should have a selector starting with \'ds-themed-\'',
    },
  },
  defaultOptions: [],
+} as DSpaceESLintRuleInfo;
+
+export const rule = ESLintUtils.RuleCreator.withoutDocs({
+  ...info,
  create(context: any): any {
    if (context.getFilename()?.endsWith('.spec.ts')) {
      return {};
@@ -35,7 +61,7 @@ export default ESLintUtils.RuleCreator.withoutDocs({
    function enforceWrapperSelector(selectorNode: any) {
      if (selectorNode?.value.startsWith('ds-themed-')) {
        context.report({
-          messageId: 'wrongSelectorThemedComponentWrapper',
+          messageId: Message.WRAPPER,
          node: selectorNode,
          fix(fixer: any) {
            return fixer.replaceText(selectorNode, stringLiteral(selectorNode.value.replace('ds-themed-', 'ds-')));
@@ -47,7 +73,7 @@ export default ESLintUtils.RuleCreator.withoutDocs({
    function enforceBaseSelector(selectorNode: any) {
      if (!selectorNode?.value.startsWith('ds-base-')) {
        context.report({
-          messageId: 'wrongSelectorUnthemedComponent',
+          messageId: Message.BASE,
          node: selectorNode,
          fix(fixer: any) {
            return fixer.replaceText(selectorNode, stringLiteral(selectorNode.value.replace('ds-', 'ds-base-')));
@@ -59,7 +85,7 @@ export default ESLintUtils.RuleCreator.withoutDocs({
    function enforceThemedSelector(selectorNode: any) {
      if (!selectorNode?.value.startsWith('ds-themed-')) {
        context.report({
-          messageId: 'wrongSelectorThemedComponentOverride',
+          messageId: Message.THEMED,
          node: selectorNode,
          fix(fixer: any) {
            return fixer.replaceText(selectorNode, stringLiteral(selectorNode.value.replace('ds-', 'ds-themed-')));
@@ -91,3 +117,130 @@ export default ESLintUtils.RuleCreator.withoutDocs({
    };
  },
 });
+
+export const tests = {
+  plugin: info.name,
+  valid: [
+    {
+      name: 'Regular non-themeable component selector',
+      code: `
+        @Component({
+          selector: 'ds-something',
+        })
+        class Something {
+        }
+      `,
+    },
+    {
+      name: 'Themeable component selector should replace the original version, unthemed version should be changed to ds-base-',
+      code: `
+@Component({
+  selector: 'ds-base-something',
+})
+class Something {
+}
+
+@Component({
+  selector: 'ds-something',
+})
+class ThemedSomething extends ThemedComponent<Something> {
+}
+
+@Component({
+  selector: 'ds-themed-something',
+})
+class OverrideSomething extends Something {
+}
+      `,
+    },
+    {
+      name: 'Other themed component wrappers should not interfere',
+      code: `
+@Component({
+  selector: 'ds-something',
+})
+class Something {
+}
+
+@Component({
+  selector: 'ds-something-else',
+})
+class ThemedSomethingElse extends ThemedComponent<SomethingElse> {
+}
+      `,
+    },
+  ],
+  invalid: [
+    {
+      name: 'Wrong selector for base component',
+      filename: fixture('src/app/test/test-themeable.component.ts'),
+      code: `
+@Component({
+  selector: 'ds-something',
+})
+class TestThemeableComponent {
+}
+        `,
+      errors: [
+        {
+          messageId: Message.BASE,
+        },
+      ],
+      output: `
+@Component({
+  selector: 'ds-base-something',
+})
+class TestThemeableComponent {
+}
+        `,
+    },
+    {
+      name: 'Wrong selector for wrapper component',
+      filename: fixture('src/app/test/themed-test-themeable.component.ts'),
+      code: `
+@Component({
+  selector: 'ds-themed-something',
+})
+class ThemedTestThemeableComponent extends ThemedComponent<TestThemeableComponent> {
+}
+        `,
+      errors: [
+        {
+          messageId: Message.WRAPPER,
+        },
+      ],
+      output: `
+@Component({
+  selector: 'ds-something',
+})
+class ThemedTestThemeableComponent extends ThemedComponent<TestThemeableComponent> {
+}
+        `,
+    },
+    {
+      name: 'Wrong selector for theme override',
+      filename: fixture('src/themes/test/app/test/test-themeable.component.ts'),
+      code: `
+@Component({
+  selector: 'ds-something',
+})
+class TestThememeableComponent extends BaseComponent {
+}
+        `,
+      errors: [
+        {
+          messageId: Message.THEMED,
+        },
+      ],
+      output: `
+@Component({
+  selector: 'ds-themed-something',
+})
+class TestThememeableComponent extends BaseComponent {
+}
+        `,
+    },
+  ],
+};
+
+export default rule;
--- a/lint/src/rules/ts/themed-component-usages.ts
+++ b/lint/src/rules/ts/themed-component-usages.ts
@@ -6,8 +6,9 @@
 * http://www.dspace.org/license/
 */
 import { ESLintUtils } from '@typescript-eslint/utils';
-
+import { fixture } from '../../../test/fixture';
 import { findUsages } from '../../util/misc';
+import { DSpaceESLintRuleInfo } from '../../util/structure';
 import {
  allThemeableComponents,
  DISALLOWED_THEME_SELECTORS,
@@ -17,17 +18,40 @@ import {
  isAllowedUnthemedUsage,
 } from '../../util/theme-support';

-export default ESLintUtils.RuleCreator.withoutDocs({
+export enum Message {
+  WRONG_CLASS = 'mustUseThemedWrapperClass',
+  WRONG_IMPORT = 'mustImportThemedWrapper',
+  WRONG_SELECTOR = 'mustUseThemedWrapperSelector',
+}
+
+export const info = {
+  name: 'themed-component-usages',
  meta: {
+    docs: {
+      description: `Themeable components should be used via their \`ThemedComponent\` wrapper class
+
+This ensures that custom themes can correctly override _all_ instances of this component.
+There are a few exceptions where the base class can still be used:
+- Class declaration expressions (otherwise we can't declare, extend or override the class in the first place)
+- Angular modules (except for routing modules)
+- Angular \`@ViewChild\` decorators
+- Type annotations
+      `,
+    },
    type: 'problem',
    schema: [],
    fixable: 'code',
    messages: {
-      mustUseThemedWrapper: 'Themeable components should be used via their ThemedComponent wrapper',
-      mustImportThemedWrapper: 'Themeable components should be used via their ThemedComponent wrapper',
+      [Message.WRONG_CLASS]: 'Themeable components should be used via their ThemedComponent wrapper',
+      [Message.WRONG_IMPORT]: 'Themeable components should be used via their ThemedComponent wrapper',
+      [Message.WRONG_SELECTOR]: 'Themeable components should be used via their ThemedComponent wrapper',
    },
  },
  defaultOptions: [],
+} as DSpaceESLintRuleInfo;
+
+export const rule = ESLintUtils.RuleCreator.withoutDocs({
+  ...info,
  create(context: any, options: any): any {
    function handleUnthemedUsagesInTypescript(node: any) {
      if (isAllowedUnthemedUsage(node)) {
@@ -42,7 +66,7 @@ export default ESLintUtils.RuleCreator.withoutDocs({
      }

      context.report({
-        messageId: 'mustUseThemedWrapper',
+        messageId: Message.WRONG_CLASS,
        node: node,
        fix(fixer: any) {
          return fixer.replaceText(node, entry.wrapperClass);
@@ -53,7 +77,7 @@ export default ESLintUtils.RuleCreator.withoutDocs({
    function handleThemedSelectorQueriesInTests(node: any) {
      context.report({
        node,
-        messageId: 'mustUseThemedWrapper',
+        messageId: Message.WRONG_SELECTOR,
        fix(fixer: any){
          const newSelector = fixSelectors(node.raw);
          return fixer.replaceText(node, newSelector);
@@ -79,7 +103,7 @@ export default ESLintUtils.RuleCreator.withoutDocs({
      }

      context.report({
-        messageId: 'mustImportThemedWrapper',
+        messageId: Message.WRONG_IMPORT,
        node: importedNode,
        fix(fixer: any) {
          const ops = [];
@@ -133,3 +157,175 @@ export default ESLintUtils.RuleCreator.withoutDocs({

  },
 });
+
+export const tests = {
+  plugin: info.name,
+  valid: [
+    {
+      name: 'allow wrapper class usages',
+      code: `
+import { ThemedTestThemeableComponent } from '../test/themed-test-themeable.component.ts';
+
+const config = {
+  a: ThemedTestThemeableComponent,
+  b: ChipsComponent,
+}
+        `,
+    },
+    {
+      name: 'allow base class in class declaration',
+      code: `
+export class TestThemeableComponent {
+}
+        `,
+    },
+    {
+      name: 'allow inheriting from base class',
+      code: `
+import { TestThemeableComponent } from '../test/test-themeable.component.ts';
+
+export class ThemedAdminSidebarComponent extends ThemedComponent<TestThemeableComponent> {
+}
+        `,
+    },
+    {
+      name: 'allow base class in ViewChild',
+      code: `
+import { TestThemeableComponent } from '../test/test-themeable.component.ts';
+
+export class Something {
+  @ViewChild(TestThemeableComponent) test: TestThemeableComponent;
+}
+        `,
+    },
+    {
+      name: 'allow wrapper selectors in test queries',
+      filename: fixture('src/app/test/test.component.spec.ts'),
+      code: `
+By.css('ds-themeable');
+By.Css('#test > ds-themeable > #nest');
+        `,
+    },
+    {
+      name: 'allow wrapper selectors in cypress queries',
+      filename: fixture('src/app/test/test.component.cy.ts'),
+      code: `
+By.css('ds-themeable');
+By.Css('#test > ds-themeable > #nest');
+        `,
+    },
+  ],
+  invalid: [
+    {
+      name: 'disallow direct usages of base class',
+      code: `
+import { TestThemeableComponent } from '../test/test-themeable.component.ts';
+import { TestComponent } from '../test/test.component.ts';
+
+const config = {
+  a: TestThemeableComponent,
+  b: TestComponent,
+}
+        `,
+      errors: [
+        {
+          messageId: Message.WRONG_IMPORT,
+        },
+        {
+          messageId: Message.WRONG_CLASS,
+        },
+      ],
+      output: `
+import { ThemedTestThemeableComponent } from '../test/themed-test-themeable.component.ts';
+import { TestComponent } from '../test/test.component.ts';
+
+const config = {
+  a: ThemedTestThemeableComponent,
+  b: TestComponent,
+}
+        `,
+    },
+    {
+      name: 'disallow override selector in test queries',
+      filename: fixture('src/app/test/test.component.spec.ts'),
+      code: `
+By.css('ds-themed-themeable');
+By.css('#test > ds-themed-themeable > #nest');
+        `,
+      errors: [
+        {
+          messageId: Message.WRONG_SELECTOR,
+        },
+        {
+          messageId: Message.WRONG_SELECTOR,
+        },
+      ],
+      output: `
+By.css('ds-themeable');
+By.css('#test > ds-themeable > #nest');
+        `,
+    },
+    {
+      name: 'disallow base selector in test queries',
+      filename: fixture('src/app/test/test.component.spec.ts'),
+      code: `
+By.css('ds-base-themeable');
+By.css('#test > ds-base-themeable > #nest');
+        `,
+      errors: [
+        {
+          messageId: Message.WRONG_SELECTOR,
+        },
+        {
+          messageId: Message.WRONG_SELECTOR,
+        },
+      ],
+      output: `
+By.css('ds-themeable');
+By.css('#test > ds-themeable > #nest');
+        `,
+    },
+    {
+      name: 'disallow override selector in cypress queries',
+      filename: fixture('src/app/test/test.component.cy.ts'),
+      code: `
+cy.get('ds-themed-themeable');
+cy.get('#test > ds-themed-themeable > #nest');
+        `,
+      errors: [
+        {
+          messageId: Message.WRONG_SELECTOR,
+        },
+        {
+          messageId: Message.WRONG_SELECTOR,
+        },
+      ],
+      output: `
+cy.get('ds-themeable');
+cy.get('#test > ds-themeable > #nest');
+        `,
+    },
+    {
+      name: 'disallow base selector in cypress queries',
+      filename: fixture('src/app/test/test.component.cy.ts'),
+      code: `
+cy.get('ds-base-themeable');
+cy.get('#test > ds-base-themeable > #nest');
+        `,
+      errors: [
+        {
+          messageId: Message.WRONG_SELECTOR,
+        },
+        {
+          messageId: Message.WRONG_SELECTOR,
+        },
+      ],
+      output: `
+cy.get('ds-themeable');
+cy.get('#test > ds-themeable > #nest');
+        `,
+    },
+  ],
+};
+
+export default rule;