Repository: eslint/eslint
Pull request #8245 (2017-03-14 03:34:38)

Fix: sort-vars crash on mixed destructuring declarations
**What is the purpose of this pull request? (put an "X" next to item)**

[x] Bug fix

**Tell us about your environment**

* **ESLint Version:** master
* **Node Version:** 7.7.2
* **npm Version:** 4.1.2

**What parser (default, Babel-ESLint, etc.) are you using?**

default

**Please show your full configuration:**

```yml
parserOptions:
  ecmaVersion: 2015
rules:
  sort-vars: [error, { ignoreCase: true }]
```

**What did you do? Please include the actual source code causing the issue.**

```js
var {a} = 1, b;
```

**What did you expect to happen?**

I expected ESLint to not crash.

**What actually happened? Please include the actual, raw output from ESLint.**

ESLint crashed.

```
Cannot read property 'toLowerCase' of undefined
TypeError: Cannot read property 'toLowerCase' of undefined
    at VariableDeclaration.node.declarations.reduce (/path/to/eslint/lib/rules/sort-vars.js:49:60)
    at Array.reduce (native)
    at EventEmitter.VariableDeclaration (/path/to/eslint/lib/rules/sort-vars.js:40:35)
    at emitOne (events.js:96:13)
    at EventEmitter.emit (events.js:191:7)
    at NodeEventGenerator.enterNode (/path/to/eslint/lib/util/node-event-generator.js:39:22)
    at CodePathAnalyzer.enterNode (/path/to/eslint/lib/code-path-analysis/code-path-analyzer.js:607:23)
    at CommentEventGenerator.enterNode (/path/to/eslint/lib/util/comment-event-generator.js:98:23)
    at Controller.enter (/path/to/eslint/lib/eslint.js:928:36)
    at Controller.__execute (/path/to/eslint/node_modules/estraverse/estraverse.js:397:31)

```

**What changes did you make? (Give an overview)**

By design, sort-vars ignores destructuring assignments. However, the rule previously didn't account for the case where the first declarator of a variable declaration was a destructuring assignment -- this led to a crash. This commit updates the rule to handle this case properly.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular

eslintbot: LGTM
mention-bot: @not-an-aardvark, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nzakas, @vitorbal and @ilyavolodin to be potential reviewers.

Patch:
diff --git a/docs/rules/arrow-parens.md b/docs/rules/arrow-parens.md
index 2e006c283..371a500ff 100644
--- a/docs/rules/arrow-parens.md
+++ b/docs/rules/arrow-parens.md
@@ -71,7 +71,7 @@ a => a;
 a => {'\n'};
 a.then(foo => {});
 a.then(foo => a);
-a(foo => { if (true) {}; });
+a(foo => { if (true) {} });
 ```
 
 Examples of **correct** code for this rule with the default `"always"` option:
@@ -85,7 +85,7 @@ Examples of **correct** code for this rule with the default `"always"` option:
 (a) => a;
 (a) => {'\n'}
 a.then((foo) => {});
-a.then((foo) => { if (true) {}; });
+a.then((foo) => { if (true) {} });
 ```
 
 #### If Statements
@@ -102,7 +102,7 @@ if (a => b) {
  console.log('bigger');
 } else {
  console.log('smaller');
-};
+}
 // outputs 'bigger', not smaller as expected
 ```
 
@@ -120,7 +120,7 @@ if ((a) => b) {
  console.log('truthy value returned');
 } else {
  console.log('falsey value returned');
-};
+}
 // outputs 'truthy value returned'
 ```
 
@@ -158,7 +158,7 @@ Examples of **incorrect** code for this rule with the `"as-needed"` option:
 (a) => {'\n'};
 a.then((foo) => {});
 a.then((foo) => a);
