Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no-extra-parens new option to avoid conflicts with no-sequences #11916

Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules

Comments

@mdjermanovic
Copy link
Member

What rule do you want to change?

no-extra-parens

Does this change cause the rule to produce more or fewer warnings?

fewer

How will the change be implemented? (New option, new default behavior, etc.)?

new option

Please provide some example code that this change will affect:

/* eslint  no-extra-parens: "error" */
/* eslint  no-sequences: "error" */

(a, b); // 'no-extra-parens' warning

if ((a, b)); // 'no-extra-parens' warning

while((a, b)); // 'no-extra-parens' warning

What does the rule currently do for this code?

Fixes it to:

/* eslint  no-extra-parens: "error" */
/* eslint  no-sequences: "error" */

a, b; // 'no-sequences' warning

if (a, b); // 'no-sequences' warning

while(a, b); // 'no-sequences' warning

What will the rule do after it's changed?

/* eslint  no-extra-parens: ["error", "all", { "sequenceExpressions": false }] */
/* eslint  no-sequences: "error" */

(a, b); // no warnings

if ((a, b)); // no warnings

while((a, b)); // no warnings

Are you willing to submit a pull request to implement this change?

Yes

@mdjermanovic mdjermanovic added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules triage An ESLint team member will look at this issue soon labels Jun 28, 2019
@mysticatea mysticatea added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jun 28, 2019
@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Jul 29, 2019
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@platinumazure
Copy link
Member

@eslint/eslint-team This just needs a champion and we could accept this!

@kaicataldo
Copy link
Member

I'll champion this.

@kaicataldo kaicataldo self-assigned this Aug 18, 2019
@kaicataldo kaicataldo added accepted There is consensus among the team that this change meets the criteria for inclusion and removed auto closed The bot closed this issue evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Aug 18, 2019
@kaicataldo kaicataldo reopened this Aug 18, 2019
@mdjermanovic
Copy link
Member Author

I'm working on this :)

What should be the name of the new option? The last was enforceForArrowConditionals

Maybe enforceForSequenceExpressions instead of just sequenceExpressions?

@platinumazure
Copy link
Member

I like enforceForSequenceExpressions personally. I generally don't like option names without a verb indicating what will happen if you turn it on. (Just my two cents; other team members may have their own opinion.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.