Update: Check computed method keys in no-extra-parens

[mdjermanovic]

What is the purpose of this pull request? (put an "X" next to item)

[X] Bug fix

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:

Configuration

module.exports = {
  parserOptions: {
    ecmaVersion: 6,
  },
  rules: {
    "no-extra-parens": "error"
  }
};

What did you do? Please include the actual source code causing the issue.

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.

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.

[aladdin-add]

Thanks for contributing, and the kindly description! :)

public instance fields and static class fields is in stage-3, it means eslint don't officially support. So I don't think we can add tests for this. But other cases LGTM!

[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.

[aladdin-add]

LGTM, thanks!

[platinumazure]

LGTM, thanks for contributing!

[mysticatea]

LGTM, thank you.

Would you resolve conflicts?

[mdjermanovic]

I''ll resolve this better when I get back from travel.

[mdjermanovic]

Rebased and ready for review now.

[platinumazure]

LGTM, thanks!

[platinumazure]

Thanks @mdjermanovic for contributing! I appreciate your patience as we sorted this one out 😄