8000 refactor: Easier RuleContext creation (#19709) · eslint/eslint@f60f276 · GitHub
[go: up one dir, main page]
Skip to content

Commit f60f276

Browse files
nzakasnzakas
authored
refactor: Easier RuleContext creation (#19709)
refs #18787
1 parent 71317eb commit f60f276

File tree

3 files changed

+238
-52
lines changed

3 files changed

+238
-52
lines changed

‎lib/linter/file-context.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,17 @@ class FileContext {
128128
getSourceCode() {
129129
return this.sourceCode;
130130
}
131+
132+
/**
133+
* Creates a new object with the current object as the prototype and
134+
* the specified properties as its own properties.
135+
* @param {Object} extension The properties to add to the new object.
136+
* @returns {FileContext} A new object with the current object as the prototype
137+
* and the specified properties as its own properties.
138+
*/
139+
extend(extension) {
140+
return Object.freeze(Object.assign(Object.create(this), extension));
141+
}
131142
}
132143

133144
exports.FileContext = FileContext;

‎lib/linter/linter.js

Lines changed: 50 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,7 +1185,7 @@ function runRules(
11851185
* All rule contexts will inherit from this object. This avoids the performance penalty of copying all the
11861186
* properties once for each rule.
11871187
*/
1188-
const sharedTraversalContext = new FileContext({
1188+
const fileContext = new FileContext({
11891189
cwd,
11901190
filename,
11911191
physicalFilename: physicalFilename || filename,
@@ -1221,63 +1221,61 @@ function runRules(
12211221

12221222
const messageIds = rule.meta && rule.meta.messages;
12231223
let reportTranslator = null;
1224-
const ruleContext = Object.freeze(
1225-
Object.assign(Object.create(sharedTraversalContext), {
1226-
id: ruleId,
1227-
options: getRuleOptions(
1228-
configuredRules[ruleId],
1229-
applyDefaultOptions ? rule.meta?.defaultOptions : void 0,
1230-
),
1231-
report(...args) {
1232-
/*
1233-
* Create a report translator lazily.
1234-
* In a vast majority of cases, any given rule reports zero errors on a given
1235-
* piece of code. Creating a translator lazily avoids the performance cost of
1236-
* creating a new translator function for each rule that usually doesn't get
1237-
* called.
1238-
*
1239-
* Using lazy report translators improves end-to-end performance by about 3%
1240-
* with Node 8.4.0.
1241-
*/
1242-
if (reportTranslator === null) {
1243-
reportTranslator = createReportTranslator({
1244-
ruleId,
1245-
severity,
1246-
sourceCode,
1247-
messageIds,
1248-
disableFixes,
1249-
language,
1250-
});
1251-
}
1252-
< 3270 span class=pl-k>const problem = reportTranslator(...args);
1224+
const ruleContext = fileContext.extend({
1225+
id: ruleId,
1226+
options: getRuleOptions(
1227+
configuredRules[ruleId],
1228+
applyDefaultOptions ? rule.meta?.defaultOptions : void 0,
1229+
),
1230+
report(...args) {
1231+
/*
1232+
* Create a report translator lazily.
1233+
* In a vast majority of cases, any given rule reports zero errors on a given
1234+
* piece of code. Creating a translator lazily avoids the performance cost of
1235+
* creating a new translator function for each rule that usually doesn't get
1236+
* called.
1237+
*
1238+
* Using lazy report translators improves end-to-end performance by about 3%
1239+
* with Node 8.4.0.
1240+
*/
1241+
if (reportTranslator === null) {
1242+
reportTranslator = createReportTranslator({
1243+
ruleId,
1244+
severity,
1245+
sourceCode,
1246+
messageIds,
1247+
disableFixes,
1248+
language,
1249+
});
1250+
}
1251+
const problem = reportTranslator(...args);
12531252

1254-
if (problem.fix && !(rule.meta && rule.meta.fixable)) {
1255-
throw new Error(
1256-
'Fixable rules must set the `meta.fixable` property to "code" or "whitespace".',
1257-
);
1258-
}
1253+
if (problem.fix && !(rule.meta && rule.meta.fixable)) {
1254+
throw new Error(
1255+
'Fixable rules must set the `meta.fixable` property to "code" or "whitespace".',
1256+
);
1257+
}
1258+
if (
1259+
problem.suggestions &&
1260+
!(rule.meta && rule.meta.hasSuggestions === true)
1261+
) {
12591262
if (
1260-
problem.suggestions &&
1261-
!(rule.meta && rule.meta.hasSuggestions === true)
1263+
rule.meta &&
1264+
rule.meta.docs &&
1265+
typeof rule.meta.docs.suggestion !== "undefined"
12621266
) {
1263-
if (
1264-
rule.meta &&
1265-
rule.meta.docs &&
1266-
typeof rule.meta.docs.suggestion !== "undefined"
1267-
) {
1268-
// Encourage migration from the former property name.
1269-
throw new Error(
1270-
"Rules with suggestions must set the `meta.hasSuggestions` property to `true`. `meta.docs.suggestion` is ignored by ESLint.",
1271-
);
1272-
}
1267+
// Encourage migration from the former property name.
12731268
throw new Error(
1274-
"Rules with suggestions must set the `meta.hasSuggestions` property to `true`.",
1269+
"Rules with suggestions must set the `meta.hasSuggestions` property to `true`. `meta.docs.suggestion` is ignored by ESLint.",
12751270
);
12761271
}
1277-
lintingProblems.push(problem);
1278-
},
1279-
}),
1280-
);
1272+
throw new Error(
1273+
"Rules with suggestions must set the `meta.hasSuggestions` property to `true`.",
1274+
);
1275+
}
1276+
lintingProblems.push(problem);
1277+
},
1278+
});
12811279

12821280
const ruleListenersReturn =
12831281
timing.enabled || stats

‎tests/lib/linter/file-context.js

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
/**
2+
* @fileoverview Tests for FileContext class.
3+
* @author Nicholas C. Zakas
4+
*/
5+
6+
"use strict";
7+
8+
//------------------------------------------------------------------------------
9+
// Requirements
10+
//------------------------------------------------------------------------------
11+
12+
const assert = require("chai").assert;
13+
const { FileContext } = require("../../../lib/linter/file-context");
14+
15+
//------------------------------------------------------------------------------
16+
// Tests
17+
//------------------------------------------------------------------------------
18+
19+
describe("FileContext", () => {
20+
const mockSourceCode = {};
21+
const defaultConfig = {
22+
cwd: "/path/to/project",
23+
filename: "test.js",
24+
physicalFilename: "/path/to/project/test.js",
25+
sourceCode: mockSourceCode,
26+
parserOptions: { ecmaVersion: 2021 },
27+
parserPath: "/path/to/parser",
28+
languageOptions: { ecmaVersion: 2022 },
29+
settings: { env: { es6: true } },
30+
};
31+
32+
describe("constructor", () => {
33+
it("should create a frozen instance with all properties set", () => {
34+
const context = new FileContext(defaultConfig);
35+
36+
assert.strictEqual(context.cwd, defaultConfig.cwd);
37+
assert.strictEqual(context.filename, defaultConfig.filename);
38+
assert.strictEqual(
39+
context.physicalFilename,
40+
defaultConfig.physicalFilename,
41+
);
42+
assert.strictEqual(context.sourceCode, defaultConfig.sourceCode);
43+
assert.deepStrictEqual(
44+
context.parserOptions,
45+
defaultConfig.parserOptions,
46+
);
47+
assert.strictEqual(context.parserPath, defaultConfig.parserPath);
48+
assert.deepStrictEqual(
49+
context.languageOptions,
50+
defaultConfig.languageOptions,
51+
);
52+
assert.deepStrictEqual(context.settings, defaultConfig.settings);
53+
54+
// Verify the instance is frozen
55+
assert.throws(() => {
56+
context.cwd = "changed";
57+
}, TypeError);
58+
});
59+
60+
it("should allow partial configuration", () => {
61+
const partialConfig = {
62+
cwd: "/path/to/project",
63+
filename: "test.js",
64+
physicalFilename: "/path/to/project/test.js",
65+
sourceCode: mockSourceCode,
66+
};
67+
68+
const context = new FileContext(partialConfig);
69+
70+
assert.strictEqual(context.cwd, partialConfig.cwd);
71+
assert.strictEqual(context.filename, partialConfig.filename);
72+
assert.strictEqual(
73+
context.physicalFilename,
74+
partialConfig.physicalFilename,
75+
);
76+
assert.strictEqual(context.sourceCode, partialConfig.sourceCode);
77+
assert.isUndefined(context.parserOptions);
78+
assert.isUndefined(context.parserPath);
79+
assert.isUndefined(context.languageOptions);
80+
assert.isUndefined(context.settings);
81+
});
82+
});
83+
84+
describe("deprecated methods", () => {
85+
let context;
86+
87+
beforeEach(() => {
88+
context = new FileContext(defaultConfig);
89+
});
90+
91+
it("getCwd() should return the cwd property", () => {
92+
assert.strictEqual(context.getCwd(), context.cwd);
93+
assert.strictEqual(context.getCwd(), defaultConfig.cwd);
94+
});
95+
96+
it("getFilename() should return the filename property", () => {
97+
assert.strictEqual(context.getFilename(), context.filename);
98+
assert.strictEqual(context.getFilename(), defaultConfig.filename);
99+
});
100+
101+
it("getPhysicalFilename() should return the physicalFilename property", () => {
102+
assert.strictEqual(
103+
context.getPhysicalFilename(),
104+
context.physicalFilename,
105+
);
106+
assert.strictEqual(
107+
context.getPhysicalFilename(),
108+
defaultConfig.physicalFilename,
109+
);
110+
});
111+
112+
it("getSourceCode() should return the sourceCode property", () => {
113+
assert.strictEqual(context.getSourceCode(), context.sourceCode);
114+
assert.strictEqual(
115+
context.getSourceCode(),
116+
defaultConfig.sourceCode,
117+
);
118+
});
119+
});
120+
121+
describe("extend()", () => {
122+
let context;
123+
124+
beforeEach(() => {
125+
context = new FileContext(defaultConfig);
126+
});
127+
128+
it("should create a new object with the original as prototype", () => {
129+
const extension = { extraProperty: "extra" };
130+
const extended = context.extend(extension);
131+
132+
// Verify new properties
133+
assert.strictEqual(extended.extraProperty, "extra");
134+
135+
// Verify inherited properties
136+
assert.strictEqual(extended.cwd, context.cwd);
137+
assert.strictEqual(extended.filename, context.filename);
138+
assert.strictEqual(
139+
extended.physicalFilename,
140+
context.physicalFilename,
141+
);
142+
assert.strictEqual(extended.sourceCode, context.sourceCode);
143+
assert.deepStrictEqual(
144+
extended.parserOptions,
145+
context.parserOptions,
146+
);
147+
assert.strictEqual(extended.parserPath, context.parserPath);
148+
assert.deepStrictEqual(
149+
extended.languageOptions,
150+
context.languageOptions,
151+
);
152+
assert.deepStrictEqual(extended.settings, context.settings);
153+
});
154+
155+
it("should freeze the extended object", () => {
156+
const extension = { extraProperty: "extra" };
157+
const extended = context.extend(extension);
158+
159+
// Verify the extended object is frozen
160+
assert.throws(() => {
161+
extended.cwd = "changed";
162+
}, TypeError);
163+
164+
assert.throws(() => {
165+
extended.extraProperty = "changed";
166+
}, TypeError);
167+
});
168+
169+
it("should throw an error when attempting to override existing properties", () => {
170+
const extension = { cwd: "newCwd" };
171+
172+
assert.throws(() => {
173+
context.extend(extension);
174+
}, TypeError);
175+
});
176+
});
177+
});

0 commit comments

Comments
 (0)
0