diff --git a/tools/tslint/noImplicitOverrideAbstractRule.ts b/tools/tslint/noImplicitOverrideAbstractRule.ts new file mode 100644 index 00000000000..96a2421886b --- /dev/null +++ b/tools/tslint/noImplicitOverrideAbstractRule.ts @@ -0,0 +1,166 @@ +/** + * @license + * Copyright Google LLC + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {Replacement, RuleFailure, WalkContext} from 'tslint/lib'; +import {TypedRule} from 'tslint/lib/rules'; +import ts from 'typescript'; + +const FAILURE_MESSAGE = + 'Missing override modifier. Members implemented as part of ' + + 'abstract classes must explicitly set the "override" modifier. ' + + 'More details: https://github.com/microsoft/TypeScript/issues/44457#issuecomment-856202843.'; + +/** + * Rule which enforces that class members implementing abstract members + * from base classes explicitly specify the `override` modifier. + * + * This ensures we follow the best-practice of applying `override` for abstract-implemented + * members so that TypeScript creates diagnostics in both scenarios where either the abstract + * class member is removed, or renamed. + * + * More details can be found here: https://github.com/microsoft/TypeScript/issues/44457. + */ +export class Rule extends TypedRule { + override applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] { + return this.applyWithFunction(sourceFile, (ctx) => visitNode(sourceFile, ctx, program)); + } +} + +/** + * For a TypeScript AST node and each of its child nodes, check whether the node is a class + * element which implements an abstract member but does not have the `override` keyword. + */ +function visitNode(node: ts.Node, ctx: WalkContext, program: ts.Program) { + // If a class element implements an abstract member but does not have the + // `override` keyword, create a lint failure. + if ( + ts.isClassElement(node) && + !hasOverrideModifier(node) && + matchesParentAbstractElement(node, program) + ) { + ctx.addFailureAtNode( + node, + FAILURE_MESSAGE, + Replacement.appendText(node.getStart(), `override `), + ); + } + + ts.forEachChild(node, (n) => visitNode(n, ctx, program)); +} + +/** + * Checks if the specified class element matches a parent abstract class element. i.e. + * whether the specified member "implements" an abstract member from a base class. + */ +function matchesParentAbstractElement(node: ts.ClassElement, program: ts.Program): boolean { + const containingClass = node.parent as ts.ClassDeclaration; + + // If the property we check does not have a property name, we cannot look for similarly-named + // members in parent classes and therefore return early. + if (node.name === undefined) { + return false; + } + + const propertyName = getPropertyNameText(node.name); + const typeChecker = program.getTypeChecker(); + + // If the property we check does not have a statically-analyzable property name, + // we cannot look for similarly-named members in parent classes and return early. + if (propertyName === null) { + return false; + } + + return checkClassForInheritedMatchingAbstractMember(containingClass, typeChecker, propertyName); +} + +/** Checks if the given class inherits an abstract member with the specified name. */ +function checkClassForInheritedMatchingAbstractMember( + clazz: ts.ClassDeclaration, + typeChecker: ts.TypeChecker, + searchMemberName: string, +): boolean { + const baseClass = getBaseClass(clazz, typeChecker); + + // If the class is not `abstract`, then all parent abstract methods would need to + // be implemented, and there is never an abstract member within the class. + if (baseClass === null || !hasAbstractModifier(baseClass)) { + return false; + } + + const matchingMember = baseClass.members.find( + (m) => m.name !== undefined && getPropertyNameText(m.name) === searchMemberName, + ); + + if (matchingMember !== undefined) { + return hasAbstractModifier(matchingMember); + } + + return checkClassForInheritedMatchingAbstractMember(baseClass, typeChecker, searchMemberName); +} + +/** Gets the base class for the given class declaration. */ +function getBaseClass( + node: ts.ClassDeclaration, + typeChecker: ts.TypeChecker, +): ts.ClassDeclaration | null { + const baseTypes = getExtendsHeritageExpressions(node); + + if (baseTypes.length > 1) { + throw Error('Class unexpectedly extends from multiple types.'); + } + + const baseClass = typeChecker.getTypeAtLocation(baseTypes[0]).getSymbol(); + const baseClassDecl = baseClass?.valueDeclaration ?? baseClass?.declarations?.[0]; + + if (baseClassDecl !== undefined && ts.isClassDeclaration(baseClassDecl)) { + return baseClassDecl; + } + + return null; +} + +/** Gets the `extends` base type expressions of the specified class. */ +function getExtendsHeritageExpressions( + classDecl: ts.ClassDeclaration, +): ts.ExpressionWithTypeArguments[] { + if (classDecl.heritageClauses === undefined) { + return []; + } + const result: ts.ExpressionWithTypeArguments[] = []; + for (const clause of classDecl.heritageClauses) { + if (clause.token === ts.SyntaxKind.ExtendsKeyword) { + result.push(...clause.types); + } + } + return result; +} + +/** Gets whether the specified node has the `abstract` modifier applied. */ +function hasAbstractModifier(node: ts.Node): boolean { + if (!ts.canHaveModifiers(node)) { + return false; + } + return !!ts + .getModifiers(node) + ?.some((s: ts.Modifier) => s.kind === ts.SyntaxKind.AbstractKeyword); +} + +/** Gets whether the specified node has the `override` modifier applied. */ +function hasOverrideModifier(node: ts.Node): boolean { + return !!(node as any).modifiers?.some( + (s: ts.Modifier) => s.kind === ts.SyntaxKind.OverrideKeyword, + ); +} + +/** Gets the property name text of the specified property name. */ +function getPropertyNameText(name: ts.PropertyName): string | null { + if (ts.isComputedPropertyName(name)) { + return null; + } + return name.text; +} diff --git a/tools/tslint/validateImportForEsmCjsInteropRule.ts b/tools/tslint/validateImportForEsmCjsInteropRule.ts new file mode 100644 index 00000000000..35833f03150 --- /dev/null +++ b/tools/tslint/validateImportForEsmCjsInteropRule.ts @@ -0,0 +1,101 @@ +/** + * @license + * Copyright Google LLC + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {RuleFailure, WalkContext} from 'tslint/lib'; +import {AbstractRule} from 'tslint/lib/rules'; +import ts from 'typescript'; + +const noNamedExportsError = + 'Named import is not allowed. The module does not expose named exports when ' + + 'imported in an ES module. Use a default import instead.'; + +const noDefaultExportError = + 'Default import is not allowed. The module does not expose a default export at ' + + 'runtime. Use a named import instead.'; + +interface RuleOptions { + /** + * List of modules without any named exports that NodeJS can statically detect when the + * CommonJS module is imported from ESM. Node only exposes named exports which are + * statically discoverable: https://nodejs.org/api/esm.html#esm_import_statements. + */ + noNamedExports?: string[]; + /** + * List of modules which appear to have named exports in the typings but do + * not have any at runtime due to NodeJS not being able to discover these + * through static analysis: https://nodejs.org/api/esm.html#esm_import_statements. + * */ + noDefaultExport?: string[]; + /** + * List of modules which are always incompatible. The rule allows for a custom + * message to be provided when it discovers an import to such a module. + */ + incompatibleModules?: Record; +} + +/** + * Rule that blocks named imports from being used for certain configured module + * specifiers. This is helpful for enforcing an ESM-compatible interop with CommonJS + * modules which do not expose named bindings at runtime. + * + * For example, consider the `typescript` module. It does not statically expose named + * exports even though the type definition suggests it. An import like the following + * will break at runtime when the `typescript` CommonJS module is imported inside an ESM. + * + * ``` + * import * as ts from 'typescript'; + * console.log(ts.SyntaxKind.CallExpression); // `SyntaxKind is undefined`. + * ``` + * + * More details here: https://nodejs.org/api/esm.html#esm_import_statements. + */ +export class Rule extends AbstractRule { + override apply(sourceFile: ts.SourceFile): RuleFailure[] { + const options = this.getOptions().ruleArguments[0]; + return this.applyWithFunction(sourceFile, (ctx) => visitNode(sourceFile, ctx, options)); + } +} + +function visitNode(node: ts.Node, ctx: WalkContext, options: RuleOptions) { + if (options.incompatibleModules && ts.isImportDeclaration(node)) { + const specifier = node.moduleSpecifier as ts.StringLiteral; + const failureMsg = options.incompatibleModules[specifier.text]; + + if (failureMsg !== undefined) { + ctx.addFailureAtNode(node, failureMsg); + return; + } + } + + if (options.noNamedExports && isNamedImportToDisallowedModule(node, options.noNamedExports)) { + ctx.addFailureAtNode(node, noNamedExportsError); + } + + if (options.noDefaultExport && isDefaultImportToDisallowedModule(node, options.noDefaultExport)) { + ctx.addFailureAtNode(node, noDefaultExportError); + } + + ts.forEachChild(node, (n) => visitNode(n, ctx, options)); +} + +function isNamedImportToDisallowedModule(node: ts.Node, disallowed: string[]): boolean { + if (!ts.isImportDeclaration(node) || node.importClause === undefined) { + return false; + } + const specifier = node.moduleSpecifier as ts.StringLiteral; + return !!node.importClause.namedBindings && disallowed.includes(specifier.text); +} + +function isDefaultImportToDisallowedModule(node: ts.Node, disallowed: string[]) { + if (!ts.isImportDeclaration(node) || node.importClause === undefined) { + return false; + } + const specifier = node.moduleSpecifier as ts.StringLiteral; + + return node.importClause.name !== undefined && disallowed.includes(specifier.text); +} diff --git a/tslint.json b/tslint.json index ec8c22c202e..8e296e2771e 100644 --- a/tslint.json +++ b/tslint.json @@ -1,7 +1,6 @@ { "rulesDirectory": [ "tools/tslint", - "node_modules/@angular/build-tooling/lint-rules/tslint", "node_modules/vrsource-tslint-rules/rules", "node_modules/tslint-eslint-rules/dist/rules", "node_modules/tslint-no-toplevel-property-access/rules"