Repository: eslint/eslint
Pull request #11444 (2019-02-26 00:30:20)

Fix: no-extra-parens crash when code is "((let))"
<!--
    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)**

[x] Bug fix

The PR title contains all of the information needed to reproduce the issue.

**Tell us about your environment**

* **ESLint Version:** master
* **Node Version:** latest
* **npm Version:** latest

**What parser (default, Babel-ESLint, etc.) are you using?**

default

**Please show your full configuration:**

```yml
rules:
  no-extra-parens: error
```

**What did you do? Please include the actual source code causing the issue.**

```js
((let))
```

**What did you expect to happen?**

I expected an error to be reported by the `no-extra-parens` rule, because the identifier `let` is unnecessarily parenthesized twice.

**What actually happened? Please include the actual, raw output from ESLint.**

The rule crashed:

```
TypeError: Cannot read property 'value' of null
    at Object.isOpeningBracketToken (/home/travis/build/eslint/eslint/lib/util/ast-utils.js:329:18)
    at checkExpressionOrExportStatement (/home/travis/build/eslint/eslint/lib/rules/no-extra-parens.js:482:65)
    at ExpressionStatement (/home/travis/build/eslint/eslint/lib/rules/no-extra-parens.js:565:42)
    at listeners.(anonymous function).forEach.listener (/home/travis/build/eslint/eslint/lib/util/safe-emitter.js:45:58)
    at Array.forEach (<anonymous>)
    at Object.emit (/home/travis/build/eslint/eslint/lib/util/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/home/travis/build/eslint/eslint/lib/util/node-event-generator.js:251:26)
    at NodeEventGenerator.applySelectors (/home/travis/build/eslint/eslint/lib/util/node-event-generator.js:280:22)
    at NodeEventGenerator.enterNode (/home/travis/build/eslint/eslint/lib/util/node-event-generator.js:294:14)
    at CodePathAnalyzer.enterNode (/home/travis/build/eslint/eslint/lib/code-path-analysis/code-path-analyzer.js:632:23)
```

**What changes did you make? (Give an overview)**

This fixes a broken check in the `no-extra-parens` rule for tokens after `let` identifiers.

This issue was detected by the fuzzer in https://travis-ci.org/eslint/eslint/jobs/498031437.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular


aladdin-add: just curious  is `(let) foo` valid? I would have thought it will cause a parsing error.

and should we add a test `(var) foo`(if it is accepted).
not-an-aardvark: `(let)\nfoo`contains two ExpressionStatements, an identifier called `let` and an identifier called `foo`. ASI inserts a semicolon after `(let)`. (`let` is a valid variable name in sloppy mode.)

Without parentheses, `let\nfoo` is a variable declaration for a variable named `foo`.
mysticatea: @aladdin-add `var` is a reserved word, but `let` is not. We can use `let` as a variable name.

Patch:
diff --git a/README.md b/README.md
index e0ee6a74f..702a6e852 100644
--- a/README.md
+++ b/README.md
@@ -266,7 +266,7 @@ The following companies, organizations, and individuals support ESLint's ongoing
 <!-- NOTE: This section is autogenerated. Do not manually edit.-->
 <!--sponsorsstart-->
 <h3>Gold Sponsors</h3>
