feat(linter): allow appending plugins in override (#7379)

follow up to https://github.com/oxc-project/oxc/issues/6896

for improved compatibility with ESLint, this tries to match the behavior of plugin overrides so that plugins can be enabled for certain paths. this does not allow disabling plugins.

crates/oxc_linter/src/config/flat.rs

@@ -113,12 +113,7 @@ impl ConfigStore {
 
     /// NOTE: this function must not borrow any entries from `self.cache` or DashMap will deadlock.
     fn apply_overrides(&self, override_ids: &[OverrideId]) -> ResolvedLinterState {
-        let plugins = self
-            .overrides
-            .iter()
-            .rev()
-            .find_map(|cfg| cfg.plugins)
-            .unwrap_or(self.base.config.plugins);
+        let mut plugins = self.base.config.plugins;
 
         let all_rules = RULES
             .iter()
@@ -135,10 +130,14 @@ impl ConfigStore {
 
         let overrides = override_ids.iter().map(|id| &self.overrides[*id]);
         for override_config in overrides {
-            if override_config.rules.is_empty() {
-                continue;
+            if !override_config.rules.is_empty() {
+                override_config.rules.override_rules(&mut rules, &all_rules);
+            }
+
+            // Append the override's plugins to the base list of enabled plugins.
+            if let Some(override_plugins) = override_config.plugins {
+                plugins |= override_plugins;
             }
-            override_config.rules.override_rules(&mut rules, &all_rules);
         }
 
         let rules = rules.into_iter().collect::<Vec<_>>();
@@ -158,7 +157,10 @@ impl ConfigStore {
 #[cfg(test)]
 mod test {
     use super::{ConfigStore, OxlintOverrides};
-    use crate::{config::LintConfig, AllowWarnDeny, LintPlugins, RuleEnum, RuleWithSeverity};
+    use crate::{
+        config::{LintConfig, OxlintEnv, OxlintGlobals, OxlintSettings},
+        AllowWarnDeny, LintPlugins, RuleEnum, RuleWithSeverity,
+    };
 
     macro_rules! from_json {
         ($json:tt) => {
@@ -171,11 +173,6 @@ mod test {
         RuleWithSeverity::new(RuleEnum::NoExplicitAny(Default::default()), AllowWarnDeny::Warn)
     }
 
-    #[allow(clippy::default_trait_access)]
-    fn no_cycle() -> RuleWithSeverity {
-        RuleWithSeverity::new(RuleEnum::NoCycle(Default::default()), AllowWarnDeny::Warn)
-    }
-
     /// an empty ruleset is a no-op
     #[test]
     fn test_no_rules() {
@@ -219,26 +216,6 @@ mod test {
         );
     }
 
-    /// removing plugins strips rules from those plugins, even if no rules are
-    /// added/removed explicitly
-    #[test]
-    fn test_no_rules_and_remove_plugins() {
-        let base_rules = vec![no_cycle()];
-        let overrides = from_json!([{
-            "files": ["*.test.{ts,tsx}"],
-            "plugins": ["jest"],
-            "rules": {}
-        }]);
-        let config = LintConfig {
-            plugins: LintPlugins::default() | LintPlugins::IMPORT,
-            ..LintConfig::default()
-        };
-        let store = ConfigStore::new(base_rules, config, overrides);
-
-        assert_eq!(store.resolve("App.tsx".as_ref()).rules.len(), 1);
-        assert_eq!(store.resolve("App.test.tsx".as_ref()).rules.len(), 0);
-    }
-
     #[test]
     fn test_remove_rule() {
         let base_rules = vec![no_explicit_any()];
@@ -300,4 +277,36 @@ mod test {
         assert_eq!(src_app.len(), 1);
         assert_eq!(src_app[0].severity, AllowWarnDeny::Deny);
     }
+
+    #[test]
+    fn test_add_plugins() {
+        let base_config = LintConfig {
+            plugins: LintPlugins::IMPORT,
+            env: OxlintEnv::default(),
+            settings: OxlintSettings::default(),
+            globals: OxlintGlobals::default(),
+        };
+        let overrides = from_json!([{
+            "files": ["*.jsx", "*.tsx"],
+            "plugins": ["react"],
+        }, {
+            "files": ["*.ts", "*.tsx"],
+            "plugins": ["typescript"],
+        }]);
+
+        let store = ConfigStore::new(vec![], base_config, overrides);
+        assert_eq!(store.base.config.plugins, LintPlugins::IMPORT);
+
+        let app = store.resolve("other.mjs".as_ref()).config;
+        assert_eq!(app.plugins, LintPlugins::IMPORT);
+
+        let app = store.resolve("App.jsx".as_ref()).config;
+        assert_eq!(app.plugins, LintPlugins::IMPORT | LintPlugins::REACT);
+
+        let app = store.resolve("App.ts".as_ref()).config;
+        assert_eq!(app.plugins, LintPlugins::IMPORT | LintPlugins::TYPESCRIPT);
+
+        let app = store.resolve("App.tsx".as_ref()).config;
+        assert_eq!(app.plugins, LintPlugins::IMPORT | LintPlugins::REACT | LintPlugins::TYPESCRIPT);
+    }
 }

crates/oxc_linter/src/config/overrides.rs

@@ -151,3 +151,31 @@ impl JsonSchema for GlobSet {
         gen.subschema_for::<Vec<String>>()
     }
 }
+
+mod test {
+    #[test]
+    fn test_parsing_plugins() {
+        use super::*;
+        use serde_json::{from_value, json};
+
+        let config: OxlintOverride = from_value(json!({
+            "files": ["*.tsx"],
+        }))
+        .unwrap();
+        assert_eq!(config.plugins, None);
+
+        let config: OxlintOverride = from_value(json!({
+            "files": ["*.tsx"],
+            "plugins": [],
+        }))
+        .unwrap();
+        assert_eq!(config.plugins, Some(LintPlugins::empty()));
+
+        let config: OxlintOverride = from_value(json!({
+            "files": ["*.tsx"],
+            "plugins": ["typescript", "react"],
+        }))
+        .unwrap();
+        assert_eq!(config.plugins, Some(LintPlugins::REACT | LintPlugins::TYPESCRIPT));
+    }
+}

crates/oxc_linter/src/config/plugins.rs

@@ -149,7 +149,7 @@ impl<S: AsRef<str>> FromIterator<S> for LintPlugins {
     fn from_iter<T: IntoIterator<Item = S>>(iter: T) -> Self {
         iter.into_iter()
             .map(|plugin| plugin.as_ref().into())
-            .fold(LintPlugins::default(), LintPlugins::union)
+            .fold(LintPlugins::empty(), LintPlugins::union)
     }
 }
 
@@ -184,18 +184,17 @@ impl<'de> Deserialize<'de> for LintPlugins {
                     // serde_json::from_value provides a String. The former is
                     // used in almost all cases, but the latter is more
                     // convenient for test cases.
-                    match seq.next_element::<&str>() {
+                    match seq.next_element::<String>() {
                         Ok(Some(next)) => {
-                            plugins |= next.into();
+                            plugins |= next.as_str().into();
                         }
                         Ok(None) => break,
-                        Err(_) => {
-                            if let Some(next) = seq.next_element::<String>()? {
-                                plugins |= next.as_str().into();
-                            } else {
-                                break;
+                        Err(_) => match seq.next_element::<&str>() {
+                            Ok(Some(next)) => {
+                                plugins |= next.into();
                             }
-                        }
+                            Ok(None) | Err(_) => break,
+                        },
                     };
                 }
 
@@ -316,7 +315,7 @@ impl LintPluginOptions {
 
 impl<S: AsRef<str>> FromIterator<(S, bool)> for LintPluginOptions {
     fn from_iter<I: IntoIterator<Item = (S, bool)>>(iter: I) -> Self {
-        let mut options = Self::default();
+        let mut options = Self::none();
         for (s, enabled) in iter {
             let flags = LintPlugins::from(s.as_ref());
             match flags {
@@ -381,11 +380,11 @@ mod test {
     fn test_collect_empty() {
         let empty: &[&str] = &[];
         let plugins: LintPluginOptions = empty.iter().copied().collect();
-        assert_eq!(plugins, LintPluginOptions::default());
+        assert_eq!(plugins, LintPluginOptions::none());
 
         let empty: Vec<(String, bool)> = vec![];
         let plugins: LintPluginOptions = empty.into_iter().collect();
-        assert_eq!(plugins, LintPluginOptions::default());
+        assert_eq!(plugins, LintPluginOptions::none());
     }
 
     #[test]
@@ -394,9 +393,9 @@ mod test {
         let plugins: LintPluginOptions = enabled.into_iter().collect();
         let expected = LintPluginOptions {
             react: true,
-            unicorn: true,
+            unicorn: false,
             typescript: true,
-            oxc: true,
+            oxc: false,
             import: false,
             jsdoc: false,
             jest: true,