Skip to content

Commit dd79abc

Browse files
ota-meshimdjermanovicnzakas
authored
fix: eslint-disable to be able to parse quoted rule names (#17612)
* fix: `eslint-disable` to be able to parse literal rule names * fix: id casing * Apply suggestions from code review Co-authored-by: Milos Djermanovic <[email protected]> Co-authored-by: Nicholas C. Zakas <[email protected]> * test: update test cases * test: add eslint-env test case * test: add test case and fix test case --------- Co-authored-by: Milos Djermanovic <[email protected]> Co-authored-by: Nicholas C. Zakas <[email protected]>
1 parent d2f6801 commit dd79abc

4 files changed

Lines changed: 290 additions & 2 deletions

File tree

‎lib/linter/apply-disable-directives.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ function createIndividualDirectivesRemoval(directives, commentToken) {
8787
return directives.map(directive => {
8888
const { ruleId } = directive;
8989

90-
const regex = new RegExp(String.raw`(?:^|\s*,\s*)${escapeRegExp(ruleId)}(?:\s*,\s*|$)`, "u");
90+
const regex = new RegExp(String.raw`(?:^|\s*,\s*)(?<quote>['"]?)${escapeRegExp(ruleId)}\k<quote>(?:\s*,\s*|$)`, "u");
9191
const match = regex.exec(listText);
9292
const matchedText = match[0];
9393
const matchStartOffset = listStartOffset + match.index;

‎lib/linter/config-comment-parser.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ module.exports = class ConfigCommentParser {
139139
const items = {};
140140

141141
string.split(",").forEach(name => {
142-
const trimmedName = name.trim();
142+
const trimmedName = name.trim().replace(/^(?<quote>['"]?)(?<ruleId>.*)\k<quote>$/us, "$<ruleId>");
143143

144144
if (trimmedName) {
145145
items[trimmedName] = true;

‎tests/lib/linter/config-comment-parser.js‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,28 @@ describe("ConfigCommentParser", () => {
224224
b: true
225225
});
226226
});
227+
228+
it("should parse list config with quoted items", () => {
229+
const code = "'a', \"b\", 'c\", \"d'";
230+
const result = commentParser.parseListConfig(code);
231+
232+
assert.deepStrictEqual(result, {
233+
a: true,
234+
b: true,
235+
"\"d'": true, // This result is correct because used mismatched quotes.
236+
"'c\"": true // This result is correct because used mismatched quotes.
237+
});
238+
});
239+
it("should parse list config with spaced items", () => {
240+
const code = " a b , 'c d' , \"e f\" ";
241+
const result = commentParser.parseListConfig(code);
242+
243+
assert.deepStrictEqual(result, {
244+
"a b": true,
245+
"c d": true,
246+
"e f": true
247+
});
248+
});
227249
});
228250

229251
});

‎tests/lib/linter/linter.js‎

Lines changed: 266 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,6 +1467,32 @@ describe("Linter", () => {
14671467
linter.verify(code, config);
14681468
assert(spy && spy.calledOnce);
14691469
});
1470+
1471+
it("variables should be available in global scope with quoted items", () => {
1472+
const code = `/*${ESLINT_ENV} 'node'*/ function f() {} /*${ESLINT_ENV} "browser", "mocha"*/`;
1473+
const config = { rules: { checker: "error" } };
1474+
let spy;
1475+
1476+
linter.defineRule("checker", {
1477+
create(context) {
1478+
spy = sinon.spy(() => {
1479+
const scope = context.getScope(),
1480+
exports = getVariable(scope, "exports"),
1481+
window = getVariable(scope, "window"),
1482+
it = getVariable(scope, "it");
1483+
1484+
assert.strictEqual(exports.writeable, true);
1485+
assert.strictEqual(window.writeable, false);
1486+
assert.strictEqual(it.writeable, false);
1487+
});
1488+
1489+
return { Program: spy };
1490+
}
1491+
});
1492+
1493+
linter.verify(code, config);
1494+
assert(spy && spy.calledOnce);
1495+
});
14701496
});
14711497

14721498
describe("when evaluating code containing /*eslint-env */ block with sloppy whitespace", () => {
@@ -2604,6 +2630,33 @@ describe("Linter", () => {
26042630
assert.strictEqual(suppressedMessages.length, 2);
26052631
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
26062632
});
2633+
2634+
it("should report a violation with quoted rule names in eslint-disable-line", () => {
2635+
const code = [
2636+
"alert('test'); // eslint-disable-line 'no-alert'",
2637+
"console.log('test');", // here
2638+
"alert('test'); // eslint-disable-line \"no-alert\""
2639+
].join("\n");
2640+
const config = {
2641+
rules: {
2642+
"no-alert": 1,
2643+
"no-console": 1
2644+
}
2645+
};
2646+
2647+
const messages = linter.verify(code, config, filename);
2648+
const suppressedMessages = linter.getSuppressedMessages();
2649+
2650+
assert.strictEqual(messages.length, 1);
2651+
assert.strictEqual(messages[0].ruleId, "no-console");
2652+
assert.strictEqual(messages[0].line, 2);
2653+
2654+
assert.strictEqual(suppressedMessages.length, 2);
2655+
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
2656+
assert.strictEqual(suppressedMessages[0].line, 1);
2657+
assert.strictEqual(suppressedMessages[1].ruleId, "no-alert");
2658+
assert.strictEqual(suppressedMessages[1].line, 3);
2659+
});
26072660
});
26082661

26092662
describe("eslint-disable-next-line", () => {
@@ -2957,6 +3010,31 @@ describe("Linter", () => {
29573010

29583011
assert.strictEqual(suppressedMessages.length, 0);
29593012
});
3013+
3014+
it("should ignore violation of specified rule on next line with quoted rule names", () => {
3015+
const code = [
3016+
"// eslint-disable-next-line 'no-alert'",
3017+
"alert('test');",
3018+
"// eslint-disable-next-line \"no-alert\"",
3019+
"alert('test');",
3020+
"console.log('test');"
3021+
].join("\n");
3022+
const config = {
3023+
rules: {
3024+
"no-alert": 1,
3025+
"no-console": 1
3026+
}
3027+
};
3028+
const messages = linter.verify(code, config, filename);
3029+
const suppressedMessages = linter.getSuppressedMessages();
3030+
3031+
assert.strictEqual(messages.length, 1);
3032+
assert.strictEqual(messages[0].ruleId, "no-console");
3033+
3034+
assert.strictEqual(suppressedMessages.length, 2);
3035+
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
3036+
assert.strictEqual(suppressedMessages[1].ruleId, "no-alert");
3037+
});
29603038
});
29613039
});
29623040

@@ -3233,6 +3311,61 @@ describe("Linter", () => {
32333311
assert.strictEqual(suppressedMessages[2].ruleId, "no-console");
32343312
assert.strictEqual(suppressedMessages[2].line, 6);
32353313
});
3314+
3315+
it("should report a violation with quoted rule names in eslint-disable", () => {
3316+
const code = [
3317+
"/*eslint-disable 'no-alert' */",
3318+
"alert('test');",
3319+
"console.log('test');", // here
3320+
"/*eslint-enable */",
3321+
"/*eslint-disable \"no-console\" */",
3322+
"alert('test');", // here
3323+
"console.log('test');"
3324+
].join("\n");
3325+
const config = { rules: { "no-alert": 1, "no-console": 1 } };
3326+
3327+
const messages = linter.verify(code, config, filename);
3328+
const suppressedMessages = linter.getSuppressedMessages();
3329+
3330+
assert.strictEqual(messages.length, 2);
3331+
assert.strictEqual(messages[0].ruleId, "no-console");
3332+
assert.strictEqual(messages[1].ruleId, "no-alert");
3333+
3334+
assert.strictEqual(suppressedMessages.length, 2);
3335+
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
3336+
assert.strictEqual(suppressedMessages[1].ruleId, "no-console");
3337+
});
3338+
3339+
it("should report a violation with quoted rule names in eslint-enable", () => {
3340+
const code = [
3341+
"/*eslint-disable no-alert, no-console */",
3342+
"alert('test');",
3343+
"console.log('test');",
3344+
"/*eslint-enable 'no-alert'*/",
3345+
"alert('test');", // here
3346+
"console.log('test');",
3347+
"/*eslint-enable \"no-console\"*/",
3348+
"console.log('test');" // here
3349+
].join("\n");
3350+
const config = { rules: { "no-alert": 1, "no-console": 1 } };
3351+
3352+
const messages = linter.verify(code, config, filename);
3353+
const suppressedMessages = linter.getSuppressedMessages();
3354+
3355+
assert.strictEqual(messages.length, 2);
3356+
assert.strictEqual(messages[0].ruleId, "no-alert");
3357+
assert.strictEqual(messages[0].line, 5);
3358+
assert.strictEqual(messages[1].ruleId, "no-console");
3359+
assert.strictEqual(messages[1].line, 8);
3360+
3361+
assert.strictEqual(suppressedMessages.length, 3);
3362+
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
3363+
assert.strictEqual(suppressedMessages[0].line, 2);
3364+
assert.strictEqual(suppressedMessages[1].ruleId, "no-console");
3365+
assert.strictEqual(suppressedMessages[1].line, 3);
3366+
assert.strictEqual(suppressedMessages[2].ruleId, "no-console");
3367+
assert.strictEqual(suppressedMessages[2].line, 6);
3368+
});
32363369
});
32373370

