From 19878f10983b2611d4cefb16b44ac73873a0d455 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Sat, 15 Feb 2025 21:04:21 -0500 Subject: [PATCH] feat(linter): add support for nested config files --- .../fixtures/nested_config/.oxlintrc.json | 6 ++ apps/oxlint/fixtures/nested_config/console.ts | 1 + .../oxlint/fixtures/nested_config/debugger.js | 1 + .../package1-empty-config/.oxlintrc.json | 5 ++ .../package1-empty-config/console.ts | 1 + .../package1-empty-config/debugger.js | 1 + .../package2-no-config/console.ts | 1 + .../package2-no-config/debugger.js | 1 + .../package3-deep-config/.oxlintrc.json | 6 ++ .../package3-deep-config/src/.oxlintrc.json | 5 ++ .../src/components/component.js | 3 + apps/oxlint/src/lint.rs | 87 ++++++++++++++++++- ...g_--experimental-nested-config@oxlint.snap | 57 ++++++++++++ crates/oxc_linter/src/lib.rs | 48 +++++++++- 14 files changed, 215 insertions(+), 8 deletions(-) create mode 100644 apps/oxlint/fixtures/nested_config/.oxlintrc.json create mode 100644 apps/oxlint/fixtures/nested_config/console.ts create mode 100644 apps/oxlint/fixtures/nested_config/debugger.js create mode 100644 apps/oxlint/fixtures/nested_config/package1-empty-config/.oxlintrc.json create mode 100644 apps/oxlint/fixtures/nested_config/package1-empty-config/console.ts create mode 100644 apps/oxlint/fixtures/nested_config/package1-empty-config/debugger.js create mode 100644 apps/oxlint/fixtures/nested_config/package2-no-config/console.ts create mode 100644 apps/oxlint/fixtures/nested_config/package2-no-config/debugger.js create mode 100644 apps/oxlint/fixtures/nested_config/package3-deep-config/.oxlintrc.json create mode 100644 apps/oxlint/fixtures/nested_config/package3-deep-config/src/.oxlintrc.json create mode 100644 apps/oxlint/fixtures/nested_config/package3-deep-config/src/components/component.js create mode 100644 apps/oxlint/src/snapshots/fixtures__nested_config_--experimental-nested-config@oxlint.snap diff --git a/apps/oxlint/fixtures/nested_config/.oxlintrc.json b/apps/oxlint/fixtures/nested_config/.oxlintrc.json new file mode 100644 index 00000000000000..7532876f3b17ea --- /dev/null +++ b/apps/oxlint/fixtures/nested_config/.oxlintrc.json @@ -0,0 +1,6 @@ +{ + "rules": { + "no-console": "error", + "no-debugger": "error" + } +} diff --git a/apps/oxlint/fixtures/nested_config/console.ts b/apps/oxlint/fixtures/nested_config/console.ts new file mode 100644 index 00000000000000..14c198cadb60d1 --- /dev/null +++ b/apps/oxlint/fixtures/nested_config/console.ts @@ -0,0 +1 @@ +console.log("test"); diff --git a/apps/oxlint/fixtures/nested_config/debugger.js b/apps/oxlint/fixtures/nested_config/debugger.js new file mode 100644 index 00000000000000..eab74692130a60 --- /dev/null +++ b/apps/oxlint/fixtures/nested_config/debugger.js @@ -0,0 +1 @@ +debugger; diff --git a/apps/oxlint/fixtures/nested_config/package1-empty-config/.oxlintrc.json b/apps/oxlint/fixtures/nested_config/package1-empty-config/.oxlintrc.json new file mode 100644 index 00000000000000..68557cac450dea --- /dev/null +++ b/apps/oxlint/fixtures/nested_config/package1-empty-config/.oxlintrc.json @@ -0,0 +1,5 @@ +{ + // this is a nested config file, but it should use the default config which + // is implicitly merged with this + "rules": {} +} diff --git a/apps/oxlint/fixtures/nested_config/package1-empty-config/console.ts b/apps/oxlint/fixtures/nested_config/package1-empty-config/console.ts new file mode 100644 index 00000000000000..14c198cadb60d1 --- /dev/null +++ b/apps/oxlint/fixtures/nested_config/package1-empty-config/console.ts @@ -0,0 +1 @@ +console.log("test"); diff --git a/apps/oxlint/fixtures/nested_config/package1-empty-config/debugger.js b/apps/oxlint/fixtures/nested_config/package1-empty-config/debugger.js new file mode 100644 index 00000000000000..eab74692130a60 --- /dev/null +++ b/apps/oxlint/fixtures/nested_config/package1-empty-config/debugger.js @@ -0,0 +1 @@ +debugger; diff --git a/apps/oxlint/fixtures/nested_config/package2-no-config/console.ts b/apps/oxlint/fixtures/nested_config/package2-no-config/console.ts new file mode 100644 index 00000000000000..14c198cadb60d1 --- /dev/null +++ b/apps/oxlint/fixtures/nested_config/package2-no-config/console.ts @@ -0,0 +1 @@ +console.log("test"); diff --git a/apps/oxlint/fixtures/nested_config/package2-no-config/debugger.js b/apps/oxlint/fixtures/nested_config/package2-no-config/debugger.js new file mode 100644 index 00000000000000..eab74692130a60 --- /dev/null +++ b/apps/oxlint/fixtures/nested_config/package2-no-config/debugger.js @@ -0,0 +1 @@ +debugger; diff --git a/apps/oxlint/fixtures/nested_config/package3-deep-config/.oxlintrc.json b/apps/oxlint/fixtures/nested_config/package3-deep-config/.oxlintrc.json new file mode 100644 index 00000000000000..673a0f96e64fc2 --- /dev/null +++ b/apps/oxlint/fixtures/nested_config/package3-deep-config/.oxlintrc.json @@ -0,0 +1,6 @@ +{ + "rules": { + // should not be used, as it is overridden by the nested config + "no-console": "warn" + } +} diff --git a/apps/oxlint/fixtures/nested_config/package3-deep-config/src/.oxlintrc.json b/apps/oxlint/fixtures/nested_config/package3-deep-config/src/.oxlintrc.json new file mode 100644 index 00000000000000..2ff50f91ec3262 --- /dev/null +++ b/apps/oxlint/fixtures/nested_config/package3-deep-config/src/.oxlintrc.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-console": "error" + } +} diff --git a/apps/oxlint/fixtures/nested_config/package3-deep-config/src/components/component.js b/apps/oxlint/fixtures/nested_config/package3-deep-config/src/components/component.js new file mode 100644 index 00000000000000..f867ad5746a3b3 --- /dev/null +++ b/apps/oxlint/fixtures/nested_config/package3-deep-config/src/components/component.js @@ -0,0 +1,3 @@ +export function Component() { + console.log("hello"); +} diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index 7cad78de57b129..60c0de271200cc 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -9,10 +9,11 @@ use cow_utils::CowUtils; use ignore::{gitignore::Gitignore, overrides::OverrideBuilder}; use oxc_diagnostics::{DiagnosticService, GraphicalReportHandler}; use oxc_linter::{ - loader::LINT_PARTIAL_LOADER_EXT, AllowWarnDeny, ConfigStoreBuilder, InvalidFilterKind, - LintFilter, LintOptions, LintService, LintServiceOptions, Linter, Oxlintrc, + loader::LINT_PARTIAL_LOADER_EXT, AllowWarnDeny, ConfigStore, ConfigStoreBuilder, + InvalidFilterKind, LintFilter, LintOptions, LintService, LintServiceOptions, Linter, Oxlintrc, }; use oxc_span::VALID_EXTENSIONS; +use rustc_hash::{FxHashMap, FxHashSet}; use serde_json::Value; use crate::{ @@ -55,6 +56,7 @@ impl Runner for LintRunner { fix_options, enable_plugins, misc_options, + experimental_nested_config, .. } = self.options; @@ -168,6 +170,56 @@ impl Runner for LintRunner { let number_of_files = paths.len(); + // TODO(perf): benchmark whether or not it is worth it to store the configurations on a + // per-file or per-directory basis, to avoid calling `.parent()` on every path. + let mut nested_oxlintrc = FxHashMap::<&Path, Oxlintrc>::default(); + let mut nested_configs = FxHashMap::::default(); + + if experimental_nested_config { + // get all of the unique directories among the paths to use for search for + // oxlint config files in those directories + // e.g. `/some/file.js` and `/some/other/file.js` would both result in `/some` + let mut directories = FxHashSet::default(); + for path in &paths { + if let Some(directory) = path.parent() { + // TODO(perf): benchmark whether or not it is worth it to produce the config files here without + // iterating over the directories again. it might cause the cache for paths to be disrupted, but + // it is unclear whether or not that is a problem in practice. + directories.insert(directory); + } + } + for directory in directories { + // TODO: how should we handle a nested config error? + if let Ok(config) = Self::find_oxlint_config_in_directory(directory) { + nested_oxlintrc.insert(directory, config); + } + } + + // iterate over each config and build the ConfigStore + for (dir, oxlintrc) in nested_oxlintrc { + // TODO(perf): figure out if we can avoid cloning `filter` + let builder = + ConfigStoreBuilder::from_oxlintrc(false, oxlintrc).with_filters(filter.clone()); + match builder.build() { + Ok(config) => nested_configs.insert(dir.to_path_buf(), config), + Err(diagnostic) => { + let handler = GraphicalReportHandler::new(); + let mut err = String::new(); + handler.render_report(&mut err, &diagnostic).unwrap(); + stdout + .write_all( + format!("Failed to parse configuration file.\n{err}\n").as_bytes(), + ) + .or_else(Self::check_for_writer_error) + .unwrap(); + stdout.flush().unwrap(); + + return CliRunResult::InvalidOptionConfig; + } + }; + } + } + enable_plugins.apply_overrides(&mut oxlintrc.plugins); let oxlintrc_for_print = if misc_options.print_config || basic_options.init { @@ -245,8 +297,11 @@ impl Runner for LintRunner { } }; - let linter = - Linter::new(LintOptions::default(), lint_config).with_fix(fix_options.fix_kind()); + let linter = if experimental_nested_config { + Linter::new_with_nested_configs(LintOptions::default(), lint_config, nested_configs) + } else { + Linter::new(LintOptions::default(), lint_config).with_fix(fix_options.fix_kind()) + }; let tsconfig = basic_options.tsconfig; if let Some(path) = tsconfig.as_ref() { @@ -381,6 +436,24 @@ impl LintRunner { Oxlintrc::from_file(&config_path).or_else(|_| Ok(Oxlintrc::default())) } + /// Looks in a directory for an oxlint config file, returns the oxlint config if it exists + /// and returns `Err` if none exists or the file is invalid. Does not apply the default + /// config file. + fn find_oxlint_config_in_directory(dir: &Path) -> Result { + let possible_config_path = dir.join(Self::DEFAULT_OXLINTRC); + if possible_config_path.is_file() { + Oxlintrc::from_file(&possible_config_path).map_err(|e| { + let handler = GraphicalReportHandler::new(); + let mut err = String::new(); + handler.render_report(&mut err, &e).unwrap(); + err + }) + } else { + // TODO: Better error handling here. + Err("No oxlint config file found".to_string()) + } + } + fn check_for_writer_error(error: std::io::Error) -> Result<(), std::io::Error> { // Do not panic when the process is killed (e.g. piping into `less`). if matches!(error.kind(), ErrorKind::Interrupted | ErrorKind::BrokenPipe) { @@ -862,4 +935,10 @@ mod test { vec![String::from("src/target"), String::from("!src/dist"), String::from("!!src/dist")] ); } + + #[test] + fn test_nested_config() { + let args = &["--experimental-nested-config"]; + Tester::new().with_cwd("fixtures/nested_config".into()).test_and_snapshot(args); + } } diff --git a/apps/oxlint/src/snapshots/fixtures__nested_config_--experimental-nested-config@oxlint.snap b/apps/oxlint/src/snapshots/fixtures__nested_config_--experimental-nested-config@oxlint.snap new file mode 100644 index 00000000000000..5038aa5b9629e2 --- /dev/null +++ b/apps/oxlint/src/snapshots/fixtures__nested_config_--experimental-nested-config@oxlint.snap @@ -0,0 +1,57 @@ +--- +source: apps/oxlint/src/tester.rs +--- +########## +arguments: --experimental-nested-config +working directory: fixtures/nested_config +---------- + + x ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-console.html\eslint(no-console)]8;;\: eslint(no-console): Unexpected console statement. + ,-[console.ts:1:1] + 1 | console.log("test"); + : ^^^^^^^^^^^ + `---- + help: Delete this console statement. + + x ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\eslint(no-debugger)]8;;\: `debugger` statement is not allowed + ,-[debugger.js:1:1] + 1 | debugger; + : ^^^^^^^^^ + `---- + help: Delete this code. + + ! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\eslint(no-debugger)]8;;\: `debugger` statement is not allowed + ,-[package1-empty-config/debugger.js:1:1] + 1 | debugger; + : ^^^^^^^^^ + `---- + help: Delete this code. + + x ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-console.html\eslint(no-console)]8;;\: eslint(no-console): Unexpected console statement. + ,-[package2-no-config/console.ts:1:1] + 1 | console.log("test"); + : ^^^^^^^^^^^ + `---- + help: Delete this console statement. + + x ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html\eslint(no-debugger)]8;;\: `debugger` statement is not allowed + ,-[package2-no-config/debugger.js:1:1] + 1 | debugger; + : ^^^^^^^^^ + `---- + help: Delete this code. + + x ]8;;https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-console.html\eslint(no-console)]8;;\: eslint(no-console): Unexpected console statement. + ,-[package3-deep-config/src/components/component.js:2:3] + 1 | export function Component() { + 2 | console.log("hello"); + : ^^^^^^^^^^^ + 3 | } + `---- + help: Delete this console statement. + +Found 1 warning and 5 errors. +Finished in ms on 7 files with 98 rules using 1 threads. +---------- +CLI result: LintFoundErrors +---------- diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index 3816ae59f6f479..a9610859698731 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -22,9 +22,14 @@ pub mod loader; pub mod rules; pub mod table; -use std::{path::Path, rc::Rc, sync::Arc}; +use std::{ + path::{Path, PathBuf}, + rc::Rc, + sync::Arc, +}; use oxc_semantic::{AstNode, Semantic}; +use rustc_hash::FxHashMap; pub use crate::{ config::{ @@ -62,11 +67,23 @@ pub struct Linter { options: LintOptions, // config: Arc, config: ConfigStore, + // TODO(refactor): remove duplication with `config` field when nested config is + // standardized, as we do not need to pass both at that point + nested_configs: FxHashMap, } impl Linter { pub fn new(options: LintOptions, config: ConfigStore) -> Self { - Self { options, config } + Self { options, config, nested_configs: FxHashMap::default() } + } + + // TODO(refactor); remove this when nested config is standardized + pub fn new_with_nested_configs( + options: LintOptions, + config: ConfigStore, + nested_configs: FxHashMap, + ) -> Self { + Self { options, config, nested_configs } } /// Set the kind of auto fixes to apply. @@ -99,8 +116,15 @@ impl Linter { semantic: Rc>, module_record: Arc, ) -> Vec> { - // Get config + rules for this file. Takes base rules and applies glob-based overrides. - let ResolvedLinterState { rules, config } = self.config.resolve(path); + // TODO(refactor): remove branch when nested config is standardized + let ResolvedLinterState { rules, config } = if self.nested_configs.is_empty() { + // Get config + rules for this file. Takes base rules and applies glob-based overrides. + self.config.resolve(path) + } else if let Some(nearest_config) = self.get_nearest_config(path) { + nearest_config.resolve(path) + } else { + self.config.resolve(path) + }; let ctx_host = Rc::new(ContextHost::new(path, semantic, module_record, self.options, config)); @@ -181,6 +205,22 @@ impl Linter { ctx_host.take_diagnostics() } + + /// Get the nearest config for the given path, in the following priority order: + /// 1. config file in the same directory as the path + /// 2. config file in the closest parent directory + fn get_nearest_config(&self, path: &Path) -> Option<&ConfigStore> { + // TODO(perf): should we cache the computed nearest config for every directory, + // so we don't have to recompute it for every file? + let mut current = path.parent(); + while let Some(dir) = current { + if let Some(config_store) = self.nested_configs.get(dir) { + return Some(config_store); + } + current = dir.parent(); + } + None + } } #[cfg(test)]