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

Support braces for no-fallthrough #7889

Closed
cowwoc opened this issue Jan 9, 2017 · 7 comments
Closed

Support braces for no-fallthrough #7889

cowwoc opened this issue Jan 9, 2017 · 7 comments
Labels
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 evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules

Comments

@cowwoc
Copy link

cowwoc commented Jan 9, 2017

What version of ESLint are you using?
3.0.1
What rule do you want to change?
no-fallthrough
What code should be flagged as correct with this change?

/*eslint no-fallback: ["error", { "commentPattern": "[\\w\\s]*fallthrough" }]*/
var foo = ...;
switch(bar) {
    case 0: {
      if (typeof(foo) == 'string')
        return doSomething();
      // Otherwise, fallthrough
    }
    default:
      doSomethingElse();
}

What happens when the rule is applied to this code now?
Errors:
Expected a 'break' statement before 'default'.

... Basically I am asking for the rule to support case statements that contain braces.

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jan 9, 2017
@platinumazure platinumazure added enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Jan 9, 2017
@platinumazure
Copy link
Member

Seems reasonable to me.

Just to make sure I understand your proposal, using ESTree terms:

  1. For a given SwitchCase,
  2. If its consequent contains one statement and that is a BlockStatement,
  3. Then the rule logic should effectively be run on the BlockStatement instead of the SwitchCase, and a // falls through label or similar at the end of the BlockStatement should be acceptable and not cause a warning.

Does that sound correct? (Note: I'm basing this on the AST seen here.)

@cowwoc
Copy link
Author

cowwoc commented Jan 9, 2017

@platinumazure Yes, that sounds about right :) Thank you.

@not-an-aardvark
Copy link
Member

Thanks for your interest in improving ESLint. Unfortunately, it looks like this issue didn't get enough support from the team and so I'm closing it. 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 consensus after a long time tend to never do it, and as such, we close those issues. This doesn't mean the idea isn't interesting, just that it's not something the team can commit to.

@cowwoc
Copy link
Author

cowwoc commented Jul 11, 2017

@not-an-aardvark I didn't get the feeling that there was a lack of consensus over this issue. Please see the last 2 comments.

@ilyavolodin
Copy link
Member

@cowwoc what @not-an-aardvark means is consensus from the team. Every proposed change has to be supported by 3 team members and championed by one. Unfortunately, in this case, none of the team members elected to support this change other then @platinumazure
As @not-an-aardvark message mentions, we found that if the issue doesn't gain support shortly after being created, it's most likely will be forgotten and will never gain support required for implementation:-( That's why we periodically close older issues.

@cowwoc
Copy link
Author

cowwoc commented Jul 11, 2017

Fair enough. Thanks for the clarification. Hopefully this will be revisited in the future.

@j-f1
Copy link
Contributor

j-f1 commented Aug 9, 2017

This has been brought up again in #9080.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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 evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

6 participants