Skip to content

Commit

Permalink
reduce max override count to 128
Browse files Browse the repository at this point in the history
  • Loading branch information
DonIsaac committed Oct 28, 2024
1 parent 4c1f24b commit aa1cfea
Show file tree
Hide file tree
Showing 15 changed files with 555 additions and 41 deletions.
23 changes: 23 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ futures = "0.3.31"
glob = "0.3.1"
globset = "0.4.15"
handlebars = "6.1.0"
heapless = "0.8.0"
humansize = "2.1.3"
ignore = "0.4.23"
indexmap = "2.6.0"
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_diagnostics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ pub struct OxcCode {
pub scope: Option<Cow<'static, str>>,
pub number: Option<Cow<'static, str>>,
}

impl OxcCode {
pub fn is_some(&self) -> bool {
self.scope.is_some() || self.number.is_some()
Expand Down
7 changes: 5 additions & 2 deletions crates/oxc_linter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ oxc_cfg = { workspace = true }
oxc_codegen = { workspace = true }
oxc_diagnostics = { workspace = true }
oxc_ecmascript = { workspace = true }
oxc_index = { workspace = true }
oxc_index = { workspace = true, features = ["serialize"] }
oxc_macros = { workspace = true }
oxc_parser = { workspace = true }
oxc_regular_expression = { workspace = true }
Expand All @@ -36,17 +36,20 @@ oxc_span = { workspace = true, features = ["schemars", "serialize"] }
oxc_syntax = { workspace = true }

aho-corasick = { workspace = true }
assert-unchecked = { workspace = true }
bitflags = { workspace = true }
convert_case = { workspace = true }
cow-utils = { workspace = true }
dashmap = { workspace = true }
globset = { workspace = true }
globset = { workspace = true, features = ["serde1"] }
heapless = { workspace = true }
itertools = { workspace = true }
json-strip-comments = { workspace = true }
language-tags = { workspace = true }
lazy_static = { workspace = true }
memchr = { workspace = true }
mime_guess = { workspace = true }
nonmax = { workspace = true }
once_cell = { workspace = true }
phf = { workspace = true, features = ["macros"] }
rayon = { workspace = true }
Expand Down
28 changes: 21 additions & 7 deletions crates/oxc_linter/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use oxc_span::CompactStr;
use rustc_hash::FxHashSet;

use crate::{
config::{ESLintRule, LintPlugins, OxlintRules},
config::{ConfigStore, ESLintRule, LintPlugins, OxlintOverrides, OxlintRules},
rules::RULES,
AllowWarnDeny, FixKind, FrameworkFlags, LintConfig, LintFilter, LintFilterKind, LintOptions,
Linter, Oxlintrc, RuleCategory, RuleEnum, RuleWithSeverity,
Expand All @@ -18,6 +18,7 @@ pub struct LinterBuilder {
pub(super) rules: FxHashSet<RuleWithSeverity>,
options: LintOptions,
config: LintConfig,
overrides: OxlintOverrides,
cache: RulesCache,
}

Expand All @@ -36,9 +37,10 @@ impl LinterBuilder {
let options = LintOptions::default();
let config = LintConfig::default();
let rules = FxHashSet::default();
let overrides = OxlintOverrides::default();
let cache = RulesCache::new(config.plugins);

Self { rules, options, config, cache }
Self { rules, options, config, overrides, cache }
}

/// Warn on all rules in all plugins and categories, including those in `nursery`.
Expand All @@ -48,6 +50,7 @@ impl LinterBuilder {
pub fn all() -> Self {
let options = LintOptions::default();
let config = LintConfig { plugins: LintPlugins::all(), ..LintConfig::default() };
let overrides = OxlintOverrides::default();
let cache = RulesCache::new(config.plugins);
Self {
rules: RULES
Expand All @@ -56,6 +59,7 @@ impl LinterBuilder {
.collect(),
options,
config,
overrides,
cache,
}
}
Expand All @@ -76,15 +80,22 @@ impl LinterBuilder {
/// ```
pub fn from_oxlintrc(start_empty: bool, oxlintrc: Oxlintrc) -> Self {
// TODO: monorepo config merging, plugin-based extends, etc.
let Oxlintrc { plugins, settings, env, globals, categories, rules: oxlintrc_rules } =
oxlintrc;
let Oxlintrc {
plugins,
settings,
env,
globals,
categories,
rules: oxlintrc_rules,
overrides,
} = oxlintrc;

let config = LintConfig { plugins, settings, env, globals };
let options = LintOptions::default();
let rules =
if start_empty { FxHashSet::default() } else { Self::warn_correctness(plugins) };
let cache = RulesCache::new(config.plugins);
let mut builder = Self { rules, options, config, cache };
let mut builder = Self { rules, options, config, overrides, cache };

if !categories.is_empty() {
builder = builder.with_filters(categories.filters());
Expand Down Expand Up @@ -222,6 +233,8 @@ impl LinterBuilder {
}
}

/// # Panics
/// If more than 128 overrides are present within the oxlint config.
#[must_use]
pub fn build(self) -> Linter {
// When a plugin gets disabled before build(), rules for that plugin aren't removed until
Expand All @@ -234,7 +247,8 @@ impl LinterBuilder {
self.rules.into_iter().collect::<Vec<_>>()
};
rules.sort_unstable_by_key(|r| r.id());
Linter::new(rules, self.options, self.config)
let config = ConfigStore::new(rules, self.config, self.overrides).unwrap();
Linter::new(self.options, config)
}

/// Warn for all correctness rules in the given set of plugins.
Expand Down Expand Up @@ -532,7 +546,7 @@ mod test {
desired_plugins.set(LintPlugins::TYPESCRIPT, false);

let linter = LinterBuilder::default().with_plugins(desired_plugins).build();
for rule in linter.rules() {
for rule in linter.rules().iter() {
let name = rule.name();
let plugin = rule.plugin_name();
assert_ne!(
Expand Down
179 changes: 179 additions & 0 deletions crates/oxc_linter/src/config/flat.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
use crate::LintPlugins;
use crate::{rules::RULES, RuleWithSeverity};
use assert_unchecked::assert_unchecked;
use oxc_diagnostics::OxcDiagnostic;
use rustc_hash::FxHashSet;
use std::{
hash::{BuildHasher, Hash, Hasher},
path::Path,
sync::Arc,
};

use super::{
overrides::{OverrideId, OxlintOverrides},
LintConfig,
};
use dashmap::DashMap;
use heapless::Vec as StackVec;
use rustc_hash::FxBuildHasher;

type AppliedOverrideHash = u64;

// bigger = more overrides in oxlintrc files, but more stack space taken when resolving configs. We
// should tune this value based on real-world usage, but we don't collect telemetry yet.
pub const MAX_OVERRIDE_COUNT: usize = 128;
const _: () = {
assert!(MAX_OVERRIDE_COUNT.is_power_of_two());
};

// TODO: support `categories` et. al. in overrides.
#[derive(Debug)]
pub(crate) struct ResolvedLinterState {
// TODO: Arc + Vec -> SyncVec? It would save a pointer dereference.
pub rules: Arc<[RuleWithSeverity]>,
pub config: Arc<LintConfig>,
}

impl Clone for ResolvedLinterState {
fn clone(&self) -> Self {
Self { rules: Arc::clone(&self.rules), config: Arc::clone(&self.config) }
}
}

/// 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: DashMap<AppliedOverrideHash, ResolvedLinterState, FxBuildHasher>,
/// "root" level configuration. In the future this may just be the first entry in `overrides`.
base: ResolvedLinterState,
/// Config deltas applied to `base`.
overrides: OxlintOverrides,
}

impl ConfigStore {
pub fn new(
base_rules: Vec<RuleWithSeverity>,
base_config: LintConfig,
overrides: OxlintOverrides,
) -> Result<Self, OxcDiagnostic> {
if overrides.len() > MAX_OVERRIDE_COUNT {
return Err(OxcDiagnostic::error(format!(
"Oxlint only supports up to {} overrides, but {} were provided",
overrides.len(),
MAX_OVERRIDE_COUNT
)));
}
let base = ResolvedLinterState {
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 = DashMap::with_capacity_and_hasher(overrides.len(), FxBuildHasher);

Ok(Self { cache, base, overrides })
}

/// Set the base rules, replacing all existing rules.
#[cfg(test)]
#[inline]
pub fn set_rules(&mut self, new_rules: Vec<RuleWithSeverity>) {
self.base.rules = Arc::from(new_rules.into_boxed_slice());
}

pub fn number_of_rules(&self) -> usize {
self.base.rules.len()
}

pub fn rules(&self) -> &Arc<[RuleWithSeverity]> {
&self.base.rules
}

pub(crate) fn resolve(&self, path: &Path) -> ResolvedLinterState {
if self.overrides.is_empty() {
return self.base.clone();
}

// SAFETY: number of overrides is checked in constructor, and overrides cannot be added
// after ConfigStore is created.
unsafe { assert_unchecked!(self.overrides.len() <= MAX_OVERRIDE_COUNT) };
// Resolution gets run in a relatively tight loop. This vec allocates on the stack, kind
// of like `int buf[MAX_OVERRIDE_COUNT]` in C (but we also keep track of a len). This
// prevents lots of malloc/free calls, reducing heap fragmentation and system call
// overhead.
let mut overrides_to_apply: StackVec<OverrideId, MAX_OVERRIDE_COUNT> = StackVec::new();
let mut hasher = FxBuildHasher.build_hasher();

for (id, override_config) in self.overrides.iter_enumerated() {
// SAFETY: we know that overrides_to_apply's length will always be less than or equal
// to the maximum override count, which is what the capacity is set to. This assertion
// helps rustc optimize away bounds checks in the loop's `.push()` calls. Rustc is
// notoriously bad at optimizing away loop bounds checks, so we do it for it.
unsafe { assert_unchecked!(overrides_to_apply.len() < overrides_to_apply.capacity()) };
if override_config.files.is_match(path) {
overrides_to_apply.push(id).unwrap();
id.hash(&mut hasher);
}
}

if overrides_to_apply.is_empty() {
return self.base.clone();
}

let key = hasher.finish();
self.cache
.entry(key)
.or_insert_with(|| self.apply_overrides(&overrides_to_apply))
.value()
.clone()
}

/// 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 all_rules = RULES
.iter()
.filter(|rule| plugins.contains(LintPlugins::from(rule.plugin_name())))
.cloned()
.collect::<Vec<_>>();
let mut rules = self.base.rules.iter().cloned().collect::<FxHashSet<_>>();

let overrides = override_ids.iter().map(|id| &self.overrides[*id]);
for override_config in overrides {
if override_config.rules.is_empty() {
continue;
}
override_config.rules.override_rules(&mut rules, &all_rules);
}

let rules = rules.into_iter().collect::<Vec<_>>();
let config = if plugins == self.base.config.plugins {
Arc::clone(&self.base.config)
} else {
let mut config = (*self.base.config.as_ref()).clone();

config.plugins = plugins;
Arc::new(config)
};

ResolvedLinterState { rules: Arc::from(rules.into_boxed_slice()), config }
}
}
Loading

0 comments on commit aa1cfea

Please sign in to comment.