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

curly and comments #7538

Closed
wwwillchen opened this issue Nov 5, 2016 · 3 comments · Fixed by #7539
Closed

curly and comments #7538

wwwillchen opened this issue Nov 5, 2016 · 3 comments · Fixed by #7539
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 rule Relates to ESLint's core rules

Comments

@wwwillchen
Copy link
Contributor

Tell us about your environment

  • ESLint Version: 3.9.1
  • Node Version: 4.5.0
  • npm Version: 2.15.11

What parser (default, Babel-ESLint, etc.) are you using?
default

Please show your full configuration:

"curly": [2, "multi-or-nest", "consistent"]

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

if (foo) {
  // some comment
  bar();
}

What did you expect to happen?
ESLint should not give an error for having a brace because the block is multi-line, however ESLint is treating it as a single-line statement and does not observe the comment.

What actually happened? Please include the actual, raw output from ESLint.
Unnecessary { after 'if' condition.

ESLint essentially wants no braces which is confusing to myself (and likely other JS readers):

if (foo)
  // some comment
  bar();

I'm happy to work on this bug as my project at work is interested in using this rule. I'm thinking it will be a fairly simple to fix, along the lines of #6396

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Nov 5, 2016
@not-an-aardvark not-an-aardvark added bug ESLint is working incorrectly rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Nov 5, 2016
@not-an-aardvark
Copy link
Member

Thanks for the report; this does seem like a bug.

@mysticatea
Copy link
Member

It seems a bug.

I felt the name multi-or-nest is confusing since the rule has the option multi and multi-line then the multi-or-nest's behavior is near to multi-line.

@wwwillchen
Copy link
Contributor Author

I agree the naming is confusing and I had to re-read the examples a few times to fully understand the differences. If it helps, the reason why our project is using multi-or-nest is because through some (strange-ish) transitive logic, we follow a variation of the Google C++ style guide [0] which specifies that "In general, curly braces are not required for single-line statements". I'm not sure what a better name is, perhaps "multi-line-statement"?

[0] https://google.github.io/styleguide/cppguide.html#Conditionals

nzakas pushed a commit that referenced this issue Nov 9, 2016
)

* Fix: Curly rule doesn't account for leading comment (fixes #7538)

* PR feedback

* simplify check

* check .length > 0
@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
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 rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants