Skip to content

Commit

Permalink
fix(linter): ignorePatterns does not work when files are provided a…
Browse files Browse the repository at this point in the history
  • Loading branch information
shulaoda authored Jan 31, 2025
1 parent 7ea99f4 commit 9cc9d5f
Show file tree
Hide file tree
Showing 8 changed files with 167 additions and 51 deletions.
Empty file.
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"ignorePatterns": [
"*.js"
]
}
Empty file.
152 changes: 123 additions & 29 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
};

use cow_utils::CowUtils;
use ignore::gitignore::Gitignore;
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,29 +62,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 {
return CliRunResult::LintNoFilesFound;
}

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

let filter = match Self::get_filters(filter) {
Ok(filter) => filter,
Err((result, message)) => {
Expand Down Expand Up @@ -115,11 +92,69 @@ 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 mut override_builder = None;

if !ignore_options.no_ignore {
let mut 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}");
builder.add(&pattern).unwrap();
}
}
if !oxlintrc.ignore_patterns.is_empty() {
let oxlint_wd = oxlintrc.path.parent().unwrap_or(&self.cwd).to_path_buf();
oxlintrc.ignore_patterns =
Self::adjust_ignore_patterns(&self.cwd, &oxlint_wd, oxlintrc.ignore_patterns);
for pattern in &oxlintrc.ignore_patterns {
let pattern = format!("!{pattern}");
builder.add(&pattern).unwrap();
}
}

let builder = 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() {
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 {
!(builder.matched(p, false).is_ignore()
|| ignore.matched(path, false).is_ignore())
}
});
}

override_builder = Some(builder);
}

if paths.is_empty() {
// If explicit paths were provided, but all have been
// filtered, return early.
if provided_path_count > 0 {
return CliRunResult::LintNoFilesFound;
}

let paths = Walk::new(&oxlint_wd, &paths, &ignore_options, &oxlintrc.ignore_patterns)
.with_extensions(Extensions(extensions))
.paths();
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 @@ -344,11 +379,35 @@ impl LintRunner {
Err(error)
}
}

fn adjust_ignore_patterns(
base: &PathBuf,
path: &PathBuf,
ignore_patterns: Vec<String>,
) -> Vec<String> {
if base == path {
ignore_patterns
} else {
let relative_ignore_path =
path.strip_prefix(base).map_or_else(|_| PathBuf::from("."), Path::to_path_buf);

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 = relative_ignore_path.join(pattern);
format!("{prefix}{}", adjusted_path.to_string_lossy().cow_replace('\\', "/"))
})
.collect()
}
}
}

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

use super::LintRunner;
use crate::tester::Tester;
Expand Down Expand Up @@ -686,6 +745,15 @@ mod test {
Tester::new().test_and_snapshot(args);
}

#[test]
fn test_ignore_patterns() {
let args = &["-c", "./test/eslintrc.json", "--ignore-pattern", "*.ts", "."];

Tester::new()
.with_cwd("fixtures/config_ignore_patterns/with_oxlintrc".into())
.test_and_snapshot(args);
}

#[test]
fn test_config_ignore_patterns_extension() {
let args = &[
Expand All @@ -697,6 +765,17 @@ mod test {
Tester::new().test_and_snapshot(args);
}

#[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",
];

Tester::new().test_and_snapshot(args);
}

#[test]
fn test_config_ignore_patterns_directory() {
let args = &["-c", "eslintrc.json"];
Expand Down Expand Up @@ -734,4 +813,19 @@ mod test {
.with_cwd("fixtures/eslint_and_typescript_alias_rules".into())
.test_and_snapshot_multiple(&[args_1, args_2]);
}

#[test]
fn test_adjust_ignore_patterns() {
let base = PathBuf::from("/project/root");
let path = PathBuf::from("/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")]
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
source: apps/oxlint/src/tester.rs
---
##########
arguments: -c fixtures/config_ignore_patterns/ignore_extension/eslintrc.json fixtures/config_ignore_patterns/ignore_extension/main.js
working directory:
----------
----------
CLI result: LintNoFilesFound
----------
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: apps/oxlint/src/tester.rs
---
##########
arguments: -c ./test/eslintrc.json --ignore-pattern *.ts .
working directory: fixtures/config_ignore_patterns/with_oxlintrc
----------

! ]8;;https://oxc.rs/docs/guide/usage/linter/rules/unicorn/no-empty-file.html\eslint-plugin-unicorn(no-empty-file)]8;;\: Empty files are not allowed.
,-[main.js:1:1]
`----
help: Delete this file or add some code to it.
Found 1 warning and 0 errors.
Finished in <variable>ms on 1 file with 97 rules using 1 threads.
----------
CLI result: LintSucceeded
----------
33 changes: 11 additions & 22 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 @@ -68,10 +68,9 @@ impl Walk {
/// Will not canonicalize paths.
/// # Panics
pub fn new(
current_path: &PathBuf,
paths: &[PathBuf],
options: &IgnoreOptions,
config_ignore_patterns: &Vec<String>,
override_builder: Option<Override>,
) -> Self {
assert!(!paths.is_empty(), "At least one path must be provided to Walk::new");

Expand All @@ -91,25 +90,11 @@ 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();
}
if let Some(override_builder) = override_builder {
inner.overrides(override_builder);
}

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

// 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 +133,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 +151,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, Some(override_builder))
.with_extensions(Extensions(["js", "vue"].to_vec()))
.paths()
.into_iter()
Expand Down

0 comments on commit 9cc9d5f

Please sign in to comment.