From 2b14a6fb5403dfe5eff8dfade3baf7725b837052 Mon Sep 17 00:00:00 2001 From: "Alexander S." Date: Thu, 2 Jan 2025 11:34:02 +0100 Subject: [PATCH] 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 --- apps/oxlint/src/lint.rs | 51 ++++++++++++++++-------- apps/oxlint/src/walk.rs | 12 +++--- crates/oxc_linter/src/config/oxlintrc.rs | 5 +-- justfile | 2 +- 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index ed4d5833e0db5..c3dd35f85bebc 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -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::>(); - 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 { 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); } diff --git a/apps/oxlint/src/walk.rs b/apps/oxlint/src/walk.rs index 10cddc7e97b02..9d5c00b27a4f9 100644 --- a/apps/oxlint/src/walk.rs +++ b/apps/oxlint/src/walk.rs @@ -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, ) -> 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() diff --git a/crates/oxc_linter/src/config/oxlintrc.rs b/crates/oxc_linter/src/config/oxlintrc.rs index 37bf6566e2414..e6cf382512afc 100644 --- a/crates/oxc_linter/src/config/oxlintrc.rs +++ b/crates/oxc_linter/src/config/oxlintrc.rs @@ -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) } diff --git a/justfile b/justfile index 3eafb588c3074..52e8d97714bf0 100755 --- a/justfile +++ b/justfile @@ -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 # or install via `cargo install cargo-binstall` # Initialize the project by installing all the necessary tools.