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 ccbd5de
Show file tree
Hide file tree
Showing 14 changed files with 152 additions and 64 deletions.
3 changes: 2 additions & 1 deletion lint/generate-docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@
* http://www.dspace.org/license/
*/

import { rmSync } from 'node:fs';

import {
existsSync,
mkdirSync,
readFileSync,
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
41 changes: 24 additions & 17 deletions lint/src/rules/html/themed-component-usages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,15 @@
*
* http://www.dspace.org/license/
*/
import { TmplAstElement } from '@angular-eslint/bundled-angular-compiler';
import { getTemplateParserServices } from '@angular-eslint/utils';
import {
ESLint,
Rule,
} from 'eslint';

import { fixture } from '../../../test/fixture';
import { getFilename } from '../../util/misc';
import { DSpaceESLintRuleInfo } from '../../util/structure';
import {
DISALLOWED_THEME_SELECTORS,
Expand Down Expand Up @@ -38,37 +46,36 @@ The only exception to this rule are unit tests, where we may want to use the bas

export const rule = {
...info,
create(context: any) {
if (context.getFilename().includes('.spec.ts')) {
create(context: Readonly<Rule.RuleContext>) {
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,
loc: parserServices.convertNodeSourceSpanToLoc(startSourceSpan),
fix(fixer: any) {
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 +84,7 @@ export const rule = {
},
};
},
};
} as ESLint.Plugin;

export const tests = {
plugin: info.name,
Expand Down
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
43 changes: 27 additions & 16 deletions lint/src/rules/ts/themed-component-selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,18 @@
*
* 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 {
getFilename,
stringLiteral,
} from '../../util/misc';
import { DSpaceESLintRuleInfo } from '../../util/structure';
import {
inThemedComponentOverrideFile,
Expand Down Expand Up @@ -53,12 +60,12 @@ 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[]>) {
if (getFilename(context).endsWith('.spec.ts')) {
return {};
}

function enforceWrapperSelector(selectorNode: any) {
function enforceWrapperSelector(selectorNode: TSESTree.StringLiteral) {
if (selectorNode?.value.startsWith('ds-themed-')) {
context.report({
messageId: Message.WRAPPER,
Expand All @@ -70,7 +77,7 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({
}
}

function enforceBaseSelector(selectorNode: any) {
function enforceBaseSelector(selectorNode: TSESTree.StringLiteral) {
if (!selectorNode?.value.startsWith('ds-base-')) {
context.report({
messageId: Message.BASE,
Expand All @@ -82,7 +89,7 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({
}
}

function enforceThemedSelector(selectorNode: any) {
function enforceThemedSelector(selectorNode: TSESTree.StringLiteral) {
if (!selectorNode?.value.startsWith('ds-themed-')) {
context.report({
messageId: Message.THEMED,
Expand All @@ -95,11 +102,15 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({
}

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 @@ -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
32 changes: 21 additions & 11 deletions lint/src/rules/ts/themed-component-usages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,17 @@
*
* 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 {
findUsages,
getFilename,
} from '../../util/misc';
import { DSpaceESLintRuleInfo } from '../../util/structure';
import {
allThemeableComponents,
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[]>): any {
const filename = getFilename(context);

function handleUnthemedUsagesInTypescript(node: TSESTree.Identifier) {
if (isAllowedUnthemedUsage(node)) {
return;
}
Expand All @@ -74,7 +84,7 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({
});
}

function handleThemedSelectorQueriesInTests(node: any) {
function handleThemedSelectorQueriesInTests(node: TSESTree.Literal) {
context.report({
node,
messageId: Message.WRONG_SELECTOR,
Expand All @@ -85,7 +95,7 @@ export const rule = ESLintUtils.RuleCreator.withoutDocs({
});
}

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 Down Expand Up @@ -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
16 changes: 12 additions & 4 deletions lint/src/util/angular.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,20 @@
*
* 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;
}
22 changes: 18 additions & 4 deletions lint/src/util/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
*
* http://www.dspace.org/license/
*/
import {
TSESLint,
TSESTree,
} from '@typescript-eslint/utils';
import { Rule } from 'eslint';

export function stringLiteral(value: string): string {
return `'${value}'`;
Expand All @@ -29,14 +34,23 @@ export function findUsages(context: any, localNode: any): any[] {
}


export function isPartOfTypeExpression(node: any): boolean {
export function isPartOfTypeExpression(node: TSESTree.Identifier): boolean {
return node.parent.type.startsWith('TSType');
}

export function isClassDeclaration(node: any): boolean {
export function isPartOfClassDeclaration(node: TSESTree.Identifier): boolean {
if (node.parent === undefined) {
return false;
}
return node.parent.type === 'ClassDeclaration';
}

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

export function getFilename(context: TSESLint.RuleContext<any, any> | Rule.RuleContext): string {
// TSESLint claims this is deprecated, but the suggested alternative is undefined (could be a version mismatch between ESLint and TSESlint?)
// eslint-disable-next-line deprecation/deprecation
return context.getFilename();
}
5 changes: 0 additions & 5 deletions lint/src/util/structure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ export interface DSpaceESLintPluginInfo {
tests: DSpaceESLintTestInfo;
}

export interface DSpaceESLintInfo {
html: DSpaceESLintPluginInfo;
ts: DSpaceESLintPluginInfo;
}

export interface RuleExports {
Message: EnumType,
info: DSpaceESLintRuleInfo,
Expand Down
4 changes: 2 additions & 2 deletions lint/src/util/theme-support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { basename } from 'path';
import ts from 'typescript';

import {
isClassDeclaration,
isPartOfClassDeclaration,
isPartOfTypeExpression,
isPartOfViewChild,
} from './misc';
Expand Down Expand Up @@ -183,7 +183,7 @@ export function getThemeableComponentByBaseClass(baseClass: string): ThemeableCo
}

export function isAllowedUnthemedUsage(usageNode: any) {
return isClassDeclaration(usageNode) || isPartOfTypeExpression(usageNode) || isPartOfViewChild(usageNode);
return isPartOfClassDeclaration(usageNode) || isPartOfTypeExpression(usageNode) || isPartOfViewChild(usageNode);
}

export const DISALLOWED_THEME_SELECTORS = 'ds-(base|themed)-';
Expand Down
Loading

0 comments on commit ccbd5de

Please sign in to comment.