Skip to content

Commit 5b11610

Browse files
committed
refactor(linter): extract config searching functions to their own file (#18673)
move the funcitons that search for config files out of sever_linter.rs/lint.rs into config_loader so that they're all in the same place
1 parent 051cfcf commit 5b11610

File tree

5 files changed

+131
-133
lines changed

5 files changed

+131
-133
lines changed

apps/oxlint/src/config_loader.rs

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,124 @@
1-
use std::path::{Path, PathBuf};
1+
use std::{
2+
ffi::OsStr,
3+
path::{Path, PathBuf},
4+
sync::{Arc, mpsc},
5+
};
26

7+
use ignore::DirEntry;
38
use oxc_diagnostics::OxcDiagnostic;
49
use oxc_linter::{
510
Config, ConfigStoreBuilder, ExternalLinter, ExternalPluginStore, LintFilter, Oxlintrc,
611
};
12+
use rustc_hash::FxHashSet;
13+
14+
use crate::DEFAULT_OXLINTRC_NAME;
15+
16+
/// Discover config files by walking UP from each file's directory to ancestors.
17+
///
18+
/// Used by CLI where we have specific files to lint and need to find configs
19+
/// that apply to them.
20+
///
21+
/// Example: For files `/project/src/foo.js` and `/project/src/bar/baz.js`:
22+
/// - Checks `/project/src/bar/`, `/project/src/`, `/project/`, `/`
23+
/// - Returns paths to any `.oxlintrc.json` files found
24+
pub fn discover_configs_in_ancestors<P: AsRef<Path>>(
25+
files: &[P],
26+
) -> impl IntoIterator<Item = PathBuf> {
27+
let mut config_paths = FxHashSet::<PathBuf>::default();
28+
let mut visited_dirs = FxHashSet::default();
29+
30+
for file in files {
31+
let path = file.as_ref();
32+
// Start from the file's parent directory and walk up the tree
33+
let mut current = path.parent();
34+
while let Some(dir) = current {
35+
// Stop if we've already checked this directory (and its ancestors)
36+
let inserted = visited_dirs.insert(dir.to_path_buf());
37+
if !inserted {
38+
break;
39+
}
40+
if let Some(config_path) = find_config_in_directory(dir) {
41+
config_paths.insert(config_path);
42+
}
43+
current = dir.parent();
44+
}
45+
}
46+
47+
config_paths.into_iter()
48+
}
49+
50+
/// Discover config files by walking DOWN from a root directory.
51+
///
52+
/// Used by LSP where we have a workspace root and need to discover all configs
53+
/// upfront for file watching and diagnostics.
54+
pub fn discover_configs_in_tree(root: &Path) -> impl IntoIterator<Item = PathBuf> {
55+
let walker = ignore::WalkBuilder::new(root)
56+
.hidden(false) // don't skip hidden files
57+
.parents(false) // disable gitignore from parent dirs
58+
.ignore(false) // disable .ignore files
59+
.git_global(false) // disable global gitignore
60+
.follow_links(true)
61+
.build_parallel();
62+
63+
let (sender, receiver) = mpsc::channel::<Vec<Arc<OsStr>>>();
64+
let mut builder = ConfigWalkBuilder { sender };
65+
walker.visit(&mut builder);
66+
drop(builder);
67+
68+
receiver.into_iter().flatten().map(|p| PathBuf::from(p.as_ref()))
69+
}
70+
71+
/// Check if a directory contains an oxlint config file.
72+
fn find_config_in_directory(dir: &Path) -> Option<PathBuf> {
73+
let config_path = dir.join(DEFAULT_OXLINTRC_NAME);
74+
if config_path.is_file() { Some(config_path) } else { None }
75+
}
76+
77+
// Helper types for parallel directory walking
78+
struct ConfigWalkBuilder {
79+
sender: mpsc::Sender<Vec<Arc<OsStr>>>,
80+
}
81+
82+
impl<'s> ignore::ParallelVisitorBuilder<'s> for ConfigWalkBuilder {
83+
fn build(&mut self) -> Box<dyn ignore::ParallelVisitor + 's> {
84+
Box::new(ConfigWalkCollector { paths: vec![], sender: self.sender.clone() })
85+
}
86+
}
87+
88+
struct ConfigWalkCollector {
89+
paths: Vec<Arc<OsStr>>,
90+
sender: mpsc::Sender<Vec<Arc<OsStr>>>,
91+
}
92+
93+
impl Drop for ConfigWalkCollector {
94+
fn drop(&mut self) {
95+
let paths = std::mem::take(&mut self.paths);
96+
self.sender.send(paths).unwrap();
97+
}
98+
}
99+
100+
impl ignore::ParallelVisitor for ConfigWalkCollector {
101+
fn visit(&mut self, entry: Result<DirEntry, ignore::Error>) -> ignore::WalkState {
102+
match entry {
103+
Ok(entry) => {
104+
if is_config_file(&entry) {
105+
self.paths.push(entry.path().as_os_str().into());
106+
}
107+
ignore::WalkState::Continue
108+
}
109+
Err(_) => ignore::WalkState::Skip,
110+
}
111+
}
112+
}
113+
114+
fn is_config_file(entry: &DirEntry) -> bool {
115+
let Some(file_type) = entry.file_type() else { return false };
116+
if file_type.is_dir() {
117+
return false;
118+
}
119+
let Some(file_name) = entry.path().file_name() else { return false };
120+
file_name == DEFAULT_OXLINTRC_NAME
121+
}
7122

