Skip to content

Commit

Permalink
fix: eslint-disable to be able to parse quoted rule names (#17612)
Browse files Browse the repository at this point in the history 
* fix: `eslint-disable` to be able to parse literal rule names

* fix: id casing

* Apply suggestions from code review

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

* test: update test cases

* test: add eslint-env test case

* test: add test case and fix test case

---------

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
  • Loading branch information
@ota-meshi @mdjermanovic @nzakas
3 people committed Oct 4, 2023
1 parent d2f6801 commit dd79abc
Show file tree
Hide file tree
Showing 4 changed files with 290 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/linter/apply-disable-directives.js
View file
Expand Up @@ -87,7 +87,7 @@ function createIndividualDirectivesRemoval(directives, commentToken) {
return directives.map(directive => {
const { ruleId } = directive;

const regex = new RegExp(String.raw`(?:^|\s*,\s*)${escapeRegExp(ruleId)}(?:\s*,\s*|$)`, "u");
const regex = new RegExp(String.raw`(?:^|\s*,\s*)(?<quote>['"]?)${escapeRegExp(ruleId)}\k<quote>(?:\s*,\s*|$)`, "u");
const match = regex.exec(listText);
const matchedText = match[0];
const matchStartOffset = listStartOffset + match.index;
Expand Down
2 changes: 1 addition & 1 deletion lib/linter/config-comment-parser.js
View file
Expand Up @@ -139,7 +139,7 @@ module.exports = class ConfigCommentParser {
const items = {};

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

if (trimmedName) {
items[trimmedName] = true;
Expand Down
22 changes: 22 additions & 0 deletions tests/lib/linter/config-comment-parser.js
View file
Expand Up @@ -224,6 +224,28 @@ describe("ConfigCommentParser", () => {
b: true
});
});

it("should parse list config with quoted items", () => {
const code = "'a', \"b\", 'c\", \"d'";
const result = commentParser.parseListConfig(code);

assert.deepStrictEqual(result, {
a: true,
b: true,
"\"d'": true, // This result is correct because used mismatched quotes.
"'c\"": true // This result is correct because used mismatched quotes.
});
});
it("should parse list config with spaced items", () => {
const code = " a b , 'c d' , \"e f\" ";
const result = commentParser.parseListConfig(code);

assert.deepStrictEqual(result, {
"a b": true,
"c d": true,
"e f": true
});
});
});

});
266 changes: 266 additions & 0 deletions tests/lib/linter/linter.js
View file
Expand Up @@ -1467,6 +1467,32 @@ describe("Linter", () => {
linter.verify(code, config);
assert(spy && spy.calledOnce);
});

it("variables should be available in global scope with quoted items", () => {
const code = `/*${ESLINT_ENV} 'node'*/ function f() {} /*${ESLINT_ENV} "browser", "mocha"*/`;
const config = { rules: { checker: "error" } };
let spy;

linter.defineRule("checker", {
create(context) {
spy = sinon.spy(() => {
const scope = context.getScope(),
exports = getVariable(scope, "exports"),
window = getVariable(scope, "window"),
it = getVariable(scope, "it");

assert.strictEqual(exports.writeable, true);
assert.strictEqual(window.writeable, false);
assert.strictEqual(it.writeable, false);
});

return { Program: spy };
}
});

linter.verify(code, config);
assert(spy && spy.calledOnce);
});
});

describe("when evaluating code containing /*eslint-env */ block with sloppy whitespace", () => {
Expand Down Expand Up @@ -2604,6 +2630,33 @@ describe("Linter", () => {
assert.strictEqual(suppressedMessages.length, 2);
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
});

it("should report a violation with quoted rule names in eslint-disable-line", () => {
const code = [
"alert('test'); // eslint-disable-line 'no-alert'",
"console.log('test');", // here
"alert('test'); // eslint-disable-line \"no-alert\""
].join("\n");
const config = {
rules: {
"no-alert": 1,
"no-console": 1
}
};

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();

assert.strictEqual(messages.length, 1);
assert.strictEqual(messages[0].ruleId, "no-console");
assert.strictEqual(messages[0].line, 2);

assert.strictEqual(suppressedMessages.length, 2);
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
assert.strictEqual(suppressedMessages[0].line, 1);
assert.strictEqual(suppressedMessages[1].ruleId, "no-alert");
assert.strictEqual(suppressedMessages[1].line, 3);
});
});

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

