From fb15e89426b95a5559a08affeb722bec1c320953 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 10 May 2019 23:29:12 +0200 Subject: [PATCH] build: add rule to warn for undecorated base classes (#15976) With Ivy, directives or components no longer properly inherit a constructor from a parent base class which is undecorated. See more details here: FW-1238 In order to avoid dependency injection issues with Ivy, a new custom TSLint rule reports a failure when a directive/component uses dependency injection in a way that breaks with Ivy. Related to #15975 --- .../noUndecoratedBaseClassDiRule.ts | 96 +++++++++++++++++++ tslint.json | 1 + 2 files changed, 97 insertions(+) create mode 100644 tools/tslint-rules/noUndecoratedBaseClassDiRule.ts diff --git a/tools/tslint-rules/noUndecoratedBaseClassDiRule.ts b/tools/tslint-rules/noUndecoratedBaseClassDiRule.ts new file mode 100644 index 00000000000..1fc7bfd23ae --- /dev/null +++ b/tools/tslint-rules/noUndecoratedBaseClassDiRule.ts @@ -0,0 +1,96 @@ +import * as Lint from 'tslint'; +import * as ts from 'typescript'; + +const RULE_FAILURE = `Class inherits constructor using dependency injection from ` + + `undecorated base class. This breaks dependency injection with Ivy and can be fixed ` + + `by creating an explicit pass-through constructor.`; + +/** + * Rule that doesn't allow inheriting a constructor using dependency injection from an + * undecorated base class. With Ivy, undecorated base classes cannot use dependency + * injection. Classes that inherit the constructor from the base class can specify + * an explicit pass-through constructor to make DI work. + */ +export class Rule extends Lint.Rules.TypedRule { + applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + return this.applyWithWalker( + new Walker(sourceFile, this.getOptions(), program.getTypeChecker())); + } +} + +class Walker extends Lint.RuleWalker { + constructor( + sourceFile: ts.SourceFile, options: Lint.IOptions, private typeChecker: ts.TypeChecker) { + super(sourceFile, options); + } + + visitClassDeclaration(node: ts.ClassDeclaration) { + if (!this.hasDirectiveDecorator(node)) { + return; + } + + // If the class already has an explicit constructor, it's not required + // for base classes to be decorated. + if (this.hasExplicitConstructor(node)) { + return; + } + + const baseClass = this.getConstructorBaseClass(node); + if (baseClass && !this.hasDirectiveDecorator(baseClass)) { + this.addFailureAtNode(node, RULE_FAILURE); + } + } + + /** Checks whether the given class declaration has an explicit constructor. */ + hasExplicitConstructor(node: ts.ClassDeclaration): boolean { + return node.members.some(ts.isConstructorDeclaration); + } + + /** Checks if the specified node has a "@Directive" or "@Component" decorator. */ + hasDirectiveDecorator(node: ts.ClassDeclaration): boolean { + return !!node.decorators && node.decorators.some(d => { + if (!ts.isCallExpression(d.expression)) { + return false; + } + + const decoratorText = d.expression.expression.getText(); + return decoratorText === 'Directive' || decoratorText === 'Component'; + }); + } + + /** + * Gets the first inherited class of the specified class that has an + * explicit constructor. + */ + getConstructorBaseClass(node: ts.ClassDeclaration): ts.ClassDeclaration|null { + let currentClass = node; + while (currentClass) { + const baseTypes = this.getBaseTypeIdentifiers(currentClass); + if (!baseTypes || baseTypes.length !== 1) { + return null; + } + const symbol = this.typeChecker.getTypeAtLocation(baseTypes[0]).getSymbol(); + if (!symbol || !ts.isClassDeclaration(symbol.valueDeclaration)) { + return null; + } + if (this.hasExplicitConstructor(symbol.valueDeclaration)) { + return symbol.valueDeclaration; + } + currentClass = symbol.valueDeclaration; + } + return null; + } + + /** Determines the base type identifiers of a specified class declaration. */ + getBaseTypeIdentifiers(node: ts.ClassDeclaration): ts.Identifier[]|null { + if (!node.heritageClauses) { + return null; + } + + return node.heritageClauses.filter(clause => clause.token === ts.SyntaxKind.ExtendsKeyword) + .reduce( + (types, clause) => types.concat(clause.types), [] as ts.ExpressionWithTypeArguments[]) + .map(typeExpression => typeExpression.expression) + .filter(ts.isIdentifier); + } +} diff --git a/tslint.json b/tslint.json index 20343782cf7..05761ee1fc1 100644 --- a/tslint.json +++ b/tslint.json @@ -98,6 +98,7 @@ "no-exposed-todo": true, "no-import-spacing": true, "no-private-getters": true, + "no-undecorated-base-class-di": true, "setters-after-getters": true, "ng-on-changes-property-access": true, "rxjs-imports": true,