fix(linter): fix ignorePattern config for windows (#8214)

Changes:

- Config `ignorePatterns` not as `Path`, because the create `ignore`
does only handle `/` and not Win-style `\`
- Passed full path to the config and do not use `canonicalize` because
it prefix the paths with `\\?\`,
  read more here: https://stackoverflow.com/a/41233992/7387397
- Updated and enabled tests for windows, some are still failing 

closes #8188

---------

Co-authored-by: Alexander Schlegel <alexander.schlegel@clicksports.de>
This commit is contained in:
Alexander S. 2025-01-02 11:34:02 +01:00 committed by GitHub
parent 0e168b8c19
commit 2b14a6fb54
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 42 additions and 28 deletions

View file

@ -6,6 +6,7 @@ use std::{
};
use ignore::gitignore::Gitignore;
use oxc_diagnostics::{DiagnosticService, GraphicalReportHandler};
use oxc_linter::{
loader::LINT_PARTIAL_LOADER_EXT, AllowWarnDeny, InvalidFilterKind, LintFilter, LintService,
@ -21,6 +22,7 @@ use crate::{
walk::{Extensions, Walk},
};
#[derive(Debug)]
pub struct LintRunner {
options: LintCommand,
cwd: PathBuf,
@ -114,13 +116,9 @@ 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 ignore_paths = oxlintrc
.ignore_patterns
.iter()
.map(|value| oxlintrc.path.parent().unwrap().join(value))
.collect::<Vec<_>>();
let paths = Walk::new(&paths, &ignore_options, &ignore_paths)
let paths = Walk::new(&oxlint_wd, &paths, &ignore_options, &oxlintrc.ignore_patterns)
.with_extensions(Extensions(extensions))
.paths();
@ -258,7 +256,10 @@ impl LintRunner {
// when no file is found, the default configuration is returned
fn find_oxlint_config(cwd: &Path, config: Option<&PathBuf>) -> Result<Oxlintrc, CliRunResult> {
if let Some(config_path) = config {
return match Oxlintrc::from_file(config_path) {
let mut full_path = cwd.to_path_buf();
full_path.push(config_path);
return match Oxlintrc::from_file(&full_path) {
Ok(config) => Ok(config),
Err(diagnostic) => {
let handler = GraphicalReportHandler::new();
@ -281,9 +282,9 @@ impl LintRunner {
}
}
#[cfg(all(test, not(target_os = "windows")))]
#[cfg(test)]
mod test {
use std::env;
use std::{env, path::MAIN_SEPARATOR_STR};
use super::LintRunner;
use crate::cli::{lint_command, CliRunResult, LintResult, Runner};
@ -301,10 +302,19 @@ mod test {
fn test_with_cwd(cwd: &str, args: &[&str]) -> LintResult {
let mut new_args = vec!["--silent"];
new_args.extend(args);
let options = lint_command().run_inner(new_args.as_slice()).unwrap();
let mut current_cwd = env::current_dir().unwrap();
current_cwd.push(cwd);
let part_cwd = if MAIN_SEPARATOR_STR == "/" {
cwd.into()
} else {
#[expect(clippy::disallowed_methods)]
cwd.replace('/', MAIN_SEPARATOR_STR)
};
current_cwd.push(part_cwd);
match LintRunner::new(options).with_cwd(current_cwd).run() {
CliRunResult::LintResult(lint_result) => lint_result,
@ -343,6 +353,8 @@ mod test {
assert_eq!(result.number_of_errors, 0);
}
// exclude because we are working with Glob, which only supports the current working directory
#[cfg(all(test, not(target_os = "windows")))]
#[test]
fn cwd() {
let args = &["debugger.js"];
@ -371,6 +383,8 @@ mod test {
assert_eq!(result.number_of_errors, 0);
}
// ToDo: lints all files under windows
#[cfg(all(test, not(target_os = "windows")))]
#[test]
fn wrong_extension() {
let args = &["foo.asdf"];
@ -679,12 +693,16 @@ mod test {
use std::fs;
let file = "fixtures/linter/fix.js";
let args = &["--fix", file];
let content = fs::read_to_string(file).unwrap();
let content_original = fs::read_to_string(file).unwrap();
#[expect(clippy::disallowed_methods)]
let content = content_original.replace("\r\n", "\n");
assert_eq!(&content, "debugger\n");
// Apply fix to the file.
let _ = test(args);
assert_eq!(fs::read_to_string(file).unwrap(), "\n");
#[expect(clippy::disallowed_methods)]
let new_content = fs::read_to_string(file).unwrap().replace("\r\n", "\n");
assert_eq!(new_content, "\n");
// File should not be modified if no fix is applied.
let modified_before = fs::metadata(file).unwrap().modified().unwrap();
@ -693,7 +711,7 @@ mod test {
assert_eq!(modified_before, modified_after);
// Write the file back.
fs::write(file, content).unwrap();
fs::write(file, content_original).unwrap();
}
#[test]
@ -779,11 +797,10 @@ mod test {
#[test]
fn test_config_ignore_patterns_directory() {
let result = test(&[
"-c",
"fixtures/config_ignore_patterns/ignore_directory/eslintrc.json",
let result = test_with_cwd(
"fixtures/config_ignore_patterns/ignore_directory",
]);
&["-c", "eslintrc.json"],
);
assert_eq!(result.number_of_files, 1);
}

View file

@ -64,14 +64,14 @@ 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: &[PathBuf],
config_ignore_patterns: &Vec<String>,
) -> Self {
assert!(!paths.is_empty(), "At least one path must be provided to Walk::new");
@ -91,7 +91,7 @@ impl Walk {
if !options.no_ignore {
inner.add_custom_ignore_filename(&options.ignore_path);
let mut override_builder = OverrideBuilder::new(Path::new("/"));
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
@ -103,7 +103,7 @@ impl Walk {
if !config_ignore_patterns.is_empty() {
for pattern in config_ignore_patterns {
let pattern = format!("!{}", pattern.to_str().unwrap());
let pattern = format!("!{pattern}");
override_builder.add(&pattern).unwrap();
}
}
@ -148,7 +148,7 @@ impl Walk {
#[cfg(test)]
mod test {
use std::{env, ffi::OsString};
use std::{env, ffi::OsString, path::PathBuf};
use super::{Extensions, Walk};
use crate::cli::IgnoreOptions;
@ -164,7 +164,7 @@ mod test {
symlinks: false,
};
let mut paths = Walk::new(&fixtures, &ignore_options, &[])
let mut paths = Walk::new(&PathBuf::from("/"), &fixtures, &ignore_options, &vec![])
.with_extensions(Extensions(["js", "vue"].to_vec()))
.paths()
.into_iter()

View file

@ -127,10 +127,7 @@ impl Oxlintrc {
OxcDiagnostic::error(format!("Failed to parse config with error {err:?}"))
})?;
// Get absolute path from `path`
let absolute_path = path.canonicalize().unwrap_or_else(|_| path.to_path_buf());
config.path = absolute_path;
config.path = path.to_path_buf();
Ok(config)
}

View file

@ -11,7 +11,7 @@ alias c := conformance
alias f := fix
alias new-typescript-rule := new-ts-rule
# Make sure you have cargo-binstall installed.
# Make sure you have cargo-binstall and pnpm installed.
# You can download the pre-compiled binary from <https://github.com/cargo-bins/cargo-binstall#installation>
# or install via `cargo install cargo-binstall`
# Initialize the project by installing all the necessary tools.