Skip to content

Commit

Permalink
feat(linter): add support for nested config files
Browse files Browse the repository at this point in the history
  • Loading branch information
camchenry authored and Boshen committed Feb 20, 2025
1 parent cded0ad commit 9194935
Show file tree
Hide file tree
Showing 14 changed files with 215 additions and 8 deletions.
6 changes: 6 additions & 0 deletions apps/oxlint/fixtures/nested_config/.oxlintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"rules": {
"no-console": "error",
"no-debugger": "error"
}
}
1 change: 1 addition & 0 deletions apps/oxlint/fixtures/nested_config/console.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("test");
1 change: 1 addition & 0 deletions apps/oxlint/fixtures/nested_config/debugger.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
debugger;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
// this is a nested config file, but it should use the default config which
// is implicitly merged with this
"rules": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("test");
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
debugger;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("test");
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
debugger;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"rules": {
// should not be used, as it is overridden by the nested config
"no-console": "warn"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"no-console": "error"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function Component() {
console.log("hello");
}
87 changes: 83 additions & 4 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -55,6 +56,7 @@ impl Runner for LintRunner {
fix_options,
enable_plugins,
misc_options,
experimental_nested_config,
..
} = self.options;

Expand Down Expand Up @@ -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::<PathBuf, ConfigStore>::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 {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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<Oxlintrc, String> {
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) {
Expand Down Expand Up @@ -866,4 +939,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);
}
}
Original file line number Diff line number Diff line change
@@ -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 <variable>ms on 7 files with 98 rules using 1 threads.
----------
CLI result: LintFoundErrors
----------
48 changes: 44 additions & 4 deletions crates/oxc_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -62,11 +67,23 @@ pub struct Linter {
options: LintOptions,
// config: Arc<LintConfig>,
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<PathBuf, ConfigStore>,
}

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<PathBuf, ConfigStore>,
) -> Self {
Self { options, config, nested_configs }
}

/// Set the kind of auto fixes to apply.
Expand Down Expand Up @@ -99,8 +116,15 @@ impl Linter {
semantic: Rc<Semantic<'a>>,
module_record: Arc<ModuleRecord>,
) -> Vec<Message<'a>> {
// 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));

Expand Down Expand Up @@ -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)]
Expand Down

0 comments on commit 9194935

Please sign in to comment.