Repository: eslint/eslint
Pull request #8339 (2017-03-26 21:18:52)

Fix: no-extra-parens false positive with objects following arrows
**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.4
* **npm Version:** 4.1.2

**What parser (default, Babel-ESLint, etc.) are you using?**

default

**Please show your full configuration:**

```yml
parserOptions:
  ecmaVersion: 6
rules:
  no-extra-parens: error
```

**What did you do? Please include the actual source code causing the issue.**

```js
() => ({ foo: 1 }).foo;
```

**What did you expect to happen?**

I expected no error to be reported.

**What actually happened? Please include the actual, raw output from ESLint.**

An error was reported. The rule suggested fixing the code to

```js
() => { foo: 1 }.foo
```

which is invalid syntax.

**What changes did you make? (Give an overview)**

Previously, the no-extra-parens rule correctly did not report an error when an arrow function returned a parenthesized object expression. However, it would still report an error if an arrow function had a parenthesized object curly as its first token. This commit updates the rule to not report a paren between an arrow token and an opening curly token.

**Is there anything you'd like reviewers to focus on?**

I'm wondering if there's a better way to check for this case.


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

Patch:
diff --git a/lib/eslint.js b/lib/eslint.js
index 3c23a7495..a9066c4c8 100755
--- a/lib/eslint.js
+++ b/lib/eslint.js
@@ -778,17 +778,18 @@ module.exports = (function() {
         // search and apply "eslint-env *".
         const envInFile = findEslintEnv(text || textOrSourceCode.text);
 
+        config = Object.assign({}, config);
+
         if (envInFile) {
-            if (!config || !config.env) {
-                config = Object.assign({}, config || {}, { env: envInFile });
-            } else {
-                config = Object.assign({}, config);
+            if (config.env) {
                 config.env = Object.assign({}, config.env, envInFile);
+            } else {
+                config.env = envInFile;
             }
         }
 
         // process initial config to make it safe to extend
-        config = prepareConfig(config || {});
+        config = prepareConfig(config);
 
         // only do this for text
         if (text !== null) {
diff --git a/lib/rules/comma-style.js b/lib/rules/comma-style.js
index 4892ae505..fcaecc662 100644
--- a/lib/rules/comma-style.js
+++ b/lib/rules/comma-style.js
@@ -205,7 +205,11 @@ module.exports = {
                                 currentItemToken, reportItem);
                     }
 
-                    previousItemToken = item ? sourceCode.getLastToken(item) : previousItemToken;
+                    if (item) {
+                        const tokenAfterItem = sourceCode.getTokenAfter(item, astUtils.isNotClosingParenToken);
+
+                        previousItemToken = tokenAfterItem ? sourceCode.getTokenBefore(tokenAfterItem) : sourceCode.ast.tokens[sourceCode.ast.tokens.length - 1];
+                    }
                 });
 
                 /*
diff --git a/lib/rules/dot-notation.js b/lib/rules/dot-notation.js
index 3f5eae0bb..abb9b4b48 100644
--- a/lib/rules/dot-notation.js
+++ b/lib/rules/dot-notation.js
@@ -79,9 +79,11 @@ module.exports = {
                                     return null;
                                 }
 
+                                const textBeforeDot = astUtils.isDecimalInteger(node.object) ? " " : "";
+
                                 return fixer.replaceTextRange(
                                     [leftBracket.range[0], rightBracket.range[1]],
-                                    `.${node.property.value}`
+                                    `${textBeforeDot}.${node.property.value}`
                                 );
                             }
                         });
diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js
index 42c2bda99..42a86484b 100644
--- a/lib/rules/no-extra-parens.js
+++ b/lib/rules/no-extra-parens.js
@@ -345,6 +345,11 @@ module.exports = {
         function report(node) {
             const leftParenToken = sourceCode.getTokenBefore(node);
             const rightParenToken = sourceCode.getTokenAfter(node);
+            const tokenBeforeLeftParen = sourceCode.getTokenBefore(leftParenToken);
+
+            if (tokenBeforeLeftParen && astUtils.isArrowToken(tokenBeforeLeftParen) && astUtils.isOpeningBraceToken(sourceCode.getFirstToken(node))) {
+                return;
+            }
 
             context.report({
                 node,
@@ -481,7 +486,7 @@ module.exports = {
                 }
 
                 if (node.body.type !== "BlockStatement") {
-                    if (sourceCode.getFirstToken(node.body).value !== "{" && hasExcessParens(node.body) && precedence(node.body) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
+                    if (hasExcessParens(node.body) && precedence(node.body) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
                         report(node.body);
                         return;
                     }


Test patch:
diff --git a/tests/lib/eslint.js b/tests/lib/eslint.js
index 406302a69..9a1456cb1 100644
--- a/tests/lib/eslint.js
+++ b/tests/lib/eslint.js
@@ -3425,6 +3425,16 @@ describe("eslint", () => {
 
             assert(result.length === 0);
         });
+
+
+        it("should not modify config object passed as argument", () => {
+            const config = {};
+
+            Object.freeze(config);
+            assert.doesNotThrow(() => {
+                eslint.verify("var foo", config);
+            });
+        });
     });
 
     describe("Variables and references", () => {
diff --git a/tests/lib/rules/comma-style.js b/tests/lib/rules/comma-style.js
index 8dcbbc883..98c765c6b 100644
--- a/tests/lib/rules/comma-style.js
+++ b/tests/lib/rules/comma-style.js
@@ -569,6 +569,11 @@ ruleTester.run("comma-style", rule, {
                 message: BAD_LN_BRK_MSG,
                 type: "Identifier"
             }]
+        },
+        {
+            code: "[(foo),\n,\nbar]",
+            output: "[(foo),,\nbar]",
+            errors: [{ message: BAD_LN_BRK_MSG }]
         }
     ]
 });
diff --git a/tests/lib/rules/dot-notation.js b/tests/lib/rules/dot-notation.js
index e228899e1..0c28c4393 100644
--- a/tests/lib/rules/dot-notation.js
+++ b/tests/lib/rules/dot-notation.js
@@ -168,6 +168,11 @@ ruleTester.run("dot-notation", rule, {
             code: "(foo)['bar']",
             output: "(foo).bar",
             errors: [{ message: "[\"bar\"] is better written in dot notation." }]
+        },
+        {
+            code: "1['toString']",
+            output: "1 .toString",
+            errors: [{ message: "[\"toString\"] is better written in dot notation." }]
         }
     ]
 });
diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js
index 9b12ccf81..967d2df05 100644
--- a/tests/lib/rules/no-extra-parens.js
+++ b/tests/lib/rules/no-extra-parens.js
@@ -410,6 +410,22 @@ ruleTester.run("no-extra-parens", rule, {
         {
             code: "const A = class extends (B=C) {}",
             parserOptions: { ecmaVersion: 2015 }
+        },
+        {
+            code: "() => ({ foo: 1 })",
+            parserOptions: { ecmaVersion: 2015 }
+        },
+        {
+            code: "() => ({ foo: 1 }).foo",
+            parserOptions: { ecmaVersion: 2015 }
+        },
+        {
+            code: "() => ({ foo: 1 }.foo().bar).baz.qux()",
+            parserOptions: { ecmaVersion: 2015 }
+        },
+        {
+            code: "() => ({ foo: 1 }.foo().bar + baz)",
+            parserOptions: { ecmaVersion: 2015 }
         }
     ],
 
@@ -959,6 +975,18 @@ ruleTester.run("no-extra-parens", rule, {
             "Identifier",
             1,
             { parserOptions: { ecmaVersion: 2015 } }
-        )
+        ),
+        {
+            code: "() => (({ foo: 1 }).foo)",
+            output: "() => ({ foo: 1 }).foo",
+            parserOptions: { ecmaVersion: 2015 },
+            errors: [
+
+                // 2 errors are reported, but fixing one gets rid of the other
+                { message: "Gratuitous parentheses around expression.", type: "MemberExpression" },
+                { message: "Gratuitous parentheses around expression.", type: "ObjectExpression" }
+            ]
+
+        }
     ]
 });