From f466894bbd979dcc37f67f378eda8b99cfa7b605 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sun, 22 Dec 2019 16:53:36 +0900 Subject: [PATCH 1/9] Breaking: description in directive comments (refs eslint/rfcs#33) --- docs/user-guide/configuring.md | 20 +++ lib/linter/linter.js | 18 +- tests/lib/linter/linter.js | 308 +++++++++++++++++++++++++++++++++ 3 files changed, 343 insertions(+), 3 deletions(-) diff --git a/docs/user-guide/configuring.md b/docs/user-guide/configuring.md index 2a47a3e9e3d..bc51018da37 100644 --- a/docs/user-guide/configuring.md +++ b/docs/user-guide/configuring.md @@ -420,6 +420,19 @@ If a rule has additional options, you can specify them using array literal synta This comment specifies the "double" option for the [`quotes`](../rules/quotes) rule. The first item in the array is always the rule severity (number or string). +If the part preceded by `--` exists in the configuration comment, ESLint ignores the part. The `--` can be longer than 2. For examples: + +```js +/* eslint eqeqeq: "off", curly: "error" -- You can clarify the reason of this configuration. */ +``` + +```js +/* eslint eqeqeq: "off", curly: "error" + -------- + You can clarify the reason of this configuration. + But mind that '*' at beginning lines may cause syntax error. */ +``` + ### Using Configuration Files To configure rules inside of a configuration file, use the `rules` key along with an error level and any options you want to use. For example: @@ -574,6 +587,13 @@ foo(); // eslint-disable-line example/rule-name foo(); /* eslint-disable-line example/rule-name */ ``` +If the part preceded by `--` exists in the inline comments, ESLint ignores the part. The `--` can be longer than 2. For examples: + +```js +// eslint-disable-next-line no-console -- this console.log makes fine the program for some reason. +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/linter.js b/lib/linter/linter.js index 6d88cb5aa12..76d35b49eef 100644 --- a/lib/linter/linter.js +++ b/lib/linter/linter.js @@ -267,6 +267,15 @@ function createDisableDirectives(options) { return result; } +/** + * Remove the ignored part from a given directive comment and trim it. + * @param {string} value The comment text to strip. + * @returns {string} The stripped text. + */ +function stripDirectiveComment(value) { + return value.split(/\s-{2,}\s/u)[0].trim(); +} + /** * Parses comments in file to extract file-specific config of rules, globals * and environments and merges them with global config; also code blocks @@ -286,7 +295,7 @@ function getDirectiveComments(filename, ast, ruleMapper, warnInlineConfig) { const disableDirectives = []; ast.comments.filter(token => token.type !== "Shebang").forEach(comment => { - const trimmedCommentText = comment.value.trim(); + const trimmedCommentText = stripDirectiveComment(comment.value); const match = /^(eslint(?:-env|-enable|-disable(?:(?:-next)?-line)?)?|exported|globals?)(?:\s|$)/u.exec(trimmedCommentText); if (!match) { @@ -444,8 +453,11 @@ function findEslintEnv(text) { eslintEnvPattern.lastIndex = 0; - while ((match = eslintEnvPattern.exec(text))) { - retv = Object.assign(retv || {}, commentParser.parseListConfig(match[1])); + while ((match = eslintEnvPattern.exec(text)) !== null) { + retv = Object.assign( + retv || {}, + commentParser.parseListConfig(stripDirectiveComment(match[1])) + ); } return retv; diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index e444aea920c..ce9a52fe6d2 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -3759,6 +3759,314 @@ describe("Linter", () => { linter.verify("x", { rules: { "foo-bar-baz": "error" } }); assert(spy.calledOnce); }); + + describe("comments in directive comments", () => { + it("should ignore the part preceded by '--' in '/*eslint*/'.", () => { + const aaa = sinon.stub().returns({}); + const bbb = sinon.stub().returns({}); + + linter.defineRule("aaa", { create: aaa }); + linter.defineRule("bbb", { create: bbb }); + const messages = linter.verify(` + /*eslint aaa:error -- bbb:error */ + console.log("hello") + `, {}); + + // Don't include syntax error of the comment. + assert.deepStrictEqual(messages, []); + + // Use only `aaa`. + assert.strictEqual(aaa.callCount, 1); + assert.strictEqual(bbb.callCount, 0); + }); + + it("should ignore the part preceded by '--' in '/*eslint-env*/'.", () => { + const messages = linter.verify(` + /*eslint-env es2015 -- es2017 */ + var Promise = {} + var Atomics = {} + `, { rules: { "no-redeclare": "error" } }); + + // Don't include `Atomics` + assert.deepStrictEqual( + messages, + [{ + column: 25, + endColumn: 32, + endLine: 3, + line: 3, + message: "'Promise' is already defined as a built-in global variable.", + messageId: "redeclaredAsBuiltin", + nodeType: "Identifier", + ruleId: "no-redeclare", + severity: 2 + }] + ); + }); + + it("should ignore the part preceded by '--' in '/*global*/'.", () => { + const messages = linter.verify(` + /*global aaa -- bbb */ + var aaa = {} + var bbb = {} + `, { rules: { "no-redeclare": "error" } }); + + // Don't include `bbb` + assert.deepStrictEqual( + messages, + [{ + column: 30, + line: 2, + message: "'aaa' is already defined by a variable declaration.", + messageId: "redeclaredBySyntax", + nodeType: "Block", + ruleId: "no-redeclare", + severity: 2 + }] + ); + }); + + it("should ignore the part preceded by '--' in '/*globals*/'.", () => { + const messages = linter.verify(` + /*globals aaa -- bbb */ + var aaa = {} + var bbb = {} + `, { rules: { "no-redeclare": "error" } }); + + // Don't include `bbb` + assert.deepStrictEqual( + messages, + [{ + column: 31, + line: 2, + message: "'aaa' is already defined by a variable declaration.", + messageId: "redeclaredBySyntax", + nodeType: "Block", + ruleId: "no-redeclare", + severity: 2 + }] + ); + }); + + it("should ignore the part preceded by '--' in '/*exported*/'.", () => { + const messages = linter.verify(` + /*exported aaa -- bbb */ + var aaa = {} + var bbb = {} + `, { rules: { "no-unused-vars": "error" } }); + + // Don't include `aaa` + assert.deepStrictEqual( + messages, + [{ + column: 25, + endColumn: 28, + endLine: 4, + line: 4, + message: "'bbb' is assigned a value but never used.", + nodeType: "Identifier", + ruleId: "no-unused-vars", + severity: 2 + }] + ); + }); + + it("should ignore the part preceded by '--' in '/*eslint-disable*/'.", () => { + const messages = linter.verify(` + /*eslint-disable no-redeclare -- no-unused-vars */ + var aaa = {} + var aaa = {} + `, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } }); + + // Do include `no-unused-vars` but not `no-redeclare` + assert.deepStrictEqual( + messages, + [{ + column: 25, + endLine: 3, + endColumn: 28, + line: 3, + message: "'aaa' is assigned a value but never used.", + nodeType: "Identifier", + ruleId: "no-unused-vars", + severity: 2 + }] + ); + }); + + it("should ignore the part preceded by '--' in '/*eslint-enable*/'.", () => { + const messages = linter.verify(` + /*eslint-disable no-redeclare, no-unused-vars */ + /*eslint-enable no-redeclare -- no-unused-vars */ + var aaa = {} + var aaa = {} + `, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } }); + + // Do include `no-redeclare` but not `no-unused-vars` + assert.deepStrictEqual( + messages, + [{ + column: 25, + endLine: 5, + endColumn: 28, + line: 5, + message: "'aaa' is already defined.", + messageId: "redeclared", + nodeType: "Identifier", + ruleId: "no-redeclare", + severity: 2 + }] + ); + }); + + it("should ignore the part preceded by '--' in '//eslint-disable-line'.", () => { + const messages = linter.verify(` + var aaa = {} //eslint-disable-line no-redeclare -- no-unused-vars + var aaa = {} //eslint-disable-line no-redeclare -- no-unused-vars + `, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } }); + + // Do include `no-unused-vars` but not `no-redeclare` + assert.deepStrictEqual( + messages, + [{ + column: 25, + endLine: 2, + endColumn: 28, + line: 2, + message: "'aaa' is assigned a value but never used.", + nodeType: "Identifier", + ruleId: "no-unused-vars", + severity: 2 + }] + ); + }); + + it("should ignore the part preceded by '--' in '/*eslint-disable-line*/'.", () => { + const messages = linter.verify(` + var aaa = {} /*eslint-disable-line no-redeclare -- no-unused-vars */ + var aaa = {} /*eslint-disable-line no-redeclare -- no-unused-vars */ + `, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } }); + + // Do include `no-unused-vars` but not `no-redeclare` + assert.deepStrictEqual( + messages, + [{ + column: 25, + endLine: 2, + endColumn: 28, + line: 2, + message: "'aaa' is assigned a value but never used.", + nodeType: "Identifier", + ruleId: "no-unused-vars", + severity: 2 + }] + ); + }); + + it("should ignore the part preceded by '--' in '//eslint-disable-next-line'.", () => { + const messages = linter.verify(` + //eslint-disable-next-line no-redeclare -- no-unused-vars + var aaa = {} + //eslint-disable-next-line no-redeclare -- no-unused-vars + var aaa = {} + `, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } }); + + // Do include `no-unused-vars` but not `no-redeclare` + assert.deepStrictEqual( + messages, + [{ + column: 25, + endLine: 3, + endColumn: 28, + line: 3, + message: "'aaa' is assigned a value but never used.", + nodeType: "Identifier", + ruleId: "no-unused-vars", + severity: 2 + }] + ); + }); + + it("should ignore the part preceded by '--' in '/*eslint-disable-next-line*/'.", () => { + const messages = linter.verify(` + /*eslint-disable-next-line no-redeclare -- no-unused-vars */ + var aaa = {} + /*eslint-disable-next-line no-redeclare -- no-unused-vars */ + var aaa = {} + `, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } }); + + // Do include `no-unused-vars` but not `no-redeclare` + assert.deepStrictEqual( + messages, + [{ + column: 25, + endLine: 3, + endColumn: 28, + line: 3, + message: "'aaa' is assigned a value but never used.", + nodeType: "Identifier", + ruleId: "no-unused-vars", + severity: 2 + }] + ); + }); + + it("should not ignore the part preceded by '--' if the '--' is not surrounded by whitespaces.", () => { + const rule = sinon.stub().returns({}); + + linter.defineRule("a--rule", { create: rule }); + const messages = linter.verify(` + /*eslint a--rule:error */ + console.log("hello") + `, {}); + + // Don't include syntax error of the comment. + assert.deepStrictEqual(messages, []); + + // Use `a--rule`. + assert.strictEqual(rule.callCount, 1); + }); + + it("should ignore the part preceded by '--' even if the '--' is longer than 2.", () => { + const aaa = sinon.stub().returns({}); + const bbb = sinon.stub().returns({}); + + linter.defineRule("aaa", { create: aaa }); + linter.defineRule("bbb", { create: bbb }); + const messages = linter.verify(` + /*eslint aaa:error -------- bbb:error */ + console.log("hello") + `, {}); + + // Don't include syntax error of the comment. + assert.deepStrictEqual(messages, []); + + // Use only `aaa`. + assert.strictEqual(aaa.callCount, 1); + assert.strictEqual(bbb.callCount, 0); + }); + + it("should ignore the part preceded by '--' with line breaks.", () => { + const aaa = sinon.stub().returns({}); + const bbb = sinon.stub().returns({}); + + linter.defineRule("aaa", { create: aaa }); + linter.defineRule("bbb", { create: bbb }); + const messages = linter.verify(` + /*eslint aaa:error + -------- + bbb:error */ + console.log("hello") + `, {}); + + // Don't include syntax error of the comment. + assert.deepStrictEqual(messages, []); + + // Use only `aaa`. + assert.strictEqual(aaa.callCount, 1); + assert.strictEqual(bbb.callCount, 0); + }); + }); }); describe("context.getScope()", () => { From ed9ca64753f33aef1d90337b82b6b45ae3c8a3b8 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 24 Dec 2019 15:17:24 +0900 Subject: [PATCH 2/9] Update docs/user-guide/configuring.md Co-Authored-By: Kai Cataldo --- docs/user-guide/configuring.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/configuring.md b/docs/user-guide/configuring.md index bc51018da37..7f59456af57 100644 --- a/docs/user-guide/configuring.md +++ b/docs/user-guide/configuring.md @@ -420,7 +420,7 @@ If a rule has additional options, you can specify them using array literal synta This comment specifies the "double" option for the [`quotes`](../rules/quotes) rule. The first item in the array is always the rule severity (number or string). -If the part preceded by `--` exists in the configuration comment, ESLint ignores the part. The `--` can be longer than 2. For examples: +Configuration comments can include descriptions to explain why the comment is necessary. The description must occur after the configuration is separated from the comment by at two or more consecutive `-` characters. For example: ```js /* eslint eqeqeq: "off", curly: "error" -- You can clarify the reason of this configuration. */ From 4246f02438a220fc48ad5ac8bc2e9b5f10056f8d Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 24 Dec 2019 15:17:31 +0900 Subject: [PATCH 3/9] Update docs/user-guide/configuring.md Co-Authored-By: Kai Cataldo --- docs/user-guide/configuring.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/configuring.md b/docs/user-guide/configuring.md index 7f59456af57..024b28edc16 100644 --- a/docs/user-guide/configuring.md +++ b/docs/user-guide/configuring.md @@ -423,7 +423,7 @@ This comment specifies the "double" option for the [`quotes`](../rules/quotes) r Configuration comments can include descriptions to explain why the comment is necessary. The description must occur after the configuration is separated from the comment by at two or more consecutive `-` characters. For example: ```js -/* eslint eqeqeq: "off", curly: "error" -- You can clarify the reason of this configuration. */ +/* eslint eqeqeq: "off", curly: "error" -- Here's a description about why this configuration is necessary. */ ``` ```js From eb32ec737f466b8ffc1237685f93b4b274742f0d Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 24 Dec 2019 15:17:40 +0900 Subject: [PATCH 4/9] Update docs/user-guide/configuring.md Co-Authored-By: Kai Cataldo --- docs/user-guide/configuring.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/configuring.md b/docs/user-guide/configuring.md index 024b28edc16..9e49be53777 100644 --- a/docs/user-guide/configuring.md +++ b/docs/user-guide/configuring.md @@ -429,7 +429,7 @@ Configuration comments can include descriptions to explain why the comment is ne ```js /* eslint eqeqeq: "off", curly: "error" -------- - You can clarify the reason of this configuration. + Here's a description about why this configuration is necessary. But mind that '*' at beginning lines may cause syntax error. */ ``` From 7808a32569e5523cf9913b0ee43c0e5fcda12d49 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 24 Dec 2019 15:17:50 +0900 Subject: [PATCH 5/9] Update docs/user-guide/configuring.md Co-Authored-By: Kai Cataldo --- docs/user-guide/configuring.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/configuring.md b/docs/user-guide/configuring.md index 9e49be53777..b650bd3f3bd 100644 --- a/docs/user-guide/configuring.md +++ b/docs/user-guide/configuring.md @@ -590,7 +590,7 @@ foo(); /* eslint-disable-line example/rule-name */ If the part preceded by `--` exists in the inline comments, ESLint ignores the part. The `--` can be longer than 2. For examples: ```js -// eslint-disable-next-line no-console -- this console.log makes fine the program for some reason. +// eslint-disable-next-line no-console -- Here's a description about why this configuration is necessary. console.log('hello'); ``` From 057befe355840425beeffb005153569040978705 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 24 Dec 2019 15:18:05 +0900 Subject: [PATCH 6/9] Update docs/user-guide/configuring.md Co-Authored-By: Kai Cataldo --- docs/user-guide/configuring.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/configuring.md b/docs/user-guide/configuring.md index b650bd3f3bd..866809cd0df 100644 --- a/docs/user-guide/configuring.md +++ b/docs/user-guide/configuring.md @@ -587,7 +587,7 @@ foo(); // eslint-disable-line example/rule-name foo(); /* eslint-disable-line example/rule-name */ ``` -If the part preceded by `--` exists in the inline comments, ESLint ignores the part. The `--` can be longer than 2. For examples: +Configuration comments can include descriptions to explain why the comment is necessary. The description must occur after the configuration is separated from the comment by at two or more consecutive `-` characters. For example: ```js // eslint-disable-next-line no-console -- Here's a description about why this configuration is necessary. From b6d61d6a4a4e0070b088d16452c943b69adb54d4 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 24 Dec 2019 15:18:15 +0900 Subject: [PATCH 7/9] Update tests/lib/linter/linter.js Co-Authored-By: Kai Cataldo --- tests/lib/linter/linter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js index ce9a52fe6d2..71bfd262a79 100644 --- a/tests/lib/linter/linter.js +++ b/tests/lib/linter/linter.js @@ -3760,7 +3760,7 @@ describe("Linter", () => { assert(spy.calledOnce); }); - describe("comments in directive comments", () => { + describe("descriptions in directive comments", () => { it("should ignore the part preceded by '--' in '/*eslint*/'.", () => { const aaa = sinon.stub().returns({}); const bbb = sinon.stub().returns({}); From 01548bfa3586adf1416a1726e8bd60917f416273 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 7 Jan 2020 20:17:33 +0900 Subject: [PATCH 8/9] Update docs/user-guide/configuring.md Co-Authored-By: Kai Cataldo --- docs/user-guide/configuring.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/configuring.md b/docs/user-guide/configuring.md index 866809cd0df..11dd88ea1e2 100644 --- a/docs/user-guide/configuring.md +++ b/docs/user-guide/configuring.md @@ -420,7 +420,7 @@ If a rule has additional options, you can specify them using array literal synta This comment specifies the "double" option for the [`quotes`](../rules/quotes) rule. The first item in the array is always the rule severity (number or string). -Configuration comments can include descriptions to explain why the comment is necessary. The description must occur after the configuration is separated from the comment by at two or more consecutive `-` characters. For example: +Configuration comments can include descriptions to explain why the comment is necessary. The description must occur after the configuration and is separated from the configuration by two or more consecutive `-` characters. For example: ```js /* eslint eqeqeq: "off", curly: "error" -- Here's a description about why this configuration is necessary. */ From df856999fd6da300b632f28c013c9667849f7234 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 7 Jan 2020 20:23:02 +0900 Subject: [PATCH 9/9] update configuring.md --- docs/user-guide/configuring.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/user-guide/configuring.md b/docs/user-guide/configuring.md index 11dd88ea1e2..aa98b657199 100644 --- a/docs/user-guide/configuring.md +++ b/docs/user-guide/configuring.md @@ -429,8 +429,14 @@ Configuration comments can include descriptions to explain why the comment is ne ```js /* eslint eqeqeq: "off", curly: "error" -------- - Here's a description about why this configuration is necessary. - But mind that '*' at beginning lines may cause syntax error. */ + Here's a description about why this configuration is necessary. */ +``` + +```js +/* eslint eqeqeq: "off", curly: "error" + * -------- + * This will not work due to the line above starting with a '*' character. + */ ``` ### Using Configuration Files @@ -587,7 +593,7 @@ foo(); // eslint-disable-line example/rule-name foo(); /* eslint-disable-line example/rule-name */ ``` -Configuration comments can include descriptions to explain why the comment is necessary. The description must occur after the configuration is separated from the comment by at two or more consecutive `-` characters. For example: +Configuration comments can include descriptions to explain why the comment is necessary. The description must occur after the configuration and is separated from the configuration by two or more consecutive `-` characters. For example: ```js // eslint-disable-next-line no-console -- Here's a description about why this configuration is necessary.