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

Bug: conflict between space-before-blocks and switch-colon-spacing #15082

Closed
1 task done
morganbarrett opened this issue Sep 19, 2021 · 4 comments · Fixed by #15093
Closed
1 task done

Bug: conflict between space-before-blocks and switch-colon-spacing #15082

morganbarrett opened this issue Sep 19, 2021 · 4 comments · Fixed by #15093
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 bug ESLint is working incorrectly repro:yes
Projects

Comments

@morganbarrett
Copy link

morganbarrett commented Sep 19, 2021

Environment

Node version: 16.0.0
npm version: 7.10.0
Local ESLint version: 7.32.0
Global ESLint version: 7.32.0
Operating System: osx 11.4

What parser are you using?

@typescript-eslint/parser

What did you do?

Configuration
{
    "switch-colon-spacing": "error",
    "space-before-blocks": ["error", "never"]
}
switch(x){
    case 9: {
        break;
    }
}

What did you expect to happen?

For space-before-blocks to acknowledge the rule settings of switch-colon-spacing and behave accordingly.

What actually happened?

The two rules conflicted leaving no state where there was not an error.

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

No response

@morganbarrett morganbarrett added bug ESLint is working incorrectly repro:needed labels Sep 19, 2021
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Sep 19, 2021
@mdjermanovic mdjermanovic moved this from Needs Triage to Triaging in Triage Sep 19, 2021
@mdjermanovic
Copy link
Member

mdjermanovic commented Sep 19, 2021

Hi @morganbarrett, thanks for the issue!

I can reproduce this, it seems that those two rules are indeed conflicting:

 /* eslint 
    "switch-colon-spacing": "error",
    "space-before-blocks": ["error", "never"]
*/

switch(x){
    case 9: { // space-before-blocks error
        break;
    }
}

switch(x){
    case 9:{ // switch-colon-spacing error
        break;
    }
}

Online Demo

Since switch-colon-spacing looks more specific for this code, we should probably update space-before-blocks to ignore blocks in this position. It already ignores blocks after => and after keywords to avoid conflicts with other rules, as documented in rule details.

That might be a breaking change, though.

@eslint/eslint-tsc thoughts about this? If updating space-before-blocks to ignore these blocks looks like the right solution, shall we plan that change for ESLint v8.0.0?

@mdjermanovic mdjermanovic added needs bikeshedding Minor details about this change need to be discussed repro:yes and removed repro:needed labels Sep 19, 2021
@morganbarrett morganbarrett changed the title Bug: confliction between space-before-blocks and switch-colon-spacing Bug: conflict between space-before-blocks and switch-colon-spacing Sep 19, 2021
@btmills
Copy link
Member

btmills commented Sep 19, 2021

Since switch-colon-spacing looks more specific for this code, we should probably update space-before-blocks to ignore blocks in this position.

I concur. Since that change would a) resolve an unintentional incompatibility between two rules and b) report fewer errors, I don’t see it as a breaking change.

@nzakas
Copy link
Member

nzakas commented Sep 21, 2021

I concur. Since that change would a) resolve an unintentional incompatibility between two rules and b) report fewer errors, I don’t see it as a breaking change.

Agree. 👍

@nzakas nzakas added accepted There is consensus among the team that this change meets the criteria for inclusion and removed needs bikeshedding Minor details about this change need to be discussed labels Sep 21, 2021
@nzakas nzakas moved this from Triaging to Ready to Implement in Triage Sep 21, 2021
@mdjermanovic
Copy link
Member

mdjermanovic commented Sep 22, 2021

I'm working on this.

Triage automation moved this from Ready to Implement to Complete Sep 24, 2021
mdjermanovic added a commit that referenced this issue Sep 24, 2021
…#15093)

* Update: space-before-blocks ignore after switch colons (fixes #15082)

* remove extra space in docs
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Mar 24, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Mar 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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 bug ESLint is working incorrectly repro:yes
Projects
Triage
Complete
Development

Successfully merging a pull request may close this issue.

4 participants