Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: allow to define eslint-disable-next-line in multiple lines #15436

Merged
merged 8 commits into from Dec 29, 2021
Merged

feat: allow to define eslint-disable-next-line in multiple lines #15436

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3b9f3dd
feat: `eslint-disable-next-line` directive with multiple lines
snitin315 Dec 18, 2021
8f8c7ef
test: add cases for multiline `disable-next-line` directives
snitin315 Dec 18, 2021
04f88e0
test: add more cases
snitin315 Dec 18, 2021
af7d3d8
fix: correct reporting location for unused disable directives
snitin315 Dec 24, 2021
7d91e9a
docs: add examples for multiline eslint-disable-next-line directive
snitin315 Dec 24, 2021
35fb63a
fix: logic
snitin315 Dec 24, 2021
e10efd6
refactor: code
snitin315 Dec 24, 2021
035b3d4
refactor: rename `position` to `unprocessedDirective`
snitin315 Dec 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/linter/linter.js
View file
Open in desktop
Expand Up @@ -305,7 +305,11 @@ function createDisableDirectives(options) {

// push to directives, if the rule is defined(including null, e.g. /*eslint enable*/)
if (ruleId === null || !!ruleMapper(ruleId)) {
result.directives.push({ parentComment, type, line: commentToken.loc.start.line, column: commentToken.loc.start.column + 1, ruleId });
if (type === "disable-next-line") {
result.directives.push({ parentComment, type, line: commentToken.loc.end.line, column: commentToken.loc.end.column + 1, ruleId });
snitin315 marked this conversation as resolved.
Show resolved Hide resolved
} else {
result.directives.push({ parentComment, type, line: commentToken.loc.start.line, column: commentToken.loc.start.column + 1, ruleId });
}
} else {
result.directiveProblems.push(createLintingProblem({ ruleId, loc: commentToken.loc }));
}
Expand Down Expand Up @@ -369,7 +373,7 @@ function getDirectiveComments(filename, ast, ruleMapper, warnInlineConfig) {
return;
}

if (lineCommentSupported && comment.loc.start.line !== comment.loc.end.line) {
if (directiveText === "eslint-disable-line" && comment.loc.start.line !== comment.loc.end.line) {
const message = `${directiveText} comment should not span multiple lines.`;

problems.push(createLintingProblem({
Expand Down
163 changes: 157 additions & 6 deletions tests/lib/linter/linter.js
View file
Open in desktop
Expand Up @@ -2186,7 +2186,7 @@ describe("Linter", () => {
assert.strictEqual(messages.length, 0);
});

it("should not ignore violation if block comment is not on a single line", () => {
it("should not ignore violation if code is not on next line", () => {
const code = [
"/* eslint-disable-next-line",
"no-alert */alert('test');"
Expand All @@ -2198,8 +2198,24 @@ describe("Linter", () => {
};
const messages = linter.verify(code, config, filename);

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

it("should ignore violation if block comment span multiple lines", () => {
const code = [
"/* eslint-disable-next-line",
"no-alert */",
"alert('test');"
].join("\n");
const config = {
rules: {
"no-alert": 1
}
};
const messages = linter.verify(code, config, filename);

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

it("should ignore violations only of specified rule", () => {
Expand Down Expand Up @@ -2240,6 +2256,28 @@ describe("Linter", () => {
assert.strictEqual(messages[0].ruleId, "no-console");
});

it("should ignore violations of multiple rules when specified in multiple lines", () => {
const code = [
"/* eslint-disable-next-line",
"no-alert,",
"quotes",
"*/",
"alert(\"test\");",
"console.log('test');"
].join("\n");
const config = {
rules: {
"no-alert": 1,
quotes: [1, "single"],
"no-console": 1
}
};
const messages = linter.verify(code, config, filename);

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

it("should ignore violations of multiple rules when specified in mixed comments", () => {
const code = [
"/* eslint-disable-next-line no-alert */ // eslint-disable-next-line quotes",
Expand All @@ -2256,6 +2294,24 @@ describe("Linter", () => {
assert.strictEqual(messages.length, 0);
});

it("should ignore violations of multiple rules when specified in mixed sinlge line and multi line comments", () => {
const code = [
"/* eslint-disable-next-line",
"no-alert",
"*/ // eslint-disable-next-line quotes",
"alert(\"test\");"
].join("\n");
const config = {
rules: {
"no-alert": 1,
quotes: [1, "single"]
}
};
const messages = linter.verify(code, config, filename);

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

it("should ignore violations of only the specified rule on next line", () => {
const code = [
"// eslint-disable-next-line quotes",
Expand Down Expand Up @@ -11224,7 +11280,7 @@ var a = "test2";
assert.strictEqual(messages.length, 0);
});

it("should not ignore violation if block comment is not on a single line", () => {
it("should not ignore violation if code is not on next line", () => {
const code = [
"/* eslint-disable-next-line",
"no-alert */alert('test');"
Expand All @@ -11236,8 +11292,43 @@ var a = "test2";
};
const messages = linter.verify(code, config, filename);

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

it("should ignore violation if block comment span multiple lines", () => {
const code = [
"/* eslint-disable-next-line",
"no-alert */",
"alert('test');"
].join("\n");
const config = {
rules: {
"no-alert": 1
}
};
const messages = linter.verify(code, config, filename);

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

// For https://github.com/eslint/eslint/issues/14284
it("should ignore violation if block comment span multiple lines with description", () => {
const code = `
/* eslint-disable-next-line no-alert --
description on why this exception is seen as appropriate but past a
comfortable reading line length
*/
alert("buzz");
`;
const config = {
rules: {
"no-alert": 1
}
};
const messages = linter.verify(code, config, filename);

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

it("should ignore violations only of specified rule", () => {
Expand All @@ -11259,6 +11350,26 @@ var a = "test2";
assert.strictEqual(messages[1].ruleId, "no-console");
});

it("should ignore violations only of specified rule when block comment span multiple lines", () => {
const code = [
"/* eslint-disable-next-line",
"no-console */",
"alert('test');",
"console.log('test');"
].join("\n");
const config = {
rules: {
"no-alert": 1,
"no-console": 1
}
};
const messages = linter.verify(code, config, filename);

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

it("should ignore violations of multiple rules when specified", () => {
const code = [
"// eslint-disable-next-line no-alert, quotes",
Expand All @@ -11278,6 +11389,28 @@ var a = "test2";
assert.strictEqual(messages[0].ruleId, "no-console");
});

it("should ignore violations of multiple rules when specified in multiple lines", () => {
const code = [
"/* eslint-disable-next-line",
"no-alert,",
"quotes",
"*/",
"alert(\"test\");",
"console.log('test');"
].join("\n");
const config = {
rules: {
"no-alert": 1,
quotes: [1, "single"],
"no-console": 1
}
};
const messages = linter.verify(code, config, filename);

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

it("should ignore violations of multiple rules when specified in mixed comments", () => {
const code = [
"/* eslint-disable-next-line no-alert */ // eslint-disable-next-line quotes",
Expand All @@ -11294,6 +11427,24 @@ var a = "test2";
assert.strictEqual(messages.length, 0);
});

it("should ignore violations of multiple rules when specified in mixed sinlge line and multi line comments", () => {
const code = [
"/* eslint-disable-next-line",
"no-alert",
"*/ // eslint-disable-next-line quotes",
"alert(\"test\");"
].join("\n");
const config = {
rules: {
"no-alert": 1,
quotes: [1, "single"]
}
};
const messages = linter.verify(code, config, filename);

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

it("should ignore violations of only the specified rule on next line", () => {
const code = [
"// eslint-disable-next-line quotes",
Expand Down