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
Fix: edge-cases of semi-style #9560
Conversation
lib/rules/semi-style.js
Outdated
t === "IfStatement" || | ||
t === "LabeledStatement" || | ||
t === "WhileStatement" || | ||
t === "WithStatement" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be !astUtils.STATEMENT_LIST_PARENTS.has(node.parent.type)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's simple. Yes, it works fine!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After I rethought, I'm wondering if I remove the check of EmptyStatement
entirely. It makes only redundant consequent semicolons... E.g. foo;;\nbar
or foo\n;;bar
. No-extra-semi
rule should remove that.
I updated this PR to simplify with dropping EmptyStatements. The behavior follows to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a question 😄
code: ` | ||
switch (a) { | ||
case 1: | ||
;foo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please explain a little why this case is valid? IMHO, it's using options: ["last"]
, so it should be foo;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. This rule checks the location of the semicolon between two statements. I.e.:
first();
second()
// OR
first()
;second()
The rule does not add/remove semicolons because it's the responsibility of semi
and no-extra-semi
rules. Instead, the rule handles whitespaces of around semicolons.
Now, the current behavior is that the case 1:\n;foo
is warned and fixed to case 1:;\nfoo
, it was wrong.
What is the purpose of this pull request? (put an "X" next to item)
[X] Bug fix
Tell us about your environment
What parser (default, Babel-ESLint, etc.) are you using?
Please show your full configuration:
What did you do? Please include the actual source code causing the issue.
What did you expect to happen?
No errors.
What actually happened? Please include the actual, raw output from ESLint.
Several errors are reported. See the demo pages.
What changes did you make? (Give an overview)
This PR fixes those false positives. Now the rule ignores semicolons at the first/last in a block/clause.
Is there anything you'd like reviewers to focus on?
Nothing in particular.