Content-Length: 324605 | pFad | https://github.com/eslint/eslint/issues/16131

01 Apparent discrepancy in the definition of a block between rules · Issue #16131 · eslint/eslint · GitHub
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

Apparent discrepancy in the definition of a block between rules #16131

Closed
1 task
kboucher opened this issue Jul 14, 2022 · 8 comments · Fixed by #16153
Closed
1 task

Apparent discrepancy in the definition of a block between rules #16131

kboucher opened this issue Jul 14, 2022 · 8 comments · Fixed by #16153
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 bug ESLint is working incorrectly repro:yes Issues with a reproducible example rule Relates to ESLint's core rules

Comments

@kboucher
Copy link

Environment

Node version: 16.14.1
npm version: 8.5.0
Local ESLint version: 7.32.0
Global ESLint version: N/A
Operating System: OS X

What parser are you using?

@babel/eslint-parser

What did you do?

For the lines-around-comment rule, setting allowBlockStart: true is not respected for switch statements.

According to this issue: #9653, that is because switch statement curly braces are not considered "blocks". See comment: #9653 (comment)

However, the padded-blocks rule appears to interpret switch statement curly braces as blocks which results in a discrepancy that can only be remedied by never including a comment inside a switch statement's opening curly brace.

Relevant rule configurations:

'lines-around-comment': [
    2,
    {
        allowArrayStart: true,
        allowBlockStart: true,
        allowObjectStart: true,
        beforeBlockComment: true,
        beforeLineComment: true,
    },
],
'padded-blocks': ['error', 'never'],

Sample code

let someValue = "a";

switch (someValue) {
    // some comment
  case 'a':
    someValue = "b";
    break;
  
  default:
    someValue = "c";
}

Link to reproduction of issue:
https://eslint.org/play/#eyJ0ZXh0IjoibGV0IHNvbWVWYWx1ZSA9IFwiYVwiO1xuXG5zd2l0Y2ggKHNvbWVWYWx1ZSkge1xuICAgIC8vIHNvbWUgY29tbWVudFxuICBjYXNlICdhJzpcbiAgICBzb21lVmFsdWUgPSBcImJcIjtcbiAgICBicmVhaztcbiAgXG4gIGRlZmF1bHQ6XG4gICAgc29tZVZhbHVlID0gXCJjXCI7XG59Iiwib3B0aW9ucyI6eyJwYXJzZXJPcHRpb25zIjp7ImVjbWFWZXJzaW9uIjoibGF0ZXN0Iiwic291cmNlVHlwZSI6InNjcmlwdCIsImVjbWFGZWF0dXJlcyI6e319LCJydWxlcyI6eyJsaW5lcy1hcm91bmQtY29tbWVudCI6WzIseyJhbGxvd0Jsb2NrU3RhcnQiOnRydWUsImJlZm9yZUxpbmVDb21tZW50Ijp0cnVlLCJiZWZvcmVCbG9ja0NvbW1lbnQiOnRydWUsImFmdGVyQmxvY2tDb21tZW50IjpmYWxzZSwiYWZ0ZXJMaW5lQ29tbWVudCI6ZmFsc2UsImFsbG93QmxvY2tFbmQiOmZhbHNlfV0sInBhZGRlZC1ibG9ja3MiOlsiZXJyb3IiLCJuZXZlciJdfSwiZW52Ijp7ImVzNiI6dHJ1ZX19fQ==

What did you expect to happen?

I expected that setting allowBlockStart: true would preclude a violation of beforeLineComment: true in the lines-around-comment rule.

Given the discussion in issue 9653, I would then expect 'padded-blocks': ['error', 'never'] to not result in an error inside a switch statement's opening curly brace.

What actually happened?

To visualize, visit the reproduction and add a line break before the comment. Note that the lines-around-comment error becomes a padded-blocks error because the two rules do not consistently interpret switch case curly braces as blocks.

