Skip to content

Conversation

@siketyan
Copy link
Member

@siketyan siketyan commented Apr 24, 2025

Summary

Initial implementation of a new rule useExhaustiveSwitchCases, which is an equivalent to typescript-eslint/switch-exhaustiveness-check. I left adding options for future improvement.

Test Plan

Added snapshot tests.

@siketyan siketyan self-assigned this Apr 24, 2025
@github-actions github-actions bot added A-Project Area: project A-Linter Area: linter L-JavaScript Language: JavaScript and super languages A-Diagnostic Area: diagnostocis labels Apr 24, 2025
@ematipico
Copy link
Member

I'm thrilled!!!! 🤩

@codspeed-hq
Copy link

codspeed-hq bot commented Apr 24, 2025

CodSpeed Performance Report

Merging #5762 will not alter performance

Comparing siketyan:feat/use-exhaustive-switch-cases (a061af1) with main (6bdffb1)

Summary

✅ 95 untouched benchmarks

@siketyan siketyan force-pushed the feat/use-exhaustive-switch-cases branch from b22c4e1 to 7a486b1 Compare April 26, 2025 06:57
@siketyan siketyan force-pushed the feat/use-exhaustive-switch-cases branch from 7a486b1 to d16049a Compare April 26, 2025 07:58
@siketyan siketyan requested review from a team April 26, 2025 07:59
@siketyan siketyan marked this pull request as ready for review April 26, 2025 07:59
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Looks good. We need to add the second pillar to the diagnostic messages

@siketyan
Copy link
Member Author

I am going to merge this after #5777, which will change some part that this rule relies on.

@github-actions github-actions bot added the A-CLI Area: CLI label Apr 26, 2025
Copy link
Contributor

@arendjr arendjr left a comment

Choose a reason for hiding this comment

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

Love it, and well done with the solid test cases!

Thanks for waiting for the refactor too 🙏

siketyan added 3 commits May 1, 2025 02:51
# Conflicts:
#	crates/biome_configuration/src/analyzer/linter/rules.rs
#	crates/biome_js_analyze/src/lint/nursery.rs
#	crates/biome_js_type_info/src/flattening.rs
#	crates/biome_js_type_info/src/type_info.rs
# Conflicts:
#	crates/biome_configuration/src/analyzer/linter/rules.rs
#	crates/biome_js_analyze/src/lint/nursery.rs
#	crates/biome_js_analyze/src/services/typed.rs
@siketyan siketyan force-pushed the feat/use-exhaustive-switch-cases branch from 37c2fbd to 3af6b6d Compare May 6, 2025 17:14
Comment on lines 123 to 124
if let Some(expr) =
JsExpressionStatement::cast_ref(node).and_then(|node| node.expression().ok())
Copy link
Member Author

@siketyan siketyan May 6, 2025

Choose a reason for hiding this comment

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

@arendjr This prevented the type of const declarations and reference expressions to be resolved. Is this condition correct? Changing this to AnyJsExpression fixed the issue, but it can occur performance regression.

Copy link
Contributor

@arendjr arendjr May 7, 2025

Choose a reason for hiding this comment

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

I had indeed avoided that for performance reasons, and this was all I needed for it to work with noFloatingPromises... but indeed that doesn't scale across the board :(

I'd be happy to relax this to support AnyJsExpression, but I'm afraid we need to resolve the performance issues first. I do have some hope my current PR may help with that 🤞

Copy link
Member Author

Choose a reason for hiding this comment

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

For now, registering types for the discriminant and the case expressions is sufficient for the rule: bdfc07b

@siketyan siketyan force-pushed the feat/use-exhaustive-switch-cases branch from 3af6b6d to 609ed92 Compare May 6, 2025 17:31
@siketyan siketyan force-pushed the feat/use-exhaustive-switch-cases branch from 609ed92 to cc5204d Compare May 6, 2025 17:34
siketyan added 2 commits May 8, 2025 23:42
# Conflicts:
#	crates/biome_configuration/src/analyzer/linter/rules.rs
#	crates/biome_js_analyze/src/lint/nursery.rs
#	crates/biome_js_type_info/src/flattening.rs
@siketyan siketyan merged commit 6a59e88 into biomejs:main May 8, 2025
14 checks passed
@siketyan siketyan deleted the feat/use-exhaustive-switch-cases branch May 8, 2025 16:25
ematipico pushed a commit that referenced this pull request May 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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 useExhaustiveSwitchCases - typescript-eslint/switch-exhaustiveness-check

4 participants