Skip to content

Commit 1eefe5f

Browse files
crisbetojelbourn
authored andcommitted
build: add tooling to avoid jit issues with empty NgModule definitions (#13843)
* Expands the decorator validation rule to allow us to run a regex against all the arguments of a decorator. This allows us to guard against cases like #13792. * Fixes a similar issue as #13792 in the `LayoutModule` which was caught after the new changes to the rule.
1 parent 6c94c06 commit 1eefe5f

File tree

3 files changed

+52
-28
lines changed

3 files changed

+52
-28
lines changed

src/cdk/layout/layout-module.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@
88
import {NgModule} from '@angular/core';
99

1010

11-
@NgModule()
11+
@NgModule({})
1212
export class LayoutModule {}

tools/tslint-rules/validateDecoratorsRule.ts

Lines changed: 49 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,16 @@ import * as minimatch from 'minimatch';
55

66
/**
77
* Rule that enforces certain decorator properties to be defined and to match a pattern.
8-
* Properties can be forbidden by prefixing their name with a `!`.
9-
* Supports whitelisting files via the third argument. E.g.
8+
* Properties can be forbidden by prefixing their name with a `!`. Supports whitelisting
9+
* files via the third argument, as well as validating all the arguments by passing in a regex. E.g.
1010
*
1111
* ```
1212
* "validate-decorators": [true, {
1313
* "Component": {
1414
* "encapsulation": "\\.None$",
1515
* "!styles": ".*"
16-
* }
16+
* },
17+
* "NgModule": "^(?!\\s*$).+"
1718
* }, "src/lib"]
1819
* ```
1920
*/
@@ -31,7 +32,7 @@ type DecoratorRuleSet = {
3132

3233
/** Represents a map between decorator names and rule sets. */
3334
type DecoratorRules = {
34-
[key: string]: DecoratorRuleSet
35+
[decorator: string]: DecoratorRuleSet | RegExp
3536
};
3637

3738
class Walker extends Lint.RuleWalker {
@@ -57,13 +58,7 @@ class Walker extends Lint.RuleWalker {
5758

5859
visitClassDeclaration(node: ts.ClassDeclaration) {
5960
if (this._enabled && node.decorators) {
60-
node.decorators
61-
.map(decorator => decorator.expression as ts.CallExpression)
62-
.filter(expression => {
63-
const args = expression.arguments;
64-
return args && args.length && (args[0] as ts.ObjectLiteralExpression).properties;
65-
})
66-
.forEach(expression => this._validatedDecorator(expression));
61+
node.decorators.forEach(decorator => this._validatedDecorator(decorator.expression));
6762
}
6863

6964
super.visitClassDeclaration(node);
@@ -82,8 +77,30 @@ class Walker extends Lint.RuleWalker {
8277
return;
8378
}
8479

80+
// If the rule is a regex, extract the arguments as a string and run it against them.
81+
if (rules instanceof RegExp) {
82+
const decoratorText: string = decorator.getText();
83+
const openParenthesisIndex = decoratorText.indexOf('(');
84+
const closeParenthesisIndex = decoratorText.lastIndexOf(')');
85+
const argumentsText = openParenthesisIndex > -1 ? decoratorText.substring(
86+
openParenthesisIndex + 1,
87+
closeParenthesisIndex > -1 ? closeParenthesisIndex : decoratorText.length) : '';
88+
89+
if (!rules.test(argumentsText)) {
90+
this.addFailureAtNode(decorator.parent, `Expected decorator arguments to match "${rules}"`);
91+
}
92+
93+
return;
94+
}
95+
96+
const args = decorator.arguments;
97+
98+
if (!args || !args.length || !args[0].properties) {
99+
return;
100+
}
101+
85102
// Extract the property names and values.
86-
const props = decorator.arguments[0].properties
103+
const props = args[0].properties
87104
.filter((node: ts.PropertyAssignment) => node.name && node.initializer)
88105
.map((node: ts.PropertyAssignment) => ({
89106
name: node.name.getText(),
@@ -125,26 +142,32 @@ class Walker extends Lint.RuleWalker {
125142
* @param config Config object passed in via the tslint.json.
126143
* @returns Sanitized rules.
127144
*/
128-
private _generateRules(config: {[key: string]: {[key: string]: string}}): DecoratorRules {
145+
private _generateRules(config: {[key: string]: string|{[key: string]: string}}): DecoratorRules {
129146
const output: DecoratorRules = {};
130147

131148
if (config) {
132149
Object.keys(config)
133150
.filter(decoratorName => Object.keys(config[decoratorName]).length > 0)
134151
.forEach(decoratorName => {
135-
output[decoratorName] = Object.keys(config[decoratorName]).reduce((accumulator, prop) => {
136-
const isForbidden = prop.startsWith('!');
137-
const cleanName = isForbidden ? prop.slice(1) : prop;
138-
const pattern = new RegExp(config[decoratorName][prop]);
139-
140-
if (isForbidden) {
141-
accumulator.forbidden[cleanName] = pattern;
142-
} else {
143-
accumulator.required[cleanName] = pattern;
144-
}
145-
146-
return accumulator;
147-
}, {required: {}, forbidden: {}} as DecoratorRuleSet);
152+
const decoratorConfig = config[decoratorName];
153+
154+
if (typeof decoratorConfig === 'string') {
155+
output[decoratorName] = new RegExp(decoratorConfig);
156+
} else {
157+
output[decoratorName] = Object.keys(decoratorConfig).reduce((rules, prop) => {
158+
const isForbidden = prop.startsWith('!');
159+
const cleanName = isForbidden ? prop.slice(1) : prop;
160+
const pattern = new RegExp(decoratorConfig[prop]);
161+
162+
if (isForbidden) {
163+
rules.forbidden[cleanName] = pattern;
164+
} else {
165+
rules.required[cleanName] = pattern;
166+
}
167+
168+
return rules;
169+
}, {required: {}, forbidden: {}} as DecoratorRuleSet);
170+
}
148171
});
149172
}
150173

tslint.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@
118118
},
119119
"Directive": {
120120
"!host": "\\[class\\]"
121-
}
121+
},
122+
"NgModule": "^(?!\\s*$).+"
122123
}, "src/+(lib|cdk|material-experimental|cdk-experimental)/**/!(*.spec).ts"],
123124
"require-license-banner": [
124125
true,

0 commit comments

Comments
 (0)