-<p><a href="https://code.facebook.com/projects/"><img src="https://images.opencollective.com/proxy/images/?src=https%3A%2F%2Fres.cloudinary.com%2Fopencollective%2Fimage%2Fupload%2Fv1508519428%2FS9gk78AS_400x400_fulq2l.jpg&height=96" alt="Facebook Open Source" height="96"></a> <a href="https://www.airbnb.com/"><img src="https://images.opencollective.com/proxy/images/?src=https%3A%2F%2Fopencollective-production.s3-us-west-1.amazonaws.com%2F098e3bd0-4d57-11e8-9324-0f6cc1f92bf1.png&height=96" alt="Airbnb" height="96"></a></p><h3>Silver Sponsors</h3>
+<p><a href="https://www.airbnb.com/"><img src="https://images.opencollective.com/proxy/images/?src=https%3A%2F%2Fopencollective-production.s3-us-west-1.amazonaws.com%2F098e3bd0-4d57-11e8-9324-0f6cc1f92bf1.png&height=96" alt="Airbnb" height="96"></a> <a href="https://code.facebook.com/projects/"><img src="https://images.opencollective.com/proxy/images/?src=https%3A%2F%2Fres.cloudinary.com%2Fopencollective%2Fimage%2Fupload%2Fv1508519428%2FS9gk78AS_400x400_fulq2l.jpg&height=96" alt="Facebook Open Source" height="96"></a> <a href="https://badoo.com/team?utm_source=eslint"><img src="https://images.opencollective.com/proxy/images/?src=https%3A%2F%2Fopencollective-production.s3-us-west-1.amazonaws.com%2Ffefc37d0-3ab3-11e9-ba2e-c312ceffa0ef.png&height=96" alt="Badoo" height="96"></a></p><h3>Silver Sponsors</h3>
 <p><a href="https://www.ampproject.org/"><img src="https://images.opencollective.com/proxy/images/?src=https%3A%2F%2Fopencollective-production.s3-us-west-1.amazonaws.com%2F68ed8b70-ebf2-11e6-9958-cb7e79408c56.png&height=96" alt="AMP Project" height="64"></a></p><h3>Bronze Sponsors</h3>
 <p><a href="http://faithlife.com/ref/about"><img src="https://images.opencollective.com/proxy/images/?src=https%3A%2F%2Flogo.clearbit.com%2Ffaithlife.com&height=96" alt="Faithlife" height="32"></a></p>
 <!--sponsorsend-->
diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js
index 3a21b6958..400ce0c76 100644
--- a/lib/rules/no-extra-parens.js
+++ b/lib/rules/no-extra-parens.js
@@ -471,6 +471,7 @@ module.exports = {
             const firstToken = isParenthesised(node) ? sourceCode.getTokenBefore(node) : sourceCode.getFirstToken(node);
             const secondToken = sourceCode.getTokenAfter(firstToken, astUtils.isNotOpeningParenToken);
             const thirdToken = secondToken ? sourceCode.getTokenAfter(secondToken) : null;
+            const tokenAfterClosingParens = secondToken ? sourceCode.getTokenAfter(secondToken, astUtils.isNotClosingParenToken) : null;
 
             if (
                 astUtils.isOpeningParenToken(firstToken) &&
@@ -479,7 +480,12 @@ module.exports = {
                     secondToken.type === "Keyword" && (
                         secondToken.value === "function" ||
                         secondToken.value === "class" ||
-                        secondToken.value === "let" && astUtils.isOpeningBracketToken(sourceCode.getTokenAfter(secondToken, astUtils.isNotClosingParenToken))
+                        secondToken.value === "let" &&
+                            tokenAfterClosingParens &&
+                            (
+                                astUtils.isOpeningBracketToken(tokenAfterClosingParens) ||
+                                tokenAfterClosingParens.type === "Identifier"
+                            )
                     ) ||
                     secondToken && secondToken.type === "Identifier" && secondToken.value === "async" && thirdToken && thirdToken.type === "Keyword" && thirdToken.value === "function"
                 )


Test patch:
diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js
index b3481d06a..366a6c39b 100644
--- a/tests/lib/rules/no-extra-parens.js
+++ b/tests/lib/rules/no-extra-parens.js
@@ -452,6 +452,7 @@ ruleTester.run("no-extra-parens", rule, {
         "({}) ? foo() : bar()",
         "({}) + foo",
         "(function(){}) + foo",
+        "(let)\nfoo",
         "(let[foo]) = 1", // setting the 'foo' property of the 'let' variable to 1
         {
             code: "((function(){}).foo.bar)();",
@@ -1092,6 +1093,18 @@ ruleTester.run("no-extra-parens", rule, {
             "Identifier",
             1
         ),
-        invalid("for (a in (b, c));", "for (a in b, c);", "SequenceExpression", null)
+        invalid("for (a in (b, c));", "for (a in b, c);", "SequenceExpression", null),
+        invalid(
+            "(let)",
+            "let",
+            "Identifier",
+            1
+        ),
+        invalid(
+            "((let))",
+            "(let)",
+            "Identifier",
+            1
+        )
     ]
 });