Repository: eslint/eslint
Pull request #8275 (2017-03-17 11:58:53)

Fix: false positive of no-extra-parens about spread and sequense
## What is the purpose of this pull request? (put an "X" next to item)

[X] Bug fix ([template](https://raw.githubusercontent.com/eslint/eslint/master/templates/bug-report.md))

**Tell us about your environment**

* **ESLint Version:** master
* **Node Version:** 7.6.0
* **npm Version:** 4.2.0

**What parser (default, Babel-ESLint, etc.) are you using?**

- default

**Please show your full configuration:**

- nothing

**What did you do? Please include the actual source code causing the issue.**

```
$ echo "let a = [ ...(b, c) ]" | eslint --stdin --env es6 --rule "no-extra-parens:2" --no-eslintrc --no-ignore
```

**What did you expect to happen?**

No errors.

**What actually happened? Please include the actual, raw output from ESLint.**

```
<text>
  1:14  error  Gratuitous parentheses around expression  no-extra-parens

✖ 1 problem (1 error, 0 warnings)
```

## What changes did you make? (Give an overview)

This PR fixes the bug of `no-extra-parens` that #8209 introduced. 
This bug has not been released yet.

`SequenseExpression` is not a `AssignmentExpression`, so `no-extra-parens` should allow `SequenseExpression` at `SpreadElement#argument`.

## Is there anything you'd like reviewers to focus on?

Nothing in particular.

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

Patch:
diff --git a/docs/rules/no-restricted-syntax.md b/docs/rules/no-restricted-syntax.md
index 2e94e5d66..ffe763c8b 100644
--- a/docs/rules/no-restricted-syntax.md
+++ b/docs/rules/no-restricted-syntax.md
@@ -58,4 +58,3 @@ If you don't want to restrict your code from using any JavaScript features or sy
 * [no-console](no-console.md)
 * [no-debugger](no-debugger.md)
 * [no-restricted-properties](no-restricted-properties.md)
-* [no-restricted-syntax](no-restricted-syntax.md)
diff --git a/docs/rules/no-unused-expressions.md b/docs/rules/no-unused-expressions.md
index 3b4a065e8..480915ee2 100644
--- a/docs/rules/no-unused-expressions.md
+++ b/docs/rules/no-unused-expressions.md
@@ -29,7 +29,8 @@ Sequence expressions (those using a comma, such as `a = 1, b = 2`) are always co
 This rule, in its default state, does not require any arguments. If you would like to enable one or more of the following you may pass an object with the options set as follows:
 
 * `allowShortCircuit` set to `true` will allow you to use short circuit evaluations in your expressions (Default: `false`).
-* `allowTernary` set to `true` will enable you use ternary operators in your expressions similarly to short circuit evaluations (Default: `false`).
+* `allowTernary` set to `true` will enable you to use ternary operators in your expressions similarly to short circuit evaluations (Default: `false`).
+* `allowTaggedTemplates` set to `true` will enable you to use tagged template literals in your expressions (Default: `false`).
 
 These options allow unused expressions *only if all* of the code paths either directly change the state (for example, assignment statement) or could have *side effects* (for example, function call).
 
@@ -56,6 +57,8 @@ a() && function namedFunctionInExpressionContext () {f();}
 
 (function anIncompleteIIFE () {});
 
+injectGlobal`body{ color: red; }`
+
 ```
 
 Note that one or more string expression statements (with or without semi-colons) will only be considered as unused if they are not in the beginning of a script, module, or function (alone and uninterrupted by other statements). Otherwise, they will be treated as part of a "directive prologue", a section potentially usable by JavaScript engines. This includes "strict mode" directives.
@@ -140,3 +143,21 @@ Examples of **correct** code for the `{ "allowShortCircuit": true, "allowTernary
 
 a ? b() || (c = d) : e()
 ```
+
+### allowTaggedTemplates
+
+Examples of **incorrect** code for the `{ "allowTaggedTemplates": true }` option:
+
+```js
+/*eslint no-unused-expressions: ["error", { "allowTaggedTemplates": true }]*/
+
+`some untagged template string`;
+```
+
+Examples of **correct** code for the `{ "allowTaggedTemplates": true }` option:
+
+```js
+/*eslint no-unused-expressions: ["error", { "allowTaggedTemplates": true }]*/
+
+tag`some tagged template string`;
+```
diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js
index 35fdfbfa6..8be8a5a49 100644
--- a/lib/rules/no-extra-parens.js
+++ b/lib/rules/no-extra-parens.js
@@ -67,6 +67,7 @@ module.exports = {
         const NESTED_BINARY = ALL_NODES && context.options[1] && context.options[1].nestedBinaryExpressions === false;
         const EXCEPT_RETURN_ASSIGN = ALL_NODES && context.options[1] && context.options[1].returnAssign === false;
         const IGNORE_JSX = ALL_NODES && context.options[1] && context.options[1].ignoreJSX;
+        const PRECEDENCE_OF_ASSIGNMENT_EXPR = precedence({ type: "AssignmentExpression" });
         const PRECEDENCE_OF_UPDATE_EXPR = precedence({ type: "UpdateExpression" });
 
         /**
@@ -394,12 +395,12 @@ module.exports = {
                 report(node.callee);
             }
             if (node.arguments.length === 1) {
-                if (hasDoubleExcessParens(node.arguments[0]) && precedence(node.arguments[0]) >= precedence({ type: "AssignmentExpression" })) {
+                if (hasDoubleExcessParens(node.arguments[0]) && precedence(node.arguments[0]) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
                     report(node.arguments[0]);
                 }
             } else {
                 [].forEach.call(node.arguments, arg => {
-                    if (hasExcessParens(arg) && precedence(arg) >= precedence({ type: "AssignmentExpression" })) {
+                    if (hasExcessParens(arg) && precedence(arg) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
                         report(arg);
                     }
                 });
@@ -456,7 +457,11 @@ module.exports = {
          * @returns {void}
          */
         function checkSpreadOperator(node) {
-            if (node.argument && hasExcessParens(node.argument)) {
+            const hasExtraParens = precedence(node.argument) >= PRECEDENCE_OF_ASSIGNMENT_EXPR
+                ? hasExcessParens(node.argument)
+                : hasDoubleExcessParens(node.argument);
+
+            if (hasExtraParens) {
                 report(node.argument);
             }
         }
@@ -464,7 +469,7 @@ module.exports = {
         return {
             ArrayExpression(node) {
                 [].forEach.call(node.elements, e => {
-                    if (e && hasExcessParens(e) && precedence(e) >= precedence({ type: "AssignmentExpression" })) {
+                    if (e && hasExcessParens(e) && precedence(e) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
                         report(e);
                     }
                 });
@@ -476,7 +481,7 @@ module.exports = {
                 }
 
                 if (node.body.type !== "BlockStatement") {
-                    if (sourceCode.getFirstToken(node.body).value !== "{" && hasExcessParens(node.body) && precedence(node.body) >= precedence({ type: "AssignmentExpression" })) {
+                    if (sourceCode.getFirstToken(node.body).value !== "{" && hasExcessParens(node.body) && precedence(node.body) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
                         report(node.body);
                         return;
                     }
@@ -510,11 +515,11 @@ module.exports = {
                     report(node.test);
                 }
 
-                if (hasExcessParens(node.consequent) && precedence(node.consequent) >= precedence({ type: "AssignmentExpression" })) {
+                if (hasExcessParens(node.consequent) && precedence(node.consequent) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
                     report(node.consequent);
                 }
 
-                if (hasExcessParens(node.alternate) && precedence(node.alternate) >= precedence({ type: "AssignmentExpression" })) {
+                if (hasExcessParens(node.alternate) && precedence(node.alternate) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
                     report(node.alternate);
                 }
             },
@@ -615,7 +620,7 @@ module.exports = {
                 [].forEach.call(node.properties, e => {
                     const v = e.value;
 
-                    if (v && hasExcessParens(v) && precedence(v) >= precedence({ type: "AssignmentExpression" })) {
+                    if (v && hasExcessParens(v) && precedence(v) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) {
                         report(v);
                     }
                 });
@@ -671,7 +676,7 @@ module.exports = {
 
             VariableDeclarator(node) {
                 if (node.init && hasExcessParens(node.init) &&
-                        precedence(node.init) >= precedence({ type: "AssignmentExpression" }) &&
+                        precedence(node.init) >= PRECEDENCE_OF_ASSIGNMENT_EXPR &&
 
                         // RegExp literal is allowed to have parens (#1589)
                         !(node.init.type === "Literal" && node.init.regex)) {
diff --git a/lib/rules/no-unused-expressions.js b/lib/rules/no-unused-expressions.js
index 548e02f46..b4e1074d5 100644
--- a/lib/rules/no-unused-expressions.js
+++ b/lib/rules/no-unused-expressions.js
@@ -25,6 +25,9 @@ module.exports = {
                     },
                     allowTernary: {
                         type: "boolean"
+                    },
+                    allowTaggedTemplates: {
+                        type: "boolean"
                     }
                 },
                 additionalProperties: false
@@ -35,7 +38,8 @@ module.exports = {
     create(context) {
         const config = context.options[0] || {},
             allowShortCircuit = config.allowShortCircuit || false,
-            allowTernary = config.allowTernary || false;
+            allowTernary = config.allowTernary || false,
+            allowTaggedTemplates = config.allowTaggedTemplates || false;
 
         /**
          * @param {ASTNode} node - any node
@@ -95,12 +99,17 @@ module.exports = {
                     return isValidExpression(node.consequent) && isValidExpression(node.alternate);
                 }
             }
+
             if (allowShortCircuit) {
                 if (node.type === "LogicalExpression") {
                     return isValidExpression(node.right);
                 }
             }
 
+            if (allowTaggedTemplates && node.type === "TaggedTemplateExpression") {
+                return true;
+            }
+
             return /^(?:Assignment|Call|New|Update|Yield|Await)Expression$/.test(node.type) ||
                 (node.type === "UnaryExpression" && ["delete", "void"].indexOf(node.operator) >= 0);
         }


Test patch:
diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js
index b1e945e5e..349c14994 100644
--- a/tests/lib/rules/no-extra-parens.js
+++ b/tests/lib/rules/no-extra-parens.js
@@ -380,6 +380,21 @@ ruleTester.run("no-extra-parens", rule, {
                 ecmaFeatures: { experimentalObjectRestSpread: true }
             }
         },
+        {
+            code: "let a = [ ...(b, c) ]",
+            parserOptions: { ecmaVersion: 2015 }
+        },
+        {
+            code: "let a = { ...(b, c) }",
+            parserOptions: {
+                ecmaVersion: 2015,
+                ecmaFeatures: { experimentalObjectRestSpread: true }
+            }
+        },
+        {
+            code: "var [x = (1, foo)] = bar",
+            parserOptions: { ecmaVersion: 2015 }
+        },
         {
             code: "class A extends B {}",
             parserOptions: { ecmaVersion: 2015 }
@@ -891,6 +906,25 @@ ruleTester.run("no-extra-parens", rule, {
                 }
             }
         ),
+        invalid(
+            "let a = [...((b, c))]",
+            "let a = [...(b, c)]",
+            "SequenceExpression",
+            1,
+            { parserOptions: { ecmaVersion: 2015 } }
+        ),
+        invalid(
+            "let a = {...((b, c))}",
+            "let a = {...(b, c)}",
+            "SequenceExpression",
+            1,
+            {
+                parserOptions: {
+                    ecmaVersion: 2015,
+                    ecmaFeatures: { experimentalObjectRestSpread: true }
+                }
+            }
+        ),
         invalid(
             "class A extends (B) {}",
             "class A extends B {}",
diff --git a/tests/lib/rules/no-unused-expressions.js b/tests/lib/rules/no-unused-expressions.js
index 57eb39ed5..520705179 100644
--- a/tests/lib/rules/no-unused-expressions.js
+++ b/tests/lib/rules/no-unused-expressions.js
@@ -60,6 +60,16 @@ ruleTester.run("no-unused-expressions", rule, {
             code: "async function foo() { foo ? await bar : await baz; }",
             options: [{ allowTernary: true }],
             parserOptions: { ecmaVersion: 8 }
+        },
+        {
+            code: "tag`tagged template literal`",
+            options: [{ allowTaggedTemplates: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "shouldNotBeAffectedByAllowTemplateTagsOption()",
+            options: [{ allowTaggedTemplates: true }],
+            parserOptions: { ecmaVersion: 6 }
         }
     ],
     invalid: [
@@ -72,6 +82,16 @@ ruleTester.run("no-unused-expressions", rule, {
         { code: "a() || false", errors: [{ message: "Expected an assignment or function call and instead saw an expression.", type: "ExpressionStatement" }] },
         { code: "a || (b = c)", errors: [{ message: "Expected an assignment or function call and instead saw an expression.", type: "ExpressionStatement" }] },
         { code: "a ? b() || (c = d) : e", errors: [{ message: "Expected an assignment or function call and instead saw an expression.", type: "ExpressionStatement" }] },
+        {
+            code: "`untagged template literal`",
+            errors: ["Expected an assignment or function call and instead saw an expression."],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "tag`tagged template literal`",
+            errors: ["Expected an assignment or function call and instead saw an expression."],
+            parserOptions: { ecmaVersion: 6 }
+        },
         { code: "a && b()", options: [{ allowTernary: true }], errors: [{ message: "Expected an assignment or function call and instead saw an expression.", type: "ExpressionStatement" }] },
         { code: "a ? b() : c()", options: [{ allowShortCircuit: true }], errors: [{ message: "Expected an assignment or function call and instead saw an expression.", type: "ExpressionStatement" }] },
         { code: "a || b", options: [{ allowShortCircuit: true }], errors: [{ message: "Expected an assignment or function call and instead saw an expression.", type: "ExpressionStatement" }] },
@@ -85,6 +105,24 @@ ruleTester.run("no-unused-expressions", rule, {
         { code: "function foo() {\"directive one\"; f(); \"directive two\"; }", errors: [{ message: "Expected an assignment or function call and instead saw an expression.", type: "ExpressionStatement" }] },
         { code: "if (0) { \"not a directive\"; f(); }", errors: [{ message: "Expected an assignment or function call and instead saw an expression.", type: "ExpressionStatement" }] },
         { code: "function foo() { var foo = true; \"use strict\"; }", errors: [{ message: "Expected an assignment or function call and instead saw an expression.", type: "ExpressionStatement" }] },
-        { code: "var foo = () => { var foo = true; \"use strict\"; }", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected an assignment or function call and instead saw an expression.", type: "ExpressionStatement" }] }
+        { code: "var foo = () => { var foo = true; \"use strict\"; }", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected an assignment or function call and instead saw an expression.", type: "ExpressionStatement" }] },
+        {
+            code: "`untagged template literal`",
+            errors: ["Expected an assignment or function call and instead saw an expression."],
+            options: [{ allowTaggedTemplates: true }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "`untagged template literal`",
+            errors: ["Expected an assignment or function call and instead saw an expression."],
+            options: [{ allowTaggedTemplates: false }],
+            parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "tag`tagged template literal`",
+            errors: ["Expected an assignment or function call and instead saw an expression."],
+            options: [{ allowTaggedTemplates: false }],
+            parserOptions: { ecmaVersion: 6 }
+        }
     ]
 });