feat(linter): implement unicorn/prefer-switch#19604
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Merging this PR will not alter performance
Comparing Footnotes
|
1d74766 to
8bad22c
Compare
57ce894 to
a52d51a
Compare
cf6c4ff to
318a547
Compare
a315cd5 to
4350365
Compare
96a2135 to
efd8c0f
Compare
52f4221 to
4ff8936
Compare
efd8c0f to
48ca8ca
Compare
4ff8936 to
c9f1e32
Compare
48ca8ca to
5f45c07
Compare
5f45c07 to
2b0552c
Compare
2b0552c to
bd929eb
Compare
d9461a6 to
46177dd
Compare
There was a problem hiding this comment.
Pull request overview
Implements the eslint-plugin-unicorn(prefer-switch) lint rule in oxc_linter, adding it to the Unicorn rule set and wiring it into the generated rule registries and snapshots.
Changes:
- Added new rule implementation
PreferSwitchwith configuration parsing and AST analysis forif/else-ifchains. - Registered the rule in
rules.rsand the generatedrules_enum.rs/rule_runner_impls.rs. - Added snapshot coverage for rule diagnostics.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
crates/oxc_linter/src/rules/unicorn/prefer_switch.rs |
New rule implementation + unit tests and snapshot generation. |
crates/oxc_linter/src/rules.rs |
Exposes the new Unicorn rule module. |
crates/oxc_linter/src/generated/rules_enum.rs |
Adds the rule to the generated enum/registry plumbing. |
crates/oxc_linter/src/generated/rule_runner_impls.rs |
Registers the rule runner for IfStatement nodes. |
crates/oxc_linter/src/snapshots/unicorn_prefer_switch.snap |
New snapshot output for failing test cases. |
| #[derive(Debug, Clone, Copy, Default, Deserialize, JsonSchema)] | ||
| #[serde(rename_all = "kebab-case")] | ||
| pub enum EmptyDefaultCase { | ||
| /// Require a comment in the default case to explain why it's empty. | ||
| #[default] | ||
| NoDefaultComment, | ||
| /// Allow an empty default case. | ||
| DoNothingComment, | ||
| /// Disallow a default case. | ||
| NoDefaultCase, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, Deserialize, JsonSchema)] | ||
| #[serde(rename_all = "camelCase", default, deny_unknown_fields)] | ||
| pub struct PreferSwitchConfig { | ||
| /// The minimum number of cases required to trigger the rule. | ||
| minimum_cases: usize, | ||
| /// How to handle an empty default case. | ||
| empty_default_case: EmptyDefaultCase, | ||
| } | ||
|
|
||
| impl Default for PreferSwitchConfig { | ||
| fn default() -> Self { | ||
| Self { minimum_cases: 3, empty_default_case: EmptyDefaultCase::NoDefaultComment } |
There was a problem hiding this comment.
PreferSwitchConfig defines empty_default_case (and EmptyDefaultCase) but the rule never reads it, so user configuration has no effect and the option-specific tests don’t actually validate behavior. Either implement the intended behavior gated on self.0.empty_default_case or remove this config field/enum until it’s supported to avoid a misleading public config surface.
| #[derive(Debug, Clone, Copy, Default, Deserialize, JsonSchema)] | |
| #[serde(rename_all = "kebab-case")] | |
| pub enum EmptyDefaultCase { | |
| /// Require a comment in the default case to explain why it's empty. | |
| #[default] | |
| NoDefaultComment, | |
| /// Allow an empty default case. | |
| DoNothingComment, | |
| /// Disallow a default case. | |
| NoDefaultCase, | |
| } | |
| #[derive(Debug, Clone, Deserialize, JsonSchema)] | |
| #[serde(rename_all = "camelCase", default, deny_unknown_fields)] | |
| pub struct PreferSwitchConfig { | |
| /// The minimum number of cases required to trigger the rule. | |
| minimum_cases: usize, | |
| /// How to handle an empty default case. | |
| empty_default_case: EmptyDefaultCase, | |
| } | |
| impl Default for PreferSwitchConfig { | |
| fn default() -> Self { | |
| Self { minimum_cases: 3, empty_default_case: EmptyDefaultCase::NoDefaultComment } | |
| #[derive(Debug, Clone, Deserialize, JsonSchema)] | |
| #[serde(rename_all = "camelCase", default, deny_unknown_fields)] | |
| pub struct PreferSwitchConfig { | |
| /// The minimum number of cases required to trigger the rule. | |
| minimum_cases: usize, | |
| } | |
| impl Default for PreferSwitchConfig { | |
| fn default() -> Self { | |
| Self { minimum_cases: 3 } |
| ("if (foo === 1) {}", None), | ||
| ( | ||
| "if (foo === 1) {} | ||
| else if (foo === 2) {} | ||
| else if (foo === 3) {} | ||
| else if (foo === 4) {} |
There was a problem hiding this comment.
This test case is in the fail list, but with the default minimum_cases of 3 a single if (foo === 1) {} should not trigger the rule (the rule only warns once the chain length reaches minimum_cases). This will cause the test to expect a diagnostic that the current implementation will never emit; move this case to pass or provide a config that lowers minimumCases if you intend it to fail.
| ("if (foo === 1) {}", None), | |
| ( | |
| "if (foo === 1) {} | |
| else if (foo === 2) {} | |
| else if (foo === 3) {} | |
| else if (foo === 4) {} | |
| ( | |
| "if (foo === 1) {} | |
| else if (foo === 2) {} | |
| else if (foo === 3) {}", | |
| None, | |
| ), |

No description provided.