Repository: eslint/eslint
Pull request #8246 (2017-03-14 04:40:17)

Fix: func-name-matching crash on non-string literal computed keys
**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
rules:
  func-name-matching: error
parserOptions:
  ecmaVersion: 2015
```

**What did you do? Please include the actual source code causing the issue.**

```js
({ [null]: function foo() {} })
```

**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 'length' of null
TypeError: Cannot read property 'length' of null
    at isIdentifierNameES6 (/path/to/eslint/node_modules/esutils/lib/keyword.js:123:15)
    at Object.isIdentifierES6 (/path/to/eslint/node_modules/esutils/lib/keyword.js:150:16)
    at isIdentifier (/path/to/eslint/lib/rules/func-name-matching.js:48:32)
    at EventEmitter.Property (/path/to/eslint/lib/rules/func-name-matching.js:174:21)
    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)
```

**What changes did you make? (Give an overview)**

In order to detect whether it should require a function name match, the `func-name-matching` rule checks to see if a computed object key is a string literal, and if so, it performs operations on the value of the string literal. However, the rule was previously only verifying that the computed object key was a literal, without verifying that it was a *string* literal. As a result, if other literals (e.g. `null`) were used as computed object keys, the rule would crash. This fix updates the rule to check that the object key is a *string* literal.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular

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

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..e33efb7aa 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
         //--------------------------------------------------------------------------
@@ -164,13 +173,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/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..667570ed6 100644
--- a/tests/lib/rules/func-name-matching.js
+++ b/tests/lib/rules/func-name-matching.js
@@ -117,6 +117,34 @@ 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 }
         }
     ],
     invalid: [