Skip to content

Commit 7b77416

Browse files
committed
Use a memoized key lookup for rules
On my project, this makes ESLint as a whole roughly 8% faster, and it seems to result in consistently faster benchmarks. Note a behavior change - member expressions used to find the first matching rule, while they now favor the more general rule if relevant. (I believe this change is an improvement: "Promise is not supported in IE 10" is a more helpful message than "Promise.reject() is not supported in IE 10".)
1 parent fbd665f commit 7b77416

File tree

4 files changed

+100
-37
lines changed

4 files changed

+100
-37
lines changed

src/helpers.ts

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
BrowsersListOpts,
1212
} from "./types";
1313
import { TargetNameMappings } from "./constants";
14+
import { RulesLookup, lookupKey } from "./rules-lookup";
1415

1516
/*
1617
3) Figures out which browsers user is targeting
@@ -51,13 +52,13 @@ function checkNotInsideIfStatementAndReport(
5152
export function lintCallExpression(
5253
context: Context,
5354
handleFailingRule: HandleFailingRule,
54-
rules: AstMetadataApiWithTargetsResolver[],
55+
rulesByObject: RulesLookup,
5556
sourceCode: SourceCode,
5657
node: ESLintNode
5758
) {
5859
if (!node.callee) return;
5960
const calleeName = node.callee.name;
60-
const failingRule = rules.find((rule) => rule.object === calleeName);
61+
const failingRule = rulesByObject.get(calleeName);
6162
if (failingRule)
6263
checkNotInsideIfStatementAndReport(
6364
context,
@@ -71,13 +72,13 @@ export function lintCallExpression(
7172
export function lintNewExpression(
7273
context: Context,
7374
handleFailingRule: HandleFailingRule,
74-
rules: Array<AstMetadataApiWithTargetsResolver>,
75+
rulesByObject: RulesLookup,
7576
sourceCode: SourceCode,
7677
node: ESLintNode
7778
) {
7879
if (!node.callee) return;
7980
const calleeName = node.callee.name;
80-
const failingRule = rules.find((rule) => rule.object === calleeName);
81+
const failingRule = rulesByObject.get(calleeName);
8182
if (failingRule)
8283
checkNotInsideIfStatementAndReport(
8384
context,
@@ -91,14 +92,12 @@ export function lintNewExpression(
9192
export function lintExpressionStatement(
9293
context: Context,
9394
handleFailingRule: HandleFailingRule,
94-
rules: AstMetadataApiWithTargetsResolver[],
95+
rulesByObject: RulesLookup,
9596
sourceCode: SourceCode,
9697
node: ESLintNode
9798
) {
9899
if (!node?.expression?.name) return;
99-
const failingRule = rules.find(
100-
(rule) => rule.object === node?.expression?.name
101-
);
100+
const failingRule = rulesByObject.get(node?.expression?.name);
102101
if (failingRule)
103102
checkNotInsideIfStatementAndReport(
104103
context,
@@ -135,7 +134,8 @@ function protoChainFromMemberExpression(node: ESLintNode): string[] {
135134
export function lintMemberExpression(
136135
context: Context,
137136
handleFailingRule: HandleFailingRule,
138-
rules: Array<AstMetadataApiWithTargetsResolver>,
137+
rulesByProtoChainId: RulesLookup,
138+
rulesByObjectProperty: RulesLookup,
139139
sourceCode: SourceCode,
140140
node: ESLintNode
141141
) {
@@ -152,9 +152,7 @@ export function lintMemberExpression(
152152
? rawProtoChain.slice(1)
153153
: rawProtoChain;
154154
const protoChainId = protoChain.join(".");
155-
const failingRule = rules.find(
156-
(rule) => rule.protoChainId === protoChainId
157-
);
155+
const failingRule = rulesByProtoChainId.get(protoChainId);
158156
if (failingRule) {
159157
checkNotInsideIfStatementAndReport(
160158
context,
@@ -167,11 +165,9 @@ export function lintMemberExpression(
167165
} else {
168166
const objectName = node.object.name;
169167
const propertyName = node.property.name;
170-
const failingRule = rules.find(
171-
(rule) =>
172-
rule.object === objectName &&
173-
(rule.property == null || rule.property === propertyName)
174-
);
168+
const failingRule =
169+
rulesByObjectProperty.get(lookupKey(objectName, null)) ||
170+
rulesByObjectProperty.get(lookupKey(objectName, propertyName));
175171
if (failingRule)
176172
checkNotInsideIfStatementAndReport(
177173
context,

src/rules-lookup.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { AstMetadataApiWithTargetsResolver } from "./types";
2+
3+
// https://stackoverflow.com/q/49752151/25507
4+
type KeysOfType<T, TProp> = keyof T &
5+
{ [P in keyof T]: T[P] extends TProp ? P : never }[keyof T];
6+
7+
export type RulesLookup = Map<
8+
string | undefined,
9+
AstMetadataApiWithTargetsResolver
10+
>;
11+
12+
export function lookupKey(...args: Array<string | null | undefined>) {
13+
return args.map((i) => (i == null ? null : i)).join("\0");
14+
}
15+
16+
export function makeLookup(
17+
rules: AstMetadataApiWithTargetsResolver[],
18+
...keys: Array<
19+
KeysOfType<Required<AstMetadataApiWithTargetsResolver>, string>
20+
>
21+
) {
22+
const lookup = new Map<
23+
string | undefined,
24+
AstMetadataApiWithTargetsResolver
25+
>();
26+
// Iterate in inverse order to favor earlier rules in case of conflict.
27+
for (let i = rules.length - 1; i >= 0; i--) {
28+
const key =
29+
keys.length === 1
30+
? rules[i][keys[0]]
31+
: lookupKey(...keys.map((k) => rules[i][k]));
32+
lookup.set(key, rules[i]);
33+
}
34+
return lookup;
35+
}

src/rules/compat.ts

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
BrowsersListOpts,
2727
} from "../types";
2828
import { nodes } from "../providers";
29+
import { RulesLookup, makeLookup } from "../rules-lookup";
2930

3031
type ESLint = {
3132
[astNodeTypeName: string]: (node: ESLintNode) => void;
@@ -112,29 +113,63 @@ type RulesFilteredByTargets = {
112113
ExpressionStatement: AstMetadataApiWithTargetsResolver[];
113114
};
114115

116+
type RuleLookupsSet = {
117+
CallExpressionsByObject: RulesLookup;
118+
NewExpressionsByObject: RulesLookup;
119+
ExpressionStatementsByObject: RulesLookup;
120+
MemberExpressionsByProtoChainId: RulesLookup;
121+
MemberExpressionsByObjectProperty: RulesLookup;
122+
};
123+
115124
/**
116125
* A small optimization that only lints APIs that are not supported by targeted browsers.
117126
* For example, if the user is targeting chrome 50, which supports the fetch API, it is
118127
* wasteful to lint calls to fetch.
119128
*/
120129
const getRulesForTargets = memoize(
121-
(targetsJSON: string, lintAllEsApis: boolean): RulesFilteredByTargets => {
122-
const result = {
123-
CallExpression: [] as AstMetadataApiWithTargetsResolver[],
124-
NewExpression: [] as AstMetadataApiWithTargetsResolver[],
125-
MemberExpression: [] as AstMetadataApiWithTargetsResolver[],
126-
ExpressionStatement: [] as AstMetadataApiWithTargetsResolver[],
130+
(targetsJSON: string, lintAllEsApis: boolean): RuleLookupsSet => {
131+
const rules: RulesFilteredByTargets = {
132+
CallExpression: [],
133+
NewExpression: [],
134+
MemberExpression: [],
135+
ExpressionStatement: [],
127136
};
128137
const targets = JSON.parse(targetsJSON);
129138

130139
nodes
131140
.filter((node) => (lintAllEsApis ? true : node.kind !== "es"))
132141
.forEach((node) => {
133142
if (!node.getUnsupportedTargets(node, targets).length) return;
134-
result[node.astNodeType].push(node);
143+
rules[node.astNodeType].push(node);
135144
});
136145

137-
return result;
146+
const expressionStatementRules = [
147+
...rules.MemberExpression,
148+
...rules.CallExpression,
149+
];
150+
const memberExpressionRules = [
151+
...rules.MemberExpression,
152+
...rules.CallExpression,
153+
...rules.NewExpression,
154+
];
155+
156+
return {
157+
CallExpressionsByObject: makeLookup(rules.CallExpression, "object"),
158+
NewExpressionsByObject: makeLookup(rules.NewExpression, "object"),
159+
ExpressionStatementsByObject: makeLookup(
160+
expressionStatementRules,
161+
"object"
162+
),
163+
MemberExpressionsByProtoChainId: makeLookup(
164+
memberExpressionRules,
165+
"protoChainId"
166+
),
167+
MemberExpressionsByObjectProperty: makeLookup(
168+
memberExpressionRules,
169+
"object",
170+
"property"
171+
),
172+
};
138173
}
139174
);
140175

@@ -220,32 +255,29 @@ export default {
220255
null,
221256
context,
222257
handleFailingRule,
223-
targetedRules.CallExpression,
258+
targetedRules.CallExpressionsByObject,
224259
sourceCode
225260
),
226261
NewExpression: lintNewExpression.bind(
227262
null,
228263
context,
229264
handleFailingRule,
230-
targetedRules.NewExpression,
265+
targetedRules.NewExpressionsByObject,
231266
sourceCode
232267
),
233268
ExpressionStatement: lintExpressionStatement.bind(
234269
null,
235270
context,
236271
handleFailingRule,
237-
[...targetedRules.MemberExpression, ...targetedRules.CallExpression],
272+
targetedRules.ExpressionStatementsByObject,
238273
sourceCode
239274
),
240275
MemberExpression: lintMemberExpression.bind(
241276
null,
242277
context,
243278
handleFailingRule,
244-
[
245-
...targetedRules.MemberExpression,
246-
...targetedRules.CallExpression,
247-
...targetedRules.NewExpression,
248-
],
279+
targetedRules.MemberExpressionsByProtoChainId,
280+
targetedRules.MemberExpressionsByObjectProperty,
249281
sourceCode
250282
),
251283
// Keep track of all the defined variables. Do not report errors for nodes that are not defined

test/e2e.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ ruleTester.run("compat", rule, {
542542
settings: { browsers: ["ie 10"] },
543543
errors: [
544544
{
545-
message: "Promise.resolve() is not supported in IE 10",
545+
message: "Promise is not supported in IE 10",
546546
type: "MemberExpression",
547547
},
548548
],
@@ -552,7 +552,7 @@ ruleTester.run("compat", rule, {
552552
settings: { browsers: ["ie 10"] },
553553
errors: [
554554
{
555-
message: "Promise.all() is not supported in IE 10",
555+
message: "Promise is not supported in IE 10",
556556
type: "MemberExpression",
557557
},
558558
],
@@ -562,7 +562,7 @@ ruleTester.run("compat", rule, {
562562
settings: { browsers: ["ie 10"] },
563563
errors: [
564564
{
565-
message: "Promise.race() is not supported in IE 10",
565+
message: "Promise is not supported in IE 10",
566566
type: "MemberExpression",
567567
},
568568
],
@@ -572,7 +572,7 @@ ruleTester.run("compat", rule, {
572572
settings: { browsers: ["ie 10"] },
573573
errors: [
574574
{
575-
message: "Promise.reject() is not supported in IE 10",
575+
message: "Promise is not supported in IE 10",
576576
type: "MemberExpression",
577577
},
578578
],

0 commit comments

Comments
 (0)