Skip to content

Commit

Permalink
Make rules more type-safe
Browse files Browse the repository at this point in the history
  • Loading branch information
ybnd committed Mar 15, 2024
1 parent c88391d commit 0876691
Show file tree
Hide file tree
Showing 15 changed files with 314 additions and 158 deletions.
2 changes: 1 addition & 1 deletion lint/generate-docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import {
existsSync,
mkdirSync,
readFileSync,
rmSync,
writeFileSync,
} from 'fs';
import { rmSync } from 'node:fs';
import { join } from 'path';

import { default as htmlPlugin } from './src/rules/html';
Expand Down
2 changes: 1 addition & 1 deletion lint/src/rules/html/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
* http://www.dspace.org/license/
*/

/* eslint-disable import/no-namespace */
import {
bundle,
RuleExports,
Expand Down
52 changes: 31 additions & 21 deletions lint/src/rules/html/themed-component-usages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,23 @@
*
* http://www.dspace.org/license/
*/
import { TmplAstElement } from '@angular-eslint/bundled-angular-compiler';
import { getTemplateParserServices } from '@angular-eslint/utils';
import {
ESLintUtils,
TSESLint,
} from '@typescript-eslint/utils';

import { fixture } from '../../../test/fixture';
import { DSpaceESLintRuleInfo } from '../../util/structure';
import {
DSpaceESLintRuleInfo,
NamedTests,
} from '../../util/structure';
import {
DISALLOWED_THEME_SELECTORS,
fixSelectors,
} from '../../util/theme-support';
import { getFilename } from '../../util/typescript';

export enum Message {
WRONG_SELECTOR = 'mustUseThemedWrapperSelector',
Expand All @@ -36,39 +47,38 @@ The only exception to this rule are unit tests, where we may want to use the bas
defaultOptions: [],
} as DSpaceESLintRuleInfo;

export const rule = {
export const rule = ESLintUtils.RuleCreator.withoutDocs({
...info,
create(context: any) {
if (context.getFilename().includes('.spec.ts')) {
create(context: TSESLint.RuleContext<Message, unknown[]>) {
if (getFilename(context).includes('.spec.ts')) {
// skip inline templates in unit tests
return {};
}

const parserServices = getTemplateParserServices(context as any);

return {
[`Element$1[name = /^${DISALLOWED_THEME_SELECTORS}/]`](node: any) {
[`Element$1[name = /^${DISALLOWED_THEME_SELECTORS}/]`](node: TmplAstElement) {
const { startSourceSpan, endSourceSpan } = node;
const openStart = startSourceSpan.start.offset as number;

context.report({
messageId: Message.WRONG_SELECTOR,
node,
fix(fixer: any) {
loc: parserServices.convertNodeSourceSpanToLoc(startSourceSpan),
fix(fixer) {
const oldSelector = node.name;
const newSelector = fixSelectors(oldSelector);

const openTagRange = [
node.startSourceSpan.start.offset + 1,
node.startSourceSpan.start.offset + 1 + oldSelector.length,
];

const ops = [
fixer.replaceTextRange(openTagRange, newSelector),
fixer.replaceTextRange([openStart + 1, openStart + 1 + oldSelector.length], newSelector),
];

// make sure we don't mangle self-closing tags
if (node.startSourceSpan.end.offset !== node.endSourceSpan.end.offset) {
const closeTagRange = [
node.endSourceSpan.start.offset + 2,
node.endSourceSpan.end.offset - 1,
];
ops.push(fixer.replaceTextRange(closeTagRange, newSelector));
if (endSourceSpan !== null && startSourceSpan.end.offset !== endSourceSpan.end.offset) {
const closeStart = endSourceSpan.start.offset as number;
const closeEnd = endSourceSpan.end.offset as number;

ops.push(fixer.replaceTextRange([closeStart + 2, closeEnd - 1], newSelector));
}

return ops;
Expand All @@ -77,7 +87,7 @@ export const rule = {
},
};
},
};
});

export const tests = {
plugin: info.name,
Expand Down Expand Up @@ -167,6 +177,6 @@ class Test {
`,
},
],
};
} as NamedTests;

export default rule;
10 changes: 9 additions & 1 deletion lint/src/rules/ts/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
/**
* 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 {
bundle,
RuleExports,
} from '../../util/structure';
import * as themedComponentUsages from './themed-component-usages';
/* eslint-disable import/no-namespace */
import * as themedComponentSelectors from './themed-component-selectors';
import * as themedComponentUsages from './themed-component-usages';

const index = [
themedComponentUsages,
Expand Down
49 changes: 30 additions & 19 deletions lint/src/rules/ts/themed-component-selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
*
* http://www.dspace.org/license/
*/
import { ESLintUtils } from '@typescript-eslint/utils';
import { fixture } from '../../../test/fixture';
import {
ESLintUtils,
TSESLint,
TSESTree,
} from '@typescript-eslint/utils';

import { fixture } from '../../../test/fixture';
import { getComponentSelectorNode } from '../../util/angular';
import { stringLiteral } from '../../util/misc';
import { DSpaceESLintRuleInfo } from '../../util/structure';
Expand All @@ -16,6 +20,7 @@ import {
isThemeableComponent,
isThemedComponentWrapper,
} from '../../util/theme-support';
import { getFilename } from '../../util/typescript';

export enum Message {
BASE = 'wrongSelectorUnthemedComponent',
Expand Down Expand Up @@ -53,53 +58,59 @@ Unit tests are exempt from this rule, because they may redefine components using

export const rule = ESLintUtils.RuleCreator.withoutDocs({
...info,
create(context: any): any {
if (context.getFilename()?.endsWith('.spec.ts')) {
create(context: TSESLint.RuleContext<Message, unknown[]>) {
const filename = getFilename(context);

if (filename.endsWith('.spec.ts')) {
return {};
}

function enforceWrapperSelector(selectorNode: any) {
function enforceWrapperSelector(selectorNode: TSESTree.StringLiteral) {
if (selectorNode?.value.startsWith('ds-themed-')) {
context.report({
messageId: Message.WRAPPER,
node: selectorNode,
fix(fixer: any) {
fix(fixer) {
return fixer.replaceText(selectorNode, stringLiteral(selectorNode.value.replace('ds-themed-', 'ds-')));
},
});
}
}

function enforceBaseSelector(selectorNode: any) {
function enforceBaseSelector(selectorNode: TSESTree.StringLiteral) {
if (!selectorNode?.value.startsWith('ds-base-')) {
context.report({
messageId: Message.BASE,
node: selectorNode,
fix(fixer: any) {
fix(fixer) {
return fixer.replaceText(selectorNode, stringLiteral(selectorNode.value.replace('ds-', 'ds-base-')));
},
});
}
}

function enforceThemedSelector(selectorNode: any) {
function enforceThemedSelector(selectorNode: TSESTree.StringLiteral) {
if (!selectorNode?.value.startsWith('ds-themed-')) {
context.report({
messageId: Message.THEMED,
node: selectorNode,
fix(fixer: any) {
fix(fixer) {
return fixer.replaceText(selectorNode, stringLiteral(selectorNode.value.replace('ds-', 'ds-themed-')));
},
});
}
}

return {
'ClassDeclaration > Decorator[expression.callee.name = "Component"]'(node: any) {
// keep track of all @Component nodes by their selector
'ClassDeclaration > Decorator[expression.callee.name = "Component"]'(node: TSESTree.Decorator) {
const selectorNode = getComponentSelectorNode(node);

if (selectorNode === undefined) {
return;
}

const selector = selectorNode?.value;
const classNode = node.parent;
const classNode = node.parent as TSESTree.ClassDeclaration;
const className = classNode.id?.name;

if (selector === undefined || className === undefined) {
Expand All @@ -108,7 +119,7 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({

if (isThemedComponentWrapper(node)) {
enforceWrapperSelector(selectorNode);
} else if (inThemedComponentOverrideFile(context)) {
} else if (inThemedComponentOverrideFile(filename)) {
enforceThemedSelector(selectorNode);
} else if (isThemeableComponent(className)) {
enforceBaseSelector(selectorNode);
Expand All @@ -124,11 +135,11 @@ export const tests = {
{
name: 'Regular non-themeable component selector',
code: `
@Component({
selector: 'ds-something',
})
class Something {
}
@Component({
selector: 'ds-something',
})
class Something {
}
`,
},
{
Expand Down
38 changes: 24 additions & 14 deletions lint/src/rules/ts/themed-component-usages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
*
* http://www.dspace.org/license/
*/
import { ESLintUtils } from '@typescript-eslint/utils';
import {
ESLintUtils,
TSESLint,
TSESTree,
} from '@typescript-eslint/utils';

import { fixture } from '../../../test/fixture';
import { findUsages } from '../../util/misc';
import { DSpaceESLintRuleInfo } from '../../util/structure';
import {
allThemeableComponents,
Expand All @@ -17,6 +21,10 @@ import {
inThemedComponentFile,
isAllowedUnthemedUsage,
} from '../../util/theme-support';
import {
findUsages,
getFilename,
} from '../../util/typescript';

export enum Message {
WRONG_CLASS = 'mustUseThemedWrapperClass',
Expand Down Expand Up @@ -52,8 +60,10 @@ There are a few exceptions where the base class can still be used:

export const rule = ESLintUtils.RuleCreator.withoutDocs({
...info,
create(context: any, options: any): any {
function handleUnthemedUsagesInTypescript(node: any) {
create(context: TSESLint.RuleContext<Message, unknown[]>) {
const filename = getFilename(context);

function handleUnthemedUsagesInTypescript(node: TSESTree.Identifier) {
if (isAllowedUnthemedUsage(node)) {
return;
}
Expand All @@ -68,24 +78,24 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({
context.report({
messageId: Message.WRONG_CLASS,
node: node,
fix(fixer: any) {
fix(fixer) {
return fixer.replaceText(node, entry.wrapperClass);
},
});
}

function handleThemedSelectorQueriesInTests(node: any) {
function handleThemedSelectorQueriesInTests(node: TSESTree.Literal) {
context.report({
node,
messageId: Message.WRONG_SELECTOR,
fix(fixer: any){
fix(fixer){
const newSelector = fixSelectors(node.raw);
return fixer.replaceText(node, newSelector);
},
});
}

function handleUnthemedImportsInTypescript(specifierNode: any) {
function handleUnthemedImportsInTypescript(specifierNode: TSESTree.ImportSpecifier) {
const allUsages = findUsages(context, specifierNode.local);
const badUsages = allUsages.filter(usage => !isAllowedUnthemedUsage(usage));

Expand All @@ -94,7 +104,7 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({
}

const importedNode = specifierNode.imported;
const declarationNode = specifierNode.parent;
const declarationNode = specifierNode.parent as TSESTree.ImportDeclaration;

const entry = getThemeableComponentByBaseClass(importedNode.name);
if (entry === undefined) {
Expand All @@ -105,7 +115,7 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({
context.report({
messageId: Message.WRONG_IMPORT,
node: importedNode,
fix(fixer: any) {
fix(fixer) {
const ops = [];

const oldImportSource = declarationNode.source.value;
Expand All @@ -128,17 +138,17 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({
}

// ignore tests and non-routing modules
if (context.getFilename()?.endsWith('.spec.ts')) {
if (filename.endsWith('.spec.ts')) {
return {
[`CallExpression[callee.object.name = "By"][callee.property.name = "css"] > Literal:first-child[value = /.*${DISALLOWED_THEME_SELECTORS}.*/]`]: handleThemedSelectorQueriesInTests,
};
} else if (context.getFilename()?.endsWith('.cy.ts')) {
} else if (filename.endsWith('.cy.ts')) {
return {
[`CallExpression[callee.object.name = "cy"][callee.property.name = "get"] > Literal:first-child[value = /.*${DISALLOWED_THEME_SELECTORS}.*/]`]: handleThemedSelectorQueriesInTests,
};
} else if (
context.getFilename()?.match(/(?!routing).module.ts$/)
|| context.getFilename()?.match(/themed-.+\.component\.ts$/)
filename.match(/(?!routing).module.ts$/)
|| filename.match(/themed-.+\.component\.ts$/)
|| inThemedComponentFile(context)
) {
// do nothing
Expand Down
20 changes: 16 additions & 4 deletions lint/src/util/angular.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,24 @@
*
* http://www.dspace.org/license/
*/
import { TSESTree } from '@typescript-eslint/utils';

export function getComponentSelectorNode(componentDecoratorNode: any): any | undefined {
for (const property of componentDecoratorNode.expression.arguments[0].properties) {
if (property.key?.name === 'selector') {
return property?.value;
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');

if (property !== undefined) {
// todo: support template literals as well
if (property.type === TSESTree.AST_NODE_TYPES.Literal && typeof property.value === 'string') {
return property as TSESTree.StringLiteral;
}
}

return undefined;
}

export function isPartOfViewChild(node: TSESTree.Identifier): boolean {
return (node.parent as any)?.callee?.name === 'ViewChild';
}
Loading

0 comments on commit 0876691

Please sign in to comment.