Repository: eslint/eslint
Pull request #11093 (2018-11-17 05:20:38)

Docs: no-console#When-Not-To-Use provides incorrect rule snippet
**What is the purpose of this pull request? (put an "X" next to item)**

- [x] Documentation update
- [ ] Bug fix ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/bug-report.md))
- [ ] New rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-proposal.md))
- [ ] Changes an existing rule ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/rule-change-proposal.md))
- [ ] Add autofixing to a rule
- [ ] Add a CLI option
- [ ] Add something to the core
- [ ] Other, please explain:

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (https://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**

The snippet provided in [no-console#When Not To Use It](https://eslint.org/docs/rules/no-console#when-not-to-use-it) aims to achieve "only receiving errors for console calls" but the selector results in a warning for `console.log()` instead of `console.foobar()`. 
<img width="1019" alt="screen shot 2018-11-17 at 12 17 05 pm" src="https://user-images.githubusercontent.com/12410942/48657361-a32f0200-ea6a-11e8-872d-c0a0d9fcff3b.png">

To fix it, the 'unexpected property' should be  selected with `!=` instead of `=`. Also, turning off "no-console" is still required to achieve "only receiving errors for console calls with unexpected property"

After change:
`console.log()` is ok
`console.logs()` is warned
<img width="1023" alt="default" src="https://user-images.githubusercontent.com/12410942/48656812-ef774380-ea64-11e8-8d31-d7c6876bfd72.png">


**Is there anything you'd like reviewers to focus on?**





Patch:
diff --git a/docs/rules/no-console.md b/docs/rules/no-console.md
index 1c36a85f9..bc163b937 100644
--- a/docs/rules/no-console.md
+++ b/docs/rules/no-console.md
@@ -78,10 +78,11 @@ However, you might not want to manually add `eslint-disable-next-line` or `eslin
 ```json
 {
     "rules": {
+        "no-console": "off",
         "no-restricted-syntax": [
             "error",
             {
-                "selector": "CallExpression[callee.object.name='console'][callee.property.name=/^(log|warn|error|info|trace)$/]",
+                "selector": "CallExpression[callee.object.name='console'][callee.property.name!=/^(log|warn|error|info|trace)$/]",
                 "message": "Unexpected property on console object was called"
             }
         ]
diff --git a/lib/linter.js b/lib/linter.js
index edcf74106..84a53784a 100644
--- a/lib/linter.js
+++ b/lib/linter.js
@@ -11,7 +11,6 @@
 
 const eslintScope = require("eslint-scope"),
     evk = require("eslint-visitor-keys"),
-    levn = require("levn"),
     lodash = require("lodash"),
     CodePathAnalyzer = require("./code-path-analysis/code-path-analyzer"),
     ConfigOps = require("./config/config-ops"),
@@ -25,6 +24,7 @@ const eslintScope = require("eslint-scope"),
     createReportTranslator = require("./report-translator"),
     Rules = require("./rules"),
     timing = require("./util/timing"),
+    ConfigCommentParser = require("./util/config-comment-parser"),
     astUtils = require("./util/ast-utils"),
     pkg = require("../package.json"),
     SourceCodeFixer = require("./util/source-code-fixer");
@@ -32,6 +32,7 @@ const eslintScope = require("eslint-scope"),
 const debug = require("debug")("eslint:linter");
 const MAX_AUTOFIX_PASSES = 10;
 const DEFAULT_PARSER_NAME = "espree";
+const commentParser = new ConfigCommentParser();
 
 //------------------------------------------------------------------------------
 // Typedefs
@@ -59,112 +60,6 @@ const DEFAULT_PARSER_NAME = "espree";
 // Helpers
 //------------------------------------------------------------------------------
 
-/**
- * Parses a list of "name:boolean_value" or/and "name" options divided by comma or
- * whitespace.
- * @param {string} string The string to parse.
- * @param {Comment} comment The comment node which has the string.
- * @returns {Object} Result map object of names and boolean values
- */
-function parseBooleanConfig(string, comment) {
-    const items = {};
-
-    // Collapse whitespace around `:` and `,` to make parsing easier
-    const trimmedString = string.replace(/\s*([:,])\s*/g, "$1");
-
-    trimmedString.split(/\s|,+/).forEach(name => {
-        if (!name) {
-            return;
-        }
-
-        // value defaults to "false" (if not provided), e.g: "foo" => ["foo", "false"]
-        const [key, value = "false"] = name.split(":");
-
-        items[key] = {
-            value: value === "true",
-            comment
-        };
-    });
-    return items;
-}
-
-/**
- * Parses a JSON-like config.
- * @param {string} string The string to parse.
- * @param {Object} location Start line and column of comments for potential error message.
- * @returns {({success: true, config: Object}|{success: false, error: Problem})} Result map object
- */
-function parseJsonConfig(string, location) {
-    let items = {};
-
-    // Parses a JSON-like comment by the same way as parsing CLI option.
-    try {
-        items = levn.parse("Object", string) || {};
-
-        // Some tests say that it should ignore invalid comments such as `/*eslint no-alert:abc*/`.
-        // Also, commaless notations have invalid severity:
-        //     "no-alert: 2 no-console: 2" --> {"no-alert": "2 no-console: 2"}
-        // Should ignore that case as well.
-        if (ConfigOps.isEverySeverityValid(items)) {
-            return {
-                success: true,
-                config: items
-            };
-        }
-    } catch (ex) {
-
-        // ignore to parse the string by a fallback.
-    }
-
-    /*
-     * Optionator cannot parse commaless notations.
-     * But we are supporting that. So this is a fallback for that.
-     */
-    items = {};
-    const normalizedString = string.replace(/([a-zA-Z0-9\-/]+):/g, "\"$1\":").replace(/(]|[0-9])\s+(?=")/, "$1,");
-
-    try {
-        items = JSON.parse(`{${normalizedString}}`);
-    } catch (ex) {
-        return {
-            success: false,
-            error: {
-                ruleId: null,
-                fatal: true,
-                severity: 2,
-                message: `Failed to parse JSON from '${normalizedString}': ${ex.message}`,
-                line: location.start.line,
-                column: location.start.column + 1
-            }
-        };
-
-    }
-
-    return {
-        success: true,
-        config: items
-    };
-}
-
-/**
- * Parses a config of values separated by comma.
- * @param {string} string The string to parse.
- * @returns {Object} Result map of values and true values
- */
-function parseListConfig(string) {
-    const items = {};
-
-    // Collapse whitespace around ,
-    string.replace(/\s*,\s*/g, ",").split(/,+/).forEach(name => {
-        const trimmedName = name.trim();
-
-        if (trimmedName) {
-            items[trimmedName] = true;
-        }
-    });
-    return items;
-}
-
 /**
  * Ensures that variables representing built-in properties of the Global Object,
  * and any globals declared by special block comments, are present in the global
@@ -243,7 +138,7 @@ function addDeclaredGlobals(globalScope, configGlobals, commentDirectives) {
  * @returns {DisableDirective[]} Directives from the comment
  */
 function createDisableDirectives(type, loc, value) {
-    const ruleIds = Object.keys(parseListConfig(value));
+    const ruleIds = Object.keys(commentParser.parseListConfig(value));
     const directiveRules = ruleIds.length ? ruleIds : [null];
 
     return directiveRules.map(ruleId => ({ type, line: loc.line, column: loc.column + 1, ruleId }));
@@ -296,12 +191,12 @@ function getDirectiveComments(filename, ast, ruleMapper) {
         } else if (comment.type === "Block") {
             switch (match[1]) {
                 case "exported":
-                    Object.assign(exportedVariables, parseBooleanConfig(directiveValue, comment));
+                    Object.assign(exportedVariables, commentParser.parseBooleanConfig(directiveValue, comment));
                     break;
 
                 case "globals":
                 case "global":
-                    Object.assign(enabledGlobals, parseBooleanConfig(directiveValue, comment));
+                    Object.assign(enabledGlobals, commentParser.parseBooleanConfig(directiveValue, comment));
                     break;
 
                 case "eslint-disable":
@@ -313,7 +208,7 @@ function getDirectiveComments(filename, ast, ruleMapper) {
                     break;
 
                 case "eslint": {
-                    const parseResult = parseJsonConfig(directiveValue, comment.loc);
+                    const parseResult = commentParser.parseJsonConfig(directiveValue, comment.loc);
 
                     if (parseResult.success) {
                         Object.keys(parseResult.config).forEach(name => {
@@ -393,7 +288,7 @@ function findEslintEnv(text) {
     eslintEnvPattern.lastIndex = 0;
 
     while ((match = eslintEnvPattern.exec(text))) {
-        retv = Object.assign(retv || {}, parseListConfig(match[1]));
+        retv = Object.assign(retv || {}, commentParser.parseListConfig(match[1]));
     }
 
     return retv;
diff --git a/lib/rules/indent.js b/lib/rules/indent.js
index dc9fbaf90..0b87412c8 100644
--- a/lib/rules/indent.js
+++ b/lib/rules/indent.js
@@ -1229,9 +1229,13 @@ module.exports = {
                 }
 
                 const fromToken = sourceCode.getLastToken(node, token => token.type === "Identifier" && token.value === "from");
+                const sourceToken = sourceCode.getLastToken(node, token => token.type === "String");
+                const semiToken = sourceCode.getLastToken(node, token => token.type === "Punctuator" && token.value === ";");
 
                 if (fromToken) {
-                    offsets.setDesiredOffsets([fromToken.range[0], node.range[1]], sourceCode.getFirstToken(node), 1);
+                    const end = semiToken && semiToken.range[1] === sourceToken.range[1] ? node.range[1] : sourceToken.range[1];
+
+                    offsets.setDesiredOffsets([fromToken.range[0], end], sourceCode.getFirstToken(node), 1);
                 }
             },
 
diff --git a/lib/util/config-comment-parser.js b/lib/util/config-comment-parser.js
new file mode 100644
index 000000000..88504caf1
--- /dev/null
+++ b/lib/util/config-comment-parser.js
@@ -0,0 +1,144 @@
+/**
+ * @fileoverview Config Comment Parser
+ * @author Nicholas C. Zakas
+ */
+
+/* eslint-disable class-methods-use-this*/
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const levn = require("levn"),
+    ConfigOps = require("../config/config-ops");
+
+const debug = require("debug")("eslint:config-comment-parser");
+
+//------------------------------------------------------------------------------
+// Public Interface
+//------------------------------------------------------------------------------
+
+/**
+ * Object to parse ESLint configuration comments inside JavaScript files.
+ * @name ConfigCommentParser
+ */
+module.exports = class ConfigCommentParser {
+
+    /**
+     * Parses a list of "name:boolean_value" or/and "name" options divided by comma or
+     * whitespace. Used for "global" and "exported" comments.
+     * @param {string} string The string to parse.
+     * @param {Comment} comment The comment node which has the string.
+     * @returns {Object} Result map object of names and boolean values
+     */
+    parseBooleanConfig(string, comment) {
+        debug("Parsing Boolean config");
+
+        const items = {};
+
+        // Collapse whitespace around `:` and `,` to make parsing easier
+        const trimmedString = string.replace(/\s*([:,])\s*/g, "$1");
+
+        trimmedString.split(/\s|,+/).forEach(name => {
+            if (!name) {
+                return;
+            }
+
+            // value defaults to "false" (if not provided), e.g: "foo" => ["foo", "false"]
+            const [key, value = "false"] = name.split(":");
+
+            items[key] = {
+                value: value === "true",
+                comment
+            };
+        });
+        return items;
+    }
+
+    /**
+     * Parses a JSON-like config.
+     * @param {string} string The string to parse.
+     * @param {Object} location Start line and column of comments for potential error message.
+     * @returns {({success: true, config: Object}|{success: false, error: Problem})} Result map object
+     */
+    parseJsonConfig(string, location) {
+        debug("Parsing JSON config");
+
+        let items = {};
+
+        // Parses a JSON-like comment by the same way as parsing CLI option.
+        try {
+            items = levn.parse("Object", string) || {};
+
+            // Some tests say that it should ignore invalid comments such as `/*eslint no-alert:abc*/`.
+            // Also, commaless notations have invalid severity:
+            //     "no-alert: 2 no-console: 2" --> {"no-alert": "2 no-console: 2"}
+            // Should ignore that case as well.
+            if (ConfigOps.isEverySeverityValid(items)) {
+                return {
+                    success: true,
+                    config: items
+                };
+            }
+        } catch (ex) {
+
+            debug("Levn parsing failed; falling back to manual parsing.");
+
+            // ignore to parse the string by a fallback.
+        }
+
+        /*
+         * Optionator cannot parse commaless notations.
+         * But we are supporting that. So this is a fallback for that.
+         */
+        items = {};
+        const normalizedString = string.replace(/([a-zA-Z0-9\-/]+):/g, "\"$1\":").replace(/(]|[0-9])\s+(?=")/, "$1,");
+
+        try {
+            items = JSON.parse(`{${normalizedString}}`);
+        } catch (ex) {
+            debug("Manual parsing failed.");
+
+            return {
+                success: false,
+                error: {
+                    ruleId: null,
+                    fatal: true,
+                    severity: 2,
+                    message: `Failed to parse JSON from '${normalizedString}': ${ex.message}`,
+                    line: location.start.line,
+                    column: location.start.column + 1
+                }
+            };
+
+        }
+
+        return {
+            success: true,
+            config: items
+        };
+    }
+
+    /**
+     * Parses a config of values separated by comma.
+     * @param {string} string The string to parse.
+     * @returns {Object} Result map of values and true values
+     */
+    parseListConfig(string) {
+        debug("Parsing list config");
+
+        const items = {};
+
+        // Collapse whitespace around commas
+        string.replace(/\s*,\s*/g, ",").split(/,+/).forEach(name => {
+            const trimmedName = name.trim();
+
+            if (trimmedName) {
+                items[trimmedName] = true;
+            }
+        });
+        return items;
+    }
+
+};


Test patch:
diff --git a/tests/lib/rules/indent.js b/tests/lib/rules/indent.js
index b25e25949..07e025f30 100644
--- a/tests/lib/rules/indent.js
+++ b/tests/lib/rules/indent.js
@@ -271,6 +271,17 @@ ruleTester.run("indent", rule, {
             `,
             options: [4]
         },
+        {
+
+            // https://github.com/eslint/eslint/issues/11802
+            code: unIndent`
+                import foo from "foo"
+
+                ;(() => {})()
+            `,
+            options: [4],
+            parserOptions: { sourceType: "module" }
+        },
         {
             code: unIndent`
                 function test() {
diff --git a/tests/lib/util/config-comment-parser.js b/tests/lib/util/config-comment-parser.js
new file mode 100644
index 000000000..6dbe9381a
--- /dev/null
+++ b/tests/lib/util/config-comment-parser.js
@@ -0,0 +1,229 @@
+/**
+ * @fileoverview Tests for ConfigCommentParser object.
+ * @author Nicholas C. Zakas
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const assert = require("chai").assert,
+    ConfigCommentParser = require("../../../lib/util/config-comment-parser");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+describe("ConfigCommentParser", () => {
+
+    let commentParser;
+    const location = {
+        start: {
+            line: 1,
+            column: 0
+        }
+    };
+
+    beforeEach(() => {
+        commentParser = new ConfigCommentParser();
+    });
+
+    describe("parseJsonConfig", () => {
+
+        it("should parse JSON config with one item", () => {
+            const code = "no-alert:0";
+            const result = commentParser.parseJsonConfig(code, location);
+
+
+            assert.deepStrictEqual(result, {
+                success: true,
+                config: {
+                    "no-alert": 0
+                }
+            });
+        });
+
+        it("should parse JSON config with two items", () => {
+            const code = "no-alert:0 semi: 2";
+            const result = commentParser.parseJsonConfig(code, location);
+
+
+            assert.deepStrictEqual(result, {
+                success: true,
+                config: {
+                    "no-alert": 0,
+                    semi: 2
+                }
+            });
+        });
+
+        it("should parse JSON config with two comma-separated items", () => {
+            const code = "no-alert:0,semi: 2";
+            const result = commentParser.parseJsonConfig(code, location);
+
+
+            assert.deepStrictEqual(result, {
+                success: true,
+                config: {
+                    "no-alert": 0,
+                    semi: 2
+                }
+            });
+        });
+
+        it("should parse JSON config with two items and a string severity", () => {
+            const code = "no-alert:off,semi: 2";
+            const result = commentParser.parseJsonConfig(code, location);
+
+
+            assert.deepStrictEqual(result, {
+                success: true,
+                config: {
+                    "no-alert": "off",
+                    semi: 2
+                }
+            });
+        });
+
+        it("should parse JSON config with two items and options", () => {
+            const code = "no-alert:off, semi: [2, always]";
+            const result = commentParser.parseJsonConfig(code, location);
+
+
+            assert.deepStrictEqual(result, {
+                success: true,
+                config: {
+                    "no-alert": "off",
+                    semi: [2, "always"]
+                }
+            });
+        });
+
+        it("should parse JSON config with two items and options from plugins", () => {
+            const code = "plugin/no-alert:off, plugin/semi: [2, always]";
+            const result = commentParser.parseJsonConfig(code, location);
+
+            assert.deepStrictEqual(result, {
+                success: true,
+                config: {
+                    "plugin/no-alert": "off",
+                    "plugin/semi": [2, "always"]
+                }
+            });
+        });
+
+
+    });
+
+    describe("parseBooleanConfig", () => {
+
+        const comment = {};
+
+        it("should parse Boolean config with one item", () => {
+            const code = "a: true";
+            const result = commentParser.parseBooleanConfig(code, comment);
+
+            assert.deepStrictEqual(result, {
+                a: {
+                    value: true,
+                    comment
+                }
+            });
+        });
+
+        it("should parse Boolean config with one item and no value", () => {
+            const code = "a";
+            const result = commentParser.parseBooleanConfig(code, comment);
+
+            assert.deepStrictEqual(result, {
+                a: {
+                    value: false,
+                    comment
+                }
+            });
+        });
+
+        it("should parse Boolean config with two items", () => {
+            const code = "a: true b:false";
+            const result = commentParser.parseBooleanConfig(code, comment);
+
+            assert.deepStrictEqual(result, {
+                a: {
+                    value: true,
+                    comment
+                },
+                b: {
+                    value: false,
+                    comment
+                }
+            });
+        });
+
+        it("should parse Boolean config with two comma-separated items", () => {
+            const code = "a: true, b:false";
+            const result = commentParser.parseBooleanConfig(code, comment);
+
+            assert.deepStrictEqual(result, {
+                a: {
+                    value: true,
+                    comment
+                },
+                b: {
+                    value: false,
+                    comment
+                }
+            });
+        });
+
+        it("should parse Boolean config with two comma-separated items and no values", () => {
+            const code = "a , b";
+            const result = commentParser.parseBooleanConfig(code, comment);
+
+            assert.deepStrictEqual(result, {
+                a: {
+                    value: false,
+                    comment
+                },
+                b: {
+                    value: false,
+                    comment
+                }
+            });
+        });
+    });
+
+    describe("parseListConfig", () => {
+
+        it("should parse list config with one item", () => {
+            const code = "a";
+            const result = commentParser.parseListConfig(code);
+
+            assert.deepStrictEqual(result, {
+                a: true
+            });
+        });
+
+        it("should parse list config with two items", () => {
+            const code = "a, b";
+            const result = commentParser.parseListConfig(code);
+
+            assert.deepStrictEqual(result, {
+                a: true,
+                b: true
+            });
+        });
+
+        it("should parse list config with two items and exta whitespace", () => {
+            const code = "  a , b  ";
+            const result = commentParser.parseListConfig(code);
+
+            assert.deepStrictEqual(result, {
+                a: true,
+                b: true
+            });
+        });
+    });
+
+});
diff --git a/tests/lib/util/npm-utils.js b/tests/lib/util/npm-utils.js
index 1b07f3715..5c8e4cc84 100644
--- a/tests/lib/util/npm-utils.js
+++ b/tests/lib/util/npm-utils.js
@@ -162,8 +162,15 @@ describe("npmUtils", () => {
         });
 
         it("should return false if package.json does not exist", () => {
-            mockFs({});
-            assert.strictEqual(npmUtils.checkPackageJson(), false);
+
+            /*
+             * Checking local file system directly, which does not
+             * contains package.json. This is necessary because mock-fs
+             * doesn't work in Node.js 11: it throws an error when
+             * a file is missing rather than returning false.
+             * TODO: Find a replacement for mock-fs that works in Node 11.
+             */
+            assert.strictEqual(npmUtils.checkPackageJson("/"), false);
         });
     });