32383371
describe("when evaluating code with comments to enable and disable multiple comma separated rules", () => {
@@ -4813,6 +4946,20 @@ var a = "test2";
48134946
output
48144947
);
48154948
});
4949+
4950+
// Test for quoted rule names
4951+
for (const testcaseForLiteral of [
4952+
{ code: code.replace(/((?:un)?used[\w-]*)/gu, '"$1"'), output: output.replace(/((?:un)?used[\w-]*)/gu, '"$1"') },
4953+
{ code: code.replace(/((?:un)?used[\w-]*)/gu, "'$1'"), output: output.replace(/((?:un)?used[\w-]*)/gu, "'$1'") }
4954+
]) {
4955+
// eslint-disable-next-line no-loop-func -- `linter` is getting updated in beforeEach()
4956+
it(testcaseForLiteral.code, () => {
4957+
assert.strictEqual(
4958+
linter.verifyAndFix(testcaseForLiteral.code, config).output,
4959+
testcaseForLiteral.output
4960+
);
4961+
});
4962+
}
48164963
}
48174964
});
48184965
});
@@ -13060,6 +13207,60 @@ var a = "test2";
1306013207
assert.strictEqual(suppressedMessages[1].line, 3);
1306113208
});
1306213209

13210+
it("should report a violation with quoted rule names in eslint-disable", () => {
13211+
const code = [
13212+
"/*eslint-disable 'no-alert' */",
13213+
"alert('test');",
13214+
"console.log('test');", // here
13215+
"/*eslint-enable */",
13216+
"/*eslint-disable \"no-console\" */",
13217+
"alert('test');", // here
13218+
"console.log('test');"
13219+
].join("\n");
13220+
const config = { rules: { "no-alert": 1, "no-console": 1 } };
13221+
13222+
const messages = linter.verify(code, config, filename);
13223+
const suppressedMessages = linter.getSuppressedMessages();
13224+
13225+
assert.strictEqual(messages.length, 2);
13226+
assert.strictEqual(messages[0].ruleId, "no-console");
13227+
assert.strictEqual(messages[1].ruleId, "no-alert");
13228+
13229+
assert.strictEqual(suppressedMessages.length, 2);
13230+
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
13231+
assert.strictEqual(suppressedMessages[1].ruleId, "no-console");
13232+
});
13233+
13234+
it("should report a violation with quoted rule names in eslint-enable", () => {
13235+
const code = [
13236+
"/*eslint-disable no-alert, no-console */",
13237+
"alert('test');",
13238+
"console.log('test');",
13239+
"/*eslint-enable 'no-alert'*/",
13240+
"alert('test');", // here
13241+
"console.log('test');",
13242+
"/*eslint-enable \"no-console\"*/",
13243+
"console.log('test');" // here
13244+
].join("\n");
13245+
const config = { rules: { "no-alert": 1, "no-console": 1 } };
13246+
13247+
const messages = linter.verify(code, config, filename);
13248+
const suppressedMessages = linter.getSuppressedMessages();
13249+
13250+
assert.strictEqual(messages.length, 2);
13251+
assert.strictEqual(messages[0].ruleId, "no-alert");
13252+
assert.strictEqual(messages[0].line, 5);
13253+
assert.strictEqual(messages[1].ruleId, "no-console");
13254+
assert.strictEqual(messages[1].line, 8);
13255+
13256+
assert.strictEqual(suppressedMessages.length, 3);
13257+
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
13258+
assert.strictEqual(suppressedMessages[0].line, 2);
13259+
assert.strictEqual(suppressedMessages[1].ruleId, "no-console");
13260+
assert.strictEqual(suppressedMessages[1].line, 3);
13261+
assert.strictEqual(suppressedMessages[2].ruleId, "no-console");
13262+
assert.strictEqual(suppressedMessages[2].line, 6);
13263+
});
1306313264
});
1306413265

