perf(linter): avoid cloning filters by refactoring functions to take references (#10247)

Author: ulrichstark

apps/oxlint/src/lint.rs

@@ -69,8 +69,8 @@ impl Runner for LintRunner {
         let provided_path_count = paths.len();
         let now = Instant::now();
 
-        let filter = match Self::get_filters(filter) {
-            Ok(filter) => filter,
+        let filters = match Self::get_filters(filter) {
+            Ok(filters) => filters,
             Err((result, message)) => {
                 stdout.write_all(message.as_bytes()).or_else(Self::check_for_writer_error).unwrap();
                 stdout.flush().unwrap();
@@ -215,8 +215,8 @@ impl Runner for LintRunner {
                         return CliRunResult::InvalidOptionConfig;
                     }
                 }
-                // TODO(perf): figure out if we can avoid cloning `filter`
-                .with_filters(filter.clone());
+                .with_filters(&filters);
+
                 match builder.build() {
                     Ok(config) => nested_configs.insert(dir.to_path_buf(), config),
                     Err(diagnostic) => {
@@ -259,7 +259,7 @@ impl Runner for LintRunner {
                 return CliRunResult::InvalidOptionConfig;
             }
         }
-        .with_filters(filter);
+        .with_filters(&filters);
 
         if let Some(basic_config_file) = oxlintrc_for_print {
             let config_file = config_builder.resolve_final_config_file(basic_config_file);

crates/oxc_language_server/src/linter/server_linter.rs

@@ -47,7 +47,7 @@ mod test {
     #[test]
     fn test_no_console() {
         let config_store = ConfigStoreBuilder::default()
-            .with_filter(LintFilter::deny(LintFilterKind::parse("no-console".into()).unwrap()))
+            .with_filter(&LintFilter::deny(LintFilterKind::parse("no-console".into()).unwrap()))
             .build()
             .unwrap();
         let linter = Linter::new(LintOptions::default(), config_store).with_fix(FixKind::SafeFix);

crates/oxc_linter/src/config/config_builder.rs

@@ -109,8 +109,8 @@ impl ConfigStoreBuilder {
         let cache = RulesCache::new(config.plugins);
         let mut builder = Self { rules, config, overrides, cache };
 
-        if !categories.is_empty() {
-            builder = builder.with_filters(categories.filters());
+        for filter in categories.filters() {
+            builder = builder.with_filter(&filter);
         }
 
         {
@@ -227,20 +227,20 @@ impl ConfigStoreBuilder {
         self
     }
 
-    pub fn with_filters<I: IntoIterator<Item = LintFilter>>(mut self, filters: I) -> Self {
+    pub fn with_filters<'a, I: IntoIterator<Item = &'a LintFilter>>(mut self, filters: I) -> Self {
         for filter in filters {
             self = self.with_filter(filter);
         }
         self
     }
 
-    pub fn with_filter(mut self, filter: LintFilter) -> Self {
+    pub fn with_filter(mut self, filter: &LintFilter) -> Self {
         let (severity, filter) = filter.into();
 
         match severity {
             AllowWarnDeny::Deny | AllowWarnDeny::Warn => match filter {
                 LintFilterKind::Category(category) => {
-                    self.upsert_where(severity, |r| r.category() == category);
+                    self.upsert_where(severity, |r| r.category() == *category);
                 }
                 LintFilterKind::Rule(_, name) => self.upsert_where(severity, |r| r.name() == name),
                 LintFilterKind::Generic(name_or_category) => {
@@ -253,7 +253,7 @@ impl ConfigStoreBuilder {
             },
             AllowWarnDeny::Allow => match filter {
                 LintFilterKind::Category(category) => {
-                    self.rules.retain(|rule| rule.category() != category);
+                    self.rules.retain(|rule| rule.category() != *category);
                 }
                 LintFilterKind::Rule(_, name) => {
                     self.rules.retain(|rule| rule.name() != name);
@@ -537,7 +537,7 @@ mod test {
         let builder = ConfigStoreBuilder::default();
         let initial_rule_count = builder.rules.len();
 
-        let builder = builder.with_filters([LintFilter::deny(RuleCategory::Correctness)]);
+        let builder = builder.with_filter(&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
@@ -566,8 +566,7 @@ mod test {
             let initial_rule_count = builder.rules.len();
 
             let builder =
-                builder
-                    .with_filters([LintFilter::new(AllowWarnDeny::Deny, filter_string).unwrap()]);
+                builder.with_filter(&LintFilter::new(AllowWarnDeny::Deny, filter_string).unwrap());
             let rule_count_after_deny = builder.rules.len();
             assert_eq!(
                 initial_rule_count, rule_count_after_deny,
@@ -591,7 +590,7 @@ mod test {
             let builder = ConfigStoreBuilder::default();
             // sanity check: not already turned on
             assert!(!builder.rules.iter().any(|r| r.name() == "no-console"));
-            let builder = builder.with_filter(filter);
+            let builder = builder.with_filter(&filter);
             let no_console = builder
                 .rules
                 .iter()
@@ -604,14 +603,11 @@ mod test {
 
     #[test]
     fn test_filter_allow_all_then_warn() {
-        let builder = ConfigStoreBuilder::default().with_filters([LintFilter::new(
-            AllowWarnDeny::Allow,
-            "all",
-        )
-        .unwrap()]);
+        let builder = ConfigStoreBuilder::default()
+            .with_filter(&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)]);
+        let builder = builder.with_filter(&LintFilter::warn(RuleCategory::Correctness));
         assert!(
             !builder.rules.is_empty(),
             "warning on categories after allowing all rules should populate the rules set"