Repository: eslint/eslint
Pull request #10373 (2018-05-18 05:45:46)

Update: Add ignoreDestructing option to camelcase rule (refs #9807)
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

**What is the purpose of this pull request? (put an "X" next to item)**

[ ] 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))
[x] 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:

**What rule do you want to change?**
The `camelcase` rule.

**Does this change cause the rule to produce more or fewer warnings?**
Fewer.

**How will the change be implemented? (New option, new default behavior, etc.)?**
A new option:
```js
"camelcase": [ "error", { "ignoreDestructuring": false }]
"camelcase": [ "error", { "ignoreDestructuring": true }]
```

**Please provide some example code that this change will affect:**

```js
const { foo_bar } = baz;
```

**What does the rule currently do for this code?**
The rule currently always fails for these expressions (with or without the properties: "never" config option set).

**What will the rule do after it's changed?**
If the "ignoreDestructuring": true option is set, the line of code above should pass the rule (while the rule should still be enforced for other identifiers in the source).

**What changes did you make? (Give an overview)**
The changes are in `lib/rules/camelcase.js`

**Is there anything you'd like reviewers to focus on?**
There are a couple of edge cases I mentioned on #9807, both of which are errors when `ignore_destructuring: true` is set in this change:

Creating a new identifier from a destructured assignment

```
const { category_id: category_alias } = query;
```

Creating a non-camelcased identifier for rest props:

```
const { category_id, ...other_props } = query;
```


platinumazure: Hi @alunny, thanks for contributing!

I don't have time to do a full review right now-- apologies-- but I do have two things that will need to change, so it's up to you if you want to fix them now or wait for a full review.
* Commit message should end with "(fixes #9807)" unless this change is only a partial fix
* Commit message should start with "Update:" since we generally only use "New:" for completely new features (i.e., only if the rule itself is completely new)

Thanks again for the contribution, hopefully we can get this reviewed and (assuming all goes well) merged soon!
leipert: @alunny Are you working on this? This is a real pain-point for us! I could also do the adjustments @platinumazure wanted you to make...
platinumazure: Friendly ping @alunny, are you still working on this? (If not, totally okay!)

If you're busy, would you be okay with someone pushing changes to your branch to help finish this off?

Thanks for all your work so far on this!
alunny: My bad @platinumazure - was a bit busy these last few weeks.

I've blocked out some time today so should be able to push some changes.

Patch:
diff --git a/README.md b/README.md
index 95c272094..5aca4dddc 100644
--- a/README.md
+++ b/README.md
@@ -145,7 +145,9 @@ ESLint has full support for ECMAScript 3, 5 (default), 2015, 2016, 2017, and 201
 
 ### What about experimental features?
 
