Skip to content

Commit 6728ede

Browse files
camchenrycamchenry
committed
refactor(linter): simplify built-in lint plugin checks
1 parent 198f2e9 commit 6728ede

File tree

6 files changed

+68
-50
lines changed

6 files changed

+68
-50
lines changed

‎crates/oxc_linter/src/config/config_builder.rs‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,10 @@ impl ConfigStoreBuilder {
346346

347347
RULES
348348
.iter()
349-
.filter(|rule| builtin_plugins.contains(LintPlugins::from(rule.plugin_name())))
349+
.filter(|rule| {
350+
LintPlugins::try_from(rule.plugin_name())
351+
.is_ok_and(|plugin_flag| builtin_plugins.contains(plugin_flag))
352+
})
350353
.cloned()
351354
.collect()
352355
}
@@ -396,7 +399,10 @@ impl ConfigStoreBuilder {
396399
let mut rules: Vec<_> = self
397400
.rules
398401
.into_iter()
399-
.filter(|(r, _)| plugins.contains(r.plugin_name().into()))
402+
.filter(|(r, _)| {
403+
LintPlugins::try_from(r.plugin_name())
404+
.is_ok_and(|plugin_name| plugins.contains(plugin_name))
405+
})
400406
.collect();
401407
rules.sort_unstable_by_key(|(r, _)| r.id());
402408

@@ -457,7 +463,8 @@ impl ConfigStoreBuilder {
457463
// NOTE: this logic means there's no way to disable ESLint
458464
// correctness rules. I think that's fine for now.
459465
rule.category() == RuleCategory::Correctness
460-
&& plugins.contains(LintPlugins::from(rule.plugin_name()))
466+
&& LintPlugins::try_from(rule.plugin_name())
467+
.is_ok_and(|plugin_flag| plugins.contains(plugin_flag))
461468
})
462469
.map(|rule| (rule.clone(), AllowWarnDeny::Warn))
463470
.collect()
@@ -535,7 +542,7 @@ impl ConfigStoreBuilder {
535542

536543
match result {
537544
PluginLoadResult::Success { name, offset, rule_names } => {
538-
if name != "eslint" && LintPlugins::from(name.as_str()) == LintPlugins::empty() {
545+
if LintPlugins::try_from(name.as_str()).is_err() {
539546
external_plugin_store.register_plugin(plugin_path, name, offset, rule_names);
540547
Ok(())
541548
} else {
@@ -650,11 +657,12 @@ mod test {
650657
for (rule, severity) in &builder.rules {
651658
assert_eq!(rule.category(), RuleCategory::Correctness);
652659
assert_eq!(*severity, AllowWarnDeny::Warn);
653-
let plugin = rule.plugin_name();
660+
let plugin_name = rule.plugin_name();
661+
let plugin = LintPlugins::try_from(plugin_name);
654662
let name = rule.name();
655663
assert!(
656-
builder.plugins().contains(plugin.into()),
657-
"{plugin}/{name} is in the default rule set but its plugin is not enabled"
664+
plugin.is_ok_and(|plugin| builder.plugins().contains(plugin)),
665+
"{plugin_name}/{name} is in the default rule set but its plugin is not enabled"
658666
);
659667
}
660668
}
@@ -683,11 +691,12 @@ mod test {
683691
assert_eq!(rule.category(), RuleCategory::Correctness);
684692
assert_eq!(*severity, AllowWarnDeny::Deny);
685693

686-
let plugin = rule.plugin_name();
694+
let plugin_name = rule.plugin_name();
695+
let plugin = LintPlugins::try_from(plugin_name);
687696
let name = rule.name();
688697
assert!(
689-
builder.plugins().contains(plugin.into()),
690-
"{plugin}/{name} is in the default rule set but its plugin is not enabled"
698+
plugin.is_ok_and(|plugin| builder.plugins().contains(plugin)),
699+
"{plugin_name}/{name} is in the default rule set but its plugin is not enabled"
691700
);
692701
}
693702
}
@@ -792,8 +801,8 @@ mod test {
792801
let name = rule.name();
793802
let plugin = rule.plugin_name();
794803
assert_ne!(
795-
LintPlugins::from(plugin),
796-
LintPlugins::TYPESCRIPT,
804+
LintPlugins::try_from(plugin),
805+
Ok(LintPlugins::TYPESCRIPT),
797806
"{plugin}/{name} is in the rules list after typescript plugin has been disabled"
798807
);
799808
}

‎crates/oxc_linter/src/config/config_store.rs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,13 +167,19 @@ impl Config {
167167
let mut rules = self
168168
.base_rules
169169
.iter()
170-
.filter(|(rule, _)| plugins.contains(LintPlugins::from(rule.plugin_name())))
170+
.filter(|(rule, _)| {
171+
LintPlugins::try_from(rule.plugin_name())
172+
.is_ok_and(|plugin| plugins.contains(plugin))
173+
})
171174
.cloned()
172175
.collect::<FxHashMap<_, _>>();
173176

174177
let all_rules = RULES
175178
.iter()
176-
.filter(|rule| plugins.contains(LintPlugins::from(rule.plugin_name())))
179+
.filter(|rule| {
180+
LintPlugins::try_from(rule.plugin_name())
181+
.is_ok_and(|plugin| plugins.contains(plugin))
182+
})
177183
.cloned()
178184
.collect::<Vec<_>>();
179185

@@ -194,7 +200,8 @@ impl Config {
194200

195201
if !unconfigured_plugins.is_empty() {
196202
for (rule, severity) in all_rules.iter().filter_map(|rule| {
197-
let rule_plugin = LintPlugins::from(rule.plugin_name());
203+
let rule_plugin = LintPlugins::try_from(rule.plugin_name())
204+
.unwrap_or(LintPlugins::empty());
198205
// Only apply categories to rules from unconfigured plugins
199206
if unconfigured_plugins.contains(rule_plugin) {
200207
self.categories

‎crates/oxc_linter/src/config/plugins.rs‎

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -75,31 +75,34 @@ impl LintPlugins {
7575
}
7676
}
7777

78-
impl From<&str> for LintPlugins {
79-
fn from(value: &str) -> Self {
78+
impl TryFrom<&str> for LintPlugins {
79+
type Error = ();
80+
81+
fn try_from(value: &str) -> Result<Self, Self::Error> {
8082
match value {
81-
"react" | "react-hooks" | "react_hooks" => LintPlugins::REACT,
82-
"unicorn" => LintPlugins::UNICORN,
83+
"react" | "react-hooks" | "react_hooks" => Ok(LintPlugins::REACT),
84+
"unicorn" => Ok(LintPlugins::UNICORN),
8385
"typescript" | "typescript-eslint" | "typescript_eslint" | "@typescript-eslint" => {
84-
LintPlugins::TYPESCRIPT
86+
Ok(LintPlugins::TYPESCRIPT)
8587
}
8688
// deepscan for backwards compatibility. Those rules have been moved into oxc
87-
"oxc" | "deepscan" => LintPlugins::OXC,
89+
"oxc" | "deepscan" => Ok(LintPlugins::OXC),
8890
// import-x has the same rules but better performance
89-
"import" | "import-x" => LintPlugins::IMPORT,
90-
"jsdoc" => LintPlugins::JSDOC,
91-
"jest" => LintPlugins::JEST,
92-
"vitest" => LintPlugins::VITEST,
93-
"jsx-a11y" | "jsx_a11y" => LintPlugins::JSX_A11Y,
94-
"nextjs" => LintPlugins::NEXTJS,
95-
"react-perf" | "react_perf" => LintPlugins::REACT_PERF,
96-
"promise" => LintPlugins::PROMISE,
97-
"node" => LintPlugins::NODE,
98-
"regex" => LintPlugins::REGEX,
99-
"vue" => LintPlugins::VUE,
91+
"import" | "import-x" => Ok(LintPlugins::IMPORT),
92+
"jsdoc" => Ok(LintPlugins::JSDOC),
93+
"jest" => Ok(LintPlugins::JEST),
94+
"vitest" => Ok(LintPlugins::VITEST),
95+
"jsx-a11y" | "jsx_a11y" => Ok(LintPlugins::JSX_A11Y),
96+
"nextjs" => Ok(LintPlugins::NEXTJS),
97+
"react-perf" | "react_perf" => Ok(LintPlugins::REACT_PERF),
98+
"promise" => Ok(LintPlugins::PROMISE),
99+
"node" => Ok(LintPlugins::NODE),
100+
"regex" => Ok(LintPlugins::REGEX),
101+
"vue" => Ok(LintPlugins::VUE),
100102
// "eslint" is not really a plugin, so it's 'empty'. This has the added benefit of
101103
// making it the default value.
102-
_ => LintPlugins::empty(),
104+
"eslint" => Ok(LintPlugins::ESLINT),
105+
_ => Err(()),
103106
}
104107
}
105108
}
@@ -134,15 +137,11 @@ impl<'de> Deserialize<'de> for LintPlugins {
134137
let mut lint_plugins = LintPlugins::empty();
135138

136139
for plugin in &plugin_names {
137-
if plugin == "eslint" {
138-
continue;
139-
}
140-
141-
let plugin_flag = LintPlugins::from(plugin.as_str());
142-
if plugin_flag == LintPlugins::empty() {
140+
if let Ok(plugin_flag) = LintPlugins::try_from(plugin.as_str()) {
141+
lint_plugins |= plugin_flag;
142+
} else {
143143
return Err(serde::de::Error::custom(format!("Unknown plugin: '{plugin}'.")));
144144
}
145-
lint_plugins |= plugin_flag;
146145
}
147146

148147
Ok(lint_plugins)
@@ -243,10 +242,10 @@ mod tests {
243242

244243
#[test]
245244
fn test_plugin_from_str() {
246-
assert_eq!(LintPlugins::from("react"), LintPlugins::REACT);
247-
assert_eq!(LintPlugins::from("typescript-eslint"), LintPlugins::TYPESCRIPT);
248-
assert_eq!(LintPlugins::from("deepscan"), LintPlugins::OXC);
249-
assert_eq!(LintPlugins::from("unknown"), LintPlugins::empty());
245+
assert_eq!(LintPlugins::try_from("react"), Ok(LintPlugins::REACT));
246+
assert_eq!(LintPlugins::try_from("typescript-eslint"), Ok(LintPlugins::TYPESCRIPT));
247+
assert_eq!(LintPlugins::try_from("deepscan"), Ok(LintPlugins::OXC));
248+
assert_eq!(LintPlugins::try_from("unknown"), Err(()));
250249
}
251250

252251
#[test]

‎crates/oxc_linter/src/config/rules.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ impl OxlintRules {
8686
let config = rule_config.config.clone().unwrap_or_default();
8787
let severity = rule_config.severity;
8888

89-
// TODO(camc314): remove the `plugin_name == "eslint"`
90-
if plugin_name == "eslint" || !LintPlugins::from(plugin_name).is_empty() {
89+
if LintPlugins::try_from(plugin_name).is_ok() {
9190
let rule = rules_map.get(&plugin_name).copied().or_else(|| {
9291
all_rules
9392
.iter()

‎crates/oxc_linter/src/tester.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,12 @@ impl Tester {
518518
)
519519
.unwrap()
520520
})
521-
.with_builtin_plugins(self.plugins | LintPlugins::from(self.plugin_name))
521+
.with_builtin_plugins(
522+
self.plugins
523+
| LintPlugins::try_from(self.plugin_name).unwrap_or_else(|()| {
524+
panic!("invalid plugin name: {}", self.plugin_name)
525+
}),
526+
)
522527
.with_rule(rule, AllowWarnDeny::Warn)
523528
.build(&external_plugin_store)
524529
.unwrap(),

‎tasks/website/src/linter/rules/doc_page.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,16 +172,15 @@ fn rule_source(rule: &RuleTableRow) -> String {
172172
/// - Example: `eslint` => true
173173
/// - Example: `jest` => false
174174
fn is_default_plugin(plugin: &str) -> bool {
175-
let plugin = LintPlugins::from(plugin);
176-
LintPlugins::default().contains(plugin)
175+
LintPlugins::try_from(plugin).is_ok_and(|plugin| LintPlugins::default().contains(plugin))
177176
}
178177

179178
/// Returns the normalized plugin name.
180179
/// - Example: `react_perf` -> `react-perf`
181180
/// - Example: `eslint` -> `eslint`
182181
/// - Example: `jsx_a11y` -> `jsx-a11y`
183182
fn get_normalized_plugin_name(plugin: &str) -> &str {
184-
LintPlugins::from(plugin).into()
183+
LintPlugins::try_from(plugin).unwrap_or(LintPlugins::empty()).into()
185184
}
186185

187186
fn how_to_use(rule: &RuleTableRow) -> String {

0 commit comments

Comments
 (0)