1306513266
describe("/*eslint-disable-line*/", () => {
@@ -13293,6 +13494,32 @@ var a = "test2";
1329313494
assert.strictEqual(suppressedMessages.length, 5);
1329413495
});
1329513496

13497+
it("should report a violation with quoted rule names in eslint-disable-line", () => {
13498+
const code = [
13499+
"alert('test'); // eslint-disable-line 'no-alert'",
13500+
"console.log('test');", // here
13501+
"alert('test'); // eslint-disable-line \"no-alert\""
13502+
].join("\n");
13503+
const config = {
13504+
rules: {
13505+
"no-alert": 1,
13506+
"no-console": 1
13507+
}
13508+
};
13509+
13510+
const messages = linter.verify(code, config, filename);
13511+
const suppressedMessages = linter.getSuppressedMessages();
13512+
13513+
assert.strictEqual(messages.length, 1);
13514+
assert.strictEqual(messages[0].ruleId, "no-console");
13515+
assert.strictEqual(messages[0].line, 2);
13516+
13517+
assert.strictEqual(suppressedMessages.length, 2);
13518+
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
13519+
assert.strictEqual(suppressedMessages[0].line, 1);
13520+
assert.strictEqual(suppressedMessages[1].ruleId, "no-alert");
13521+
assert.strictEqual(suppressedMessages[1].line, 3);
13522+
});
1329613523
});
1329713524

