-
-
Notifications
You must be signed in to change notification settings - Fork 476
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
feat(lint): useCollapsedElseIf #160
Conversation
crates/rome_js_analyze/src/analyzers/nursery/use_collapsed_else_if.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_collapsed_else_if.rs
Outdated
Show resolved
Hide resolved
fix: comment detection, feat: differentiate between safe and suggested code fix
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.
Looks great! After the minor changes I proposed I think we are ready for merging your contribution :)
crates/rome_js_analyze/src/analyzers/nursery/use_collapsed_else_if.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_collapsed_else_if.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_collapsed_else_if.rs
Outdated
Show resolved
Hide resolved
Thanks! Great contribution :) I had to change the description of the rule: it caused a fail when the website is bundled (this seems to be a bug of Astro). |
I have to thank for accepting my first PR! 🙌 Cool to already see it in action in the playground! The issue (#43) is still open. Can be closed, right? |
@n-gude This is a great first contribution! Looking forward for your future ones :) |
Summary
This PR implements the
useCollapsedElseIf
lint rule (#43).It provides a code fix unless the else-block statement contains a comment. The placement of the comment would be questionable. (ESLint handles it the same way.)
Test Plan
Tests included.