-a((foo) => { if (true) {}; });
+a((foo) => { if (true) {} });
 ```
 
 Examples of **correct** code for this rule with the `"as-needed"` option:
@@ -172,7 +172,7 @@ a => {};
 a => a;
 a => {'\n'};
 a.then(foo => {});
-a.then(foo => { if (true) {}; });
+a.then(foo => { if (true) {} });
 (a, b, c) => a;
 (a = 10) => a;
 ([a, b]) => a;
@@ -209,8 +209,8 @@ a => ({});
 () => {};
 a => a;
 a.then((foo) => {});
-a.then((foo) => { if (true) {}; });
-a((foo) => { if (true) {}; });
+a.then((foo) => { if (true) {} });
+a((foo) => { if (true) {} });
 (a, b, c) => a;
 (a = 10) => a;
 ([a, b]) => a;
diff --git a/lib/rules/func-name-matching.js b/lib/rules/func-name-matching.js
index fc6fa8588..db06d5d46 100644
--- a/lib/rules/func-name-matching.js
+++ b/lib/rules/func-name-matching.js
@@ -132,6 +132,15 @@ module.exports = {
             });
         }
 
+        /**
+         * Determines whether a given node is a string literal
+         * @param {ASTNode} node The node to check
+         * @returns {boolean} `true` if the node is a string literal
+         */
+        function isStringLiteral(node) {
+            return node.type === "Literal" && typeof node.value === "string";
+        }
+
         //--------------------------------------------------------------------------
         // Public
         //--------------------------------------------------------------------------
@@ -139,7 +148,7 @@ module.exports = {
         return {
 
             VariableDeclarator(node) {
-                if (!node.init || node.init.type !== "FunctionExpression") {
+                if (!node.init || node.init.type !== "FunctionExpression" || node.id.type !== "Identifier") {
                     return;
                 }
                 if (node.init.id && shouldWarn(node.id.name, node.init.id.name)) {
@@ -148,10 +157,12 @@ module.exports = {
             },
 
             AssignmentExpression(node) {
-                if (node.right.type !== "FunctionExpression" ||
-                   (node.left.computed && node.left.property.type !== "Literal") ||
-                    (!includeModuleExports && isModuleExports(node.left))
-                   ) {
+                if (
+                    node.right.type !== "FunctionExpression" ||
+                    (node.left.computed && node.left.property.type !== "Literal") ||
+                    (!includeModuleExports && isModuleExports(node.left)) ||
+                    (node.left.type !== "Identifier" && node.left.type !== "MemberExpression")
+                ) {
                     return;
                 }
 
@@ -164,13 +175,13 @@ module.exports = {
             },
 
             Property(node) {
-                if (node.value.type !== "FunctionExpression" || !node.value.id || node.computed && node.key.type !== "Literal") {
+                if (node.value.type !== "FunctionExpression" || !node.value.id || node.computed && !isStringLiteral(node.key)) {
                     return;
                 }
                 if (node.key.type === "Identifier" && shouldWarn(node.key.name, node.value.id.name)) {
                     report(node, node.key.name, node.value.id.name, true);
                 } else if (
-                    node.key.type === "Literal" &&
+                    isStringLiteral(node.key) &&
                     isIdentifier(node.key.value, ecmaVersion) &&
                     shouldWarn(node.key.value, node.value.id.name)
                 ) {
diff --git a/lib/rules/sort-vars.js b/lib/rules/sort-vars.js
index 6bee22103..1fdfc9ace 100644
--- a/lib/rules/sort-vars.js
+++ b/lib/rules/sort-vars.js
@@ -37,11 +37,9 @@ module.exports = {
 
         return {
             VariableDeclaration(node) {
-                node.declarations.reduce((memo, decl) => {
-                    if (decl.id.type === "ObjectPattern" || decl.id.type === "ArrayPattern") {
-                        return memo;
-                    }
+                const idDeclarations = node.declarations.filter(decl => decl.id.type === "Identifier");
 
+                idDeclarations.slice(1).reduce((memo, decl) => {
                     let lastVariableName = memo.id.name,
                         currenVariableName = decl.id.name;
 
@@ -56,7 +54,7 @@ module.exports = {
                     }
                     return decl;
 
-                }, node.declarations[0]);
+                }, idDeclarations[0]);
             }
         };
     }
diff --git a/package.json b/package.json
index 1352ef3d2..4fd3423cc 100644
--- a/package.json
+++ b/package.json
@@ -36,7 +36,7 @@
   "dependencies": {
     "babel-code-frame": "^6.16.0",
     "chalk": "^1.1.3",
-    "concat-stream": "^1.4.6",
+    "concat-stream": "^1.5.2",
     "debug": "^2.1.1",
     "doctrine": "^1.2.2",
     "escope": "^3.6.0",


Test patch:
diff --git a/tests/lib/rules/func-name-matching.js b/tests/lib/rules/func-name-matching.js
index 5d98cd388..8f0d81425 100644
--- a/tests/lib/rules/func-name-matching.js
+++ b/tests/lib/rules/func-name-matching.js
@@ -117,6 +117,66 @@ ruleTester.run("func-name-matching", rule, {
         {
             code: "({[foo]: function bar() {}})",
             parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[null]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[1]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[true]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[`x`]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[/abc/]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[[1, 2, 3]]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({[{x: 1}]: function foo() {}})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "[] = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({} = function foo() {})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "[a] = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "({a} = function foo() {})",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "var [] = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "var {} = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "var [a] = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "var {a} = function foo() {}",
+            parserOptions: { ecmaVersion: 6 }
         }
     ],
     invalid: [
diff --git a/tests/lib/rules/sort-vars.js b/tests/lib/rules/sort-vars.js
index 3994f52cd..4411c35ef 100644
--- a/tests/lib/rules/sort-vars.js
+++ b/tests/lib/rules/sort-vars.js
@@ -77,7 +77,13 @@ ruleTester.run("sort-vars", rule, {
             "            c;",
             "    }",
             "}"
-        ].join("\n"), env: { es6: true }, parserOptions: { sourceType: "module" } }
+        ].join("\n"), env: { es6: true }, parserOptions: { sourceType: "module" } },
+
+        {
+            code: "var {} = 1, a",
+            options: ignoreCaseArgs,
+            parserOptions: { ecmaVersion: 6 }
+        }
     ],
     invalid: [
         { code: "var b, a", errors: [expectedError] },
@@ -93,6 +99,13 @@ ruleTester.run("sort-vars", rule, {
         { code: "var d, a, [b, c] = {};", options: ignoreCaseArgs,
             parserOptions: { ecmaVersion: 6 }, errors: [expectedError] },
         { code: "var d, a, [b, {x: {c, e}}] = {};", options: ignoreCaseArgs,
-            parserOptions: { ecmaVersion: 6 }, errors: [expectedError] }
+            parserOptions: { ecmaVersion: 6 }, errors: [expectedError] },
+
+        {
+            code: "var {} = 1, b, a",
+            options: ignoreCaseArgs,
+            parserOptions: { ecmaVersion: 6 },
+            errors: [expectedError]
+        }
     ]
 });