diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index 048c496a07928..1f54645da0d4e 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -180,7 +180,11 @@ impl Runner for LintRunner { FxHashMap::default() }; - enable_plugins.apply_overrides(&mut oxlintrc.plugins); + { + let mut plugins = oxlintrc.plugins.unwrap_or_default(); + enable_plugins.apply_overrides(&mut plugins); + oxlintrc.plugins = Some(plugins); + } let oxlintrc_for_print = if misc_options.print_config || basic_options.init { Some(oxlintrc.clone()) diff --git a/apps/oxlint/src/snapshots/fixtures__extends_config_--config relative_paths__extends_extends_config.json console.js@oxlint.snap b/apps/oxlint/src/snapshots/fixtures__extends_config_--config relative_paths__extends_extends_config.json console.js@oxlint.snap index f895571d05880..d36d7e290b588 100644 --- a/apps/oxlint/src/snapshots/fixtures__extends_config_--config relative_paths__extends_extends_config.json console.js@oxlint.snap +++ b/apps/oxlint/src/snapshots/fixtures__extends_config_--config relative_paths__extends_extends_config.json console.js@oxlint.snap @@ -14,7 +14,7 @@ working directory: fixtures/extends_config help: Delete this console statement. Found 0 warnings and 1 error. -Finished in ms on 1 file with 103 rules using 1 threads. +Finished in ms on 1 file with 102 rules using 1 threads. ---------- CLI result: LintFoundErrors ---------- diff --git a/apps/oxlint/src/snapshots/fixtures__extends_config_overrides@oxlint.snap b/apps/oxlint/src/snapshots/fixtures__extends_config_overrides@oxlint.snap index dd0cf95ea3c52..d470982ed2650 100644 --- a/apps/oxlint/src/snapshots/fixtures__extends_config_overrides@oxlint.snap +++ b/apps/oxlint/src/snapshots/fixtures__extends_config_overrides@oxlint.snap @@ -1,6 +1,5 @@ --- source: apps/oxlint/src/tester.rs -snapshot_kind: text --- ########## arguments: overrides @@ -22,6 +21,15 @@ working directory: fixtures/extends_config `---- help: Use `unknown` instead, this will force you to explicitly, and safely, assert the type is correct. + ! ]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. + ,-[overrides/test.tsx:2:11] + 1 | function component(): any { + 2 | return click here; + : ^ + 3 | } + `---- + help: Provide an `href` for the `a` element. + 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. ,-[overrides/test.tsx:2:10] 1 | function component(): any { @@ -31,7 +39,7 @@ working directory: fixtures/extends_config `---- help: Avoid using ambiguous text like "click here", replace it with more descriptive text that provides context. -Found 0 warnings and 3 errors. +Found 1 warning and 3 errors. Finished in ms on 2 files using 1 threads. ---------- CLI result: LintFoundErrors diff --git a/crates/oxc_linter/src/config/categories.rs b/crates/oxc_linter/src/config/categories.rs index 5985be28324a6..7565471432793 100644 --- a/crates/oxc_linter/src/config/categories.rs +++ b/crates/oxc_linter/src/config/categories.rs @@ -1,4 +1,7 @@ -use std::{borrow::Cow, ops::Deref}; +use std::{ + borrow::Cow, + ops::{Deref, DerefMut}, +}; use rustc_hash::FxHashMap; use schemars::JsonSchema; @@ -18,6 +21,12 @@ impl Deref for OxlintCategories { } } +impl DerefMut for OxlintCategories { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + impl OxlintCategories { pub fn filters(&self) -> impl Iterator + '_ { self.iter().map(|(category, severity)| LintFilter::new(*severity, *category).unwrap()) diff --git a/crates/oxc_linter/src/config/config_builder.rs b/crates/oxc_linter/src/config/config_builder.rs index 9639bb4dbe21d..d6346b8cf2e29 100644 --- a/crates/oxc_linter/src/config/config_builder.rs +++ b/crates/oxc_linter/src/config/config_builder.rs @@ -78,99 +78,70 @@ impl ConfigStoreBuilder { start_empty: bool, oxlintrc: Oxlintrc, ) -> Result { - // TODO(refactor); can we make this function infallible, and move all the error handling to - // the `build` method? - let Oxlintrc { - plugins, - settings, - env, - globals, - categories, - rules: oxlintrc_rules, - overrides, - path, - ignore_patterns: _, - extends, - } = oxlintrc; - - let config = LintConfig { plugins, settings, env, globals, path: Some(path) }; - let rules = - if start_empty { FxHashMap::default() } else { Self::warn_correctness(plugins) }; - let cache = RulesCache::new(config.plugins); - let mut builder = Self { rules, config, overrides, cache }; + // TODO: this can be cached to avoid re-computing the same oxlintrc + fn resolve_oxlintrc_config(config: Oxlintrc) -> Result { + let path = config.path.clone(); + let root_path = path.parent(); + let extends = config.extends.clone(); + + let mut oxlintrc = config; + + for path in extends.iter().rev() { + if path.starts_with("eslint:") || path.starts_with("plugin:") { + // `eslint:` and `plugin:` named configs are not supported + continue; + } + // if path does not include a ".", then we will heuristically skip it since it + // kind of looks like it might be a named config + if !path.to_string_lossy().contains('.') { + continue; + } - for filter in categories.filters() { - builder = builder.with_filter(&filter); - } + let path = match root_path { + Some(p) => &p.join(path), + None => path, + }; - { - if !extends.is_empty() { - let config_path = builder.config.path.clone(); - let config_path_parent = config_path.as_ref().and_then(|p| p.parent()); - - for path in &extends { - if path.starts_with("eslint:") || path.starts_with("plugin:") { - // eslint: and plugin: named configs are not supported - continue; - } - // if path does not include a ".", then we will heuristically skip it since it - // kind of looks like it might be a named config - if !path.to_string_lossy().contains('.') { - continue; + let extends_oxlintrc = Oxlintrc::from_file(path).map_err(|e| { + ConfigBuilderError::InvalidConfigFile { + file: path.display().to_string(), + reason: e.to_string(), } + })?; - // resolve path relative to config path - let path = match config_path_parent { - Some(config_file_path) => &config_file_path.join(path), - None => path, - }; - // TODO: throw an error if this is a self-referential extend - // TODO(perf): use a global config cache to avoid re-parsing the same file multiple times - match Oxlintrc::from_file(path) { - Ok(extended_config) => { - // TODO(refactor): can we merge this together? seems redundant to use `override_rules` and then - // use `ConfigStoreBuilder`, but we don't have a better way of loading rules from config files other than that. - // Use `override_rules` to apply rule configurations and add/remove rules as needed - extended_config - .rules - .override_rules(&mut builder.rules, &builder.cache.borrow()); - // Use `ConfigStoreBuilder` to load extended config files and then apply rules from those - let mut extended_config_store = - ConfigStoreBuilder::from_oxlintrc(true, extended_config)?; - let rules = std::mem::take(&mut extended_config_store.rules); - builder = builder.with_rules(rules); - - // Handle plugin inheritance - let parent_plugins = extended_config_store.plugins(); - let child_plugins = builder.plugins(); - - if child_plugins == LintPlugins::default() { - // If child has default plugins, inherit from parent - builder = builder.with_plugins(parent_plugins); - } else if child_plugins != LintPlugins::empty() { - // If child specifies plugins, combine with parent's plugins - builder = builder.with_plugins(child_plugins.union(parent_plugins)); - } - - if !extended_config_store.overrides.is_empty() { - let overrides = - std::mem::take(&mut extended_config_store.overrides); - builder = builder.with_overrides(overrides); - } - } - Err(err) => { - return Err(ConfigBuilderError::InvalidConfigFile { - file: path.display().to_string(), - reason: err.to_string(), - }); - } - } - } + let extends = resolve_oxlintrc_config(extends_oxlintrc)?; + + oxlintrc = oxlintrc.merge(extends); } + Ok(oxlintrc) + } + + let oxlintrc = resolve_oxlintrc_config(oxlintrc)?; + let rules = if start_empty { + FxHashMap::default() + } else { + Self::warn_correctness(oxlintrc.plugins.unwrap_or_default()) + }; + + let config = LintConfig { + plugins: oxlintrc.plugins.unwrap_or_default(), + settings: oxlintrc.settings, + env: oxlintrc.env, + globals: oxlintrc.globals, + path: Some(oxlintrc.path), + }; + let cache = RulesCache::new(config.plugins); + let mut builder = Self { rules, config, overrides: oxlintrc.overrides, cache }; + + for filter in oxlintrc.categories.filters() { + builder = builder.with_filter(&filter); + } + + { let all_rules = builder.cache.borrow(); - oxlintrc_rules.override_rules(&mut builder.rules, all_rules.as_slice()); + oxlintrc.rules.override_rules(&mut builder.rules, all_rules.as_slice()); } Ok(builder) @@ -218,14 +189,6 @@ impl ConfigStoreBuilder { self } - pub(crate) fn with_rules>( - mut self, - rules: R, - ) -> Self { - self.rules.extend(rules); - self - } - /// Appends an override to the end of the current list of overrides. pub fn with_overrides>(mut self, overrides: O) -> Self { self.overrides.extend(overrides); diff --git a/crates/oxc_linter/src/config/config_store.rs b/crates/oxc_linter/src/config/config_store.rs index fd9175009ce53..8f8bf7836d891 100644 --- a/crates/oxc_linter/src/config/config_store.rs +++ b/crates/oxc_linter/src/config/config_store.rs @@ -142,11 +142,14 @@ impl Config { } } -/// Resolves a lint configuration for a given file, by applying overrides based on the file's path. +/// Stores the configuration state for the linter including: +/// 1. the root configuration (base) +/// 2. any nested configurations (`nested_configs`) +/// +/// If an explicit config has been provided `-c config.json`, then `nested_configs` will be empty #[derive(Debug, Clone)] pub struct ConfigStore { base: Config, - nested_configs: FxHashMap, } diff --git a/crates/oxc_linter/src/config/mod.rs b/crates/oxc_linter/src/config/mod.rs index fab4d0472f9f5..4cac7ced47d9f 100644 --- a/crates/oxc_linter/src/config/mod.rs +++ b/crates/oxc_linter/src/config/mod.rs @@ -36,7 +36,7 @@ pub struct LintConfig { impl From for LintConfig { fn from(config: Oxlintrc) -> Self { Self { - plugins: config.plugins, + plugins: config.plugins.unwrap_or_default(), settings: config.settings, env: config.env, globals: config.globals, diff --git a/crates/oxc_linter/src/config/overrides.rs b/crates/oxc_linter/src/config/overrides.rs index 0761713d268aa..fab21d083a45b 100644 --- a/crates/oxc_linter/src/config/overrides.rs +++ b/crates/oxc_linter/src/config/overrides.rs @@ -92,7 +92,7 @@ pub struct OxlintOverride { /// /// Thin wrapper around [`globset::GlobSet`] because that struct doesn't implement Serialize or schemars /// traits. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Default)] pub struct GlobSet { /// Raw patterns from the config. Inefficient, but required for [serialization](Serialize), /// which in turn is required for `--print-config`. @@ -126,6 +126,12 @@ impl GlobSet { } } +impl std::fmt::Debug for GlobSet { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_tuple("GlobSet").field(&self.raw).finish() + } +} + impl Serialize for GlobSet { fn serialize(&self, serializer: S) -> Result { self.raw.serialize(serializer) diff --git a/crates/oxc_linter/src/config/oxlintrc.rs b/crates/oxc_linter/src/config/oxlintrc.rs index 51647b5579856..620ff565e6787 100644 --- a/crates/oxc_linter/src/config/oxlintrc.rs +++ b/crates/oxc_linter/src/config/oxlintrc.rs @@ -3,6 +3,7 @@ use std::{ path::{Path, PathBuf}, }; +use rustc_hash::FxHashMap; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -62,7 +63,7 @@ use super::{ #[serde(default)] #[non_exhaustive] pub struct Oxlintrc { - pub plugins: LintPlugins, + pub plugins: Option, pub categories: OxlintCategories, /// Example /// @@ -147,6 +148,53 @@ impl Oxlintrc { Ok(config) } + + /// Merges two [Oxlintrc] files together + /// [Self] takes priority over `other` + #[must_use] + pub fn merge(&self, other: Oxlintrc) -> Oxlintrc { + let mut categories = other.categories.clone(); + categories.extend(self.categories.iter()); + + let rules = self + .rules + .rules + .iter() + .chain(&other.rules.rules) + .fold(FxHashMap::default(), |mut rules_set, rule| { + if rules_set.contains_key(&(&rule.plugin_name, &rule.rule_name)) { + return rules_set; + } + rules_set.insert((&rule.plugin_name, &rule.rule_name), rule); + rules_set + }) + .values() + .map(|rule| (**rule).clone()) + .collect::>(); + + let settings = self.settings.clone(); + let env = self.env.clone(); + let globals = self.globals.clone(); + + let mut overrides = self.overrides.clone(); + overrides.extend(other.overrides); + + Oxlintrc { + plugins: self.plugins.map_or_else( + || other.plugins, + |p| Some(other.plugins.map_or_else(|| p, |p2| p2.union(p))), + ), + categories, + rules: OxlintRules::new(rules), + settings, + env, + globals, + overrides, + path: self.path.clone(), + ignore_patterns: self.ignore_patterns.clone(), + extends: self.extends.clone(), + } + } } fn is_json_ext(ext: &str) -> bool { @@ -162,7 +210,7 @@ mod test { #[test] fn test_oxlintrc_de_empty() { let config: Oxlintrc = serde_json::from_value(json!({})).unwrap(); - assert_eq!(config.plugins, LintPlugins::default()); + assert_eq!(config.plugins, None); assert_eq!(config.rules, OxlintRules::default()); assert!(config.rules.is_empty()); assert_eq!(config.settings, OxlintSettings::default()); @@ -174,29 +222,29 @@ mod test { #[test] fn test_oxlintrc_de_plugins_empty_array() { let config: Oxlintrc = serde_json::from_value(json!({ "plugins": [] })).unwrap(); - assert_eq!(config.plugins, LintPlugins::empty()); + assert_eq!(config.plugins, Some(LintPlugins::empty())); } #[test] fn test_oxlintrc_empty_config_plugins() { let config: Oxlintrc = serde_json::from_str(r"{}").unwrap(); - assert_eq!(config.plugins, LintPlugins::default()); + assert_eq!(config.plugins, None); } #[test] fn test_oxlintrc_specifying_plugins_will_override() { let config: Oxlintrc = serde_json::from_str(r#"{ "plugins": ["react", "oxc"] }"#).unwrap(); - assert_eq!(config.plugins, LintPlugins::REACT.union(LintPlugins::OXC)); + assert_eq!(config.plugins, Some(LintPlugins::REACT.union(LintPlugins::OXC))); let config: Oxlintrc = serde_json::from_str(r#"{ "plugins": ["typescript", "unicorn"] }"#).unwrap(); - assert_eq!(config.plugins, LintPlugins::TYPESCRIPT.union(LintPlugins::UNICORN)); + assert_eq!(config.plugins, Some(LintPlugins::TYPESCRIPT.union(LintPlugins::UNICORN))); let config: Oxlintrc = serde_json::from_str(r#"{ "plugins": ["typescript", "unicorn", "react", "oxc", "import", "jsdoc", "jest", "vitest", "jsx-a11y", "nextjs", "react-perf", "promise", "node"] }"#).unwrap(); - assert_eq!(config.plugins, LintPlugins::all()); + assert_eq!(config.plugins, Some(LintPlugins::all())); let config: Oxlintrc = serde_json::from_str(r#"{ "plugins": ["typescript", "@typescript-eslint"] }"#).unwrap(); - assert_eq!(config.plugins, LintPlugins::TYPESCRIPT); + assert_eq!(config.plugins, Some(LintPlugins::TYPESCRIPT)); } #[test] diff --git a/crates/oxc_linter/src/snapshots/schema_json.snap b/crates/oxc_linter/src/snapshots/schema_json.snap index 3ea196fb60124..8ab4cef206ee1 100644 --- a/crates/oxc_linter/src/snapshots/schema_json.snap +++ b/crates/oxc_linter/src/snapshots/schema_json.snap @@ -60,15 +60,13 @@ expression: json ] }, "plugins": { - "default": [ - "react", - "unicorn", - "typescript", - "oxc" - ], - "allOf": [ + "default": null, + "anyOf": [ { "$ref": "#/definitions/LintPlugins" + }, + { + "type": "null" } ] }, diff --git a/npm/oxlint/configuration_schema.json b/npm/oxlint/configuration_schema.json index 850b434d321ba..6413dd8ea49e9 100644 --- a/npm/oxlint/configuration_schema.json +++ b/npm/oxlint/configuration_schema.json @@ -56,15 +56,13 @@ ] }, "plugins": { - "default": [ - "react", - "unicorn", - "typescript", - "oxc" - ], - "allOf": [ + "default": null, + "anyOf": [ { "$ref": "#/definitions/LintPlugins" + }, + { + "type": "null" } ] }, diff --git a/tasks/website/src/linter/snapshots/schema_markdown.snap b/tasks/website/src/linter/snapshots/schema_markdown.snap index 99396743ba0f5..6efb85194c70f 100644 --- a/tasks/website/src/linter/snapshots/schema_markdown.snap +++ b/tasks/website/src/linter/snapshots/schema_markdown.snap @@ -225,15 +225,6 @@ type: `object` See [Oxlint Rules](https://oxc.rs/docs/guide/usage/linter/rules.html) -## plugins - -type: `string[]` - -default: `["react", "unicorn", "typescript", "oxc"]` - - - - ## rules type: `object`