refactor(linter): simplify ConfigStore to prep for nested configs (#…

…8032)

crates/oxc_linter/src/config/flat.rs

@@ -1,21 +1,9 @@
-use std::{
-    hash::{BuildHasher, Hash, Hasher},
-    path::Path,
-    sync::Arc,
-};
+use std::{path::Path, sync::Arc};
 
-use dashmap::DashMap;
-use rustc_hash::{FxBuildHasher, FxHashSet};
+use rustc_hash::FxHashSet;
 
-use crate::{rules::RULES, LintPlugins, RuleWithSeverity};
-
-use super::{
-    overrides::{OverrideId, OxlintOverrides},
-    LintConfig,
-};
-
-type AppliedOverrideHash = u64;
-type FxDashMap<K, V> = DashMap<K, V, FxBuildHasher>;
+use super::{overrides::OxlintOverrides, LintConfig, LintPlugins};
+use crate::{rules::RULES, RuleWithSeverity};
 
 // TODO: support `categories` et. al. in overrides.
 #[derive(Debug)]
@@ -31,24 +19,21 @@ impl Clone for ResolvedLinterState {
     }
 }
 
-/// Keeps track of a list of config deltas, lazily applying them to a base config as requested by
-/// [`ConfigStore::resolve`]. This struct is [`Sync`] + [`Send`] since the linter runs on each file
-/// in parallel.
 #[derive(Debug)]
-pub struct ConfigStore {
-    // TODO: flatten base config + overrides into a single "flat" config. Similar idea to ESLint's
-    // flat configs, but we would still support v8 configs. Doing this could open the door to
-    // supporting flat configs (e.g. eslint.config.js). Still need to figure out how this plays
-    // with nested configs.
-    /// Resolved override cache. The key is a hash of each override's ID that matched the list of
-    /// file globs in order to avoid re-allocating the same set of rules multiple times.
-    cache: FxDashMap<AppliedOverrideHash, ResolvedLinterState>,
-    /// "root" level configuration. In the future this may just be the first entry in `overrides`.
+struct Config {
+    /// The basic linter state for this configuration.
     base: ResolvedLinterState,
-    /// Config deltas applied to `base`.
+
+    /// An optional set of overrides to apply to the base state depending on the file being linted.
     overrides: OxlintOverrides,
 }
 
+/// Resolves a lint configuration for a given file, by applying overrides based on the file's path.
+#[derive(Debug)]
+pub struct ConfigStore {
+    base: Config,
+}
+
 impl ConfigStore {
     pub fn new(
         base_rules: Vec<RuleWithSeverity>,
@@ -59,102 +44,76 @@ impl ConfigStore {
             rules: Arc::from(base_rules.into_boxed_slice()),
             config: Arc::new(base_config),
         };
-        // best-best case: no overrides are provided & config is initialized with 0 capacity best
-        // case: each file matches only a single override, so we only need `overrides.len()`
-        // capacity worst case: files match more than one override. In the most ridiculous case, we
-        // could end up needing (overrides.len() ** 2) capacity. I don't really want to
-        // pre-allocate that much space unconditionally. Better to re-alloc if we end up needing
-        // it.
-        let cache = FxDashMap::with_capacity_and_hasher(overrides.len(), FxBuildHasher);
-
-        Self { cache, base, overrides }
+        Self { base: Config { base, overrides } }
     }
 
     pub fn number_of_rules(&self) -> usize {
-        self.base.rules.len()
+        self.base.base.rules.len()
     }
 
     pub fn rules(&self) -> &Arc<[RuleWithSeverity]> {
-        &self.base.rules
+        &self.base.base.rules
     }
 
     pub(crate) fn resolve(&self, path: &Path) -> ResolvedLinterState {
-        if self.overrides.is_empty() {
-            return self.base.clone();
-        }
-
-        let mut overrides_to_apply: Vec<OverrideId> = Vec::new();
-        let mut hasher = FxBuildHasher.build_hasher();
-
-        // Compute the path of the file relative to the configuration file for glob matching. Globs should match
-        // relative to the location of the configuration file.
-        // - path: /some/path/like/this/to/file.js
-        // - config_path: /some/path/like/.oxlintrc.json
-        // => relative_path: this/to/file.js
-        // TODO: Handle nested configuration file paths.
-        let relative_path = if let Some(config_path) = &self.base.config.path {
-            if let Some(parent) = config_path.parent() {
-                path.strip_prefix(parent).unwrap_or(path)
-            } else {
-                path
-            }
-        } else {
-            path
-        };
+        // TODO: based on the `path` provided, resolve the configuration file to use.
+        let resolved_config = &self.base;
+        Self::apply_overrides(resolved_config, path)
+    }
 
-        for (id, override_config) in self.overrides.iter_enumerated() {
-            if override_config.files.is_match(relative_path) {
-                overrides_to_apply.push(id);
-                id.hash(&mut hasher);
-            }
+    fn apply_overrides(config: &Config, path: &Path) -> ResolvedLinterState {
+        if config.overrides.is_empty() {
+            return config.base.clone();
         }
 
-        if overrides_to_apply.is_empty() {
-            return self.base.clone();
-        }
+        let relative_path = config
+            .base
+            .config
+            .path
+            .as_ref()
+            .and_then(|config_path| {
+                config_path.parent().map(|parent| path.strip_prefix(parent).unwrap_or(path))
+            })
+            .unwrap_or(path);
 
-        let key = hasher.finish();
-        self.cache
-            .entry(key)
-            .or_insert_with(|| self.apply_overrides(&overrides_to_apply))
-            .value()
-            .clone()
-    }
+        let overrides_to_apply =
+            config.overrides.iter().filter(|config| config.files.is_match(relative_path));
 
-    /// NOTE: this function must not borrow any entries from `self.cache` or DashMap will deadlock.
-    fn apply_overrides(&self, override_ids: &[OverrideId]) -> ResolvedLinterState {
-        let mut plugins = self.base.config.plugins;
+        let mut overrides_to_apply = overrides_to_apply.peekable();
+
+        if overrides_to_apply.peek().is_none() {
+            return config.base.clone();
+        }
 
+        let mut plugins = config.base.config.plugins;
         let all_rules = RULES
             .iter()
             .filter(|rule| plugins.contains(LintPlugins::from(rule.plugin_name())))
             .cloned()
             .collect::<Vec<_>>();
-        let mut rules = self
+        let mut rules = config
             .base
             .rules
             .iter()
             .filter(|rule| plugins.contains(LintPlugins::from(rule.plugin_name())))
             .cloned()
             .collect::<FxHashSet<_>>();
 
-        let overrides = override_ids.iter().map(|id| &self.overrides[*id]);
-        for override_config in overrides {
+        for override_config in overrides_to_apply {
             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;
             }
         }
 
         let rules = rules.into_iter().collect::<Vec<_>>();
-        let config = if plugins == self.base.config.plugins {
-            Arc::clone(&self.base.config)
+        let config = if plugins == config.base.config.plugins {
+            Arc::clone(&config.base.config)
         } else {
-            let mut config = (*self.base.config.as_ref()).clone();
+            let mut config = (*config.base.config).clone();
 
             config.plugins = plugins;
             Arc::new(config)
@@ -309,7 +268,7 @@ mod test {
         }]);
 
         let store = ConfigStore::new(vec![], base_config, overrides);
-        assert_eq!(store.base.config.plugins, LintPlugins::IMPORT);
+        assert_eq!(store.base.base.config.plugins, LintPlugins::IMPORT);
 
         let app = store.resolve("other.mjs".as_ref()).config;
         assert_eq!(app.plugins, LintPlugins::IMPORT);