diff --git a/crates/oxc_linter/src/builder.rs b/crates/oxc_linter/src/builder.rs index c7bfb60dd57e0..dc3d0aa90e4ec 100644 --- a/crates/oxc_linter/src/builder.rs +++ b/crates/oxc_linter/src/builder.rs @@ -13,7 +13,7 @@ use crate::{ #[must_use = "You dropped your builder without building a Linter! Did you mean to call .build()?"] pub struct LinterBuilder { - rules: FxHashSet, + pub(super) rules: FxHashSet, options: LintOptions, config: LintConfig, cache: RulesCache, @@ -119,6 +119,11 @@ impl LinterBuilder { self } + #[inline] + pub fn plugins(&self) -> LintPlugins { + self.options.plugins + } + #[cfg(test)] pub(crate) fn with_rule(mut self, rule: RuleWithSeverity) -> Self { self.rules.insert(rule); @@ -139,12 +144,15 @@ impl LinterBuilder { match severity { AllowWarnDeny::Deny | AllowWarnDeny::Warn => match filter { LintFilterKind::Category(category) => { - self.rules.extend( - all_rules - .iter() - .filter(|rule| rule.category() == category) - .map(|rule| RuleWithSeverity::new(rule.clone(), severity)), - ); + let rules_to_configure = all_rules.iter().filter(|r| r.category() == category); + for rule in rules_to_configure { + if let Some(mut existing_rule) = self.rules.take(rule) { + existing_rule.severity = severity; + self.rules.insert(existing_rule); + } else { + self.rules.insert(RuleWithSeverity::new(rule.clone(), severity)); + } + } } LintFilterKind::Rule(_, name) => { self.rules.extend( @@ -292,3 +300,140 @@ impl RulesCache { *self.0.borrow_mut() = Some(all_rules); } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_builder_default() { + let builder = LinterBuilder::default(); + assert_eq!(builder.plugins(), LintPlugins::default()); + + // populated with all correctness-level ESLint rules at a "warn" severity + assert!(!builder.rules.is_empty()); + for rule in &builder.rules { + assert_eq!(rule.category(), RuleCategory::Correctness); + assert_eq!(rule.severity, AllowWarnDeny::Warn); + let plugin = rule.rule.plugin_name(); + let name = rule.name(); + assert!( + builder.plugins().contains(plugin.into()), + "{plugin}/{name} is in the default rule set but its plugin is not enabled" + ); + } + } + + #[test] + fn test_builder_empty() { + let builder = LinterBuilder::empty(); + assert_eq!(builder.plugins(), LintPlugins::default()); + assert!(builder.rules.is_empty()); + } + + #[test] + fn test_filter_deny_on_default() { + let builder = LinterBuilder::default(); + let initial_rule_count = builder.rules.len(); + + let builder = builder.with_filters([LintFilter::deny(RuleCategory::Correctness)]); + let rule_count_after_deny = builder.rules.len(); + + // By default, all correctness rules are set to warn. the above filter should only + // re-configure those rules, and not add/remove any others. + assert!(!builder.rules.is_empty()); + assert_eq!(initial_rule_count, rule_count_after_deny); + + for rule in &builder.rules { + assert_eq!(rule.category(), RuleCategory::Correctness); + assert_eq!(rule.severity, AllowWarnDeny::Deny); + + let plugin = rule.plugin_name(); + let name = rule.name(); + assert!( + builder.plugins().contains(plugin.into()), + "{plugin}/{name} is in the default rule set but its plugin is not enabled" + ); + } + } + + #[test] + fn test_filter_allow_all_then_warn() { + let builder = + LinterBuilder::default() + .with_filters([LintFilter::new(AllowWarnDeny::Allow, "all").unwrap()]); + assert!(builder.rules.is_empty(), "Allowing all rules should empty out the rules list"); + + let builder = builder.with_filters([LintFilter::warn(RuleCategory::Correctness)]); + assert!( + !builder.rules.is_empty(), + "warning on categories after allowing all rules should populate the rules set" + ); + for rule in &builder.rules { + let plugin = rule.plugin_name(); + let name = rule.name(); + assert_eq!( + rule.severity, + AllowWarnDeny::Warn, + "{plugin}/{name} should have a warning severity" + ); + assert_eq!( + rule.category(), + RuleCategory::Correctness, + "{plugin}/{name} should not be enabled b/c we only enabled correctness rules" + ); + } + } + + #[test] + fn test_rules_after_plugin_added() { + let builder = LinterBuilder::default(); + let initial_rule_count = builder.rules.len(); + + let builder = builder.and_plugins(LintPlugins::IMPORT, true); + assert_eq!(initial_rule_count, builder.rules.len(), "Enabling a plugin should not add any rules, since we don't know which categories to turn on."); + } + + #[test] + fn test_plugin_configuration() { + let builder = LinterBuilder::default(); + let initial_plugins = builder.plugins(); + + // ========================================================================================== + // Test LinterBuilder::and_plugins, which deltas the plugin list instead of overriding it + // ========================================================================================== + + // Enable eslint plugin. Since it's already enabled, this does nothing. + assert!(initial_plugins.contains(LintPlugins::ESLINT)); // sanity check that eslint is + // enabled + let builder = builder.and_plugins(LintPlugins::ESLINT, true); + assert_eq!(initial_plugins, builder.plugins()); + + // Disable import plugin. Since it's not already enabled, this is also a no-op. + assert!(!builder.plugins().contains(LintPlugins::IMPORT)); // sanity check that it's not + // already enabled + let builder = builder.and_plugins(LintPlugins::IMPORT, false); + assert_eq!(initial_plugins, builder.plugins()); + + // Enable import plugin. Since it's not already enabled, this turns it on. + let builder = builder.and_plugins(LintPlugins::IMPORT, true); + assert_eq!(LintPlugins::default().union(LintPlugins::IMPORT), builder.plugins()); + assert_ne!(initial_plugins, builder.plugins()); + + // Turn import back off, resetting plugins to the initial state + let builder = builder.and_plugins(LintPlugins::IMPORT, false); + assert_eq!(initial_plugins, builder.plugins()); + + // ========================================================================================== + // Test LinterBuilder::with_plugins, which _does_ override plugins + // ========================================================================================== + + let builder = builder.with_plugins(LintPlugins::ESLINT); + assert_eq!(LintPlugins::ESLINT, builder.plugins()); + + let expected_plugins = + LintPlugins::ESLINT.union(LintPlugins::TYPESCRIPT).union(LintPlugins::NEXTJS); + let builder = builder.with_plugins(expected_plugins); + assert_eq!(expected_plugins, builder.plugins()); + } +} diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 4d45b793db9e6..348f9b6dae172 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -1,5 +1,5 @@ use std::{ - borrow::Cow, + borrow::{Borrow, Cow}, fmt, hash::{Hash, Hasher}, ops::Deref, @@ -261,6 +261,12 @@ impl Deref for RuleWithSeverity { } } +impl Borrow for RuleWithSeverity { + fn borrow(&self) -> &RuleEnum { + &self.rule + } +} + impl RuleWithSeverity { pub fn new(rule: RuleEnum, severity: AllowWarnDeny) -> Self { Self { rule, severity }