diff --git a/docs/user-guide/configuring/rules.md b/docs/user-guide/configuring/rules.md index 4f45b4a876a..c33a64ad4f2 100644 --- a/docs/user-guide/configuring/rules.md +++ b/docs/user-guide/configuring/rules.md @@ -200,6 +200,13 @@ alert('foo'); /* eslint-disable-line no-alert, quotes, semi */ /* eslint-disable-next-line no-alert, quotes, semi */ alert('foo'); + +/* eslint-disable-next-line + no-alert, + quotes, + semi +*/ +alert('foo'); ``` All of the above methods also work for plugin rules. For example, to disable `eslint-plugin-example`'s `rule-name` rule, combine the plugin's name (`example`) and the rule's name (`rule-name`) into `example/rule-name`: @@ -214,6 +221,12 @@ Configuration comments can include descriptions to explain why the comment is ne ```js // eslint-disable-next-line no-console -- Here's a description about why this configuration is necessary. console.log('hello'); + +/* eslint-disable-next-line no-console -- + * Here's a very long description about why this configuration is necessary + * along with some additional information +**/ +console.log('hello'); ``` **Note:** Comments that disable warnings for a portion of a file tell ESLint not to report rule violations for the disabled code. ESLint still parses the entire file, however, so disabled code still needs to be syntactically valid JavaScript. diff --git a/lib/linter/apply-disable-directives.js b/lib/linter/apply-disable-directives.js index e5f2e528ef8..f6b432399cf 100644 --- a/lib/linter/apply-disable-directives.js +++ b/lib/linter/apply-disable-directives.js @@ -43,7 +43,7 @@ function groupByParentComment(directives) { * Creates removal details for a set of directives within the same comment. * @param {Directive[]} directives Unused directives to be removed. * @param {Token} commentToken The backing Comment token. - * @returns {{ description, fix, position }[]} Details for later creation of output Problems. + * @returns {{ description, fix, unprocessedDirective }[]} Details for later creation of output Problems. */ function createIndividualDirectivesRemoval(directives, commentToken) { @@ -138,7 +138,7 @@ function createIndividualDirectivesRemoval(directives, commentToken) { ], text: "" }, - position: directive.unprocessedDirective + unprocessedDirective: directive.unprocessedDirective }; }); } @@ -147,7 +147,7 @@ function createIndividualDirectivesRemoval(directives, commentToken) { * Creates a description of deleting an entire unused disable comment. * @param {Directive[]} directives Unused directives to be removed. * @param {Token} commentToken The backing Comment token. - * @returns {{ description, fix, position }} Details for later creation of an output Problem. + * @returns {{ description, fix, unprocessedDirective }} Details for later creation of an output Problem. */ function createCommentRemoval(directives, commentToken) { const { range } = commentToken; @@ -161,14 +161,14 @@ function createCommentRemoval(directives, commentToken) { range, text: " " }, - position: directives[0].unprocessedDirective + unprocessedDirective: directives[0].unprocessedDirective }; } /** * Parses details from directives to create output Problems. * @param {Directive[]} allDirectives Unused directives to be removed. - * @returns {{ description, fix, position }[]} Details for later creation of output Problems. + * @returns {{ description, fix, unprocessedDirective }[]} Details for later creation of output Problems. */ function processUnusedDisableDirectives(allDirectives) { const directiveGroups = groupByParentComment(allDirectives); @@ -261,17 +261,21 @@ function applyDirectives(options) { const processed = processUnusedDisableDirectives(unusedDisableDirectivesToReport); const unusedDisableDirectives = processed - .map(({ description, fix, position }) => ({ - ruleId: null, - message: description - ? `Unused eslint-disable directive (no problems were reported from ${description}).` - : "Unused eslint-disable directive (no problems were reported).", - line: position.line, - column: position.column, - severity: options.reportUnusedDisableDirectives === "warn" ? 1 : 2, - nodeType: null, - ...options.disableFixes ? {} : { fix } - })); + .map(({ description, fix, unprocessedDirective }) => { + const { parentComment, type, line, column } = unprocessedDirective; + + return { + ruleId: null, + message: description + ? `Unused eslint-disable directive (no problems were reported from ${description}).` + : "Unused eslint-disable directive (no problems were reported).", + line: type === "disable-next-line" ? parentComment.commentToken.loc.start.line : line, + column: type === "disable-next-line" ? parentComment.commentToken.loc.start.column + 1 : column, + severity: options.reportUnusedDisableDirectives === "warn" ? 1 : 2, + nodeType: null, + ...options.disableFixes ? {} : { fix } + }; + }); return { problems, unusedDisableDirectives }; } diff --git a/lib/linter/linter.js b/lib/linter/linter.js index f897b8ddb8c..999da0183f3 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -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 }); + } 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 })); } @@ -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({ diff --git a/tests/lib/linter/apply-disable-directives.js b/tests/lib/linter/apply-disable-directives.js index 72708bcbddc..3e24b866c18 100644 --- a/tests/lib/linter/apply-disable-directives.js +++ b/tests/lib/linter/apply-disable-directives.js @@ -25,7 +25,20 @@ const applyDisableDirectives = require("../../../lib/linter/apply-disable-direct */ function createParentComment(range, value, ruleIds = []) { return { - commentToken: { range, value }, + commentToken: { + range, + loc: { + start: { + line: 1, + column: 1 + }, + end: { + line: 1, + column: value ? value.length : 10 + } + }, + value + }, ruleIds }; } @@ -1294,7 +1307,7 @@ describe("apply-disable-directives", () => { parentComment: createParentComment([0, 27]), type: "disable-next-line", line: 1, - column: 1, + column: 2, ruleId: null }], problems: [], @@ -1305,7 +1318,7 @@ describe("apply-disable-directives", () => { ruleId: null, message: "Unused eslint-disable directive (no problems were reported).", line: 1, - column: 1, + column: 2, fix: { range: [0, 27], text: " " diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index 33434614610..2df92e5a6c5 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -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');" @@ -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", () => { @@ -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", @@ -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", @@ -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');" @@ -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", () => { @@ -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", @@ -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", @@ -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", @@ -12047,6 +12198,132 @@ var a = "test2"; ); }); + it("reports problems for unused eslint-disable-next-line comments (in config)", () => { + assert.deepStrictEqual( + linter.verify("// eslint-disable-next-line", { + linterOptions: { + reportUnusedDisableDirectives: true + } + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported).", + line: 1, + column: 1, + fix: { + range: [0, 27], + text: " " + }, + severity: 1, + nodeType: null + } + ] + ); + }); + + it("reports problems for unused multiline eslint-disable-next-line comments (in config)", () => { + assert.deepStrictEqual( + linter.verify("/* \neslint-disable-next-line\n */", { + linterOptions: { + reportUnusedDisableDirectives: true + } + }), + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported).", + line: 1, + column: 1, + fix: { + range: [0, 32], + text: " " + }, + severity: 1, + nodeType: null + } + ] + ); + }); + + it("reports problems for partially unused eslint-disable-next-line comments (in config)", () => { + const code = "// eslint-disable-next-line no-alert, no-redeclare \nalert('test');"; + const config = { + linterOptions: { + reportUnusedDisableDirectives: true + }, + rules: { + "no-alert": 1, + "no-redeclare": 1 + } + }; + + const messages = linter.verify(code, config, { + filename, + allowInlineConfig: true + }); + + assert.deepStrictEqual( + messages, + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from 'no-redeclare').", + line: 1, + column: 1, + fix: { + range: [36, 50], + text: "" + }, + severity: 1, + nodeType: null + } + ] + ); + }); + + it("reports problems for partially unused multiline eslint-disable-next-line comments (in config)", () => { + const code = ` + /* eslint-disable-next-line no-alert, no-redeclare -- + * Here's a very long description about why this configuration is necessary + * along with some additional information + **/ + alert('test'); + `; + const config = { + linterOptions: { + reportUnusedDisableDirectives: true + }, + rules: { + "no-alert": 1, + "no-redeclare": 1 + } + }; + + const messages = linter.verify(code, config, { + filename, + allowInlineConfig: true + }); + + assert.deepStrictEqual( + messages, + [ + { + ruleId: null, + message: "Unused eslint-disable directive (no problems were reported from 'no-redeclare').", + line: 2, + column: 21, + fix: { + range: [57, 71], + text: "" + }, + severity: 1, + nodeType: null + } + ] + ); + }); + describe("autofix", () => { const alwaysReportsRule = { create(context) {