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

feat(lint): useCollapsedIf JS lint rule #4179

Merged
merged 15 commits into from
Oct 16, 2024

Conversation

siketyan
Copy link
Contributor

@siketyan siketyan commented Oct 5, 2024

Summary

Closes #3944

This pull requests adds useCollapsedIf rule that suggests to merge two nested if statements into one if the parent has only one statement and the child has no else clauses.

The implementation is largely based on useCollapsedElseIf (#160).

This is my first time to make changes to Biome, so please let me know if missing something to do. 🙏

Test Plan

See test snapshots.

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project A-Linter Area: linter L-JavaScript Language: JavaScript and super languages A-Diagnostic Area: diagnostocis labels Oct 5, 2024
@siketyan siketyan changed the title feat(lint): Add useCollapsedIf JS lint rule feat(lint): useCollapsedIf JS lint rule Oct 5, 2024
Signed-off-by: Naoki Ikeguchi <[email protected]>
Copy link

codspeed-hq bot commented Oct 5, 2024

CodSpeed Performance Report

Merging #4179 will improve performances by 18.61%

Comparing siketyan:feat/useCollapsedIf (0847315) with main (89d34b2)

Summary

⚡ 1 improvements
✅ 104 untouched benchmarks

Benchmarks breakdown

Benchmark main siketyan:feat/useCollapsedIf Change
big5-added_15586211152145260264.json[uncached] 650.7 µs 548.6 µs +18.61%

Copy link
Contributor

@togami2864 togami2864 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@@ -0,0 +1,81 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you check the original test file and add cases to increase coverage?
https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/test/no-lonely-if.mjs

@@ -0,0 +1,18 @@
if (condition && anotherCondition) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Conflicts:
#	crates/biome_configuration/src/analyzer/linter/rules.rs
…eck the expr needs parens

Signed-off-by: Naoki Ikeguchi <[email protected]>
# Conflicts:
#	crates/biome_configuration/src/analyzer/linter/rules.rs
# Conflicts:
#	crates/biome_configuration/src/analyzer/linter/rules.rs
#	crates/biome_js_analyze/src/lint/nursery.rs
#	packages/@biomejs/backend-jsonrpc/src/workspace.ts
#	packages/@biomejs/biome/configuration_schema.json
@siketyan
Copy link
Contributor Author

@togami2864 @Conaclos
Thank you for your review, I applied all suggestions. Sorry for my late response.

@siketyan siketyan force-pushed the feat/useCollapsedIf branch 2 times, most recently from bc81ebf to ac9ff95 Compare October 16, 2024 02:44
@siketyan
Copy link
Contributor Author

@togami2864 Merged main branch and re-run just gen-lint. 🙏

@togami2864 togami2864 merged commit bbb93b9 into biomejs:main Oct 16, 2024
11 checks passed
@Conaclos Conaclos added the A-Changelog Area: changelog label Oct 17, 2024
Conaclos pushed a commit that referenced this pull request Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-CLI Area: CLI A-Diagnostic Area: diagnostocis A-Linter Area: linter A-Project Area: project L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

📎 Implement useCollapsedIf - clippy/collapsible_if, unicorn/no-lonely-if
3 participants