-ESLint doesn't natively support experimental ECMAScript language features. You can use [babel-eslint](https://github.com/babel/babel-eslint) to use any option available in Babel.
+ESLint's parser only officially supports the latest final ECMAScript standard. We will make changes to core rules in order to avoid crashes on stage 3 ECMAScript syntax proposals (as long as they are implemented using the correct experimental ESTree syntax). We may make changes to core rules to better work with language extensions (such as JSX, Flow, and TypeScript) on a case-by-case basis.
+
+In other cases (including if rules need to warn on more or fewer cases due to new syntax, rather than just not crashing), we recommend you use other parsers and/or rule plugins. If you are using Babel, you can use the [babel-eslint](https://github.com/babel/babel-eslint) parser and [eslint-plugin-babel](https://github.com/babel/eslint-plugin-babel) to use any option available in Babel.
 
 Once a language feature has been adopted into the ECMAScript standard (stage 4 according to the [TC39 process](https://tc39.github.io/process-document/)), we will accept issues and pull requests related to the new feature, subject to our [contributing guidelines](https://eslint.org/docs/developer-guide/contributing). Until then, please use the appropriate parser and plugin(s) for your experimental feature.
 
diff --git a/docs/README.md b/docs/README.md
new file mode 100644
index 000000000..8f5223a74
--- /dev/null
+++ b/docs/README.md
@@ -0,0 +1,17 @@
+# Documentation
+
+Welcome to our documentation pages! What would you like to view?
+
+## [User Guide](user-guide)
+
+Intended for end users of ESLint. Contains information about core rules, configuration, command line options, formatters, and integrations,
+as well as guides for migrating from earlier versions of ESLint.
+
+## [Developer Guide](developer-guide)
+
+Intended for contributors to ESLint and people who wish to extend ESLint. Contains information about contributing to ESLint; creating custom
+rules, configurations, plugins, and formatters; and information about our architecture and Node.js API.
+
+## [Maintainer Guide](maintainer-guide)
+
+Intended for maintainers of ESLint.
diff --git a/docs/rules/camelcase.md b/docs/rules/camelcase.md
index faeb92398..cce3fd924 100644
--- a/docs/rules/camelcase.md
+++ b/docs/rules/camelcase.md
@@ -12,8 +12,10 @@ This rule has an object option:
 
 * `"properties": "always"` (default) enforces camelcase style for property names
 * `"properties": "never"` does not check property names
+* `"ignoreDestructuring": false` (default) enforces camelcase style for destructured identifiers
+* `"ignoreDestructuring": true` does not check destructured identifiers
 
-### always
+### properties: "always"
 
 Examples of **incorrect** code for this rule with the default `{ "properties": "always" }` option:
 
@@ -95,7 +97,7 @@ var { foo: isCamelCased = 1 } = quz;
 
 ```
 
-### never
+### properties: "never"
 
 Examples of **correct** code for this rule with the `{ "properties": "never" }` option:
 
@@ -107,6 +109,48 @@ var obj = {
 };
 ```
 
+### ignoreDestructuring: false
+
+Examples of **incorrect** code for this rule with the default `{ "ignoreDestructuring": false }` option:
+
+```js
+/*eslint camelcase: "error"*/
+
+var { category_id } = query;
+
+var { category_id = 1 } = query;
+
+var { category_id: category_id } = query;
+
+var { category_id: category_alias } = query;
+
+var { category_id: categoryId, ...other_props } = query;
+```
+
+### ignoreDestructuring: true
+
+Examples of **incorrect** code for this rule with the `{ "ignoreDestructuring": true }` option:
+
+```js
+/*eslint camelcase: ["error", {ignoreDestructuring: true}]*/
+
+var { category_id: category_alias } = query;
+
+var { category_id, ...other_props } = query;
+```
+
+Examples of **correct** code for this rule with the `{ "ignoreDestructuring": true }` option:
+
+```js
+/*eslint camelcase: ["error", {ignoreDestructuring: true}]*/
+
+var { category_id } = query;
+
+var { category_id = 1 } = query;
+
+var { category_id: category_id } = query;
+```
+
 ## When Not To Use It
 
 If you have established coding standards using a different naming convention (separating words with underscores), turn this rule off.
diff --git a/lib/config.js b/lib/config.js
index 7ba5cd6e2..2eeaad181 100644
--- a/lib/config.js
+++ b/lib/config.js
@@ -209,7 +209,7 @@ class Config {
             const localConfigHierarchyCache = this.configCache.getHierarchyLocalConfigs(localConfigDirectory);
 
             if (localConfigHierarchyCache) {
-                const localConfigHierarchy = localConfigHierarchyCache.concat(configs.reverse());
+                const localConfigHierarchy = localConfigHierarchyCache.concat(configs);
 
                 this.configCache.setHierarchyLocalConfigs(searched, localConfigHierarchy);
                 return localConfigHierarchy;
@@ -232,7 +232,7 @@ class Config {
             }
 
             debug(`Using ${localConfigFile}`);
-            configs.push(localConfig);
+            configs.unshift(localConfig);
             searched.push(localConfigDirectory);
 
             // Stop traversing if a config is found with the root flag set
@@ -248,7 +248,7 @@ class Config {
             const personalConfig = this.getPersonalConfig();
 
             if (personalConfig) {
-                configs.push(personalConfig);
+                configs.unshift(personalConfig);
             } else if (!hasRules(this.options) && !this.options.baseConfig) {
 
                 // No config file, no manual configuration, and no rules, so error.
@@ -265,7 +265,7 @@ class Config {
         }
 
         // Set the caches for the parent directories
-        this.configCache.setHierarchyLocalConfigs(searched, configs.reverse());
+        this.configCache.setHierarchyLocalConfigs(searched, configs);
 
         return configs;
     }
diff --git a/lib/ignored-paths.js b/lib/ignored-paths.js
index 0371e5e10..8134d6d32 100644
--- a/lib/ignored-paths.js
+++ b/lib/ignored-paths.js
@@ -236,7 +236,7 @@ class IgnoredPaths {
     /**
      * Determine whether a file path is included in the default or custom ignore patterns
      * @param {string} filepath Path to check
-     * @param {string} [category=null] check 'default', 'custom' or both (null)
+     * @param {string} [category=undefined] check 'default', 'custom' or both (undefined)
      * @returns {boolean} true if the file path matches one or more patterns, false otherwise
      */
     contains(filepath, category) {
@@ -245,12 +245,11 @@ class IgnoredPaths {
         const absolutePath = path.resolve(this.options.cwd, filepath);
         const relativePath = pathUtil.getRelativePath(absolutePath, this.baseDir);
 
-        if ((typeof category === "undefined") || (category === "default")) {
-            result = result || (this.ig.default.filter([relativePath]).length === 0);
-        }
-
-        if ((typeof category === "undefined") || (category === "custom")) {
-            result = result || (this.ig.custom.filter([relativePath]).length === 0);
+        if (typeof category === "undefined") {
+            result = (this.ig.default.filter([relativePath]).length === 0) ||
+                (this.ig.custom.filter([relativePath]).length === 0);
+        } else {
+            result = (this.ig[category].filter([relativePath]).length === 0);
         }
 
         return result;
diff --git a/lib/rules/camelcase.js b/lib/rules/camelcase.js
index 84db54ff2..ec6116621 100644
--- a/lib/rules/camelcase.js
+++ b/lib/rules/camelcase.js
@@ -22,6 +22,9 @@ module.exports = {
             {
                 type: "object",
                 properties: {
+                    ignoreDestructuring: {
+                        type: "boolean"
+                    },
                     properties: {
                         enum: ["always", "never"]
                     }
@@ -57,6 +60,26 @@ module.exports = {
             return name.indexOf("_") > -1 && name !== name.toUpperCase();
         }
 
+        /**
+         * Checks if a parent of a node is an ObjectPattern.
+         * @param {ASTNode} node The node to check.
+         * @returns {boolean} if the node is inside an ObjectPattern
+         * @private
+         */
+        function isInsideObjectPattern(node) {
+            let { parent } = node;
+
+            while (parent) {
+                if (parent.type === "ObjectPattern") {
+                    return true;
+                }
+
+                parent = parent.parent;
+            }
+
+            return false;
+        }
+
         /**
          * Reports an AST node as a rule violation.
          * @param {ASTNode} node The node to report.
@@ -72,6 +95,7 @@ module.exports = {
 
         const options = context.options[0] || {};
         let properties = options.properties || "";
+        const ignoreDestructuring = options.ignoreDestructuring || false;
 
         if (properties !== "always" && properties !== "never") {
             properties = "always";
@@ -113,24 +137,28 @@ module.exports = {
                 } else if (node.parent.type === "Property" || node.parent.type === "AssignmentPattern") {
 
                     if (node.parent.parent && node.parent.parent.type === "ObjectPattern") {
-
                         if (node.parent.shorthand && node.parent.value.left && isUnderscored(name)) {
 
                             report(node);
                         }
 
+                        const assignmentKeyEqualsValue = node.parent.key.name === node.parent.value.name;
+
                         // prevent checking righthand side of destructured object
-                        if (node.parent.key === node && node.parent.value !== node) {
+                        if (!assignmentKeyEqualsValue && node.parent.key === node) {
                             return;
                         }
 
-                        if (node.parent.value.name && isUnderscored(name)) {
+                        const valueIsUnderscored = node.parent.value.name && isUnderscored(name);
+
+                        // ignore destructuring if the option is set, unless a new identifier is created
+                        if (valueIsUnderscored && !(assignmentKeyEqualsValue && ignoreDestructuring)) {
                             report(node);
                         }
                     }
 
-                    // "never" check properties
-                    if (properties === "never") {
+                    // "never" check properties or always ignore destructuring
+                    if (properties === "never" || (ignoreDestructuring && isInsideObjectPattern(node))) {
                         return;
                     }
 
diff --git a/lib/rules/no-use-before-define.js b/lib/rules/no-use-before-define.js
index 37b8e11ca..64d825702 100644
--- a/lib/rules/no-use-before-define.js
+++ b/lib/rules/no-use-before-define.js
@@ -219,49 +219,14 @@ module.exports = {
                     data: reference.identifier
                 });
             });
-        }
 
-        /**
-         * Validates variables inside of a node's scope.
-         * @param {ASTNode} node The node to check.
-         * @returns {void}
-         * @private
-         */
-        function findVariables() {
-            const scope = context.getScope();
-
-            findVariablesInScope(scope);
+            scope.childScopes.forEach(findVariablesInScope);
         }
 
-        const ruleDefinition = {
-            "Program:exit"(node) {
-                const scope = context.getScope(),
-                    ecmaFeatures = context.parserOptions.ecmaFeatures || {};
-
-                findVariablesInScope(scope);
-
-                // both Node.js and Modules have an extra scope
-                if (ecmaFeatures.globalReturn || node.sourceType === "module") {
-                    findVariablesInScope(scope.childScopes[0]);
-                }
+        return {
+            Program() {
+                findVariablesInScope(context.getScope());
             }
         };
-
-        if (context.parserOptions.ecmaVersion >= 6) {
-            ruleDefinition["BlockStatement:exit"] =
-                ruleDefinition["SwitchStatement:exit"] = findVariables;
-
-            ruleDefinition["ArrowFunctionExpression:exit"] = function(node) {
-                if (node.body.type !== "BlockStatement") {
-                    findVariables();
-                }
-            };
-        } else {
-            ruleDefinition["FunctionExpression:exit"] =
-                ruleDefinition["FunctionDeclaration:exit"] =
-                ruleDefinition["ArrowFunctionExpression:exit"] = findVariables;
-        }
-
-        return ruleDefinition;
     }
 };
diff --git a/lib/rules/no-warning-comments.js b/lib/rules/no-warning-comments.js
index ce58f9ac8..4e05cc411 100644
--- a/lib/rules/no-warning-comments.js
+++ b/lib/rules/no-warning-comments.js
@@ -57,6 +57,8 @@ module.exports = {
          */
         function convertToRegExp(term) {
             const escaped = term.replace(/[-/\\$^*+?.()|[\]{}]/g, "\\$&");
+            const wordBoundary = "\\b";
+            const eitherOrWordBoundary = `|${wordBoundary}`;
             let prefix;
 
             /*
@@ -79,12 +81,27 @@ module.exports = {
                  */
                 prefix = "^\\s*";
             } else if (/^\w/.test(term)) {
-                prefix = "\\b";
+                prefix = wordBoundary;
             } else {
                 prefix = "";
             }
 
-            return new RegExp(prefix + escaped + suffix, "i");
+            if (location === "start") {
+
+                /*
+                 * For location "start" the regex should be
+                 * ^\s*TERM\b.  This checks the word boundary
+                 * at the beginning of the comment.
+                 */
+                return new RegExp(prefix + escaped + suffix, "i");
+            }
+
+            /*
+             * For location "anywhere" the regex should be
+             * \bTERM\b|\bTERM\b, this checks the entire comment
+             * for the term.
+             */
+            return new RegExp(prefix + escaped + suffix + eitherOrWordBoundary + term + wordBoundary, "i");
         }
 
         const warningRegExps = warningTerms.map(convertToRegExp);
diff --git a/package.json b/package.json
index 0d6b322e7..af65434cb 100644
--- a/package.json
+++ b/package.json
@@ -15,6 +15,7 @@
     "ci-release": "node Makefile.js ciRelease",
     "alpharelease": "node Makefile.js prerelease -- alpha",
     "betarelease": "node Makefile.js prerelease -- beta",
+    "rcrelease": "node Makefile.js prerelease -- rc",
     "docs": "node Makefile.js docs",
     "gensite": "node Makefile.js gensite",
     "browserify": "node Makefile.js browserify",
@@ -40,9 +41,9 @@
     "cross-spawn": "^6.0.5",
     "debug": "^3.1.0",
     "doctrine": "^2.1.0",
-    "eslint-scope": "^4.0.0-alpha.0",
+    "eslint-scope": "^4.0.0-rc.0",
     "eslint-visitor-keys": "^1.0.0",
-    "espree": "^4.0.0-alpha.1",
+    "espree": "^4.0.0-rc.0",
     "esquery": "^1.0.1",
     "esutils": "^2.0.2",
     "file-entry-cache": "^2.0.0",


Test patch:
diff --git a/tests/lib/rules/camelcase.js b/tests/lib/rules/camelcase.js
index 78921bc50..f21459085 100644
--- a/tests/lib/rules/camelcase.js
+++ b/tests/lib/rules/camelcase.js
@@ -91,6 +91,21 @@ ruleTester.run("camelcase", rule, {
             code: "obj.foo_bar = function(){};",
             options: [{ properties: "never" }]
         },
+        {
+            code: "var { category_id } = query;",
+            options: [{ ignoreDestructuring: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "var { category_id: category_id } = query;",
+            options: [{ ignoreDestructuring: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "var { category_id = 1 } = query;",
+            options: [{ ignoreDestructuring: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
         {
             code: "var { category_id: category } = query;",
             parserOptions: { ecmaVersion: 6 }
@@ -292,12 +307,36 @@ ruleTester.run("camelcase", rule, {
             ]
         },
         {
-            code: "var { category_id: category_id } = query;",
+            code: "var { category_id: category_alias } = query;",
             parserOptions: { ecmaVersion: 6 },
             errors: [
                 {
                     messageId: "notCamelCase",
-                    data: { name: "category_id" },
+                    data: { name: "category_alias" },
+                    type: "Identifier"
+                }
+            ]
+        },
+        {
+            code: "var { category_id: category_alias } = query;",
+            options: [{ ignoreDestructuring: true }],
+            parserOptions: { ecmaVersion: 6 },
+            errors: [
+                {
+                    messageId: "notCamelCase",
+                    data: { name: "category_alias" },
+                    type: "Identifier"
+                }
+            ]
+        },
+        {
+            code: "var { category_id: categoryId, ...other_props } = query;",
+            options: [{ ignoreDestructuring: true }],
+            parserOptions: { ecmaVersion: 2018 },
+            errors: [
+                {
+                    messageId: "notCamelCase",
+                    data: { name: "other_props" },
                     type: "Identifier"
                 }
             ]
diff --git a/tests/lib/rules/no-use-before-define.js b/tests/lib/rules/no-use-before-define.js
index f7f9382bb..1d1df968b 100644
--- a/tests/lib/rules/no-use-before-define.js
+++ b/tests/lib/rules/no-use-before-define.js
@@ -113,6 +113,55 @@ ruleTester.run("no-use-before-define", rule, {
             code: "foo; var foo;",
             options: [{ variables: false }],
             errors: [{ message: "'foo' was used before it was defined.", type: "Identifier" }]
+        },
+
+        // https://github.com/eslint/eslint/issues/10227
+        {
+            code: "for (let x = x;;); let x = 0",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: ["'x' was used before it was defined."]
+        },
+        {
+            code: "for (let x in xs); let xs = []",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: ["'xs' was used before it was defined."]
+        },
+        {
+            code: "for (let x of xs); let xs = []",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: ["'xs' was used before it was defined."]
+        },
+        {
+            code: "try {} catch ({message = x}) {} let x = ''",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: ["'x' was used before it was defined."]
+        },
+        {
+            code: "with (obj) x; let x = {}",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: ["'x' was used before it was defined."]
+        },
+
+        // WithStatements.
+        {
+            code: "with (x); let x = {}",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: ["'x' was used before it was defined."]
+        },
+        {
+            code: "with (obj) { x } let x = {}",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: ["'x' was used before it was defined."]
+        },
+        {
+            code: "with (obj) { if (a) { x } } let x = {}",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: ["'x' was used before it was defined."]
+        },
+        {
+            code: "with (obj) { (() => { if (a) { x } })() } let x = {}",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: ["'x' was used before it was defined."]
         }
     ]
 });
diff --git a/tests/lib/rules/no-warning-comments.js b/tests/lib/rules/no-warning-comments.js
index ba9f654e1..3484d0a45 100644
--- a/tests/lib/rules/no-warning-comments.js
+++ b/tests/lib/rules/no-warning-comments.js
@@ -51,10 +51,15 @@ ruleTester.run("no-warning-comments", rule, {
         { code: "/* any fixme or todo */", options: [{ terms: ["fixme", "todo"], location: "anywhere" }], errors: [{ message: "Unexpected 'fixme' comment." }, { message: "Unexpected 'todo' comment." }] },
         { code: "/* any fixme or todo */", options: [{ location: "anywhere" }], errors: [{ message: "Unexpected 'todo' comment." }, { message: "Unexpected 'fixme' comment." }] },
         { code: "/* fixme and todo */", errors: [{ message: "Unexpected 'fixme' comment." }] },
+        { code: "/* fixme and todo */", options: [{ location: "anywhere" }], errors: [{ message: "Unexpected 'todo' comment." }, { message: "Unexpected 'fixme' comment." }] },
         { code: "/* any fixme */", options: [{ location: "anywhere" }], errors: [{ message: "Unexpected 'fixme' comment." }] },
         { code: "/* fixme! */", options: [{ terms: ["fixme"] }], errors: [{ message: "Unexpected 'fixme' comment." }] },
         { code: "// regex [litera|$]", options: [{ terms: ["[litera|$]"], location: "anywhere" }], errors: [{ message: "Unexpected '[litera|$]' comment." }] },
         { code: "/* eslint one-var: 2 */", options: [{ terms: ["eslint"] }], errors: [{ message: "Unexpected 'eslint' comment." }] },
-        { code: "/* eslint one-var: 2 */", options: [{ terms: ["one"], location: "anywhere" }], errors: [{ message: "Unexpected 'one' comment." }] }
+        { code: "/* eslint one-var: 2 */", options: [{ terms: ["one"], location: "anywhere" }], errors: [{ message: "Unexpected 'one' comment." }] },
+        { code: "/* any block comment with TODO, FIXME or XXX */", options: [{ location: "anywhere" }], errors: [{ message: "Unexpected 'todo' comment." }, { message: "Unexpected 'fixme' comment." }, { message: "Unexpected 'xxx' comment." }] },
+        { code: "/* any block comment with (TODO, FIXME's or XXX!) */", options: [{ location: "anywhere" }], errors: [{ message: "Unexpected 'todo' comment." }, { message: "Unexpected 'fixme' comment." }, { message: "Unexpected 'xxx' comment." }] },
+        { code: "/** \n *any block comment \n*with (TODO, FIXME's or XXX!) **/", options: [{ location: "anywhere" }], errors: [{ message: "Unexpected 'todo' comment." }, { message: "Unexpected 'fixme' comment." }, { message: "Unexpected 'xxx' comment." }] },
+        { code: "// any comment with TODO, FIXME or XXX", options: [{ location: "anywhere" }], errors: [{ message: "Unexpected 'todo' comment." }, { message: "Unexpected 'fixme' comment." }, { message: "Unexpected 'xxx' comment." }] }
     ]
 });