Skip to content

Commit 0c66a20

Browse files
authored
build(awslint): Add RuleFilterSet class to cache include/exclude rules. (#27319)
Incremental fix to awslint performance. The `shouldEvaluate()` method is invoked for every context of every rule of every linter. The rule filter matching that was called from this method was iterating over the include/exclude list each time, and since we now have 2k+ rule filters, this was taking a long time( 55s for `aws-cdk-lib`). This PR caches those rule filters and causes most lookups to be O(1), taking the evaluation time from 55s to 18ms. One minor limitation in the cache is that you can no longer use a 'startsWith' asterisk in the code portion of the filter, though you can still use an 'anyString' wildcard for the whole. This is explained better in the the comments: ``` /** * Rule filter format is code:scope e.g. 'docs-public-apis:aws-cdk-lib.TagType'. * Wildcards can be used in the following ways: * * 1. Match any code e.g.: '*:aws-cdk-lib.TagType' * 2. Match any scope e.g.: 'docs-public-apis:*' * 3. Match any scope prefix e.g.: 'docs-public-apis:aws-cdk-lib.Tag*' */ ``` ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 93fc662 commit 0c66a20

File tree

4 files changed

+126
-38
lines changed

4 files changed

+126
-38
lines changed

packages/awslint/bin/awslint.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as chalk from 'chalk';
66
import * as fs from 'fs-extra';
77
import * as reflect from 'jsii-reflect';
88
import * as yargs from 'yargs';
9-
import { ALL_RULES_LINTER, DiagnosticLevel } from '../lib';
9+
import { ALL_RULES_LINTER, DiagnosticLevel, RuleFilterSet } from '../lib';
1010

1111
let stackTrace = false;
1212

@@ -37,7 +37,7 @@ async function main() {
3737
.example('awslint', 'lints the current module against all rules')
3838
.example('awslint -v -i "resource*" -i "import*"', 'lints against all rules that start with "resource" or "import" and print successes')
3939
.example('awslint -x "*:@aws-cdk/aws-s3*"', 'evaluate all rules in all scopes besides ones that begin with "@aws-cdk/aws-s3"')
40-
.example('awslint --save', 'updated "package.json" with "exclude"s for all failing rules');
40+
.example('awslint --save', 'updated "awslint.json" with "exclude"s for all failing rules');
4141

4242
if (!process.stdout.isTTY) {
4343
// Disable chalk color highlighting
@@ -118,11 +118,14 @@ async function main() {
118118
const excludesToSave = new Array<string>();
119119
let errors = 0;
120120

121+
const includeRules = new RuleFilterSet(args.include);
122+
const excludeRules = new RuleFilterSet(args.exclude);
123+
121124
const results = [];
122125

123126
results.push(...ALL_RULES_LINTER.eval(assembly, {
124-
include: args.include,
125-
exclude: args.exclude,
127+
includeRules: includeRules,
128+
excludeRules: excludeRules,
126129
}));
127130

128131
// Sort errors to the top (highest severity first)

packages/awslint/lib/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
export * from './linter';
22
export * from './rules';
3+
export * from './rule-specs';

packages/awslint/lib/linter.ts

Lines changed: 8 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
import * as util from 'util';
22
import { PrimitiveType } from '@jsii/spec';
33
import * as reflect from 'jsii-reflect';
4+
import { RuleFilterSet } from './rule-specs';
45

56
export interface LinterOptions {
67
/**
78
* List of rules to include.
89
* @default all rules
910
*/
10-
include?: string[];
11+
includeRules?: RuleFilterSet;
1112

1213
/**
1314
* List of rules to exclude (takes precedence on "include")
1415
* @default none
1516
*/
16-
exclude?: string[];
17+
excludeRules?: RuleFilterSet;
1718
}
1819

1920
export abstract class LinterBase {
@@ -222,47 +223,20 @@ export class Evaluation<T> {
222223
* Evaluates whether the rule should be evaluated based on the filters applied.
223224
*/
224225
private shouldEvaluate(code: string, scope: string) {
225-
if (!this.options.include || this.options.include.length === 0) {
226+
if (!this.options.includeRules || this.options.includeRules.isEmpty()) {
226227
return true;
227228
}
228229

229-
for (const include of this.options.include) {
230-
// match include
231-
if (matchRule(include)) {
232-
for (const exclude of this.options.exclude || []) {
233-
// match exclude
234-
if (matchRule(exclude)) {
235-
return false;
236-
}
237-
}
238-
return true;
230+
if (this.options.includeRules.matches(code, scope)) {
231+
if (this.options.excludeRules?.matches(code, scope)) {
232+
return false;
239233
}
234+
return true;
240235
}
241236

242237
return false;
243-
244-
function matchRule(filter: string) {
245-
if (filter.indexOf(':') === -1) {
246-
filter += ':*'; // add "*" scope filter if there isn't one
247-
}
248-
249-
// filter format is "code:scope" and both support "*" suffix to indicate startsWith
250-
const [codeFilter, scopeFilter] = filter.split(':');
251-
return matchPattern(code, codeFilter) && matchPattern(scope, scopeFilter);
252-
}
253-
254-
function matchPattern(s: string, pattern: string) {
255-
if (pattern.endsWith('*')) {
256-
const prefix = pattern.slice(0, -1);
257-
return s.startsWith(prefix);
258-
} else {
259-
return s === pattern;
260-
}
261-
}
262238
}
263-
264239
}
265-
266240
export interface Rule {
267241
code: string,
268242
message: string;

packages/awslint/lib/rule-specs.ts

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/**
2+
* Caches the list of rule filters that are specified in the include and exclude options in awslint.json.
3+
*/
4+
export class RuleFilterSet {
5+
/**
6+
* Rule filter format is code:scope e.g. 'docs-public-apis:aws-cdk-lib.TagType'.
7+
* Wildcards can be used in the following ways:
8+
*
9+
* 1. Match any code e.g.: '*:aws-cdk-lib.TagType'
10+
* 2. Match any scope e.g.: 'docs-public-apis:*'
11+
* 3. Match any scope prefix e.g.: 'docs-public-apis:aws-cdk-lib.Tag*'
12+
*/
13+
14+
private codesToScopes = new Map<string, Set<string>>(); // code -> list of scopes
15+
16+
private codesToScopePrefixes = new Map<string, Set<string>>(); // code -> list of scope prefixes (wildcards)
17+
18+
private scopes = new Set<string>(); // list of scopes with a wilcard code
19+
20+
private scopePrefixes = new Set<string>(); // list of scope prefixes with a wildcard code
21+
22+
private _isEmpty: boolean = false;
23+
24+
constructor(ruleFilterList: string[]) {
25+
if (!ruleFilterList || ruleFilterList.length == 0) {
26+
this._isEmpty = true;
27+
}
28+
29+
for (var filter of ruleFilterList) {
30+
if (filter.indexOf(':') === -1) {
31+
filter += ':*'; // add "*" scope filter if there isn't one
32+
}
33+
34+
const [code, scope] = filter.split(':');
35+
36+
if (this.hasWildcard(code)) {
37+
if (code.length > 1) {
38+
throw new Error(`Error parsing rule filter ${filter}: rule code can only be a single wildcard, or a complete string.`);
39+
} else {
40+
if (this.hasWildcard(scope)) {
41+
this.scopePrefixes.add(scope);
42+
} else {
43+
this.scopes.add(scope);
44+
}
45+
}
46+
} else {
47+
if (this.hasWildcard(scope)) {
48+
this.addToMap(this.codesToScopePrefixes, code, scope);
49+
} else {
50+
this.addToMap(this.codesToScopes, code, scope);
51+
}
52+
}
53+
}
54+
}
55+
56+
public matches(code: string, scope: string) {
57+
// Check complete filter
58+
const completeScopes = this.codesToScopes.get(code);
59+
if (completeScopes && completeScopes.has(scope)) {
60+
return true;
61+
}
62+
63+
// Check if scope matches a prefix e.g. 'docs-public-apis:aws-cdk-lib.Tag*'
64+
const scopePrefixesForCode = this.codesToScopePrefixes.get(code);
65+
if (scopePrefixesForCode) {
66+
if (this.containsPrefix(scopePrefixesForCode, scope)) {
67+
return true;
68+
}
69+
}
70+
71+
// Check if scope has a wildcard code e.g. '*:aws-cdk-lib.TagType'
72+
if (this.scopes.has(scope)) {
73+
return true;
74+
}
75+
76+
// Check if scope matches a prefix with a wildcard code e.g. '*:aws-cdk-lib.TagType*'
77+
if (this.containsPrefix(this.scopePrefixes, scope)) {
78+
return true;
79+
}
80+
81+
return false;
82+
}
83+
84+
private containsPrefix(prefixes: Set<string>, value: string) {
85+
for (const prefix of prefixes) {
86+
const prefixStr = prefix.slice(0, -1); // Strip the asterisk
87+
if (value.startsWith(prefixStr)) {
88+
return true;
89+
}
90+
}
91+
92+
return false;
93+
}
94+
95+
public isEmpty() {
96+
return this._isEmpty;
97+
}
98+
99+
private addToMap(map: Map<string, Set<string>>, code: string, scope: string) {
100+
if (!map.has(code)) {
101+
map.set(code, new Set<string>());
102+
}
103+
104+
map.get(code)?.add(scope);
105+
}
106+
107+
private hasWildcard(value: string) {
108+
return value.indexOf('*') !== -1;
109+
}
110+
}

0 commit comments

Comments
 (0)