From e40b6ae612035135fe417ba9ec551c28fbbd5dce Mon Sep 17 00:00:00 2001 From: Yury Bondarenko Date: Thu, 21 Mar 2024 10:37:20 +0100 Subject: [PATCH] Update plugins to support standalone components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ThemedComponent wrappers should always import their base component. This ensures that it's always enough to only import the wrapper when we use it. - This implies that all themeable components must be standalone → added rules to enforce this → updated usage rule to improve declaration/import handling --- .eslintrc.json | 1 + lint/src/rules/ts/index.ts | 4 +- lint/src/rules/ts/themed-component-classes.ts | 378 ++++++++++++++++++ lint/src/rules/ts/themed-component-usages.ts | 201 +++++++--- lint/src/util/angular.ts | 59 ++- lint/src/util/fix.ts | 125 ++++++ lint/src/util/theme-support.ts | 25 +- lint/src/util/typescript.ts | 74 ++++ .../src/app/test/test-themeable.component.ts | 1 + .../test/themed-test-themeable.component.ts | 2 + .../app/test/other-themeable.component.ts | 16 + .../fixture/src/themes/test/test.module.ts | 2 + lint/test/fixture/tsconfig.json | 1 + lint/test/testing.ts | 1 + 14 files changed, 835 insertions(+), 55 deletions(-) create mode 100644 lint/src/rules/ts/themed-component-classes.ts create mode 100644 lint/src/util/fix.ts create mode 100644 lint/test/fixture/src/themes/test/app/test/other-themeable.component.ts diff --git a/.eslintrc.json b/.eslintrc.json index ce5d1225b0..147aaeb463 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -245,6 +245,7 @@ "rxjs/no-nested-subscribe": "off", // todo: go over _all_ cases // Custom DSpace Angular rules + "dspace-angular-ts/themed-component-classes": "error", "dspace-angular-ts/themed-component-selectors": "error", "dspace-angular-ts/themed-component-usages": "error" } diff --git a/lint/src/rules/ts/index.ts b/lint/src/rules/ts/index.ts index 4ff38bd0c3..a7fdfe41ef 100644 --- a/lint/src/rules/ts/index.ts +++ b/lint/src/rules/ts/index.ts @@ -10,12 +10,14 @@ import { RuleExports, } from '../../util/structure'; /* eslint-disable import/no-namespace */ +import * as themedComponentClasses from './themed-component-classes'; import * as themedComponentSelectors from './themed-component-selectors'; import * as themedComponentUsages from './themed-component-usages'; const index = [ - themedComponentUsages, + themedComponentClasses, themedComponentSelectors, + themedComponentUsages, ] as unknown as RuleExports[]; export = { diff --git a/lint/src/rules/ts/themed-component-classes.ts b/lint/src/rules/ts/themed-component-classes.ts new file mode 100644 index 0000000000..727785c6d7 --- /dev/null +++ b/lint/src/rules/ts/themed-component-classes.ts @@ -0,0 +1,378 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +import { + ESLintUtils, + TSESLint, + TSESTree, +} from '@typescript-eslint/utils'; + +import { fixture } from '../../../test/fixture'; +import { + getComponentImportNode, + getComponentInitializer, + getComponentStandaloneNode, +} from '../../util/angular'; +import { appendObjectProperties } from '../../util/fix'; +import { DSpaceESLintRuleInfo } from '../../util/structure'; +import { + getBaseComponentClassName, + inThemedComponentOverrideFile, + isThemeableComponent, + isThemedComponentWrapper, +} from '../../util/theme-support'; +import { getFilename } from '../../util/typescript'; + +export enum Message { + NOT_STANDALONE = 'mustBeStandalone', + NOT_STANDALONE_IMPORTS_BASE = 'mustBeStandaloneAndImportBase', + WRAPPER_IMPORTS_BASE = 'wrapperShouldImportBase', +} + +export const info = { + name: 'themed-component-classes', + meta: { + docs: { + description: `Formatting rules for themeable component classes`, + }, + type: 'problem', + fixable: 'code', + schema: [], + messages: { + [Message.NOT_STANDALONE]: 'Themeable components must be standalone', + [Message.NOT_STANDALONE_IMPORTS_BASE]: 'Themeable component wrapper classes must be standalone and import the base class', + [Message.WRAPPER_IMPORTS_BASE]: 'Themed component wrapper classes must import the base class', + }, + }, + defaultOptions: [], +} as DSpaceESLintRuleInfo; + +export const rule = ESLintUtils.RuleCreator.withoutDocs({ + ...info, + create(context: TSESLint.RuleContext) { + const filename = getFilename(context); + + if (filename.endsWith('.spec.ts')) { + return {}; + } + + function enforceStandalone(decoratorNode: TSESTree.Decorator, withBaseImport = false) { + const standaloneNode = getComponentStandaloneNode(decoratorNode); + + if (standaloneNode === undefined) { + // We may need to add these properties in one go + if (!withBaseImport) { + context.report({ + messageId: Message.NOT_STANDALONE, + node: decoratorNode, + fix(fixer) { + const initializer = getComponentInitializer(decoratorNode); + return appendObjectProperties(context, fixer, initializer, ['standalone: true']); + }, + }); + } + } else if (!standaloneNode.value) { + context.report({ + messageId: Message.NOT_STANDALONE, + node: standaloneNode, + fix(fixer) { + return fixer.replaceText(standaloneNode, 'true'); + }, + }); + } + + if (withBaseImport) { + const baseClass = getBaseComponentClassName(decoratorNode); + + if (baseClass === undefined) { + return; + } + + const importsNode = getComponentImportNode(decoratorNode); + + if (importsNode === undefined) { + if (standaloneNode === undefined) { + context.report({ + messageId: Message.NOT_STANDALONE_IMPORTS_BASE, + node: decoratorNode, + fix(fixer) { + const initializer = getComponentInitializer(decoratorNode); + return appendObjectProperties(context, fixer, initializer, ['standalone: true', `imports: [${baseClass}]`]); + }, + }); + } else { + context.report({ + messageId: Message.WRAPPER_IMPORTS_BASE, + node: decoratorNode, + fix(fixer) { + const initializer = getComponentInitializer(decoratorNode); + return appendObjectProperties(context, fixer, initializer, [`imports: [${baseClass}]`]); + }, + }); + } + } else { + // If we have an imports node, standalone: true will be enforced by another rule + + const imports = importsNode.elements.map(e => (e as TSESTree.Identifier).name); + + if (!imports.includes(baseClass) || imports.length > 1) { + // The wrapper should _only_ import the base component + context.report({ + messageId: Message.WRAPPER_IMPORTS_BASE, + node: importsNode, + fix(fixer) { + // todo: this may leave unused imports, but that's better than mangling things + return fixer.replaceText(importsNode, `[${baseClass}]`); + }, + }); + } + } + } + } + + return { + 'ClassDeclaration > Decorator[expression.callee.name = "Component"]'(node: TSESTree.Decorator) { + const classNode = node.parent as TSESTree.ClassDeclaration; + const className = classNode.id?.name; + + if (className === undefined) { + return; + } + + if (isThemedComponentWrapper(node)) { + enforceStandalone(node, true); + } else if (inThemedComponentOverrideFile(filename)) { + enforceStandalone(node); + } else if (isThemeableComponent(className)) { + enforceStandalone(node); + } + }, + }; + }, +}); + +export const tests = { + plugin: info.name, + valid: [ + { + name: 'Regular non-themeable component', + code: ` +@Component({ + selector: 'ds-something', + standalone: true, +}) +class Something { +} + `, + }, + { + name: 'Base component', + code: ` +@Component({ + selector: 'ds-base-test-themable', + standalone: true, +}) +class TestThemeableTomponent { +} + `, + }, + { + name: 'Wrapper component', + filename: fixture('src/app/test/themed-test-themeable.component.ts'), + code: ` +@Component({ + selector: 'ds-test-themable', + standalone: true, + imports: [ + TestThemeableComponent, + ], +}) +class ThemedTestThemeableTomponent extends ThemedComponent { +} + `, + }, + { + name: 'Override component', + filename: fixture('src/themes/test/app/test/test-themeable.component.ts'), + code: ` +@Component({ + selector: 'ds-themed-test-themable', + standalone: true, +}) +class Override extends BaseComponent { +} + `, + }, + ], + invalid: [ + { + name: 'Base component must be standalone', + code: ` +@Component({ + selector: 'ds-base-test-themable', +}) +class TestThemeableComponent { +} + `, + errors:[ + { + messageId: Message.NOT_STANDALONE, + }, + ], + output: ` +@Component({ + selector: 'ds-base-test-themable', + standalone: true, +}) +class TestThemeableComponent { +} + `, + }, + { + name: 'Wrapper component must be standalone and import base component', + filename: fixture('src/app/test/themed-test-themeable.component.ts'), + code: ` +@Component({ + selector: 'ds-test-themable', +}) +class ThemedTestThemeableComponent extends ThemedComponent { +} + `, + errors:[ + { + messageId: Message.NOT_STANDALONE_IMPORTS_BASE, + }, + ], + output: ` +@Component({ + selector: 'ds-test-themable', + standalone: true, + imports: [TestThemeableComponent], +}) +class ThemedTestThemeableComponent extends ThemedComponent { +} + `, + }, + + { + name: 'Wrapper component must import base component (array present but empty)', + filename: fixture('src/app/test/themed-test-themeable.component.ts'), + code: ` +@Component({ + selector: 'ds-test-themable', + standalone: true, + imports: [], +}) +class ThemedTestThemeableComponent extends ThemedComponent { +} + `, + errors:[ + { + messageId: Message.WRAPPER_IMPORTS_BASE, + }, + ], + output: ` +@Component({ + selector: 'ds-test-themable', + standalone: true, + imports: [TestThemeableComponent], +}) +class ThemedTestThemeableComponent extends ThemedComponent { +} + `, + }, + { + name: 'Wrapper component must import base component (array is wrong)', + filename: fixture('src/app/test/themed-test-themeable.component.ts'), + code: ` +import { SomethingElse } from './somewhere-else'; + +@Component({ + selector: 'ds-test-themable', + standalone: true, + imports: [ + SomethingElse, + ], +}) +class ThemedTestThemeableComponent extends ThemedComponent { +} + `, + errors:[ + { + messageId: Message.WRAPPER_IMPORTS_BASE, + }, + ], + output: ` +import { SomethingElse } from './somewhere-else'; + +@Component({ + selector: 'ds-test-themable', + standalone: true, + imports: [TestThemeableComponent], +}) +class ThemedTestThemeableComponent extends ThemedComponent { +} + `, + }, { + name: 'Wrapper component must import base component (array is wrong)', + filename: fixture('src/app/test/themed-test-themeable.component.ts'), + code: ` +import { Something, SomethingElse } from './somewhere-else'; + +@Component({ + selector: 'ds-test-themable', + standalone: true, + imports: [ + SomethingElse, + ], +}) +class ThemedTestThemeableComponent extends ThemedComponent { +} + `, + errors:[ + { + messageId: Message.WRAPPER_IMPORTS_BASE, + }, + ], + output: ` +import { Something, SomethingElse } from './somewhere-else'; + +@Component({ + selector: 'ds-test-themable', + standalone: true, + imports: [TestThemeableComponent], +}) +class ThemedTestThemeableComponent extends ThemedComponent { +} + `, + }, + { + name: 'Override component must be standalone', + filename: fixture('src/themes/test/app/test/test-themeable.component.ts'), + code: ` +@Component({ + selector: 'ds-themed-test-themable', +}) +class Override extends BaseComponent { +} + `, + errors:[ + { + messageId: Message.NOT_STANDALONE, + }, + ], + output: ` +@Component({ + selector: 'ds-themed-test-themable', + standalone: true, +}) +class Override extends BaseComponent { +} + `, + }, + ], +}; diff --git a/lint/src/rules/ts/themed-component-usages.ts b/lint/src/rules/ts/themed-component-usages.ts index 1263e44b48..2387ea76dd 100644 --- a/lint/src/rules/ts/themed-component-usages.ts +++ b/lint/src/rules/ts/themed-component-usages.ts @@ -12,6 +12,10 @@ import { } from '@typescript-eslint/utils'; import { fixture } from '../../../test/fixture'; +import { + removeWithCommas, + replaceOrRemoveArrayIdentifier, +} from '../../util/fix'; import { DSpaceESLintRuleInfo } from '../../util/structure'; import { allThemeableComponents, @@ -22,14 +26,18 @@ import { isAllowedUnthemedUsage, } from '../../util/theme-support'; import { + findImportSpecifier, findUsages, + findUsagesByName, getFilename, + relativePath, } from '../../util/typescript'; export enum Message { WRONG_CLASS = 'mustUseThemedWrapperClass', WRONG_IMPORT = 'mustImportThemedWrapper', WRONG_SELECTOR = 'mustUseThemedWrapperSelector', + BASE_IN_MODULE = 'baseComponentNotNeededInModule', } export const info = { @@ -53,6 +61,7 @@ There are a few exceptions where the base class can still be used: [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', + [Message.BASE_IN_MODULE]: 'Base themeable components shouldn\'t be declared in modules', }, }, defaultOptions: [], @@ -79,7 +88,11 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({ messageId: Message.WRONG_CLASS, node: node, fix(fixer) { - return fixer.replaceText(node, entry.wrapperClass); + if (node.parent.type === TSESTree.AST_NODE_TYPES.ArrayExpression) { + return replaceOrRemoveArrayIdentifier(context, fixer, node, entry.wrapperClass); + } else { + return fixer.replaceText(node, entry.wrapperClass); + } }, }); } @@ -118,18 +131,36 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({ fix(fixer) { const ops = []; - const oldImportSource = declarationNode.source.value; - const newImportLine = `import { ${entry.wrapperClass} } from '${oldImportSource.replace(entry.baseFileName, entry.wrapperFileName)}';`; + const wrapperImport = findImportSpecifier(context, entry.wrapperClass); - if (declarationNode.specifiers.length === 1) { - if (allUsages.length === badUsages.length) { - ops.push(fixer.replaceText(declarationNode, newImportLine)); + if (findUsagesByName(context, entry.wrapperClass).length === 0) { + // Wrapper is not present in this file, safe to add import + + const newImportLine = `import { ${entry.wrapperClass} } from '${relativePath(filename, entry.wrapperPath)}';`; + + if (declarationNode.specifiers.length === 1) { + if (allUsages.length === badUsages.length) { + ops.push(fixer.replaceText(declarationNode, newImportLine)); + } else if (wrapperImport === undefined) { + ops.push(fixer.insertTextAfter(declarationNode, '\n' + newImportLine)); + } } else { - ops.push(fixer.insertTextAfter(declarationNode, newImportLine)); + ops.push(...removeWithCommas(context, fixer, specifierNode)); + if (wrapperImport === undefined) { + ops.push(fixer.insertTextAfter(declarationNode, '\n' + newImportLine)); + } } } else { - ops.push(fixer.replaceText(specifierNode, entry.wrapperClass)); - ops.push(fixer.insertTextAfter(declarationNode, newImportLine)); + // Wrapper already present in the file, remove import instead + + if (allUsages.length === badUsages.length) { + if (declarationNode.specifiers.length === 1) { + // Make sure we remove the newline as well + ops.push(fixer.removeRange([declarationNode.range[0], declarationNode.range[1] + 1])); + } else { + ops.push(...removeWithCommas(context, fixer, specifierNode)); + } + } } return ops; @@ -147,9 +178,8 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({ [`CallExpression[callee.object.name = "cy"][callee.property.name = "get"] > Literal:first-child[value = /.*${DISALLOWED_THEME_SELECTORS}.*/]`]: handleThemedSelectorQueriesInTests, }; } else if ( - filename.match(/(?!routing).module.ts$/) + filename.match(/(?!src\/themes\/).*(?!routing).module.ts$/) || filename.match(/themed-.+\.component\.ts$/) - || inThemedComponentFile(context) ) { // do nothing return {}; @@ -174,7 +204,7 @@ export const tests = { { name: 'allow wrapper class usages', code: ` -import { ThemedTestThemeableComponent } from '../test/themed-test-themeable.component.ts'; +import { ThemedTestThemeableComponent } from './app/test/themed-test-themeable.component'; const config = { a: ThemedTestThemeableComponent, @@ -192,7 +222,7 @@ export class TestThemeableComponent { { name: 'allow inheriting from base class', code: ` -import { TestThemeableComponent } from '../test/test-themeable.component.ts'; +import { TestThemeableComponent } from './app/test/test-themeable.component'; export class ThemedAdminSidebarComponent extends ThemedComponent { } @@ -201,7 +231,7 @@ export class ThemedAdminSidebarComponent extends ThemedComponent ds-themeable > #nest'); { name: 'disallow direct usages of base class', code: ` -import { TestThemeableComponent } from '../test/test-themeable.component.ts'; -import { TestComponent } from '../test/test.component.ts'; +import { TestThemeableComponent } from './app/test/test-themeable.component'; +import { TestComponent } from './app/test/test.component'; const config = { a: TestThemeableComponent, @@ -246,8 +276,8 @@ const config = { }, ], output: ` -import { ThemedTestThemeableComponent } from '../test/themed-test-themeable.component.ts'; -import { TestComponent } from '../test/test.component.ts'; +import { ThemedTestThemeableComponent } from './app/test/themed-test-themeable.component'; +import { TestComponent } from './app/test/test.component'; const config = { a: ThemedTestThemeableComponent, @@ -255,6 +285,61 @@ const config = { } `, }, + { + name: 'disallow direct usages of base class, keep other imports', + code: ` +import { Something, TestThemeableComponent } from './app/test/test-themeable.component'; +import { TestComponent } from './app/test/test.component'; + +const config = { + a: TestThemeableComponent, + b: TestComponent, + c: Something, +} + `, + errors: [ + { + messageId: Message.WRONG_IMPORT, + }, + { + messageId: Message.WRONG_CLASS, + }, + ], + output: ` +import { Something } from './app/test/test-themeable.component'; +import { ThemedTestThemeableComponent } from './app/test/themed-test-themeable.component'; +import { TestComponent } from './app/test/test.component'; + +const config = { + a: ThemedTestThemeableComponent, + b: TestComponent, + c: Something, +} + `, + }, + { + name: 'handle array replacements correctly', + code: ` +const DECLARATIONS = [ + Something, + TestThemeableComponent, + Something, + ThemedTestThemeableComponent, +]; + `, + errors: [ + { + messageId: Message.WRONG_CLASS, + }, + ], + output: ` +const DECLARATIONS = [ + Something, + Something, + ThemedTestThemeableComponent, +]; + `, + }, { name: 'disallow override selector in test queries', filename: fixture('src/app/test/test.component.spec.ts'), @@ -337,30 +422,18 @@ cy.get('#test > ds-themeable > #nest'); }, { name: 'edge case: unable to find usage node through usage token, but import is still flagged and fixed', + filename: fixture('src/themes/test/app/test/other-themeable.component.ts'), code: ` import { Component } from '@angular/core'; -import { Context } from '../../core/shared/context.model'; -import { TestThemeableComponent } from '../test/test-themeable.component.ts'; +import { Context } from './app/core/shared/context.model'; +import { TestThemeableComponent } from '../../../../app/test/test-themeable.component'; @Component({ - selector: 'ds-admin-search-page', - templateUrl: './admin-search-page.component.html', - styleUrls: ['./admin-search-page.component.scss'], standalone: true, - imports: [ - TestThemeableComponent - ], + imports: [TestThemeableComponent], }) - -/** - * Component that represents a search page for administrators - */ -export class AdminSearchPageComponent { - /** - * The context of this page - */ - context: Context = Context.AdminSearch; +export class UsageComponent { } `, errors: [ @@ -374,27 +447,53 @@ export class AdminSearchPageComponent { output: ` import { Component } from '@angular/core'; -import { Context } from '../../core/shared/context.model'; -import { ThemedTestThemeableComponent } from '../test/themed-test-themeable.component.ts'; +import { Context } from './app/core/shared/context.model'; +import { ThemedTestThemeableComponent } from '../../../../app/test/themed-test-themeable.component'; @Component({ - selector: 'ds-admin-search-page', - templateUrl: './admin-search-page.component.html', - styleUrls: ['./admin-search-page.component.scss'], standalone: true, - imports: [ - ThemedTestThemeableComponent - ], + imports: [ThemedTestThemeableComponent], }) +export class UsageComponent { +} + `, + }, + { + name: 'edge case edge case: both are imported, only wrapper is retained', + filename: fixture('src/themes/test/app/test/other-themeable.component.ts'), + code: ` +import { Component } from '@angular/core'; -/** - * Component that represents a search page for administrators - */ -export class AdminSearchPageComponent { - /** - * The context of this page - */ - context: Context = Context.AdminSearch; +import { Context } from './app/core/shared/context.model'; +import { TestThemeableComponent } from '../../../../app/test/test-themeable.component'; +import { ThemedTestThemeableComponent } from '../../../../app/test/themed-test-themeable.component'; + +@Component({ + standalone: true, + imports: [TestThemeableComponent, ThemedTestThemeableComponent], +}) +export class UsageComponent { +} + `, + errors: [ + { + messageId: Message.WRONG_IMPORT, + }, + { + messageId: Message.WRONG_CLASS, + }, + ], + output: ` +import { Component } from '@angular/core'; + +import { Context } from './app/core/shared/context.model'; +import { ThemedTestThemeableComponent } from '../../../../app/test/themed-test-themeable.component'; + +@Component({ + standalone: true, + imports: [ThemedTestThemeableComponent], +}) +export class UsageComponent { } `, }, diff --git a/lint/src/util/angular.ts b/lint/src/util/angular.ts index 7bff24718c..70ee903fb8 100644 --- a/lint/src/util/angular.ts +++ b/lint/src/util/angular.ts @@ -10,8 +10,7 @@ import { TSESTree } from '@typescript-eslint/utils'; import { getObjectPropertyNodeByName } from './typescript'; export function getComponentSelectorNode(componentDecoratorNode: TSESTree.Decorator): TSESTree.StringLiteral | undefined { - const initializer = (componentDecoratorNode.expression as TSESTree.CallExpression).arguments[0] as TSESTree.ObjectExpression; - const property = getObjectPropertyNodeByName(initializer, 'selector'); + const property = getComponentInitializerNodeByName(componentDecoratorNode, 'selector'); if (property !== undefined) { // todo: support template literals as well @@ -23,6 +22,62 @@ export function getComponentSelectorNode(componentDecoratorNode: TSESTree.Decora return undefined; } +export function getComponentStandaloneNode(componentDecoratorNode: TSESTree.Decorator): TSESTree.BooleanLiteral | undefined { + const property = getComponentInitializerNodeByName(componentDecoratorNode, 'standalone'); + + if (property !== undefined) { + if (property.type === TSESTree.AST_NODE_TYPES.Literal && typeof property.value === 'boolean') { + return property as TSESTree.BooleanLiteral; + } + } + + return undefined; +} +export function getComponentImportNode(componentDecoratorNode: TSESTree.Decorator): TSESTree.ArrayExpression | undefined { + const property = getComponentInitializerNodeByName(componentDecoratorNode, 'imports'); + + if (property !== undefined) { + if (property.type === TSESTree.AST_NODE_TYPES.ArrayExpression) { + return property as TSESTree.ArrayExpression; + } + } + + return undefined; +} + +export function getComponentClassName(decoratorNode: TSESTree.Decorator): string | undefined { + if (decoratorNode.parent.type !== TSESTree.AST_NODE_TYPES.ClassDeclaration) { + return undefined; + } + + if (decoratorNode.parent.id?.type !== TSESTree.AST_NODE_TYPES.Identifier) { + return undefined; + } + + return decoratorNode.parent.id.name; +} + +export function getComponentSuperClassName(decoratorNode: TSESTree.Decorator): string | undefined { + if (decoratorNode.parent.type !== TSESTree.AST_NODE_TYPES.ClassDeclaration) { + return undefined; + } + + if (decoratorNode.parent.superClass?.type !== TSESTree.AST_NODE_TYPES.Identifier) { + return undefined; + } + + return decoratorNode.parent.superClass.name; +} + +export function getComponentInitializer(componentDecoratorNode: TSESTree.Decorator): TSESTree.ObjectExpression { + return (componentDecoratorNode.expression as TSESTree.CallExpression).arguments[0] as TSESTree.ObjectExpression; +} + +export function getComponentInitializerNodeByName(componentDecoratorNode: TSESTree.Decorator, name: string): TSESTree.Node | undefined { + const initializer = getComponentInitializer(componentDecoratorNode); + return getObjectPropertyNodeByName(initializer, name); +} + export function isPartOfViewChild(node: TSESTree.Identifier): boolean { return (node.parent as any)?.callee?.name === 'ViewChild'; } diff --git a/lint/src/util/fix.ts b/lint/src/util/fix.ts new file mode 100644 index 0000000000..10408cc316 --- /dev/null +++ b/lint/src/util/fix.ts @@ -0,0 +1,125 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +import { TSESTree } from '@typescript-eslint/utils'; +import { + RuleContext, + RuleFix, + RuleFixer, +} from '@typescript-eslint/utils/ts-eslint'; + +import { getSourceCode } from './typescript'; + + + +export function appendObjectProperties(context: RuleContext, fixer: RuleFixer, objectNode: TSESTree.ObjectExpression, properties: string[]): RuleFix { + // todo: may not handle empty objects too well + const lastProperty = objectNode.properties[objectNode.properties.length - 1]; + const source = getSourceCode(context); + const nextToken = source.getTokenAfter(lastProperty); + + // todo: newline & indentation are hardcoded for @Component({}) + // todo: we're assuming that we need trailing commas, what if we don't? + const newPart = '\n' + properties.map(p => ` ${p},`).join('\n'); + + if (nextToken !== null && nextToken.value === ',') { + return fixer.insertTextAfter(nextToken, newPart); + } else { + return fixer.insertTextAfter(lastProperty, ',' + newPart); + } +} + +export function appendArrayElement(context: RuleContext, fixer: RuleFixer, arrayNode: TSESTree.ArrayExpression, value: string): RuleFix { + const source = getSourceCode(context); + + if (arrayNode.elements.length === 0) { + // This is the first element + const openArray = source.getTokenByRangeStart(arrayNode.range[0]); + + if (openArray == null) { + throw new Error('Unexpected null token for opening square bracket'); + } + + // safe to assume the list is single-line + return fixer.insertTextAfter(openArray, `${value}`); + } else { + const lastElement = arrayNode.elements[arrayNode.elements.length - 1]; + + if (lastElement == null) { + throw new Error('Unexpected null node in array'); + } + + const nextToken = source.getTokenAfter(lastElement); + + // todo: we don't know if the list is chopped or not, so we can't make any assumptions -- may produce output that will be flagged by other rules on the next run! + // todo: we're assuming that we need trailing commas, what if we don't? + if (nextToken !== null && nextToken.value === ',') { + return fixer.insertTextAfter(nextToken, ` ${value},`); + } else { + return fixer.insertTextAfter(lastElement, `, ${value},`); + } + } + +} + +export function isLast(elementNode: TSESTree.Node): boolean { + if (!elementNode.parent) { + return false; + } + + let siblingNodes: (TSESTree.Node | null)[] = [null]; + if (elementNode.parent.type === TSESTree.AST_NODE_TYPES.ArrayExpression) { + siblingNodes = elementNode.parent.elements; + } else if (elementNode.parent.type === TSESTree.AST_NODE_TYPES.ImportDeclaration) { + siblingNodes = elementNode.parent.specifiers; + } + + return elementNode === siblingNodes[siblingNodes.length - 1]; +} + +export function removeWithCommas(context: RuleContext, fixer: RuleFixer, elementNode: TSESTree.Node): RuleFix[] { + const ops = []; + + const source = getSourceCode(context); + let nextToken = source.getTokenAfter(elementNode); + let prevToken = source.getTokenBefore(elementNode); + + if (nextToken !== null && prevToken !== null) { + if (nextToken.value === ',') { + nextToken = source.getTokenAfter(nextToken); + if (nextToken !== null) { + ops.push(fixer.removeRange([elementNode.range[0], nextToken.range[0]])); + } + } + if (isLast(elementNode) && prevToken.value === ',') { + prevToken = source.getTokenBefore(prevToken); + if (prevToken !== null) { + ops.push(fixer.removeRange([prevToken.range[1], elementNode.range[1]])); + } + } + } else if (nextToken !== null) { + ops.push(fixer.removeRange([elementNode.range[0], nextToken.range[0]])); + } + + return ops; +} + +export function replaceOrRemoveArrayIdentifier(context: RuleContext, fixer: RuleFixer, identifierNode: TSESTree.Identifier, newValue: string): RuleFix[] { + if (identifierNode.parent.type !== TSESTree.AST_NODE_TYPES.ArrayExpression) { + throw new Error('Parent node is not an array expression!'); + } + + const array = identifierNode.parent as TSESTree.ArrayExpression; + + for (const element of array.elements) { + if (element !== null && element.type === TSESTree.AST_NODE_TYPES.Identifier && element.name === newValue) { + return removeWithCommas(context, fixer, identifierNode); + } + } + + return [fixer.replaceText(identifierNode, newValue)]; +} diff --git a/lint/src/util/theme-support.ts b/lint/src/util/theme-support.ts index 6a3807a536..2458b3f665 100644 --- a/lint/src/util/theme-support.ts +++ b/lint/src/util/theme-support.ts @@ -11,7 +11,10 @@ import { readFileSync } from 'fs'; import { basename } from 'path'; import ts, { Identifier } from 'typescript'; -import { isPartOfViewChild } from './angular'; +import { + getComponentClassName, + isPartOfViewChild, +} from './angular'; import { AnyRuleContext, getFilename, @@ -74,12 +77,14 @@ function findImportDeclaration(source: ts.SourceFile, identifierName: string): t class ThemeableComponentRegistry { public readonly entries: Set; public readonly byBaseClass: Map; + public readonly byWrapperClass: Map; public readonly byBasePath: Map; public readonly byWrapperPath: Map; constructor() { this.entries = new Set(); this.byBaseClass = new Map(); + this.byWrapperClass = new Map(); this.byBasePath = new Map(); this.byWrapperPath = new Map(); } @@ -157,6 +162,7 @@ class ThemeableComponentRegistry { private add(entry: ThemeableComponentRegistryEntry) { this.entries.add(entry); this.byBaseClass.set(entry.baseClass, entry); + this.byWrapperClass.set(entry.wrapperClass, entry); this.byBasePath.set(entry.basePath, entry); this.byWrapperPath.set(entry.wrapperPath, entry); } @@ -206,6 +212,23 @@ export function isThemedComponentWrapper(decoratorNode: TSESTree.Decorator): boo return (decoratorNode.parent.superClass as any)?.name === 'ThemedComponent'; } +export function getBaseComponentClassName(decoratorNode: TSESTree.Decorator): string | undefined { + const wrapperClass = getComponentClassName(decoratorNode); + + if (wrapperClass === undefined) { + return; + } + + themeableComponents.initialize(); + const entry = themeableComponents.byWrapperClass.get(wrapperClass); + + if (entry === undefined) { + return undefined; + } + + return entry.baseClass; +} + export function isThemeableComponent(className: string): boolean { themeableComponents.initialize(); return themeableComponents.byBaseClass.has(className); diff --git a/lint/src/util/typescript.ts b/lint/src/util/typescript.ts index 90b0f2c49f..dca83637d7 100644 --- a/lint/src/util/typescript.ts +++ b/lint/src/util/typescript.ts @@ -64,6 +64,24 @@ export function findUsages(context: AnyRuleContext, localNode: TSESTree.Identifi return usages; } +export function findUsagesByName(context: AnyRuleContext, identifier: string): TSESTree.Identifier[] { + const source = getSourceCode(context); + + const usages: TSESTree.Identifier[] = []; + + for (const token of source.ast.tokens) { + if (token.type === 'Identifier' && token.value === identifier) { + const node = source.getNodeByRangeIndex(token.range[0]); + // todo: in some cases, the resulting node can actually be the whole program (!) + if (node !== null) { + usages.push(node as TSESTree.Identifier); + } + } + } + + return usages; +} + export function isPartOfTypeExpression(node: TSESTree.Identifier): boolean { return node.parent?.type?.startsWith('TSType'); } @@ -71,3 +89,59 @@ export function isPartOfTypeExpression(node: TSESTree.Identifier): boolean { export function isPartOfClassDeclaration(node: TSESTree.Identifier): boolean { return node.parent?.type === 'ClassDeclaration'; } + +function fromSrc(path: string): string { + const m = path.match(/^.*(src\/.+)(\.(ts|json|js)?)$/); + + if (m) { + return m[1]; + } else { + throw new Error(`Can't infer project-absolute TS/resource path from: ${path}`); + } +} + + +export function relativePath(thisFile: string, importFile: string): string { + const fromParts = fromSrc(thisFile).split('/'); + const toParts = fromSrc(importFile).split('/'); + + let lastCommon = 0; + for (let i = 0; i < fromParts.length - 1; i++) { + if (fromParts[i] === toParts[i]) { + lastCommon++; + } else { + break; + } + } + + const path = toParts.slice(lastCommon, toParts.length).join('/'); + const backtrack = fromParts.length - lastCommon - 1; + + let prefix: string; + if (backtrack > 0) { + prefix = '../'.repeat(backtrack); + } else { + prefix = './'; + } + + return prefix + path; +} + + +export function findImportSpecifier(context: AnyRuleContext, identifier: string): TSESTree.ImportSpecifier | undefined { + const source = getSourceCode(context); + + const usages: TSESTree.Identifier[] = []; + + for (const token of source.ast.tokens) { + if (token.type === 'Identifier' && token.value === identifier) { + const node = source.getNodeByRangeIndex(token.range[0]); + // todo: in some cases, the resulting node can actually be the whole program (!) + if (node && node.parent && node.parent.type === TSESTree.AST_NODE_TYPES.ImportSpecifier) { + return node.parent; + } + } + } + + return undefined; +} diff --git a/lint/test/fixture/src/app/test/test-themeable.component.ts b/lint/test/fixture/src/app/test/test-themeable.component.ts index bd731d8afa..b445040539 100644 --- a/lint/test/fixture/src/app/test/test-themeable.component.ts +++ b/lint/test/fixture/src/app/test/test-themeable.component.ts @@ -10,6 +10,7 @@ import { Component } from '@angular/core'; @Component({ selector: 'ds-base-test-themeable', template: '', + standalone: true, }) export class TestThemeableComponent { } diff --git a/lint/test/fixture/src/app/test/themed-test-themeable.component.ts b/lint/test/fixture/src/app/test/themed-test-themeable.component.ts index a45f89b606..2697a8c598 100644 --- a/lint/test/fixture/src/app/test/themed-test-themeable.component.ts +++ b/lint/test/fixture/src/app/test/themed-test-themeable.component.ts @@ -13,6 +13,8 @@ import { TestThemeableComponent } from './test-themeable.component'; @Component({ selector: 'ds-test-themeable', template: '', + standalone: true, + imports: [TestThemeableComponent], }) export class ThemedTestThemeableComponent extends ThemedComponent { protected getComponentName(): string { diff --git a/lint/test/fixture/src/themes/test/app/test/other-themeable.component.ts b/lint/test/fixture/src/themes/test/app/test/other-themeable.component.ts new file mode 100644 index 0000000000..f72161b2bf --- /dev/null +++ b/lint/test/fixture/src/themes/test/app/test/other-themeable.component.ts @@ -0,0 +1,16 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +import { Component } from '@angular/core'; + +@Component({ + selector: 'ds-themed-test-themeable', + template: '', +}) +export class OtherThemeableComponent { + +} diff --git a/lint/test/fixture/src/themes/test/test.module.ts b/lint/test/fixture/src/themes/test/test.module.ts index 7aac91b07a..ff6ec3b2c0 100644 --- a/lint/test/fixture/src/themes/test/test.module.ts +++ b/lint/test/fixture/src/themes/test/test.module.ts @@ -8,11 +8,13 @@ // @ts-ignore import { NgModule } from '@angular/core'; +import { OtherThemeableComponent } from './app/test/other-themeable.component'; import { TestThemeableComponent } from './app/test/test-themeable.component'; @NgModule({ declarations: [ TestThemeableComponent, + OtherThemeableComponent, ], }) export class TestModule { diff --git a/lint/test/fixture/tsconfig.json b/lint/test/fixture/tsconfig.json index 1fd3745ec8..0b61883e35 100644 --- a/lint/test/fixture/tsconfig.json +++ b/lint/test/fixture/tsconfig.json @@ -1,6 +1,7 @@ { "extends": "../../../tsconfig.json", "include": [ + "src/**/*.ts", "src/**/*.ts" ], "exclude": ["dist"] diff --git a/lint/test/testing.ts b/lint/test/testing.ts index f86870ec29..cfa54c5b85 100644 --- a/lint/test/testing.ts +++ b/lint/test/testing.ts @@ -20,6 +20,7 @@ import { themeableComponents.initialize(FIXTURE); TypeScriptRuleTester.itOnly = fit; +TypeScriptRuleTester.itSkip = xit; export const tsRuleTester = new TypeScriptRuleTester({ parser: '@typescript-eslint/parser',