mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 04:08:41 +00:00
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:
parent
0e168b8c19
commit
2b14a6fb54
4 changed files with 42 additions and 28 deletions
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
|
|
|
|||
2
justfile
2
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 <https://github.com/cargo-bins/cargo-binstall#installation>
|
||||
# or install via `cargo install cargo-binstall`
|
||||
# Initialize the project by installing all the necessary tools.
|
||||
|
|
|
|||
Loading…
Reference in a new issue