Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 47 additions & 88 deletions crates/oxc_linter/src/config/flat.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand All @@ -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>,
Expand All @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
Loading