8000 fix(linter): delay merging of oxlintrc configs (#10835) · oxc-project/oxc@e8470d9 · GitHub
[go: up one dir, main page]
Skip to content

Commit e8470d9

Browse files
camc314camc314
committed
fix(linter): delay merging of oxlintrc configs (#10835)
fixes #10760
1 parent f014174 commit e8470d9

File tree

12 files changed

+161
-133
lines changed

12 files changed

+161
-133
lines changed

‎apps/oxlint/src/lint.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,11 @@ impl Runner for LintRunner {
180180
FxHashMap::default()
181181
};
182182

183-
enable_plugins.apply_overrides(&mut oxlintrc.plugins);
183+
{
184+
let mut plugins = oxlintrc.plugins.unwrap_or_default();
185+
enable_plugins.apply_overrides(&mut plugins);
186+
oxlintrc.plugins = Some(plugins);
187+
}
184188

185189
let oxlintrc_for_print = if misc_options.print_config || basic_options.init {
186190
Some(oxlintrc.clone())

‎apps/oxlint/src/snapshots/fixtures__extends_config_--config relative_paths__extends_extends_config.json console.js@oxlint.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ working directory: fixtures/extends_config
1414
help: Delete this console statement.
1515
1616
Found 0 warnings and 1 error.
17-
Finished in <variable>ms on 1 file with 103 rules using 1 threads.
17+
Finished in <variable>ms on 1 file with 102 rules using 1 threads.
1818
----------
1919
CLI result: LintFoundErrors
2020
----------

‎apps/oxlint/src/snapshots/fixtures__extends_config_overrides@oxlint.snap

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
---
22
source: apps/oxlint/src/tester.rs
3-
snapshot_kind: text
43
---
54
##########
65
arguments: overrides
@@ -22,6 +21,15 @@ working directory: fixtures/extends_config
2221
`----
2322
help: Use `unknown` instead, this will force you to explicitly, and safely, assert the type is correct.
2423
24+
! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/jsx_a11y/anchor-is-valid.html\eslint-plugin-jsx-a11y(anchor-is-valid)]8;;\: Missing `href` attribute for the `a` element.
25+
,-[overrides/test.tsx:2:11]
26+
1 | function component(): any {
27+
2 | return <a>click here</a>;
28+
: ^
29+
3 | }
30+
`----
31+
help: Provide an `href` for the `a` element.
32+
2533
x ]8;;https://oxc.rs/docs/guide/usage/linter/rules/jsx_a11y/anchor-ambiguous-text.html\eslint-plugin-jsx-a11y(anchor-ambiguous-text)]8;;\: Ambiguous text within anchor, screen reader users rely on link text for context.
2634
,-[overrides/test.tsx:2:10]
2735
1 | function component(): any {
@@ -31,7 +39,7 @@ working directory: fixtures/extends_config
3139
`----
3240
help: Avoid using ambiguous text like "click here", replace it with more descriptive text that provides context.
3341
34-
Found 0 warnings and 3 errors.
42+
Found 1 warning and 3 errors.
3543
Finished in <variable>ms on 2 files using 1 threads.
3644
----------
3745
CLI result: LintFoundErrors

‎crates/oxc_linter/src/config/categories.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
use std::{borrow::Cow, ops::Deref};
1+
use std::{
2+
borrow::Cow,
3+
ops::{Deref, DerefMut},
4+
};
25

36
use rustc_hash::FxHashMap;
47
use schemars::JsonSchema;
@@ -18,6 +21,12 @@ impl Deref for OxlintCategories {
1821
}
1922
}
2023

24+
impl DerefMut for OxlintCategories {
25+
fn deref_mut(&mut self) -> &mut Self::Target {
26+
&mut self.0
27+
}
28+
}
29+
2130
impl OxlintCategories {
2231
pub fn filters(&self) -> impl Iterator<Item = LintFilter> + '_ {
2332
self.iter().map(|(category, severity)| LintFilter::new(*severity, *category).unwrap())

‎crates/oxc_linter/src/config/config_builder.rs

Lines changed: 56 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -78,99 +78,70 @@ impl ConfigStoreBuilder {
7878
start_empty: bool,
7979
oxlintrc: Oxlintrc,
8080
) -> Result<Self, ConfigBuilderError> {
81-
// TODO(refactor); can we make this function infallible, and move all the error handling to
82-
// the `build` method?
83-
let Oxlintrc {
84-
plugins,
85-
settings,
86-
env,
87-
globals,
88-
categories,
89-
rules: oxlintrc_rules,
90-
overrides,
91-
path,
92-
ignore_patterns: _,
93-
extends,
94-
} = oxlintrc;
95-
96-
let config = LintConfig { plugins, settings, env, globals, path: Some(path) };
97-
let rules =
98-
if start_empty { FxHashMap::default() } else { Self::warn_correctness(plugins) };
99-
let cache = RulesCache::new(config.plugins);
100-
let mut builder = Self { rules, config, overrides, cache };
81+
// TODO: this can be cached to avoid re-computing the same oxlintrc
82+
fn resolve_oxlintrc_config(config: Oxlintrc) -> Result<Oxlintrc, ConfigBuilderError> {
83+
let path = config.path.clone();
84+
let root_path = path.parent();
85+
let extends = config.extends.clone();
86+
87+
let mut oxlintrc = config;
88+
89+
for path in extends.iter().rev() {
90+
if path.starts_with("eslint:") || path.starts_with("plugin:") {
91+
// `eslint:` and `plugin:` named configs are not supported
92+
continue;
93+
}
94+
// if path does not include a ".", then we will heuristically skip it since it
95+
// kind of looks like it might be a named config
96+
if !path.to_string_lossy().contains('.') {
97+
continue;
98+
}
10199

102-
for filter in categories.filters() {
103-
builder = builder.with_filter(&filter);
104-
}
100+
let path = match root_path {
101+
Some(p) => &p.join(path),
102+
None => path,
103+
};
105104

106-
{
107-
if !extends.is_empty() {
108-
let config_path = builder.config.path.clone();
109-
let config_path_parent = config_path.as_ref().and_then(|p| p.parent());
110-
111-
for path in &extends {
112-
if path.starts_with("eslint:") || path.starts_with("plugin:") {
113-
// eslint: and plugin: named configs are not supported
114-
continue;
115-
}
116-
// if path does not include a ".", then we will heuristically skip it since it
117-
// kind of looks like it might be a named config
118-
if !path.to_string_lossy().contains('.') {
119-
continue;
105+
let extends_oxlintrc = Oxlintrc::from_file(path).map_err(|e| {
106+
ConfigBuilderError::InvalidConfigFile {
107+
file: path.display().to_string(),
108+
reason: e.to_string(),
120109
}
110+
})?;
121111

122-
// resolve path relative to config path
123-
let path = match config_path_parent {
124-
Some(config_file_path) => &config_file_path.join(path),
125-
None => path,
126-
};
127-
// TODO: throw an error if this is a self-referential extend
128-
// TODO(perf): use a global config cache to avoid re-parsing the same file multiple times
129-
match Oxlintrc::from_file(path) {
130-
Ok(extended_config) => {
131-
// TODO(refactor): can we merge this together? seems redundant to use `override_rules` and then
132-
// use `ConfigStoreBuilder`, but we don't have a better way of loading rules from config files other than that.
133-
// Use `override_rules` to apply rule configurations and add/remove rules as needed
134-
extended_config
135-
.rules
136-
.override_rules(&mut builder.rules, &builder.cache.borrow());
137-
// Use `ConfigStoreBuilder` to load extended config files and then apply rules from those
138-
let mut extended_config_store =
139-
ConfigStoreBuilder::from_oxlintrc(true, extended_config)?;
140-
let rules = std::mem::take(&mut extended_config_store.rules);
141-
builder = builder.with_rules(rules);
142-
143-
// Handle plugin inheritance
144-
let parent_plugins = extended_config_store.plugins();
145-
let child_plugins = builder.plugins();
146-
147-
if child_plugins == LintPlugins::default() {
148-
// If child has default plugins, inherit from parent
149-
builder = builder.with_plugins(parent_plugins);
150-
} else if child_plugins != LintPlugins::empty() {
151-
// If child specifies plugins, combine with parent's plugins
152-
builder = builder.with_plugins(child_plugins.union(parent_plugins));
153-
}
154-
155-
if !extended_config_store.overrides.is_empty() {
156-
let overrides =
157-
std::mem::take(&mut extended_config_store.overrides);
158-
builder = builder.with_overrides(overrides);
159-
}
160-
}
161-
Err(err) => {
162-
return Err(ConfigBuilderError::InvalidConfigFile {
163-
file: path.display().to_string(),
164-
reason: err.to_string(),
165-
});
166-
}
167-
}
168-
}
112+
let extends = resolve_oxlintrc_config(extends_oxlintrc)?;
113+
114+
oxlintrc = oxlintrc.merge(extends);
169115
}
116+
Ok(oxlintrc)
117+
}
118+
119+
let oxlintrc = resolve_oxlintrc_config(oxlintrc)?;
170120

121+
let rules = if start_empty {
122+
FxHashMap::default()
123+
} else {
124+
Self::warn_correctness(oxlintrc.plugins.unwrap_or_default())
125+
};
126+
127+
let config = LintConfig {
128+
plugins: oxlintrc.plugins.unwrap_or_default(),
129+
settings: oxlintrc.settings,
130+
env: oxlintrc.env,
131+
globals: oxlintrc.globals,
132+
path: Some(oxlintrc.path),
133+
};
134+
let cache = RulesCache::new(config.plugins);
135+
let mut builder = Self { rules, config, overrides: oxlintrc.overrides, cache };
136+
137+
for filter in oxlintrc.categories.filters() {
138+
builder = builder.with_filter(&filter);
139+
}
140+
141+
{
171142
let all_rules = builder.cache.borrow();
172143

173-
oxlintrc_rules.override_rules(&mut builder.rules, all_rules.as_slice());
144+
oxlintrc.rules.override_rules(&mut builder.rules, all_rules.as_slice());
174145
}
175146

176147
Ok(builder)
@@ -218,14 +189,6 @@ impl ConfigStoreBuilder {
218189
self
219190
}
220191

221-
pub(crate) fn with_rules<R: IntoIterator<Item = (RuleEnum, AllowWarnDeny)>>(
222-
mut self,
223-
rules: R,
224-
) -> Self {
225-
self.rules.extend(rules);
226-
self
227-
}
228-
229192
/// Appends an override to the end of the current list of overrides.
230193
pub fn with_overrides<O: IntoIterator<Item = OxlintOverride>>(mut self, overrides: O) -> Self {
231194
self.overrides.extend(overrides);

‎crates/oxc_linter/src/config/config_store.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,14 @@ impl Config {
142142
}
143143
}
144144

145-
/// Resolves a lint configuration for a given file, by applying overrides based on the file's path.
145+
/// Stores the configuration state for the linter including:
146+
/// 1. the root configuration (base)
147+
/// 2. any nested configurations (`nested_configs`)
148+
///
149+
/// If an explicit config has been provided `-c config.json`, then `nested_configs` will be empty
146150
#[derive(Debug, Clone)]
147151
pub struct ConfigStore {
148152
base: Config,
149-
150153
nested_configs: FxHashMap<PathBuf, Config>,
151154
}
152155

‎crates/oxc_linter/src/config/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ pub struct LintConfig {
3636
impl From<Oxlintrc> for LintConfig {
3737
fn from(config: Oxlintrc) -> Self {
3838
Self {
39-
plugins: config.plugins,
39+
plugins: config.plugins.unwrap_or_default(),
4040
settings: config.settings,
4141
env: config.env,
4242
globals: config.globals,

‎crates/oxc_linter/src/config/overrides.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ pub struct OxlintOverride {
9292
///
9393
/// Thin wrapper around [`globset::GlobSet`] because that struct doesn't implement Serialize or schemars
9494
/// traits.
95-
#[derive(Clone, Debug, Default)]
95+
#[derive(Clone, Default)]
9696
pub struct GlobSet {
9797
/// Raw patterns from the config. Inefficient, but required for [serialization](Serialize),
9898
/// which in turn is required for `--print-config`.
@@ -126,6 +126,12 @@ impl GlobSet {
126126
}
127127
}
128128

129+
impl std::fmt::Debug for GlobSet {
130+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
131+
f.debug_tuple("GlobSet").field(&self.raw).finish()
132+
}
133+
}
134+
129135
impl Serialize for GlobSet {
130136
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
131137
self.raw.serialize(serializer)

0 commit comments

Comments
 (0)
0