Skip to content

Commit 1e7ef91

Browse files
committed
build: add lint rule to enforce coercion static properties for setters
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 #17528.
1 parent 3fdab10 commit 1e7ef91

File tree

2 files changed

+214
-0
lines changed

2 files changed

+214
-0
lines changed
Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
import * as ts from 'typescript';
2+
import * as Lint from 'tslint';
3+
import * as tsutils from 'tsutils';
4+
5+
/**
6+
* TSLint rule that verifies that classes declare corresponding `ngAcceptInputType_*`
7+
* static fields for inputs that use coercion inside of their setters. Also handles
8+
* inherited class members and members that come from an interface.
9+
*/
10+
export class Rule extends Lint.Rules.TypedRule {
11+
applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
12+
const walker = new Walker(sourceFile, this.getOptions(), program.getTypeChecker());
13+
return this.applyWithWalker(walker);
14+
}
15+
}
16+
17+
class Walker extends Lint.RuleWalker {
18+
/** Names of the coercion functions that we should be looking for. */
19+
private _coercionFunctions: Set<string>;
20+
21+
/** Mapping of interfaces known to have coercion properties and the property names themselves. */
22+
private _coercionInterfaces: {[interfaceName: string]: string[]};
23+
24+
constructor(sourceFile: ts.SourceFile,
25+
options: Lint.IOptions,
26+
private _typeChecker: ts.TypeChecker) {
27+
super(sourceFile, options);
28+
this._coercionFunctions = new Set(options.ruleArguments[0] || []);
29+
this._coercionInterfaces = options.ruleArguments[1] || {};
30+
}
31+
32+
visitClassDeclaration(node: ts.ClassDeclaration) {
33+
if (this._shouldLintClass(node)) {
34+
this._lintClass(node, node);
35+
this._lintInheritedProperties(node);
36+
this._lintInterfaces(node);
37+
}
38+
super.visitClassDeclaration(node);
39+
}
40+
41+
/**
42+
* Goes thrown the own setters of a class declaration and checks whether they use coercion.
43+
* @param node Class declaration to be checked.
44+
* @param sourceClass Class declaration on which to look for static properties that declare the
45+
* accepted values for the setter.
46+
*/
47+
private _lintClass(node: ts.ClassDeclaration, sourceClass: ts.ClassDeclaration): void {
48+
node.members.forEach(member => {
49+
if (ts.isSetAccessor(member) && usesCoercion(member, this._coercionFunctions) &&
50+
this._shouldCheckSetter(member)) {
51+
this._checkForStaticMember(sourceClass, member.name.getText());
52+
}
53+
});
54+
}
55+
56+
/**
57+
* Goes up the inheritance chain of a class declaration and
58+
* checks whether it has any setters using coercion.
59+
* @param node Class declaration to be checked.
60+
*/
61+
private _lintInheritedProperties(node: ts.ClassDeclaration): void {
62+
let currentClass: ts.ClassDeclaration|null = node;
63+
64+
while (currentClass) {
65+
const baseType = getBaseTypeIdentifier(currentClass);
66+
67+
if (!baseType) {
68+
break;
69+
}
70+
71+
const symbol = this._typeChecker.getTypeAtLocation(baseType).getSymbol();
72+
currentClass = symbol && ts.isClassDeclaration(symbol.valueDeclaration) ?
73+
symbol.valueDeclaration : null;
74+
75+
if (currentClass) {
76+
this._lintClass(currentClass, node);
77+
}
78+
}
79+
}
80+
81+
/**
82+
* Checks whether the interfaces that a class implements contain any known coerced properties.
83+
* @param node Class declaration to be checked.
84+
*/
85+
private _lintInterfaces(node: ts.ClassDeclaration): void {
86+
if (!node.heritageClauses) {
87+
return;
88+
}
89+
90+
node.heritageClauses.forEach(clause => {
91+
if (clause.token === ts.SyntaxKind.ImplementsKeyword) {
92+
clause.types.forEach(clauseType => {
93+
if (ts.isIdentifier(clauseType.expression)) {
94+
const propNames = this._coercionInterfaces[clauseType.expression.text];
95+
96+
if (propNames) {
97+
propNames.forEach(propName => this._checkForStaticMember(node, propName));
98+
}
99+
}
100+
});
101+
}
102+
});
103+
}
104+
105+
/**
106+
* Checks whether a class declaration has a static member, corresponding
107+
* to the specified setter name, and logs a failure if it doesn't.
108+
* @param node
109+
* @param setterName
110+
*/
111+
private _checkForStaticMember(node: ts.ClassDeclaration, setterName: string) {
112+
const coercionPropertyName = `ngAcceptInputType_${setterName}`;
113+
const correspondingCoercionProperty = node.members.find(member => {
114+
return ts.isPropertyDeclaration(member) &&
115+
tsutils.hasModifier(member.modifiers, ts.SyntaxKind.StaticKeyword) &&
116+
member.name.getText() === coercionPropertyName;
117+
});
118+
119+
if (!correspondingCoercionProperty) {
120+
this.addFailureAtNode(node.name || node, `Class must declare static coercion ` +
121+
`property called ${coercionPropertyName}.`);
122+
}
123+
}
124+
125+
/** Checks whether this rule should lint a class declaration. */
126+
private _shouldLintClass(node: ts.ClassDeclaration): boolean {
127+
// We don't need to lint undecorated classes.
128+
if (!node.decorators) {
129+
return false;
130+
}
131+
132+
// If the class is a component we should lint.
133+
if (node.decorators.some(decorator => isDecoratorCalled(decorator, 'Component'))) {
134+
return true;
135+
}
136+
137+
const directiveDecorator =
138+
node.decorators.find(decorator => isDecoratorCalled(decorator, 'Directive'));
139+
140+
// We should lint non-abstract directives.
141+
if (directiveDecorator) {
142+
const args = (directiveDecorator.expression as ts.CallExpression).arguments;
143+
const directiveConfig = args.length && ts.isObjectLiteralExpression(args[0]) ?
144+
args[0] as ts.ObjectLiteralExpression : null;
145+
const selector = directiveConfig ? directiveConfig.properties.find(prop => {
146+
return prop.name && ts.isIdentifier(prop.name) && prop.name.text === 'selector';
147+
}) : null;
148+
149+
return !!selector && ts.isPropertyAssignment(selector) && ts.isIdentifier(selector.name) &&
150+
!selector.name.text.startsWith('do-not-use-abstract-');
151+
}
152+
153+
return false;
154+
}
155+
156+
/** Determines whether a setter node should be checked by the lint rule. */
157+
private _shouldCheckSetter(node: ts.SetAccessorDeclaration): boolean {
158+
const param = node.parameters[0];
159+
const types = this._typeChecker.typeToString(this._typeChecker.getTypeAtLocation(param))
160+
.split('|').map(name => name.trim());
161+
// We shouldn't check setters which accept `any` or a `string`.
162+
return types.every(typeName => typeName !== 'any' && typeName !== 'string');
163+
}
164+
}
165+
166+
/**
167+
* Checks whether a setter uses coercion.
168+
* @param setter Setter node that should be checked.
169+
* @param coercionFunctions Names of the coercion functions that we should be looking for.
170+
*/
171+
function usesCoercion(setter: ts.SetAccessorDeclaration, coercionFunctions: Set<string>): boolean {
172+
let coercionWasUsed = false;
173+
174+
setter.forEachChild(function walk(node: ts.Node) {
175+
if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) &&
176+
coercionFunctions.has(node.expression.text)) {
177+
coercionWasUsed = true;
178+
}
179+
180+
if (!coercionWasUsed) {
181+
node.forEachChild(walk);
182+
}
183+
});
184+
185+
return coercionWasUsed;
186+
}
187+
188+
/** Gets the identifier node of the base type that a class is extending. */
189+
function getBaseTypeIdentifier(node: ts.ClassDeclaration): ts.Identifier|null {
190+
if (node.heritageClauses) {
191+
for (let clause of node.heritageClauses) {
192+
if (clause.token === ts.SyntaxKind.ExtendsKeyword && clause.types.length &&
193+
ts.isIdentifier(clause.types[0].expression)) {
194+
return clause.types[0].expression;
195+
}
196+
}
197+
}
198+
199+
return null;
200+
}
201+
202+
/** Checks whether a node is a decorator with a particular name. */
203+
function isDecoratorCalled(node: ts.Decorator, name: string): boolean {
204+
return ts.isCallExpression(node.expression) &&
205+
ts.isIdentifier(node.expression.expression) &&
206+
node.expression.expression.text === name;
207+
}

tslint.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,13 @@
109109
"rxjs-imports": true,
110110
"require-breaking-change-version": true,
111111
"class-list-signatures": true,
112+
"coercion-types": [false, // Disabled until #17528 gets in.
113+
["coerceBooleanProperty", "coerceCssPixelValue", "coerceNumberProperty"],
114+
{
115+
"CanDisable": ["disabled"],
116+
"CanDisableRipple": ["disableRipple"]
117+
}
118+
],
112119
"no-host-decorator-in-concrete": [
113120
true,
114121
"HostBinding",

0 commit comments

Comments
 (0)