assert.strictEqual(suppressedMessages.length, 0);
});

it("should ignore violation of specified rule on next line with quoted rule names", () => {
const code = [
"// eslint-disable-next-line 'no-alert'",
"alert('test');",
"// eslint-disable-next-line \"no-alert\"",
"alert('test');",
"console.log('test');"
].join("\n");
const config = {
rules: {
"no-alert": 1,
"no-console": 1
}
};
const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();

assert.strictEqual(messages.length, 1);
assert.strictEqual(messages[0].ruleId, "no-console");

assert.strictEqual(suppressedMessages.length, 2);
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
assert.strictEqual(suppressedMessages[1].ruleId, "no-alert");
});
});
});

Expand Down Expand Up @@ -3233,6 +3311,61 @@ describe("Linter", () => {
assert.strictEqual(suppressedMessages[2].ruleId, "no-console");
assert.strictEqual(suppressedMessages[2].line, 6);
});

it("should report a violation with quoted rule names in eslint-disable", () => {
const code = [
"/*eslint-disable 'no-alert' */",
"alert('test');",
"console.log('test');", // here
"/*eslint-enable */",
"/*eslint-disable \"no-console\" */",
"alert('test');", // here
"console.log('test');"
].join("\n");
const config = { rules: { "no-alert": 1, "no-console": 1 } };

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();

assert.strictEqual(messages.length, 2);
assert.strictEqual(messages[0].ruleId, "no-console");
assert.strictEqual(messages[1].ruleId, "no-alert");

assert.strictEqual(suppressedMessages.length, 2);
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
assert.strictEqual(suppressedMessages[1].ruleId, "no-console");
});

it("should report a violation with quoted rule names in eslint-enable", () => {
const code = [
"/*eslint-disable no-alert, no-console */",
"alert('test');",
"console.log('test');",
"/*eslint-enable 'no-alert'*/",
"alert('test');", // here
"console.log('test');",
"/*eslint-enable \"no-console\"*/",
"console.log('test');" // here
].join("\n");
const config = { rules: { "no-alert": 1, "no-console": 1 } };

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();

assert.strictEqual(messages.length, 2);
assert.strictEqual(messages[0].ruleId, "no-alert");
assert.strictEqual(messages[0].line, 5);
assert.strictEqual(messages[1].ruleId, "no-console");
assert.strictEqual(messages[1].line, 8);

assert.strictEqual(suppressedMessages.length, 3);
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
assert.strictEqual(suppressedMessages[0].line, 2);
assert.strictEqual(suppressedMessages[1].ruleId, "no-console");
assert.strictEqual(suppressedMessages[1].line, 3);
assert.strictEqual(suppressedMessages[2].ruleId, "no-console");
assert.strictEqual(suppressedMessages[2].line, 6);
});
});

describe("when evaluating code with comments to enable and disable multiple comma separated rules", () => {
Expand Down Expand Up @@ -4813,6 +4946,20 @@ var a = "test2";
output
);
});

// Test for quoted rule names
for (const testcaseForLiteral of [
{ code: code.replace(/((?:un)?used[\w-]*)/gu, '"$1"'), output: output.replace(/((?:un)?used[\w-]*)/gu, '"$1"') },
{ code: code.replace(/((?:un)?used[\w-]*)/gu, "'$1'"), output: output.replace(/((?:un)?used[\w-]*)/gu, "'$1'") }
]) {
// eslint-disable-next-line no-loop-func -- `linter` is getting updated in beforeEach()
it(testcaseForLiteral.code, () => {
assert.strictEqual(
linter.verifyAndFix(testcaseForLiteral.code, config).output,
testcaseForLiteral.output
);
});
}
}
});
});
Expand Down Expand Up @@ -13060,6 +13207,60 @@ var a = "test2";
assert.strictEqual(suppressedMessages[1].line, 3);
});