https://eslint.org/play/#eyJ0ZXh0IjoibGV0IHNvbWVWYWx1ZSA9IFwiYVwiO1xuXG5zd2l0Y2ggKHNvbWVWYWx1ZSkge1xuICAgIFxuICAgIC8vIHNvbWUgY29tbWVudFxuICBjYXNlICdhJzpcbiAgICBzb21lVmFsdWUgPSBcImJcIjtcbiAgICBicmVhaztcbiAgXG4gIGRlZmF1bHQ6XG4gICAgc29tZVZhbHVlID0gXCJjXCI7XG59Iiwib3B0aW9ucyI6eyJwYXJzZXJPcHRpb25zIjp7ImVjbWFWZXJzaW9uIjoibGF0ZXN0Iiwic291cmNlVHlwZSI6InNjcmlwdCIsImVjbWFGZWF0dXJlcyI6e319LCJydWxlcyI6eyJsaW5lcy1hcm91bmQtY29tbWVudCI6WzIseyJhbGxvd0Jsb2NrU3RhcnQiOnRydWUsImJlZm9yZUxpbmVDb21tZW50Ijp0cnVlLCJiZWZvcmVCbG9ja0NvbW1lbnQiOnRydWUsImFmdGVyQmxvY2tDb21tZW50IjpmYWxzZSwiYWZ0ZXJMaW5lQ29tbWVudCI6ZmFsc2UsImFsbG93QmxvY2tFbmQiOmZhbHNlfV0sInBhZGRlZC1ibG9ja3MiOlsiZXJyb3IiLCJuZXZlciJdfSwiZW52Ijp7ImVzNiI6dHJ1ZX19fQ==

Participation

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

Additional comments

No response

@kboucher kboucher added bug ESLint is working incorrectly repro:needed This issue should include a reproducible example labels Jul 14, 2022
@nzakas
Copy link
Member

nzakas commented Jul 15, 2022

Yeah, that definitely seems inconsistent. Unfortunately, these things happen when you have hundreds of rules to manage.

It seems like the best way forward here would be to add the options suggested in #9653 (comment): allowSwitchStart and allowSwitchEnd.

Let’s see what the rest of the team thinks.

@nzakas nzakas added repro:yes Issues with a reproducible example and removed repro:needed This issue should include a reproducible example labels Jul 15, 2022
@kboucher
Copy link
Author

Unfortunately, these things happen when you have hundreds of rules to manage.

Totally not judging. 😁

I would just like to be able to add a comment before the first case in a switch statement without having to turn off the padded-blocks rule.

Looking forward to a possible solution. Thanks!

@mdjermanovic
Copy link
Member

allowBlockEnd: true already applies to switch statements so it would be consistent that allowBlockStart: true applies as well.

/* eslint lines-around-comment: ["error", {
  beforeLineComment: true,
  afterLineComment: true,
  allowBlockStart: true,
  allowBlockEnd: true
}]*/

switch (foo) {
  // this comment isn't allowed by allowBlockStart: true 
    
  case 1:    
    bar();
    break;
    
  // this comment is allowed by allowBlockEnd: true
}

Playground link

@mdjermanovic mdjermanovic added the rule Relates to ESLint's core rules label Jul 16, 2022
@nzakas
Copy link
Member

nzakas commented Jul 19, 2022

Oh wow, didn’t realize that. In that case, I agree it’s a bug and we should fix it so allowBlockStart also applies to switch statements.

@nzakas nzakas added the accepted There is consensus among the team that this change meets the criteria for inclusion label Jul 19, 2022
@snitin315 snitin315 self-assigned this Jul 19, 2022
@snitin315
Copy link
Contributor

I'll prepare a fix for it.

@nzakas
Copy link
Member

nzakas commented Jul 20, 2022

Thanks @snitin315

@kboucher
Copy link
Author

kboucher commented Jul 27, 2022

Thank you @snitin315!

Any idea when this fix will be released?

@snitin315
Copy link
Contributor

@kboucher the next release is scheduled on 29th July.

@nzakas nzakas moved this to Complete in Triage Jan 3, 2023
@nzakas nzakas added this to Triage Jan 3, 2023
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jan 22, 2023
@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 Jan 22, 2023
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 Issues with a reproducible example rule Relates to ESLint's core rules
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/eslint/eslint/issues/16131

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy