Skip to content

Commit

Permalink
build: add lint rule to enforce coercion static properties for setters
Browse files Browse the repository at this point in the history
Adds a custom tslint rule to enforce that properties which use coercion in a setter also declare a static property to indicate the accepted types to ngtsc. Also handles inherited setters and properties coming from an interface being implemented (necessary to support mixins).

Relates to angular#17528.
  • Loading branch information
crisbeto committed Oct 29, 2019
1 parent 3fdab10 commit 24d0781
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 0 deletions.
157 changes: 157 additions & 0 deletions tools/tslint-rules/coercionTypesRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
import * as ts from 'typescript';
import * as Lint from 'tslint';
import * as tsutils from 'tsutils';

/**
* TSLint rule that verifies that classes declare corresponding `ngAcceptInputType_*`
* static fields for inputs that use coercion inside of their setters. Also handles
* inherited class members and members that come from an interface.
*/
export class Rule extends Lint.Rules.TypedRule {
applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
const walker = new Walker(sourceFile, this.getOptions(), program.getTypeChecker());
return this.applyWithWalker(walker);
}
}

class Walker extends Lint.RuleWalker {
/** Names of the coercion functions that we should be looking for. */
private _coercionFunctions: Set<string>;

/** Mapping of interfaces known to have coercion properties and the property names themselves. */
private _coercionInterfaces: {[interfaceName: string]: string[]};

constructor(sourceFile: ts.SourceFile,
options: Lint.IOptions,
private _typeChecker: ts.TypeChecker) {
super(sourceFile, options);
this._coercionFunctions = new Set(options.ruleArguments[0] || []);
this._coercionInterfaces = options.ruleArguments[1] || {};
}

visitClassDeclaration(node: ts.ClassDeclaration) {
this._lintClass(node, node);
this._lintInheritedProperties(node);
this._lintInterfaces(node);
super.visitClassDeclaration(node);
}

/**
* Goes thrown the own setters of a class declaration and checks whether they use coercion.
* @param node Class declaration to be checked.
* @param sourceClass Class declaration on which to look for static properties that declare the
* accepted values for the setter.
*/
private _lintClass(node: ts.ClassDeclaration, sourceClass: ts.ClassDeclaration): void {
node.members.forEach(member => {
if (ts.isSetAccessor(member) && usesCoercion(member, this._coercionFunctions)) {
this._checkForStaticMember(sourceClass, member.name.getText());
}
});
}

/**
* Goes up the inheritance chain of a class declaration and
* checks whether it has any setters using coercion.
* @param node Class declaration to be checked.
*/
private _lintInheritedProperties(node: ts.ClassDeclaration): void {
let currentClass: ts.ClassDeclaration|null = node;

while (currentClass) {
const baseType = getBaseTypeIdentifier(currentClass);

if (!baseType) {
break;
}

const symbol = this._typeChecker.getTypeAtLocation(baseType).getSymbol();
currentClass = symbol && ts.isClassDeclaration(symbol.valueDeclaration) ?
symbol.valueDeclaration : null;

if (currentClass) {
this._lintClass(currentClass, node);
}
}
}

/**
* Checks whether the interfaces that a class implements contain any known coerced properties.
* @param node Class declaration to be checked.
*/
private _lintInterfaces(node: ts.ClassDeclaration): void {
if (!node.heritageClauses) {
return;
}

node.heritageClauses.forEach(clause => {
if (clause.token === ts.SyntaxKind.ImplementsKeyword) {
clause.types.forEach(clauseType => {
if (ts.isIdentifier(clauseType.expression)) {
const propNames = this._coercionInterfaces[clauseType.expression.text];

if (propNames) {
propNames.forEach(propName => this._checkForStaticMember(node, propName));
}
}
});
}
});
}

/**
* Checks whether a class declaration has a static member, corresponding
* to the specified setter name, and logs a failure if it doesn't.
* @param node
* @param setterName
*/
private _checkForStaticMember(node: ts.ClassDeclaration, setterName: string) {
const coercionPropertyName = `ngAcceptInputType_${setterName}`;
const correspondingCoercionProperty = node.members.find(member => {
return ts.isPropertyDeclaration(member) &&
tsutils.hasModifier(member.modifiers, ts.SyntaxKind.StaticKeyword) &&
member.name.getText() === coercionPropertyName;
});

if (!correspondingCoercionProperty) {
this.addFailureAtNode(node.name || node, `Class must declare static coercion ` +
`property called ${coercionPropertyName}.`);
}
}
}

/**
* Checks whether a setter uses coercion.
* @param setter Setter node that should be checked.
* @param coercionFunctions Names of the coercion functions that we should be looking for.
*/
function usesCoercion(setter: ts.SetAccessorDeclaration, coercionFunctions: Set<string>): boolean {
let coercionWasUsed = false;

setter.forEachChild(function walk(node: ts.Node) {
if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) &&
coercionFunctions.has(node.expression.text)) {
coercionWasUsed = true;
}

if (!coercionWasUsed) {
node.forEachChild(walk);
}
});

return coercionWasUsed;
}

/** Gets the identifier node of the base type that a class is extending. */
function getBaseTypeIdentifier(node: ts.ClassDeclaration): ts.Identifier|null {
if (node.heritageClauses) {
for (let clause of node.heritageClauses) {
if (clause.token === ts.SyntaxKind.ExtendsKeyword && clause.types.length &&
ts.isIdentifier(clause.types[0].expression)) {
return clause.types[0].expression;
}
}
}

return null;
}
7 changes: 7 additions & 0 deletions tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@
"rxjs-imports": true,
"require-breaking-change-version": true,
"class-list-signatures": true,
"coercion-types": [false, // Disabled until #17528 gets in.
["coerceBooleanProperty", "coerceCssPixelValue", "coerceNumberProperty"],
{
"CanDisable": ["disabled"],
"CanDisableRipple": ["disableRipple"]
}
],
"no-host-decorator-in-concrete": [
true,
"HostBinding",
Expand Down

0 comments on commit 24d0781

Please sign in to comment.