it("should report a violation with quoted rule names in eslint-disable", () => {
const code = [
"/*eslint-disable 'no-alert' */",
"alert('test');",
"console.log('test');", // here
"/*eslint-enable */",
"/*eslint-disable \"no-console\" */",
"alert('test');", // here
"console.log('test');"
].join("\n");
const config = { rules: { "no-alert": 1, "no-console": 1 } };

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();

assert.strictEqual(messages.length, 2);
assert.strictEqual(messages[0].ruleId, "no-console");
assert.strictEqual(messages[1].ruleId, "no-alert");

assert.strictEqual(suppressedMessages.length, 2);
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
assert.strictEqual(suppressedMessages[1].ruleId, "no-console");
});

it("should report a violation with quoted rule names in eslint-enable", () => {
const code = [
"/*eslint-disable no-alert, no-console */",
"alert('test');",
"console.log('test');",
"/*eslint-enable 'no-alert'*/",
"alert('test');", // here
"console.log('test');",
"/*eslint-enable \"no-console\"*/",
"console.log('test');" // here
].join("\n");
const config = { rules: { "no-alert": 1, "no-console": 1 } };

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();

assert.strictEqual(messages.length, 2);
assert.strictEqual(messages[0].ruleId, "no-alert");
assert.strictEqual(messages[0].line, 5);
assert.strictEqual(messages[1].ruleId, "no-console");
assert.strictEqual(messages[1].line, 8);

assert.strictEqual(suppressedMessages.length, 3);
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
assert.strictEqual(suppressedMessages[0].line, 2);
assert.strictEqual(suppressedMessages[1].ruleId, "no-console");
assert.strictEqual(suppressedMessages[1].line, 3);
assert.strictEqual(suppressedMessages[2].ruleId, "no-console");
assert.strictEqual(suppressedMessages[2].line, 6);
});
});

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

it("should report a violation with quoted rule names in eslint-disable-line", () => {
const code = [
"alert('test'); // eslint-disable-line 'no-alert'",
"console.log('test');", // here
"alert('test'); // eslint-disable-line \"no-alert\""
].join("\n");
const config = {
rules: {
"no-alert": 1,
"no-console": 1
}
};

const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();

assert.strictEqual(messages.length, 1);
assert.strictEqual(messages[0].ruleId, "no-console");
assert.strictEqual(messages[0].line, 2);

assert.strictEqual(suppressedMessages.length, 2);
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
assert.strictEqual(suppressedMessages[0].line, 1);
assert.strictEqual(suppressedMessages[1].ruleId, "no-alert");
assert.strictEqual(suppressedMessages[1].line, 3);
});
});

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

assert.strictEqual(suppressedMessages.length, 0);
});

it("should ignore violation of specified rule on next line with quoted rule names", () => {
const code = [
"// eslint-disable-next-line 'no-alert'",
"alert('test');",
"// eslint-disable-next-line \"no-alert\"",
"alert('test');",
"console.log('test');"
].join("\n");
const config = {
rules: {
"no-alert": 1,
"no-console": 1
}
};
const messages = linter.verify(code, config, filename);
const suppressedMessages = linter.getSuppressedMessages();

assert.strictEqual(messages.length, 1);
assert.strictEqual(messages[0].ruleId, "no-console");

assert.strictEqual(suppressedMessages.length, 2);
assert.strictEqual(suppressedMessages[0].ruleId, "no-alert");
assert.strictEqual(suppressedMessages[1].ruleId, "no-alert");
});
});

describe("descriptions in directive comments", () => {
Expand Down Expand Up @@ -15366,6 +15618,20 @@ var a = "test2";
output
);
});

// Test for quoted rule names
for (const testcaseForLiteral of [
{ code: code.replace(/(test\/[\w-]+)/gu, '"$1"'), output: output.replace(/(test\/[\w-]+)/gu, '"$1"') },
{ code: code.replace(/(test\/[\w-]+)/gu, "'$1'"), output: output.replace(/(test\/[\w-]+)/gu, "'$1'") }
]) {
// eslint-disable-next-line no-loop-func -- `linter` is getting updated in beforeEach()
it(testcaseForLiteral.code, () => {
assert.strictEqual(
linter.verifyAndFix(testcaseForLiteral.code, config).output,
testcaseForLiteral.output
);
});
}
}
});
});
Expand Down

0 comments on commit dd79abc

Please sign in to comment.