Skip to content

Commit f3caefb

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.
1 parent f51dad7 commit f3caefb

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
@@ -10,6 +10,7 @@ import {
1010
BrowsersListOpts,
1111
} from "./types";
1212
import { TargetNameMappings } from "./constants";
13+
import { RulesLookup, lookupKey } from "./rules-lookup";
1314

1415
/*
1516
3) Figures out which browsers user is targeting
@@ -40,12 +41,12 @@ function checkNotInsideIfStatementAndReport(
4041
export function lintCallExpression(
4142
context: Context,
4243
handleFailingRule: HandleFailingRule,
43-
rules: AstMetadataApiWithTargetsResolver[],
44+
rulesByObject: RulesLookup,
4445
node: ESLintNode
4546
) {
4647
if (!node.callee) return;
4748
const calleeName = node.callee.name;
48-
const failingRule = rules.find((rule) => rule.object === calleeName);
49+
const failingRule = rulesByObject.get(calleeName);
4950
if (failingRule)
5051
checkNotInsideIfStatementAndReport(
5152
context,
@@ -58,12 +59,12 @@ export function lintCallExpression(
5859
export function lintNewExpression(
5960
context: Context,
6061
handleFailingRule: HandleFailingRule,
61-
rules: Array<AstMetadataApiWithTargetsResolver>,
62+
rulesByObject: RulesLookup,
6263
node: ESLintNode
6364
) {
6465
if (!node.callee) return;
6566
const calleeName = node.callee.name;
66-
const failingRule = rules.find((rule) => rule.object === calleeName);
67+
const failingRule = rulesByObject.get(calleeName);
6768
if (failingRule)
6869
checkNotInsideIfStatementAndReport(
6970
context,
@@ -76,13 +77,11 @@ export function lintNewExpression(
7677
export function lintExpressionStatement(
7778
context: Context,
7879
handleFailingRule: HandleFailingRule,
79-
rules: AstMetadataApiWithTargetsResolver[],
80+
rulesByObject: RulesLookup,
8081
node: ESLintNode
8182
) {
8283
if (!node?.expression?.name) return;
83-
const failingRule = rules.find(
84-
(rule) => rule.object === node?.expression?.name
85-
);
84+
const failingRule = rulesByObject.get(node?.expression?.name);
8685
if (failingRule)
8786
checkNotInsideIfStatementAndReport(
8887
context,
@@ -118,7 +117,8 @@ function protoChainFromMemberExpression(node: ESLintNode): string[] {
118117
export function lintMemberExpression(
119118
context: Context,
120119
handleFailingRule: HandleFailingRule,
121-
rules: Array<AstMetadataApiWithTargetsResolver>,
120+
rulesByProtoChainId: RulesLookup,
121+
rulesByObjectProperty: RulesLookup,
122122
node: ESLintNode
123123
) {
124124
if (!node.object || !node.property) return;
@@ -134,9 +134,7 @@ export function lintMemberExpression(
134134
? rawProtoChain.slice(1)
135135
: rawProtoChain;
136136
const protoChainId = protoChain.join(".");
137-
const failingRule = rules.find(
138-
(rule) => rule.protoChainId === protoChainId
139-
);
137+
const failingRule = rulesByProtoChainId.get(protoChainId);
140138
if (failingRule) {
141139
checkNotInsideIfStatementAndReport(
142140
context,
@@ -148,11 +146,9 @@ export function lintMemberExpression(
148146
} else {
149147
const objectName = node.object.name;
150148
const propertyName = node.property.name;
151-
const failingRule = rules.find(
152-
(rule) =>
153-
rule.object === objectName &&
154-
(rule.property == null || rule.property === propertyName)
155-
);
149+
const failingRule =
150+
rulesByObjectProperty.get(lookupKey(objectName, null)) ||
151+
rulesByObjectProperty.get(lookupKey(objectName, propertyName));
156152
if (failingRule)
157153
checkNotInsideIfStatementAndReport(
158154
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

@@ -217,29 +252,26 @@ export default {
217252
null,
218253
context,
219254
handleFailingRule,
220-
targetedRules.CallExpression
255+
targetedRules.CallExpressionsByObject
221256
),
222257
NewExpression: lintNewExpression.bind(
223258
null,
224259
context,
225260
handleFailingRule,
226-
targetedRules.NewExpression
261+
targetedRules.NewExpressionsByObject
227262
),
228263
ExpressionStatement: lintExpressionStatement.bind(
229264
null,
230265
context,
231266
handleFailingRule,
232-
[...targetedRules.MemberExpression, ...targetedRules.CallExpression]
267+
targetedRules.ExpressionStatementsByObject
233268
),
234269
MemberExpression: lintMemberExpression.bind(
235270
null,
236271
context,
237272
handleFailingRule,
238-
[
239-
...targetedRules.MemberExpression,
240-
...targetedRules.CallExpression,
241-
...targetedRules.NewExpression,
242-
]
273+
targetedRules.MemberExpressionsByProtoChainId,
274+
targetedRules.MemberExpressionsByObjectProperty
243275
),
244276
// Keep track of all the defined variables. Do not report errors for nodes that are not defined
245277
Identifier(node: ESLintNode) {

test/e2e.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ ruleTester.run("compat", rule, {
540540
settings: { browsers: ["ie 10"] },
541541
errors: [
542542
{
543-
message: "Promise.resolve() is not supported in IE 10",
543+
message: "Promise is not supported in IE 10",
544544
type: "MemberExpression",
545545
},
546546
],
@@ -550,7 +550,7 @@ ruleTester.run("compat", rule, {
550550
settings: { browsers: ["ie 10"] },
551551
errors: [
552552
{
553-
message: "Promise.all() is not supported in IE 10",
553+
message: "Promise is not supported in IE 10",
554554
type: "MemberExpression",
555555
},
556556
],
@@ -560,7 +560,7 @@ ruleTester.run("compat", rule, {
560560
settings: { browsers: ["ie 10"] },
561561
errors: [
562562
{
563-
message: "Promise.race() is not supported in IE 10",
563+
message: "Promise is not supported in IE 10",
564564
type: "MemberExpression",
565565
},
566566
],
@@ -570,7 +570,7 @@ ruleTester.run("compat", rule, {
570570
settings: { browsers: ["ie 10"] },
571571
errors: [
572572
{
573-
message: "Promise.reject() is not supported in IE 10",
573+
message: "Promise is not supported in IE 10",
574574
type: "MemberExpression",
575575
},
576576
],

0 commit comments

Comments
 (0)