Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
51 changes: 34 additions & 17 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -21,6 +22,7 @@ use crate::{
walk::{Extensions, Walk},
};

#[derive(Debug)]
pub struct LintRunner {
options: LintCommand,
cwd: PathBuf,
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();
Expand All @@ -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};
Expand All @@ -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,
Expand Down Expand Up @@ -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"];
Expand Down Expand Up @@ -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"];
Expand Down Expand Up @@ -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();
Expand All @@ -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]
Expand Down Expand Up @@ -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);
}

Expand Down
12 changes: 6 additions & 6 deletions apps/oxlint/src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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
Expand All @@ -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();
}
}
Expand Down Expand Up @@ -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;
Expand All @@ -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()
Expand Down
5 changes: 1 addition & 4 deletions crates/oxc_linter/src/config/oxlintrc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion justfile
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading