diff --git a/.eslintrc.js b/.eslintrc.js index 8982a6e71d46..5c298459ce2c 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -188,5 +188,11 @@ module.exports = { "@zwave-js/consistent-cc-classes": "error", }, }, + { + files: ["packages/**/*.ts"], + rules: { + "@zwave-js/no-forbidden-imports": "error", + }, + }, ], }; diff --git a/packages/eslint-plugin/src/index.ts b/packages/eslint-plugin/src/index.ts index be9e383650ed..29cb3a8d0e8c 100644 --- a/packages/eslint-plugin/src/index.ts +++ b/packages/eslint-plugin/src/index.ts @@ -1,11 +1,13 @@ import { ccAPIValidateArgs } from "./rules/ccapi-validate-args.js"; import { consistentCCClasses } from "./rules/consistent-cc-classes.js"; import { noDebugInTests } from "./rules/no-debug-in-tests.js"; +import { noForbiddenImports } from "./rules/no-forbidden-imports.js"; module.exports = { rules: { "no-debug-in-tests": noDebugInTests, "ccapi-validate-args": ccAPIValidateArgs, "consistent-cc-classes": consistentCCClasses, + "no-forbidden-imports": noForbiddenImports, }, }; diff --git a/packages/maintenance/src/lintForbiddenImports.ts b/packages/eslint-plugin/src/rules/no-forbidden-imports.ts similarity index 50% rename from packages/maintenance/src/lintForbiddenImports.ts rename to packages/eslint-plugin/src/rules/no-forbidden-imports.ts index 3393e92ff711..cd6432dc30ff 100644 --- a/packages/maintenance/src/lintForbiddenImports.ts +++ b/packages/eslint-plugin/src/rules/no-forbidden-imports.ts @@ -1,14 +1,7 @@ -/*! - * This scripts ensures that files annotated with @forbiddenImports don't import - * anything they are not supposed to. - */ - -import { bold, red } from "ansi-colors"; +import { ESLintUtils, type TSESTree } from "@typescript-eslint/utils"; import fs from "node:fs"; import path from "node:path"; import ts from "typescript"; -import { reportProblem } from "./reportProblem"; -import { loadTSConfig, projectRoot } from "./tsAPITools"; // Whitelist some imports that are known not to import forbidden modules const whitelistedImports = [ @@ -107,14 +100,43 @@ function getImports( return output; } -interface LinterContext { +function resolveImport( + node: ts.Node, + checker: ts.TypeChecker, +): ResolvedImport | undefined { + const sourceFile = node.getSourceFile(); + + const moduleNameExpr = getExternalModuleName(node); + // if they have a name, that is a string, i.e. not alias definition `import x = y` + if ( + moduleNameExpr + && moduleNameExpr.kind === ts.SyntaxKind.StringLiteral + ) { + // Ask the checker about the "symbol: for this module name + // it would be undefined if the module was not found (i.e. error) + const moduleSymbol = checker.getSymbolAtLocation(moduleNameExpr); + const file = moduleSymbol?.getDeclarations()?.[0]?.getSourceFile(); + if (file) { + return { + name: moduleNameExpr.getText(sourceFile), + line: ts.getLineAndCharacterOfPosition( + sourceFile, + moduleNameExpr.getStart(), + ).line + 1, + sourceFile: file, + }; + } + } +} + +interface ResolverContext { program: ts.Program; resolvedSourceFiles: Map; } /** Given a definition file, this tries to resolve the original source file */ function resolveSourceFileFromDefinition( - context: LinterContext, + context: ResolverContext, file: ts.SourceFile, ): ts.SourceFile { if (context.resolvedSourceFiles.has(file.fileName)) { @@ -161,44 +183,94 @@ function resolveSourceFileFromDefinition( return bail(); } -function relativeToProject(filename: string): string { - return path.relative(projectRoot, filename).replaceAll("\\", "/"); -} - -const forbiddenImportsRegex = - /^\/\* @forbiddenImports (?.*?) \*\/$/gm; +const forbiddenImportsRegex = /^@forbiddenImports (?.*?)$/; -export function lintForbiddenImports(): Promise { - // Create a Program to represent the project, then pull out the - // source file to parse its AST. - - const tsConfig = loadTSConfig(undefined, false); - const program = ts.createProgram(tsConfig.fileNames, { - ...tsConfig.options, - preserveSymlinks: false, - }); - const checker = program.getTypeChecker(); +export const noForbiddenImports = ESLintUtils.RuleCreator.withoutDocs({ + create(context) { + // And only those with at least one /* @forbiddenImports ... */ comment + const comments = context.sourceCode.getAllComments() + .filter((c) => c.type === "Block") + .map((c) => c.value.trim()); + const forbidden = comments.map((c) => c.match(forbiddenImportsRegex)) + .filter((match): match is RegExpMatchArray => !!match) + .map((match) => match.groups?.forbidden?.trim()) + .filter((forbidden): forbidden is string => !!forbidden) + .flatMap((forbidden) => forbidden.split(" ")); - const context: LinterContext = { - program, - resolvedSourceFiles: new Map(), - }; + // No comments, nothing to do + if (!forbidden.length) return {}; - let numErrors = 0; + const services = ESLintUtils.getParserServices(context); + const checker = services.program.getTypeChecker(); + const projectRoot = services.program.getCurrentDirectory(); - // Scan all source files - for (const sourceFile of program.getSourceFiles()) { - const relativePath = relativeToProject(sourceFile.fileName); + function relativeToProject(filename: string): string { + return path.relative(projectRoot, filename).replaceAll( + /[\\\/]/g, + path.sep, + ); + } - // Only look at files inside the packages directory - if (!relativePath.startsWith("packages/")) continue; + // Remember which source files we have already visited + const visitedSourceFiles = new Set(context.getFilename()); + let todo: { file: ts.SourceFile; importStack: string[] }[] = []; + const resolverContext: ResolverContext = { + program: services.program, + resolvedSourceFiles: new Map(), + }; + + function addTodo(imp: ResolvedImport, importStack: string[]) { + // try to resolve the original source file for declaration files + const next: ts.SourceFile = imp.sourceFile + .isDeclarationFile + ? resolveSourceFileFromDefinition( + resolverContext, + imp.sourceFile, + ) + : imp.sourceFile; + + if (!visitedSourceFiles.has(next.fileName)) { + todo.push({ + file: next, + importStack, + }); + } + } - // And only those with at least one @forbiddenImports comment - const forbidden = [...sourceFile.text.matchAll(forbiddenImportsRegex)] - .map((match) => match.groups?.forbidden?.trim()) - .filter((forbidden): forbidden is string => !!forbidden) - .flatMap((forbidden) => forbidden.split(" ")); - if (!forbidden.length) continue; + function reportForbiddenImport( + node: TSESTree.Node, + imp: ResolvedImport, + importStack: string[], + ) { + if (importStack.length === 0) { + context.report({ + loc: node.loc, + messageId: "forbidden-import", + data: { + name: imp.name, + }, + }); + } else { + context.report({ + loc: node.loc, + messageId: "forbidden-import-transitive", + data: { + name: imp.name, + stack: [ + ...importStack, + `❌ ${imp.name}`, + ] + .map((file, indent) => + indent === 0 + ? file + : `${" ".repeat(indent - 1)}└─ ${file}` + ) + .map((line) => `\n${line}`) + .join(""), + }, + }); + } + } function checkImport( imp: ResolvedImport, @@ -221,96 +293,73 @@ export function lintForbiddenImports(): Promise { return forbidden.includes(trimmedImport) ? "forbidden" : "ok"; } - // Resolve the import tree - const visitedSourceFiles = new Set(); - const todo: { file: ts.SourceFile; importStack: string[] }[] = [ - { file: sourceFile, importStack: [] }, - ]; - while (todo.length > 0) { - const current = todo.shift()!; - visitedSourceFiles.add(current.file.fileName); - const importStack = [ - ...current.importStack, - relativeToProject(current.file.fileName), - ]; - - const imports = getImports(current.file, checker); - for (const imp of imports) { - switch (checkImport(imp)) { - case "ignored": - continue; - + return { + "ImportDeclaration,ExportNamedDeclaration,ExportAllDeclaration"( + node: + | TSESTree.ImportDeclaration + | TSESTree.ExportAllDeclaration + | TSESTree.ExportNamedDeclaration, + ) { + const tsNode = services.esTreeNodeToTSNodeMap.get(node); + const resolvedImport = resolveImport(tsNode, checker); + if (!resolvedImport) return; + + // First test the import itself + switch (checkImport(resolvedImport)) { case "forbidden": { - numErrors++; - - const message = - `Found forbidden import of module ${imp.name}: -${ - [...importStack, `❌ ${imp.name}`] - .map((file, indent) => - indent === 0 - ? file - : `${ - " ".repeat(indent - 1) - }└─ ${file}` - ) - .join("\n") - }`; - - reportProblem({ - severity: "error", - // The line number is relative to the declaration file, so we cannot resolve it to the .ts file here - filename: path - .relative(projectRoot, current.file.fileName) - .replaceAll("\\", "/"), - line: imp.line, - message: message, - }); - break; + reportForbiddenImport(node, resolvedImport, []); + // No need to report 200 errors on a single import + return; } case "ok": { - // try to resolve the original source file for declaration files - const next: ts.SourceFile = imp.sourceFile - .isDeclarationFile - ? resolveSourceFileFromDefinition( - context, - imp.sourceFile, - ) - : imp.sourceFile; + // Import is okay, add it to the TODO list + addTodo(resolvedImport, []); + } + } - if (!visitedSourceFiles.has(next.fileName)) { - todo.push({ - file: next, - importStack, - }); + // Continue resolving the import tree until it is empty + todo: while (todo.length > 0) { + const current = todo.shift()!; + visitedSourceFiles.add(current.file.fileName); + const importStack = [ + ...current.importStack, + relativeToProject(current.file.fileName), + ]; + + const imports = getImports(current.file, checker); + for (const imp of imports) { + switch (checkImport(imp)) { + case "ignored": + continue; + + case "forbidden": { + reportForbiddenImport(node, imp, importStack); + // No need to report 200 errors on a single import + break todo; + } + case "ok": { + addTodo(imp, importStack); + } } } } - } - } - } - - if (numErrors) { - console.error(); - console.error( - red( - `Found ${bold(numErrors.toString())} error${ - numErrors !== 1 ? "s" : "" - }!`, - ), - ); - return Promise.reject( - new Error( - "The forbiddenImports rule had an error! See log output for details.", - ), - ); - } else { - return Promise.resolve(); - } -} -if (require.main === module) { - lintForbiddenImports() - .then(() => process.exit(0)) - .catch(() => process.exit(1)); -} + todo = []; + }, + }; + }, + meta: { + docs: { + description: + "Ensures that certain files or modules are not transitively imported", + }, + type: "problem", + schema: [], + messages: { + "forbidden-import-transitive": + "This import transitively imports forbidden import {{name}}:\nImport stack:{{stack}}", + "forbidden-import": "Found forbidden import of module {{name}}", + }, + }, + defaultOptions: [], +}); diff --git a/packages/maintenance/package.json b/packages/maintenance/package.json index 40d47cade63b..4e14842e6f7b 100644 --- a/packages/maintenance/package.json +++ b/packages/maintenance/package.json @@ -33,8 +33,7 @@ "build": "tsc -b tsconfig.build.json --pretty", "clean": "del-cli build/ \"*.tsbuildinfo\"", "lint:ts": "eslint --cache --ext .ts \"src/**/*.ts\"", - "lint:ts:fix": "yarn run lint:ts --fix", - "lint:zwave": "yarn ts src/lintForbiddenImports.ts" + "lint:ts:fix": "yarn run lint:ts --fix" }, "devDependencies": { "@dprint/formatter": "^0.2.0", diff --git a/packages/maintenance/src/codefind.ts b/packages/maintenance/src/codefind.ts index b58790add94a..83367d2be8bf 100644 --- a/packages/maintenance/src/codefind.ts +++ b/packages/maintenance/src/codefind.ts @@ -1,6 +1,5 @@ /*! - * This scripts ensures that files annotated with @noExternalImports don't import - * anything from outside the monorepo. + * This scripts helps find certain code patterns via the CLI */ import { diff --git a/packages/maintenance/src/lintNoExternalImports.ts b/packages/maintenance/src/lintNoExternalImports.ts deleted file mode 100644 index c44e17810582..000000000000 --- a/packages/maintenance/src/lintNoExternalImports.ts +++ /dev/null @@ -1,290 +0,0 @@ -/*! - * This scripts ensures that files annotated with @noExternalImports don't import - * anything from outside the monorepo. - */ - -import { bold, red } from "ansi-colors"; -import fs from "node:fs"; -import path from "node:path"; -import ts from "typescript"; -import { reportProblem } from "./reportProblem"; -import { loadTSConfig, projectRoot } from "./tsAPITools"; - -// Whitelist some imports that are known not to import forbidden modules -const whitelistedImports = [ - "reflect-metadata", - "alcalzone-shared/arrays", - "alcalzone-shared/async", - "alcalzone-shared/comparable", - "alcalzone-shared/deferred-promise", - "alcalzone-shared/math", - "alcalzone-shared/objects", - "alcalzone-shared/sorted-list", - "alcalzone-shared/strings", - "alcalzone-shared/typeguards", -]; - -// Whitelist some more imports that should be ignored in the checking -const ignoredImports = ["@zwave-js/transformers"]; - -function getExternalModuleName(node: ts.Node): ts.Expression | undefined { - if ( - ts.isImportEqualsDeclaration(node) - && ts.isExternalModuleReference(node.moduleReference) - ) { - return node.moduleReference.expression; - } else if (ts.isImportDeclaration(node)) { - // Only return import declarations where there is at least one non-typeonly import specifier - if (!node.importClause) { - // import "bar" - return node.moduleSpecifier; - } else if ( - !node.importClause.isTypeOnly - // import foo from "bar" - && (!node.importClause.namedBindings - // import * as foo from "bar" - || ts.isNamespaceImport(node.importClause.namedBindings) - // import {foo, type baz} from "bar" - || (ts.isNamedImports(node.importClause.namedBindings) - && node.importClause.namedBindings.elements.some( - (e) => !e.isTypeOnly, - ))) - ) { - return node.moduleSpecifier; - } - } else if (ts.isExportDeclaration(node)) { - // Only return export declarations where there is at least one non-typeonly export specifier - if ( - !node.isTypeOnly - // export * from "bar" - && (!node.exportClause - // export * as foo from "bar" - || ts.isNamespaceExport(node.exportClause) - // export {foo, type baz} from "bar" - || (ts.isNamedExports(node.exportClause) - && node.exportClause.elements.some((e) => !e.isTypeOnly))) - ) { - return node.moduleSpecifier; - } - } -} - -interface ResolvedImport { - name: string; - line: number; - sourceFile: ts.SourceFile; -} - -function getImports( - sourceFile: ts.SourceFile, - checker: ts.TypeChecker, -): ResolvedImport[] { - const output: ResolvedImport[] = []; - ts.forEachChild(sourceFile, (node) => { - // Vist top-level import nodes - const moduleNameExpr = getExternalModuleName(node); - // if they have a name, that is a string, i.e. not alias definition `import x = y` - if ( - moduleNameExpr - && moduleNameExpr.kind === ts.SyntaxKind.StringLiteral - ) { - // Ask the checker about the "symbol: for this module name - // it would be undefined if the module was not found (i.e. error) - const moduleSymbol = checker.getSymbolAtLocation(moduleNameExpr); - const file = moduleSymbol?.getDeclarations()?.[0]?.getSourceFile(); - if (file) { - output.push({ - name: moduleNameExpr.getText(sourceFile), - line: ts.getLineAndCharacterOfPosition( - sourceFile, - moduleNameExpr.getStart(), - ).line + 1, - sourceFile: file, - }); - } - } - }); - return output; -} - -interface LinterContext { - program: ts.Program; - resolvedSourceFiles: Map; -} - -/** Given a definition file, this tries to resolve the original source file */ -function resolveSourceFileFromDefinition( - context: LinterContext, - file: ts.SourceFile, -): ts.SourceFile { - if (context.resolvedSourceFiles.has(file.fileName)) { - return ( - context.program.getSourceFile( - context.resolvedSourceFiles.get(file.fileName)!, - ) ?? file - ); - } - - function bail() { - context.resolvedSourceFiles.set(file.fileName, file.fileName); - return file; - } - - const sourceMappingURL = /^\/\/# sourceMappingURL=(.*)$/gm.exec( - file.text, - )?.[1]; - if (!sourceMappingURL) return file; - - const mapPath = path.resolve(path.dirname(file.fileName), sourceMappingURL); - let map: any; - try { - map = JSON.parse(fs.readFileSync(mapPath, "utf8")); - } catch { - return bail(); - } - - let originalFileName = map.sources?.[0]; - if (typeof originalFileName !== "string") { - return bail(); - } - - originalFileName = path.resolve( - path.dirname(file.fileName), - originalFileName, - ); - const originalFile = context.program.getSourceFile(originalFileName); - if (originalFile) { - context.resolvedSourceFiles.set(file.fileName, originalFile.fileName); - return originalFile; - } - - return bail(); -} - -// function dtsToTs(filename: string): string { -// return filename.replace(/\/build\/(.*?)\.d\.ts$/, "/src/$1.ts"); -// } - -function relativeToProject(filename: string): string { - return path.relative(projectRoot, filename).replaceAll("\\", "/"); -} - -export function lintNoExternalImports(): Promise { - // Create a Program to represent the project, then pull out the - // source file to parse its AST. - - const tsConfig = loadTSConfig(undefined, false); - const program = ts.createProgram(tsConfig.fileNames, { - ...tsConfig.options, - preserveSymlinks: false, - }); - const checker = program.getTypeChecker(); - - const context: LinterContext = { - program, - resolvedSourceFiles: new Map(), - }; - - let numErrors = 0; - - // Scan all source files - for (const sourceFile of program.getSourceFiles()) { - const relativePath = relativeToProject(sourceFile.fileName); - - // Only look at files inside the packages directory - if (!relativePath.startsWith("packages/")) continue; - - // And only those with a @noExternalImports comment - if (!/^\/\* @noExternalImports \*\//m.test(sourceFile.text)) continue; - - // Resolve the import tree - const visitedSourceFiles = new Set(); - const todo: { file: ts.SourceFile; importStack: string[] }[] = [ - { file: sourceFile, importStack: [] }, - ]; - while (todo.length > 0) { - const current = todo.shift()!; - visitedSourceFiles.add(current.file.fileName); - const importStack = [ - ...current.importStack, - relativeToProject(current.file.fileName), - ]; - - const imports = getImports(current.file, checker); - for (const imp of imports) { - const trimmedImport = imp.name.replaceAll("\"", ""); - if (ignoredImports.includes(trimmedImport)) continue; - - if ( - imp.sourceFile.fileName.includes("node_modules") - && !whitelistedImports.includes(trimmedImport) - ) { - numErrors++; - - const message = - `Found forbidden import of external module ${imp.name}: -${ - [...importStack, `❌ ${imp.name}`] - .map((file, indent) => - indent === 0 - ? file - : `${ - " ".repeat(indent - 1) - }└─ ${file}` - ) - .join("\n") - }`; - - reportProblem({ - severity: "error", - // The line number is relative to the declaration file, so we cannot resolve it to the .ts file here - filename: path - .relative(projectRoot, current.file.fileName) - .replaceAll("\\", "/"), - line: imp.line, - message: message, - }); - } else { - // try to resolve the original source file for declaration files - const next: ts.SourceFile = imp.sourceFile.isDeclarationFile - ? resolveSourceFileFromDefinition( - context, - imp.sourceFile, - ) - : imp.sourceFile; - - if (!visitedSourceFiles.has(next.fileName)) { - todo.push({ - file: next, - importStack, - }); - } - } - } - } - } - - if (numErrors) { - console.error(); - console.error( - red( - `Found ${bold(numErrors.toString())} error${ - numErrors !== 1 ? "s" : "" - }!`, - ), - ); - return Promise.reject( - new Error( - "The noExternalImports rule had an error! See log output for details.", - ), - ); - } else { - return Promise.resolve(); - } -} - -if (require.main === module) { - lintNoExternalImports() - .then(() => process.exit(0)) - .catch(() => process.exit(1)); -}