Skip to content

Commit

Permalink
refactor(linter): remove glob for windows (#8390)
Browse files Browse the repository at this point in the history
The current implementations does not work. Under linux it tells me 0
files, under windows:

```
> oxc-vscode@0.15.5 lint C:\dev\oxc\editors\vscode
> npx oxlint --config=oxlint.json --tsconfig=tsconfig.json client/*.js

Finished in 5ms on 5 files with 101 rules using 24 threads.
Found 0 warnings and 0 errors.
```

I do not think this glob is needed. we are using `ignore` in our
`Walker`, which should already covering the use case.

---------

Co-authored-by: Sysix <alexander.schlegel@clicksports.de>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Jan 10, 2025
1 parent 0a1ffc0 commit 4e05e66
Show file tree
Hide file tree
Showing 6 changed files with 1 addition and 61 deletions.
7 changes: 0 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ env_logger = { version = "0.11.5", default-features = false }
fast-glob = "0.4.0"
flate2 = "1.0.35"
futures = "0.3.31"
glob = "0.3.1"
globset = "0.4.15"
handlebars = "6.2.0"
hashbrown = "0.15.2"
Expand Down
1 change: 0 additions & 1 deletion apps/oxlint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ oxc_linter = { workspace = true }
oxc_span = { workspace = true }

bpaf = { workspace = true, features = ["autocomplete", "bright-color", "derive"] }
glob = { workspace = true }
ignore = { workspace = true, features = ["simd-accel"] }
miette = { workspace = true }
rayon = { workspace = true }
Expand Down
24 changes: 1 addition & 23 deletions apps/oxlint/src/command/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use bpaf::Bpaf;
use oxc_linter::{AllowWarnDeny, FixKind, LintPlugins};

use super::{
expand_glob,
ignore::{ignore_options, IgnoreOptions},
misc_options, validate_paths, MiscOptions, PATHS_ERROR_MESSAGE, VERSION,
};
Expand Down Expand Up @@ -41,7 +40,7 @@ pub struct LintCommand {
pub misc_options: MiscOptions,

/// Single file, single path or list of paths
#[bpaf(positional("PATH"), many, guard(validate_paths, PATHS_ERROR_MESSAGE), map(expand_glob))]
#[bpaf(positional("PATH"), many, guard(validate_paths, PATHS_ERROR_MESSAGE))]
pub paths: Vec<PathBuf>,
}

Expand Down Expand Up @@ -480,27 +479,6 @@ mod lint_options {
assert_eq!(options.paths, [file_foo, file_bar, file_baz]);
}

#[cfg(target_os = "windows")]
#[test]
#[allow(clippy::similar_names)]
fn wildcard_expansion() {
let temp_dir = tempfile::tempdir().expect("Could not create a temp dir");
let file_foo = temp_dir.path().join("foo.js");
File::create(&file_foo).expect("Could not create foo.js temp file");
let file_bar = temp_dir.path().join("bar.js");
File::create(&file_bar).expect("Could not create bar.js temp file");
let file_baz = temp_dir.path().join("baz");
File::create(&file_baz).expect("Could not create baz temp file");

let js_files_wildcard = temp_dir.path().join("*.js");
let options = get_lint_options(
js_files_wildcard.to_str().expect("could not get js files wildcard path"),
);
assert!(options.paths.contains(&file_foo));
assert!(options.paths.contains(&file_bar));
assert!(!options.paths.contains(&file_baz));
}

#[test]
fn no_parent_path() {
match lint_command().run_inner(&["../parent_dir"]) {
Expand Down
25 changes: 0 additions & 25 deletions apps/oxlint/src/command/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,31 +43,6 @@ fn validate_paths(paths: &Vec<PathBuf>) -> bool {

const PATHS_ERROR_MESSAGE: &str = "PATH must not contain \"..\"";

#[cfg(target_os = "windows")]
#[allow(clippy::needless_pass_by_value)]
fn expand_glob(paths: Vec<PathBuf>) -> Vec<PathBuf> {
let match_options = glob::MatchOptions {
case_sensitive: true,
require_literal_separator: false,
require_literal_leading_dot: false,
};

paths
.iter()
.filter_map(|path| path.to_str())
.filter_map(|path| glob::glob_with(path, match_options).ok())
.flatten()
.filter_map(Result::ok)
.collect()
}

#[cfg(not(target_os = "windows"))]
#[allow(clippy::needless_pass_by_value)]
fn expand_glob(paths: Vec<PathBuf>) -> Vec<PathBuf> {
// no-op on any os other than windows, since they expand globs
paths
}

#[cfg(test)]
mod misc_options {
use super::{lint::lint_command, MiscOptions};
Expand Down
4 changes: 0 additions & 4 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,6 @@ 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 @@ -395,8 +393,6 @@ 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

0 comments on commit 4e05e66

Please sign in to comment.