mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 12:19:15 +00:00
fix(linter): ignorePatterns does not work when files are provided as command arguments (#8590)
closes #8505
Here is why this issue occurs:
c15af02e52/apps/oxlint/src/lint.rs (L66-L70)
This commit is contained in:
parent
7ea99f40ac
commit
9cc9d5f1d1
8 changed files with 167 additions and 51 deletions
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"ignorePatterns": [
|
||||
"*.js"
|
||||
]
|
||||
}
|
||||
|
|
@ -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,
|
||||
|
|
@ -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)) => {
|
||||
|
|
@ -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;
|
||||
|
||||
let paths = Walk::new(&oxlint_wd, &paths, &ignore_options, &oxlintrc.ignore_patterns)
|
||||
.with_extensions(Extensions(extensions))
|
||||
.paths();
|
||||
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;
|
||||
}
|
||||
|
||||
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();
|
||||
|
||||
|
|
@ -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;
|
||||
|
|
@ -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 = &[
|
||||
|
|
@ -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"];
|
||||
|
|
@ -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")]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
----------
|
||||
|
|
@ -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
|
||||
----------
|
||||
|
|
@ -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;
|
||||
|
|
@ -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");
|
||||
|
||||
|
|
@ -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 let Some(override_builder) = override_builder {
|
||||
inner.overrides(override_builder);
|
||||
}
|
||||
|
||||
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());
|
||||
}
|
||||
|
||||
// Turning off `follow_links` because:
|
||||
// * following symlinks is a really slow syscall
|
||||
// * it is super rare to have symlinked source code
|
||||
|
|
@ -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;
|
||||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Reference in a new issue