1329813525
describe("/*eslint-disable-next-line*/", () => {
@@ -13689,6 +13916,31 @@ var a = "test2";
1368913916

1369013917
assert.strictEqual(suppressedMessages.length, 0);
1369113918
});
13919+
13920+
it("should ignore violation of specified rule on next line with quoted rule names", () => {
13921+
const code = [
13922+
"// eslint-disable-next-line 'no-alert'",
13923+
"alert('test');",
13924+
"// eslint-disable-next-line \"no-alert\"",
13925+
"alert('test');",
13926+
"console.log('test');"
13927+
].join("\n");
13928+
const config = {
13929+
rules: {
13930+
"no-alert": 1,
13931+
"no-console": 1
13932+
}
13933+
};
13934+
const messages = linter.verify(code, config, filename);
13935+
const suppressedMessages = linter.getSuppressedMessages();
13936+
13937+
assert.strictEqual(messages.length, 1);
13938+
assert.strictEqual(messages[0].ruleId, "no-console");
13939+
13940+
assert.strictEqual(suppressedMessages.length, 2);
13941+
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
13942+
assert.strictEqual(suppressedMessages[1].ruleId, "no-alert");
13943+
});
1369213944
});
1369313945

1369413946
describe("descriptions in directive comments", () => {
@@ -15366,6 +15618,20 @@ var a = "test2";
1536615618
output
1536715619
);
1536815620
});
15621+
15622+
// Test for quoted rule names
15623+
for (const testcaseForLiteral of [
15624+
{ code: code.replace(/(test\/[\w-]+)/gu, '"$1"'), output: output.replace(/(test\/[\w-]+)/gu, '"$1"') },
15625+
{ code: code.replace(/(test\/[\w-]+)/gu, "'$1'"), output: output.replace(/(test\/[\w-]+)/gu, "'$1'") }
15626+
]) {
15627+
// eslint-disable-next-line no-loop-func -- `linter` is getting updated in beforeEach()
15628+
it(testcaseForLiteral.code, () => {
15629+
assert.strictEqual(
15630+
linter.verifyAndFix(testcaseForLiteral.code, config).output,
15631+
testcaseForLiteral.output
15632+
);
15633+
});
15634+
}
1536915635
}
1537015636
});
1537115637
});

0 commit comments

Comments
 (0)