Breaking: allow all directives in line comments (fixes #14575) (#14656)

* Breaking: allow all directives in line comments (fixes #14575) Chore: fix linting problems Update linter.js Update linter.js Update: support eslint-env fix: rm "\n" in the regex * Update lib/linter/linter.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * chore: add more tests * chore: rename var to mustBeOnSingleLine * fix: false postives of no-inline-comments & no-warning-comments * Update lib/rules/utils/ast-utils.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update ast-utils.js * chore: add some tests * chore: move a var def to outer scope * chore: add //eslint-enable tests * chore: add more tests for //eslint-env * Update lib/rules/utils/ast-utils.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * Update tests/lib/rules/no-warning-comments.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * chore: add more tests (+1 squashed commit) Squashed commits: [9a4c7d7] chore: add more tests * docs: update config comments usage * Update ast-utils.js * Update ast-utils.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
eslint · Aug 5, 2021 · 4c841b8 · 4c841b8
1 parent 6bd747b
commit 4c841b8
Show file tree

Hide file tree

Showing 15 changed files with 206 additions and 45 deletions.
diff --git a/docs/rules/no-unused-vars.md b/docs/rules/no-unused-vars.md
@@ -82,24 +82,28 @@ function getY([, y]) {
 
 ### exported
 
-In environments outside of CommonJS or ECMAScript modules, you may use `var` to create a global variable that may be used by other scripts. You can use the `/* exported variableName */` comment block to indicate that this variable is being exported and therefore should not be considered unused.
+In environments outside of CommonJS or ECMAScript modules, you may use `var` to create a global variable that may be used by other scripts. You can use the comment `/* exported variableName */` or `// exported variableName` to indicate that this variable is being exported and therefore should not be considered unused.
 
-Note that `/* exported */` has no effect for any of the following:
+Note that the comment has no effect for any of the following:
 
 * when the environment is `node` or `commonjs`
 * when `parserOptions.sourceType` is `module`
 * when `ecmaFeatures.globalReturn` is `true`
 
-The line comment `// exported variableName` will not work as `exported` is not line-specific.
-
-Examples of **correct** code for `/* exported variableName */` operation:
+Examples of **correct** code for `exported` operation:
 
 ```js
 /* exported global_var */
 
 var global_var = 42;
 ```
 
+```js
+// exported global_var
+
+var global_var = 42;
+```
+
 ## Options
 
 This rule takes one argument which can be a string or an object. The string settings are the same as those of the `vars` property (explained below).

diff --git a/docs/user-guide/configuring/language-options.md b/docs/user-guide/configuring/language-options.md
@@ -44,12 +44,16 @@ Environments can be specified inside of a file, in configuration files or using
 
 ### Using configuration comments
 
-To specify environments using a comment inside of your JavaScript file, use the following format:
+To specify environments using a line or block comment inside of your JavaScript file, use the following format:
 
 ```js
 /* eslint-env node, mocha */
 ```
 
+```js
+// eslint-env node, mocha
+```
+
 This enables Node.js and Mocha environments.
 
 ### Using configuration files
@@ -123,12 +127,16 @@ Some of ESLint's core rules rely on knowledge of the global variables available
 
 ### Using configuration comments
 
-To specify globals using a comment inside of your JavaScript file, use the following format:
+To specify globals using a line or block comment inside of your JavaScript file, use the following format:
 
 ```js
 /* global var1, var2 */
 ```
 
+```js
+// global var1, var2
+```
+
 This defines two global variables, `var1` and `var2`. If you want to optionally specify that these global variables can be written to (rather than only being read), then you can set each with a `"writable"` flag:
 
 ```js

diff --git a/docs/user-guide/configuring/rules.md b/docs/user-guide/configuring/rules.md
@@ -14,12 +14,16 @@ ESLint comes with a large number of built-in rules and you can add more rules th
 
 ### Using configuration comments
 
-To configure rules inside of a file using configuration comments, use a comment in the following format:
+To configure rules inside of a file using configuration comments, use a line or block comment in the following format:
 
 ```js
 /* eslint eqeqeq: "off", curly: "error" */
 ```
 
+```js
+// eslint eqeqeq: "off", curly: "error"
+```
+
 In this example, [`eqeqeq`](https://eslint.org/docs/rules/eqeqeq) is turned off and [`curly`](.https://eslint.org/docs/rules/curly) is turned on as an error. You can also use the numeric equivalent for the rule severity:
 
 ```js
@@ -124,7 +128,7 @@ In these configuration files, the rule `plugin1/rule1` comes from the plugin nam
 
 ### Using configuration comments
 
-To temporarily disable rule warnings in your file, use block comments in the following format:
+To temporarily disable rule warnings in your file, use line or block comments in the following format:
 
 ```js
 /* eslint-disable */
@@ -134,6 +138,14 @@ alert('foo');
 /* eslint-enable */
 ```
 
+```js
+// eslint-disable
+
+alert('foo');
+
+// eslint-enable
+```
+
 You can also disable or enable warnings for specific rules:
 
 ```js
@@ -145,7 +157,7 @@ console.log('bar');
 /* eslint-enable no-alert, no-console */
 ```
 
-To disable rule warnings in an entire file, put a `/* eslint-disable */` block comment at the top of the file:
+To disable rule warnings in an entire file, put a `/* eslint-disable */` or `// eslint-disable` at the top of the file:
 
 ```js
 /* eslint-disable */

diff --git a/lib/linter/linter.js b/lib/linter/linter.js
@@ -307,11 +307,7 @@ function getDirectiveComments(filename, ast, ruleMapper, warnInlineConfig) {
             return;
         }
         const directiveText = match[1];
-        const lineCommentSupported = /^eslint-disable-(next-)?line$/u.test(directiveText);
-
-        if (comment.type === "Line" && !lineCommentSupported) {
-            return;
-        }
+        const mustBeOnSingleLine = /^eslint-disable-(next-)?line$/u.test(directiveText);
 
         if (warnInlineConfig) {
             const kind = comment.type === "Block" ? `/*${directiveText}*/` : `//${directiveText}`;
@@ -325,7 +321,7 @@ function getDirectiveComments(filename, ast, ruleMapper, warnInlineConfig) {
             return;
         }
 
-        if (lineCommentSupported && comment.loc.start.line !== comment.loc.end.line) {
+        if (mustBeOnSingleLine && comment.loc.start.line !== comment.loc.end.line) {
             const message = `${directiveText} comment should not span multiple lines.`;
 
             problems.push(createLintingProblem({
@@ -452,7 +448,10 @@ function normalizeEcmaVersion(parser, ecmaVersion) {
     return ecmaVersion >= 2015 ? ecmaVersion - 2009 : ecmaVersion;
 }
 
-const eslintEnvPattern = /\/\*\s*eslint-env\s(.+?)\*\//gsu;
+const eslintEnvPatterns = [
+    /\/\*\s*eslint-env\s(.+?)\*\//gsu, // block comments
+    /\/\/[^\S\r\n\u2028\u2029]*eslint-env[^\S\r\n\u2028\u2029](.+)/gu // line comments
+];
 
 /**
  * Checks whether or not there is a comment which has "eslint-env *" in a given text.
@@ -462,15 +461,16 @@ const eslintEnvPattern = /\/\*\s*eslint-env\s(.+?)\*\//gsu;
 function findEslintEnv(text) {
     let match, retv;
 
-    eslintEnvPattern.lastIndex = 0;
+    for (const eslintEnvPattern of eslintEnvPatterns) {
+        eslintEnvPattern.lastIndex = 0;
 
-    while ((match = eslintEnvPattern.exec(text)) !== null) {
-        retv = Object.assign(
-            retv || {},
-            commentParser.parseListConfig(stripDirectiveComment(match[1]))
-        );
+        while ((match = eslintEnvPattern.exec(text)) !== null) {
+            retv = Object.assign(
+                retv || {},
+                commentParser.parseListConfig(stripDirectiveComment(match[1]))
+            );
+        }
     }
-
     return retv;
 }
 

diff --git a/lib/rules/newline-before-return.js b/lib/rules/newline-before-return.js
@@ -134,7 +134,7 @@ module.exports = {
             if (tokenBefore) {
                 lineNumTokenBefore = tokenBefore.loc.end.line;
             } else {
-                lineNumTokenBefore = 0; // global return at beginning of script
+                lineNumTokenBefore = 0; // Global return at beginning of script
             }
 
             return lineNumTokenBefore;

diff --git a/lib/rules/utils/ast-utils.js b/lib/rules/utils/ast-utils.js
@@ -875,14 +875,7 @@ module.exports = {
     isDirectiveComment(node) {
         const comment = node.value.trim();
 
-        return (
-            node.type === "Line" && comment.indexOf("eslint-") === 0 ||
-            node.type === "Block" && (
-                comment.indexOf("global ") === 0 ||
-                comment.indexOf("eslint ") === 0 ||
-                comment.indexOf("eslint-") === 0
-            )
-        );
+        return /^(?:eslint[- ]|(?:globals?|exported) )/u.test(comment);
     },
 
     /**

diff --git a/tests/lib/linter/linter.js b/tests/lib/linter/linter.js
@@ -23,6 +23,7 @@ const { Linter } = require("../../../lib/linter");
 
 const TEST_CODE = "var answer = 6 * 7;",
     BROKEN_TEST_CODE = "var;";
+const linebreaks = ["\n", "\r\n", "\r", "\u2028", "\u2029"];
 
 //------------------------------------------------------------------------------
 // Helpers
@@ -1360,15 +1361,16 @@ describe("Linter", () => {
     describe("when evaluating code containing a line comment", () => {
         const code = "//global a \n function f() {}";
 
-        it("should not introduce a global variable", () => {
+        it("should introduce a global variable", () => {
             const config = { rules: { checker: "error" } };
             let spy;
 
             linter.defineRule("checker", context => {
                 spy = sinon.spy(() => {
                     const scope = context.getScope();
+                    const a = getVariable(scope, "a");
 
-                    assert.strictEqual(getVariable(scope, "a"), null);
+                    assert.strictEqual(a.eslintExplicitGlobal, true);
                 });
 
                 return { Program: spy };
@@ -5521,4 +5523,122 @@ var a = "test2";
             assert.strictEqual(messages.length, 0);
         });
     });
+
+    // https://github.com/eslint/eslint/issues/14575
+    describe("directives in line comments", () => {
+        let messages;
+
+        it("//eslint-disable", () => {
+            const codes = [
+                ...linebreaks.map(linebreak => `//eslint-disable no-debugger${linebreak}debugger;`),
+                ...linebreaks.map(linebreak => `// eslint-disable no-debugger${linebreak}debugger;`)
+            ];
+
+            for (const code of codes) {
+                messages = linter.verify(code, { rules: { "no-debugger": 2 } });
+                assert.strictEqual(messages.length, 0);
+
+                messages = linter.verify(code, { rules: { "no-debugger": 0 } });
+                assert.strictEqual(messages.length, 0);
+            }
+        });
+
+        it("//eslint-enable", () => {
+            const codes = [
+                ...linebreaks.map(linebreak => `//eslint-disable no-debugger${linebreak}//eslint-enable no-debugger${linebreak}debugger;`),
+                ...linebreaks.map(linebreak => `// eslint-disable no-debugger${linebreak}//eslint-enable no-debugger${linebreak}debugger;`)
+            ];
+
+            for (const code of codes) {
+                messages = linter.verify(code, { rules: { "no-debugger": 2 } });
+                assert.strictEqual(messages.length, 1);
+            }
+        });
+
+        it("//eslint", () => {
+            const codes = [
+                ...linebreaks.map(linebreak => `//eslint no-debugger:'error'${linebreak}debugger;`),
+                ...linebreaks.map(linebreak => `// eslint no-debugger:'error'${linebreak}debugger;`)
+            ];
+
+            for (const code of codes) {
+                messages = linter.verify(code, { rules: { "no-debugger": 0 } });
+                assert.strictEqual(messages.length, 1);
+                messages = linter.verify(code, { rules: { "no-debugger": 2 } });
+                assert.strictEqual(messages.length, 1);
+            }
+        });
+
+        it("//global(s)", () => {
+            const config = { rules: { "no-undef": 2 } };
+            const codes = [
+                ...linebreaks.map(linebreak => `//globals foo: true${linebreak}foo;`),
+                ...linebreaks.map(linebreak => `// globals foo: true${linebreak}foo;`),
+                ...linebreaks.map(linebreak => `//global foo: true${linebreak}foo;`),
+                ...linebreaks.map(linebreak => `// global foo: true${linebreak}foo;`)
+            ];
+
+            for (const code of codes) {
+                messages = linter.verify(code, config, filename);
+                assert.strictEqual(messages.length, 0);
+            }
+        });
+        it("//exported", () => {
+            const codes = [
+                ...linebreaks.map(linebreak => `//exported foo${linebreak}var foo = 0;`),
+                ...linebreaks.map(linebreak => `// exported foo${linebreak}var foo = 0;`)
+            ];
+            const config = { rules: { "no-unused-vars": 2 } };
+
+            for (const code of codes) {
+                messages = linter.verify(code, config, filename);
+                assert.strictEqual(messages.length, 0);
+            }
+        });
+
+        describe("//eslint-env", () => {
+            const config = { rules: { "no-undef": 2 } };
+
+            it("enable one env with different line breaks", () => {
+                const codes = [
+                    ...linebreaks.map(linebreak => `//${ESLINT_ENV} browser${linebreak}window;`),
+                    ...linebreaks.map(linebreak => `//  ${ESLINT_ENV}  browser${linebreak}window;`),
+                    `window;//${ESLINT_ENV} browser` // no linebreaks
+                ];
+
+                for (const code of codes) {
+                    messages = linter.verify(code, config, filename);
+                    assert.strictEqual(messages.length, 0);
+                }
+            });
+
+            it("multiple envs enabled with different line breaks", () => {
+                const codes = [
+                    ...linebreaks.map(linebreak => `//${ESLINT_ENV} browser,es6${linebreak}window;Promise;`),
+                    ...linebreaks.map(linebreak => `// ${ESLINT_ENV} browser,es6${linebreak}window;Promise;`),
+                    ...linebreaks.map(linebreak => `//${ESLINT_ENV} browser${linebreak}//${ESLINT_ENV} es6${linebreak}window;Promise;`),
+                    ...linebreaks.map(linebreak => `// ${ESLINT_ENV} browser${linebreak}//${ESLINT_ENV} es6${linebreak}window;Promise;`)
+                ];
+
+                for (const code of codes) {
+                    messages = linter.verify(code, config, filename);
+                    assert.strictEqual(messages.length, 0);
+                }
+            });
+
+            it("no env enabled with different linebreaks", () => {
+                const codes = [
+                    ...linebreaks.map(linebreak => `//${ESLINT_ENV}${linebreak}browser${linebreak}window;`),
+                    ...linebreaks.map(linebreak => `// ${ESLINT_ENV}${linebreak}browser${linebreak}window;`),
+                    ...linebreaks.map(linebreak => `//${ESLINT_ENV}browser${linebreak}window;window;`),
+                    ...linebreaks.map(linebreak => `// ${ESLINT_ENV}browser${linebreak}window;window;`)
+                ];
+
+                for (const code of codes) {
+                    messages = linter.verify(code, config, filename);
+                    assert.strictEqual(messages.length, 2);
+                }
+            });
+        });
+    });
 });
diff --git a/tests/lib/rules/id-blacklist.js b/tests/lib/rules/id-blacklist.js
@@ -1010,7 +1010,7 @@ ruleTester.run("id-blacklist", rule, {
             ]
         },
 
-        // globals declared in the given source code are not excluded from consideration
+        // Globals declared in the given source code are not excluded from consideration
         {
             code: "const foo = 1; bar = foo;",
             options: ["foo"],

diff --git a/tests/lib/rules/id-denylist.js b/tests/lib/rules/id-denylist.js
@@ -1022,7 +1022,7 @@ ruleTester.run("id-denylist", rule, {
             ]
         },
 
-        // globals declared in the given source code are not excluded from consideration
+        // Globals declared in the given source code are not excluded from consideration
         {
             code: "const foo = 1; bar = foo;",
             options: ["foo"],

diff --git a/tests/lib/rules/no-inline-comments.js b/tests/lib/rules/no-inline-comments.js
@@ -40,6 +40,16 @@ ruleTester.run("no-inline-comments", rule, {
         "var a = 1; // eslint-disable-line no-debugger",
         "var a = 1; /* eslint-disable-line no-debugger */",
 
+        // https://github.com/eslint/eslint/issues/14575
+        "var i;//eslint no-debugger:2",
+        "var i;// eslint no-debugger:2",
+        "var i;//eslint-disable no-debugger",
+        "var i;// eslint-disable no-debugger",
+        "var i;//eslint-enable no-debugger",
+        "var i;// eslint-enable no-debugger",
+        "var i;//global foo",
+        "var i;// global foo",
+
         // JSX exception
         `var a = (
             <div>