Skip to content

Commit

Permalink
fix(linter): ignorePatterns does not work when files are provided as …
Browse files Browse the repository at this point in the history
…command arguments
  • Loading branch information
shulaoda committed Jan 26, 2025
1 parent 03229c5 commit f4257ad
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 56 deletions.
8 changes: 8 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ num-bigint = "0.4.6"
num-traits = "0.2.19"
petgraph = "0.7.0"
phf = "0.11.2"
pathdiff = "0.2.3"
pico-args = "0.5.0"
prettyplease = "0.2.25"
project-root = "0.2.2"
Expand Down
2 changes: 2 additions & 0 deletions apps/oxlint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,11 @@ bpaf = { workspace = true, features = ["autocomplete", "bright-color", "derive"]
ignore = { workspace = true, features = ["simd-accel"] }
miette = { workspace = true }
rayon = { workspace = true }
cow-utils = {workspace = true }
rustc-hash = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
pathdiff = { workspace = true }
tempfile = { workspace = true }
tracing-subscriber = { workspace = true, features = [] } # Omit the `regex` feature

Expand Down
135 changes: 107 additions & 28 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use std::{
time::Instant,
};

use ignore::gitignore::Gitignore;
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,
Expand Down Expand Up @@ -62,30 +63,6 @@ impl Runner for LintRunner {
let provided_path_count = paths.len();
let now = Instant::now();

// The ignore crate whitelists explicit paths, but priority
// should be given to the ignore file. Many users lint
// automatically and pass a list of changed files explicitly.
// To accommodate this, unless `--no-ignore` is passed,
// pre-filter the paths.
if !paths.is_empty() && !ignore_options.no_ignore {
let (ignore, _err) = Gitignore::new(&ignore_options.ignore_path);
paths.retain(|p| if p.is_dir() { true } else { !ignore.matched(p, false).is_ignore() });
}

// Append cwd to all paths
paths = paths.into_iter().map(|x| self.cwd.join(x)).collect();

if paths.is_empty() {
// If explicit paths were provided, but all have been
// filtered, return early.
if provided_path_count > 0 {
// ToDo: when oxc_linter (config) validates the configuration, we can use exit_code = 1 to fail
return CliRunResult::LintResult(LintResult::default());
}

paths.push(self.cwd.clone());
}

let filter = match Self::get_filters(filter) {
Ok(filter) => filter,
Err(e) => return e,
Expand All @@ -107,9 +84,64 @@ impl Runner for LintRunner {
let mut oxlintrc = config_search_result.unwrap();
let oxlint_wd = oxlintrc.path.parent().unwrap_or(&self.cwd).to_path_buf();

let paths = Walk::new(&oxlint_wd, &paths, &ignore_options, &oxlintrc.ignore_patterns)
.with_extensions(Extensions(extensions))
.paths();
let mut override_builder = OverrideBuilder::new(&self.cwd);
if !ignore_options.ignore_pattern.is_empty() {
for pattern in &ignore_options.ignore_pattern {
// Meaning of ignore pattern is reversed
// <https://docs.rs/ignore/latest/ignore/overrides/struct.OverrideBuilder.html#method.add>
let pattern = format!("!{pattern}");
override_builder.add(&pattern).unwrap();
}
}
if !oxlintrc.ignore_patterns.is_empty() {
oxlintrc.ignore_patterns = Self::adjust_ignore_patterns(
&self.cwd.to_string_lossy(),
&oxlint_wd.to_string_lossy(),
oxlintrc.ignore_patterns,
);
for pattern in &oxlintrc.ignore_patterns {
let pattern = format!("!{pattern}");
override_builder.add(&pattern).unwrap();
}
}
let override_builder = override_builder.build().unwrap();

// The ignore crate whitelists explicit paths, but priority
// should be given to the ignore file. Many users lint
// automatically and pass a list of changed files explicitly.
// To accommodate this, unless `--no-ignore` is passed,
// pre-filter the paths.
if !paths.is_empty() && !ignore_options.no_ignore {
let (ignore, _err) = Gitignore::new(&ignore_options.ignore_path);

paths.retain_mut(|p| {
// Append cwd to all paths
let mut path = self.cwd.join(&p);

std::mem::swap(p, &mut path);

if path.is_dir() {
true
} else {
!(override_builder.matched(p, false).is_ignore()
|| ignore.matched(path, false).is_ignore())
}
});
}

if paths.is_empty() {
// If explicit paths were provided, but all have been
// filtered, return early.
if provided_path_count > 0 {
// ToDo: when oxc_linter (config) validates the configuration, we can use exit_code = 1 to fail
return CliRunResult::LintResult(LintResult::default());
}

paths.push(self.cwd.clone());
}

let walker = Walk::new(&paths, &ignore_options, override_builder);
let paths = walker.with_extensions(Extensions(extensions)).paths();

let number_of_files = paths.len();

Expand Down Expand Up @@ -326,6 +358,26 @@ impl LintRunner {
Err(error)
}
}

fn adjust_ignore_patterns(base: &str, path: &str, ignore_patterns: Vec<String>) -> Vec<String> {
if base == path {
ignore_patterns
} else {
let relative_ignore_path =
pathdiff::diff_paths(path, base).unwrap_or_else(|| PathBuf::from("."));

ignore_patterns
.into_iter()
.map(|pattern| {
let prefix_len = pattern.chars().take_while(|&c| c == '!').count();
let (prefix, pattern) = pattern.split_at(prefix_len);

let adjusted_path = Path::new(&relative_ignore_path).join(pattern);
format!("{prefix}{}", adjusted_path.to_string_lossy().cow_replace('\\', "/"))
})
.collect()
}
}
}

#[cfg(test)]
Expand Down Expand Up @@ -800,6 +852,18 @@ mod test {
assert_eq!(result.number_of_files, 1);
}

#[test]
fn test_config_ignore_patterns_special_extension() {
let args = &[
"-c",
"fixtures/config_ignore_patterns/ignore_extension/eslintrc.json",
"fixtures/config_ignore_patterns/ignore_extension/main.js",
];
let result = Tester::new().get_lint_result(args);

assert_eq!(result.number_of_files, 0);
}

#[test]
fn test_config_ignore_patterns_directory() {
let result = Tester::new()
Expand Down Expand Up @@ -875,4 +939,19 @@ mod test {
let args = &["--print-config"];
Tester::new().test_and_snapshot(args);
}

#[test]
fn test_adjust_ignore_patterns() {
let base = "/project/root";
let path = "\\project\\root\\src";
let ignore_patterns =
vec![String::from("target"), String::from("!dist"), String::from("!!dist")];

let adjusted_patterns = LintRunner::adjust_ignore_patterns(base, path, ignore_patterns);

assert_eq!(
adjusted_patterns,
vec![String::from("src/target"), String::from("!src/dist"), String::from("!!src/dist")]
);
}
}
38 changes: 10 additions & 28 deletions apps/oxlint/src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{
sync::mpsc,
};

use ignore::{overrides::OverrideBuilder, DirEntry};
use ignore::{overrides::Override, DirEntry};
use oxc_span::VALID_EXTENSIONS;

use crate::cli::IgnoreOptions;
Expand Down Expand Up @@ -67,12 +67,7 @@ impl ignore::ParallelVisitor for WalkCollector {
impl Walk {
/// Will not canonicalize paths.
/// # Panics
pub fn new(
current_path: &PathBuf,
paths: &[PathBuf],
options: &IgnoreOptions,
config_ignore_patterns: &Vec<String>,
) -> Self {
pub fn new(paths: &[PathBuf], options: &IgnoreOptions, override_builder: Override) -> Self {
assert!(!paths.is_empty(), "At least one path must be provided to Walk::new");

let mut inner = ignore::WalkBuilder::new(
Expand All @@ -90,26 +85,9 @@ impl Walk {

if !options.no_ignore {
inner.add_custom_ignore_filename(&options.ignore_path);

let mut override_builder = OverrideBuilder::new(current_path);
if !options.ignore_pattern.is_empty() {
for pattern in &options.ignore_pattern {
// Meaning of ignore pattern is reversed
// <https://docs.rs/ignore/latest/ignore/overrides/struct.OverrideBuilder.html#method.add>
let pattern = format!("!{pattern}");
override_builder.add(&pattern).unwrap();
}
}

if !config_ignore_patterns.is_empty() {
for pattern in config_ignore_patterns {
let pattern = format!("!{pattern}");
override_builder.add(&pattern).unwrap();
}
}

inner.overrides(override_builder.build().unwrap());
inner.overrides(override_builder);
}

// Turning off `follow_links` because:
// * following symlinks is a really slow syscall
// * it is super rare to have symlinked source code
Expand Down Expand Up @@ -148,7 +126,9 @@ impl Walk {

#[cfg(test)]
mod test {
use std::{env, ffi::OsString, path::PathBuf};
use std::{env, ffi::OsString};

use ignore::overrides::OverrideBuilder;

use super::{Extensions, Walk};
use crate::cli::IgnoreOptions;
Expand All @@ -164,7 +144,9 @@ mod test {
symlinks: false,
};

let mut paths = Walk::new(&PathBuf::from("/"), &fixtures, &ignore_options, &vec![])
let override_builder = OverrideBuilder::new("/").build().unwrap();

let mut paths = Walk::new(&fixtures, &ignore_options, override_builder)
.with_extensions(Extensions(["js", "vue"].to_vec()))
.paths()
.into_iter()
Expand Down

0 comments on commit f4257ad

Please sign in to comment.