Repository: eslint/eslint
Pull request #11973 (2019-07-10 00:47:36)

Update: Check computed method keys in no-extra-parens
<!--
    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

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->

**Tell us about your environment**

* **ESLint Version:** 6.0.1
* **Node Version:** 10.16.0
* **npm Version:** 6.9.0

**What parser (default, Babel-ESLint, etc.) are you using?**

default

**Please show your full configuration:**

<details>
<summary>Configuration</summary>

<!-- Paste your configuration below: -->
```js
module.exports = {
  parserOptions: {
    ecmaVersion: 6,
  },
  rules: {
    "no-extra-parens": "error"
  }
};
```

</details>

**What did you do? Please include the actual source code causing the issue.**

```js
class foo {
  *[(Symbol.iterator)]() {}

  get [(a)]() {}
  set [(a)](bar){}

  static [(b)](){} 
}
```

**What did you expect to happen?**

4 errors.

**What actually happened? Please include the actual, raw output from ESLint.**

No errors.

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (https://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**

Added `ClassBody()` to `no-extra-parens`.

**Is there anything you'd like reviewers to focus on?**

* I guess this is a bug fix that adds more reported warnings, rather than a new behavior.
* The same could be done with `MethodDefinition()`, but `ClassBody()` allows to easily check computed
public instance fields and static class fields (both are `babel-eslint`'s `"ClassProperty"`) in the future.
Actually, it can be easily done now (without the `member.type === "MethodDefinition"` condition the code would simply check anything in the class body that has `computed === true` ), but I'm not sure is it OK to do it at this moment.
* `class a { [b, c](){} }` is indeed a syntax error without parens around the sequence expression.




mdjermanovic: I could copy output from `babel-eslint` and paste it somewhere in `../tests/fixtures/parsers`

My comment was probably confusing. The question is - should this rule, at this moment, remove parens around computed keys of stage 3 class members, or simply ignore them? This commit ignores them (`member.type === "MethodDefinition"'` filter).
platinumazure: > The question is - should this rule, at this moment, remove parens around computed keys of stage 3 class members, or simply ignore them?

I think it's okay to ignore those cases for now. eslint-plugin-babel or similar plugins could extend the rule to support those stage 3 cases.
mdjermanovic: > > The question is - should this rule, at this moment, remove parens around computed keys of stage 3 class members, or simply ignore them?
> 
> I think it's okay to ignore those cases for now. eslint-plugin-babel or similar plugins could extend the rule to support those stage 3 cases.

OK, then there is nothing to change now, the commit is in line with this.

Maybe just to add a couple of babel test cases to show that the rule does not crash on such input?
platinumazure: > Maybe just to add a couple of babel test cases to show that the rule does not crash on such input?

I don't think that's necessary at this point, but other team members might feel differently.
mdjermanovic: I''ll resolve this better when I get back from travel.
mdjermanovic: Rebased and ready for review now.

Patch:
diff --git a/docs/rules/no-restricted-globals.md b/docs/rules/no-restricted-globals.md
index e3d612412..dd3e15b7b 100644
--- a/docs/rules/no-restricted-globals.md
+++ b/docs/rules/no-restricted-globals.md
@@ -77,7 +77,7 @@ Examples of **incorrect** code for a sample `"event"` global variable name, alon
 
 ```js
 /*global event*/
-/* eslint no-restricted-globals: ["error", { name: "error", message: "Use local parameter instead." }] */
+/* eslint no-restricted-globals: ["error", { name: "event", message: "Use local parameter instead." }] */
 
 function onClick() {
     console.log(event);    // Unexpected global variable 'event'. Use local parameter instead.
diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js
index 6c3198b5f..c9278f2d0 100644
--- a/lib/rules/no-extra-parens.js
+++ b/lib/rules/no-extra-parens.js
@@ -686,6 +686,13 @@ module.exports = {
 
             CallExpression: checkCallNew,
 
+            ClassBody(node) {
+                node.body
+                    .filter(member => member.type === "MethodDefinition" && member.computed &&
+                        member.key && hasExcessParens(member.key) && precedence(member.key) >= PRECEDENCE_OF_ASSIGNMENT_EXPR)
+                    .forEach(member => report(member.key));
+            },
+
             ConditionalExpression(node) {
                 if (isReturnAssignException(node)) {
                     return;


Test patch:
diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js
index 7be7d6e9e..6c6eeb2a1 100644
--- a/tests/lib/rules/no-extra-parens.js
+++ b/tests/lib/rules/no-extra-parens.js
@@ -160,6 +160,13 @@ ruleTester.run("no-extra-parens", rule, {
         "const {[a]:b=1} = {}",
         "const {[(a, b)]:c} = {}",
         "const {a, ...b} = {}",
+        "class foo {}",
+        "class foo { constructor(){} a(){} get b(){} set b(bar){} get c(){} set d(baz){} static e(){} }",
+        "class foo { [a](){} get [b](){} set [b](bar){} get [c](){} set [d](baz){} static [e](){} }",
+        "class foo { [(a,b)](){} }",
+        "class foo { a(){} [b](){} c(){} [(d,e)](){} }",
+        "class foo { [(a,b)](){} c(){} [d](){} e(){} }",
+        "const foo = class { constructor(){} a(){} get b(){} set b(bar){} get c(){} set d(baz){} static e(){} }",
 
         // ExpressionStatement restricted productions
         "({});",
@@ -553,6 +560,22 @@ ruleTester.run("no-extra-parens", rule, {
         invalid("const {a, b:c, [(d+e)]:f, [(g,h)]:i, [j]:k} = {}", "const {a, b:c, [d+e]:f, [(g,h)]:i, [j]:k} = {}", "BinaryExpression", 1),
         invalid("const {[a+(b*c)]:d} = {}", "const {[a+b*c]:d} = {}", "BinaryExpression", 1),
         invalid("const {[(a, (b+c))]:d} = {}", "const {[(a, b+c)]:d} = {}", "BinaryExpression", 1),
+        invalid("class foo { [(a)](){} }", "class foo { [a](){} }", "Identifier"),
+        invalid("class foo {*[(Symbol.iterator)]() {}}", "class foo {*[Symbol.iterator]() {}}", "MemberExpression"),
+        invalid("class foo { get [(a)](){} }", "class foo { get [a](){} }", "Identifier"),
+        invalid("class foo { set [(a)](bar){} }", "class foo { set [a](bar){} }", "Identifier"),
+        invalid("class foo { static [(a)](bar){} }", "class foo { static [a](bar){} }", "Identifier"),
+        invalid("class foo { [(a=b)](){} }", "class foo { [a=b](){} }", "AssignmentExpression"),
+        invalid("class foo { constructor (){} [(a+b)](){} }", "class foo { constructor (){} [a+b](){} }", "BinaryExpression"),
+        invalid("class foo { [(a+b)](){} constructor (){} }", "class foo { [a+b](){} constructor (){} }", "BinaryExpression"),
+        invalid("class foo { [(a+b)](){} c(){} }", "class foo { [a+b](){} c(){} }", "BinaryExpression"),
+        invalid("class foo { a(){} [(b+c)](){} d(){} }", "class foo { a(){} [b+c](){} d(){} }", "BinaryExpression"),
+        invalid("class foo { [(a+b)](){} [c](){} }", "class foo { [a+b](){} [c](){} }", "BinaryExpression"),
+        invalid("class foo { [a](){} [(b+c)](){} [d](){} }", "class foo { [a](){} [b+c](){} [d](){} }", "BinaryExpression"),
+        invalid("class foo { [(a+b)](){} [(c,d)](){} }", "class foo { [a+b](){} [(c,d)](){} }", "BinaryExpression"),
+        invalid("class foo { [(a,b)](){} [(c+d)](){} }", "class foo { [(a,b)](){} [c+d](){} }", "BinaryExpression"),
+        invalid("class foo { [a+(b*c)](){} }", "class foo { [a+b*c](){} }", "BinaryExpression"),
+        invalid("const foo = class { [(a)](){} }", "const foo = class { [a](){} }", "Identifier"),
         invalid(
             "var foo = (function*() { if ((yield foo())) { return; } }())",
             "var foo = (function*() { if (yield foo()) { return; } }())",