8123
pub struct LoadedConfig {
9124
/// The directory this config applies to

apps/oxlint/src/lint.rs

Lines changed: 12 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use std::{
99

1010
use cow_utils::CowUtils;
1111
use ignore::{gitignore::Gitignore, overrides::OverrideBuilder};
12-
use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet};
12+
use rustc_hash::{FxBuildHasher, FxHashMap};
1313

1414
use oxc_diagnostics::{DiagnosticSender, DiagnosticService, GraphicalReportHandler, OxcDiagnostic};
1515
use oxc_linter::{
@@ -20,7 +20,7 @@ use oxc_linter::{
2020
use crate::{
2121
DEFAULT_OXLINTRC_NAME,
2222
cli::{CliRunResult, LintCommand, MiscOptions, ReportUnusedDirectives, WarningOptions},
23-
config_loader::{ConfigLoadError, ConfigLoader},
23+
config_loader::{ConfigLoadError, ConfigLoader, discover_configs_in_ancestors},
2424
output_formatter::{LintCommandInfo, OutputFormatter},
2525
walk::Walk,
2626
};
@@ -474,40 +474,18 @@ impl CliRunner {
474474
external_plugin_store: &mut ExternalPluginStore,
475475
nested_ignore_patterns: &mut Vec<(Vec<String>, PathBuf)>,
476476
) -> Result<FxHashMap<PathBuf, Config>, CliRunResult> {
477-
// TODO(perf): benchmark whether or not it is worth it to store the configurations on a
478-
// per-file or per-directory basis, to avoid calling `.parent()` on every path.
479-
let mut nested_oxlintrc = FxHashSet::<PathBuf>::default();
480-
// get all of the unique directories among the paths to use for search for
481-
// oxlint config files in those directories and their ancestors
482-
// e.g. `/some/file.js` will check `/some` and `/`
483-
// `/some/other/file.js` will check `/some/other`, `/some`, and `/`
484-
let mut directories = FxHashSet::default();
485-
for path in paths {
486-
let path = Path::new(path);
487-
// Start from the file's parent directory and walk up the tree
488-
let mut current = path.parent();
489-
while let Some(dir) = current {
490-
// NOTE: Initial benchmarking showed that it was faster to iterate over the directories twice
491-
// rather than constructing the configs in one iteration. It's worth re-benchmarking that though.
492-
let inserted = directories.insert(dir);
493-
if !inserted {
494-
break;
495-
}
496-
current = dir.parent();
497-
}
498-
}
499-
for directory in directories {
500-
if let Some(path) = Self::find_oxlint_config_path_in_directory(directory) {
501-
nested_oxlintrc.insert(path);
502-
}
503-
}
477+
// Discover config files by walking up from each file's directory
478+
let config_paths: Vec<_> =
479+
paths.iter().map(|p| Path::new(p.as_ref()).to_path_buf()).collect();
480+
let discovered_configs = discover_configs_in_ancestors(&config_paths);
504481

505482
// Load all discovered configs
506483
let mut loader = ConfigLoader::new(external_linter, external_plugin_store, filters);
507-
let (configs, errors) = loader.load_many(nested_oxlintrc);
484+
let (configs, errors) = loader.load_many(discovered_configs);
508485

509-
if let Some(error) = errors.first() {
510-
let message = match error {
486+
// Fail on first error (CLI requires all configs to be valid)
487+
if let Some(error) = errors.into_iter().next() {
488+
let message = match &error {
511489
ConfigLoadError::Parse { path, error } => {
512490
format!(
513491
"Failed to parse oxlint configuration file at {}.\n{}\n",
@@ -517,17 +495,17 @@ impl CliRunner {
517495
}
518496
ConfigLoadError::Build { path, error } => {
519497
format!(
520-
"Failed to build oxlint configuration file at {}.\n{}\n",
498+
"Failed to build configuration from {}.\n{}\n",
521499
path.to_string_lossy().cow_replace('\\', "/"),
522500
render_report(handler, &OxcDiagnostic::error(error.clone()))
523501
)
524502
}
525503
};
526-
527504
print_and_flush_stdout(stdout, &message);
528505
return Err(CliRunResult::InvalidOptionConfig);
529506
}
530507

508+
// Convert loaded configs to nested config format
531509
let mut nested_configs =
532510
FxHashMap::<PathBuf, Config>::with_capacity_and_hasher(configs.len(), FxBuildHasher);
533511
for loaded in configs {
@@ -551,13 +529,6 @@ impl CliRunner {
551529
}
552530
Ok(Oxlintrc::default())
553531
}
554-
555-
/// Looks in a directory for an oxlint config file and returns the path if it exists.
556-
/// Does not validate the file or apply the default config file.
557-
fn find_oxlint_config_path_in_directory(dir: &Path) -> Option<PathBuf> {
558-
let possible_config_path = dir.join(DEFAULT_OXLINTRC_NAME);
559-
if possible_config_path.is_file() { Some(possible_config_path) } else { None }
560-
}
561532
}
562533

563534
pub fn print_and_flush_stdout(stdout: &mut dyn Write, message: &str) {

apps/oxlint/src/lsp/config_walker.rs

Lines changed: 0 additions & 87 deletions
This file was deleted.

apps/oxlint/src/lsp/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
mod code_actions;
22
mod commands;
3-
mod config_walker;
43
mod error_with_position;
54
mod lsp_file_system;
65
mod options;

apps/oxlint/src/lsp/server_linter.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ use oxc_language_server::{
2929
use crate::{
3030
DEFAULT_OXLINTRC_NAME,
3131
config_loader::ConfigLoader,
32+
config_loader::discover_configs_in_tree,
3233
lsp::{
3334
code_actions::{
3435
CODE_ACTION_KIND_SOURCE_FIX_ALL_OXC, apply_all_fix_code_action, apply_fix_code_actions,
3536
fix_all_text_edit,
3637
},
3738
commands::{FIX_ALL_COMMAND_ID, FixAllCommandArgs},
38-
config_walker::ConfigWalker,
3939
error_with_position::{
4040
DiagnosticReport, LinterCodeAction, create_unused_directives_messages,
4141
generate_inverted_diagnostics, message_to_lsp_diagnostic,
@@ -276,10 +276,10 @@ impl ServerLinterBuilder {
276276
nested_ignore_patterns: &mut Vec<(Vec<String>, PathBuf)>,
277277
extended_paths: &mut FxHashSet<PathBuf>,
278278
) -> FxHashMap<PathBuf, Config> {
279-
let paths = ConfigWalker::new(root_path).paths();
279+
let config_paths = discover_configs_in_tree(root_path);
280280

281281
let mut loader = ConfigLoader::new(external_linter, external_plugin_store, &[]);
282-
let (configs, errors) = loader.load_many(paths.iter().map(Path::new));
282+
let (configs, errors) = loader.load_many(config_paths);
283283

284284
for error in errors {
285285
warn!("Skipping config file {}: {:?}", error.path().display(), error);

0 commit comments

Comments
 (0)