From 992b81ecfdae4e45abb424ae896511849d6f212f Mon Sep 17 00:00:00 2001 From: Sysix Date: Sun, 29 Dec 2024 15:43:09 +0100 Subject: [PATCH 1/6] refactor(language_server): simplify IsolatedLintHandler --- .../src/linter/isolated_lint_handler.rs | 99 +++++++++---------- 1 file changed, 49 insertions(+), 50 deletions(-) diff --git a/crates/oxc_language_server/src/linter/isolated_lint_handler.rs b/crates/oxc_language_server/src/linter/isolated_lint_handler.rs index fec99c4e32ee8..15c157075bd43 100644 --- a/crates/oxc_language_server/src/linter/isolated_lint_handler.rs +++ b/crates/oxc_language_server/src/linter/isolated_lint_handler.rs @@ -1,6 +1,6 @@ use std::{ fs, - path::{Path, PathBuf}, + path::Path, rc::Rc, sync::{Arc, OnceLock}, }; @@ -38,56 +38,57 @@ impl IsolatedLintHandler { path: &Path, content: Option, ) -> Option> { - if Self::should_lint_path(path) { - Some(self.lint_path(path, content).map_or(vec![], |(p, errors)| { - let mut diagnostics: Vec = - errors.into_iter().map(|e| e.into_diagnostic_report(&p)).collect(); - // a diagnostics connected from related_info to original diagnostic - let mut inverted_diagnostics = vec![]; - for d in &diagnostics { - let Some(ref related_info) = d.diagnostic.related_information else { + if !Self::should_lint_path(path) { + return None; + } + + Some(self.lint_path(path, content).map_or(vec![], |errors| { + let mut diagnostics: Vec = + errors.into_iter().map(|e| e.into_diagnostic_report(&path.to_path_buf())).collect(); + + // a diagnostics connected from related_info to original diagnostic + let mut inverted_diagnostics = vec![]; + for d in &diagnostics { + let Some(ref related_info) = d.diagnostic.related_information else { + continue; + }; + let related_information = Some(vec![DiagnosticRelatedInformation { + location: lsp_types::Location { + uri: lsp_types::Url::from_file_path(path).unwrap(), + range: d.diagnostic.range, + }, + message: "original diagnostic".to_string(), + }]); + for r in related_info { + if r.location.range == d.diagnostic.range { continue; - }; - let related_information = Some(vec![DiagnosticRelatedInformation { - location: lsp_types::Location { - uri: lsp_types::Url::from_file_path(path).unwrap(), - range: d.diagnostic.range, - }, - message: "original diagnostic".to_string(), - }]); - for r in related_info { - if r.location.range == d.diagnostic.range { - continue; - } - inverted_diagnostics.push(DiagnosticReport { - diagnostic: lsp_types::Diagnostic { - range: r.location.range, - severity: Some(DiagnosticSeverity::HINT), - code: None, - message: r.message.clone(), - source: d.diagnostic.source.clone(), - code_description: None, - related_information: related_information.clone(), - tags: None, - data: None, - }, - fixed_content: None, - }); } + inverted_diagnostics.push(DiagnosticReport { + diagnostic: lsp_types::Diagnostic { + range: r.location.range, + severity: Some(DiagnosticSeverity::HINT), + code: None, + message: r.message.clone(), + source: d.diagnostic.source.clone(), + code_description: None, + related_information: related_information.clone(), + tags: None, + data: None, + }, + fixed_content: None, + }); } - diagnostics.append(&mut inverted_diagnostics); - diagnostics - })) - } else { - None - } + } + diagnostics.append(&mut inverted_diagnostics); + diagnostics + })) } fn lint_path( &self, path: &Path, source_text: Option, - ) -> Option<(PathBuf, Vec)> { + ) -> Option> { if !Loader::can_load(path) { debug!("extension not supported yet."); return None; @@ -170,12 +171,10 @@ impl IsolatedLintHandler { ErrorReport { error: Error::from(msg.error), fixed_content } }) .collect::>(); - let (_, errors_with_position) = - Self::wrap_diagnostics(path, &source_text, reports, start); - diagnostics.extend(errors_with_position); + diagnostics.extend(Self::wrap_diagnostics(path, &source_text, reports, start)); } - Some((path.to_path_buf(), diagnostics)) + Some(diagnostics) } fn should_lint_path(path: &Path) -> bool { @@ -194,9 +193,10 @@ impl IsolatedLintHandler { source_text: &str, reports: Vec, start: u32, - ) -> (PathBuf, Vec) { + ) -> Vec { let source = Arc::new(NamedSource::new(path.to_string_lossy(), source_text.to_owned())); - let diagnostics = reports + + reports .into_iter() .map(|report| { ErrorWithPosition::new( @@ -206,7 +206,6 @@ impl IsolatedLintHandler { start as usize, ) }) - .collect(); - (path.to_path_buf(), diagnostics) + .collect() } } From f88ba2aa585912434c3e11c340ad2e3f096128ab Mon Sep 17 00:00:00 2001 From: Sysix Date: Thu, 9 Jan 2025 16:12:59 +0100 Subject: [PATCH 2/6] refactor(linter): remove glob for windows --- apps/oxlint/src/command/lint.rs | 24 +----------------------- apps/oxlint/src/command/mod.rs | 25 ------------------------- apps/oxlint/src/lint.rs | 2 -- 3 files changed, 1 insertion(+), 50 deletions(-) diff --git a/apps/oxlint/src/command/lint.rs b/apps/oxlint/src/command/lint.rs index 68ba5eb017891..a02ca8f609db0 100644 --- a/apps/oxlint/src/command/lint.rs +++ b/apps/oxlint/src/command/lint.rs @@ -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, }; @@ -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, } @@ -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"]) { diff --git a/apps/oxlint/src/command/mod.rs b/apps/oxlint/src/command/mod.rs index 1252a2e187578..a2e1733f8211f 100644 --- a/apps/oxlint/src/command/mod.rs +++ b/apps/oxlint/src/command/mod.rs @@ -43,31 +43,6 @@ fn validate_paths(paths: &Vec) -> 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) -> Vec { - 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) -> Vec { - // no-op on any os other than windows, since they expand globs - paths -} - #[cfg(test)] mod misc_options { use super::{lint::lint_command, MiscOptions}; diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index 48091425846a5..1e3422ac4887b 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -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"]; From db2e88aaafb7c1735405f1f1961e5a5b8eb6a317 Mon Sep 17 00:00:00 2001 From: Sysix Date: Thu, 9 Jan 2025 16:13:30 +0100 Subject: [PATCH 3/6] Revert "refactor(language_server): simplify IsolatedLintHandler" This reverts commit 992b81ecfdae4e45abb424ae896511849d6f212f. --- .../src/linter/isolated_lint_handler.rs | 99 ++++++++++--------- 1 file changed, 50 insertions(+), 49 deletions(-) diff --git a/crates/oxc_language_server/src/linter/isolated_lint_handler.rs b/crates/oxc_language_server/src/linter/isolated_lint_handler.rs index 15c157075bd43..fec99c4e32ee8 100644 --- a/crates/oxc_language_server/src/linter/isolated_lint_handler.rs +++ b/crates/oxc_language_server/src/linter/isolated_lint_handler.rs @@ -1,6 +1,6 @@ use std::{ fs, - path::Path, + path::{Path, PathBuf}, rc::Rc, sync::{Arc, OnceLock}, }; @@ -38,57 +38,56 @@ impl IsolatedLintHandler { path: &Path, content: Option, ) -> Option> { - if !Self::should_lint_path(path) { - return None; - } - - Some(self.lint_path(path, content).map_or(vec![], |errors| { - let mut diagnostics: Vec = - errors.into_iter().map(|e| e.into_diagnostic_report(&path.to_path_buf())).collect(); - - // a diagnostics connected from related_info to original diagnostic - let mut inverted_diagnostics = vec![]; - for d in &diagnostics { - let Some(ref related_info) = d.diagnostic.related_information else { - continue; - }; - let related_information = Some(vec![DiagnosticRelatedInformation { - location: lsp_types::Location { - uri: lsp_types::Url::from_file_path(path).unwrap(), - range: d.diagnostic.range, - }, - message: "original diagnostic".to_string(), - }]); - for r in related_info { - if r.location.range == d.diagnostic.range { + if Self::should_lint_path(path) { + Some(self.lint_path(path, content).map_or(vec![], |(p, errors)| { + let mut diagnostics: Vec = + errors.into_iter().map(|e| e.into_diagnostic_report(&p)).collect(); + // a diagnostics connected from related_info to original diagnostic + let mut inverted_diagnostics = vec![]; + for d in &diagnostics { + let Some(ref related_info) = d.diagnostic.related_information else { continue; - } - inverted_diagnostics.push(DiagnosticReport { - diagnostic: lsp_types::Diagnostic { - range: r.location.range, - severity: Some(DiagnosticSeverity::HINT), - code: None, - message: r.message.clone(), - source: d.diagnostic.source.clone(), - code_description: None, - related_information: related_information.clone(), - tags: None, - data: None, + }; + let related_information = Some(vec![DiagnosticRelatedInformation { + location: lsp_types::Location { + uri: lsp_types::Url::from_file_path(path).unwrap(), + range: d.diagnostic.range, }, - fixed_content: None, - }); + message: "original diagnostic".to_string(), + }]); + for r in related_info { + if r.location.range == d.diagnostic.range { + continue; + } + inverted_diagnostics.push(DiagnosticReport { + diagnostic: lsp_types::Diagnostic { + range: r.location.range, + severity: Some(DiagnosticSeverity::HINT), + code: None, + message: r.message.clone(), + source: d.diagnostic.source.clone(), + code_description: None, + related_information: related_information.clone(), + tags: None, + data: None, + }, + fixed_content: None, + }); + } } - } - diagnostics.append(&mut inverted_diagnostics); - diagnostics - })) + diagnostics.append(&mut inverted_diagnostics); + diagnostics + })) + } else { + None + } } fn lint_path( &self, path: &Path, source_text: Option, - ) -> Option> { + ) -> Option<(PathBuf, Vec)> { if !Loader::can_load(path) { debug!("extension not supported yet."); return None; @@ -171,10 +170,12 @@ impl IsolatedLintHandler { ErrorReport { error: Error::from(msg.error), fixed_content } }) .collect::>(); - diagnostics.extend(Self::wrap_diagnostics(path, &source_text, reports, start)); + let (_, errors_with_position) = + Self::wrap_diagnostics(path, &source_text, reports, start); + diagnostics.extend(errors_with_position); } - Some(diagnostics) + Some((path.to_path_buf(), diagnostics)) } fn should_lint_path(path: &Path) -> bool { @@ -193,10 +194,9 @@ impl IsolatedLintHandler { source_text: &str, reports: Vec, start: u32, - ) -> Vec { + ) -> (PathBuf, Vec) { let source = Arc::new(NamedSource::new(path.to_string_lossy(), source_text.to_owned())); - - reports + let diagnostics = reports .into_iter() .map(|report| { ErrorWithPosition::new( @@ -206,6 +206,7 @@ impl IsolatedLintHandler { start as usize, ) }) - .collect() + .collect(); + (path.to_path_buf(), diagnostics) } } From 98e2fda7e790b48ac6a8ec9f308eacba3bc15b24 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 9 Jan 2025 15:17:44 +0000 Subject: [PATCH 4/6] [autofix.ci] apply automated fixes --- Cargo.toml | 1 - apps/oxlint/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6867f593fc185..e77557de9aff0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" diff --git a/apps/oxlint/Cargo.toml b/apps/oxlint/Cargo.toml index 676ebf59fe45a..e49afd6666293 100644 --- a/apps/oxlint/Cargo.toml +++ b/apps/oxlint/Cargo.toml @@ -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 } From 94073860177f0ac4bc4a9d91d3bd11ec7446047f Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 9 Jan 2025 15:18:32 +0000 Subject: [PATCH 5/6] [autofix.ci] apply automated fixes (attempt 2/3) --- Cargo.lock | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 59aeb93a12208..7016a372ad4b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -709,12 +709,6 @@ version = "0.31.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f" -[[package]] -name = "glob" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8d1add55171497b4705a648c6b583acafb01d58050a51727785f0b2c8e0a2b2" - [[package]] name = "globset" version = "0.4.15" @@ -2134,7 +2128,6 @@ name = "oxlint" version = "0.15.5" dependencies = [ "bpaf", - "glob", "ignore", "jemallocator", "mimalloc", From 0f7701abc7396f14def1d679452d0a7fc40832bf Mon Sep 17 00:00:00 2001 From: Sysix Date: Thu, 9 Jan 2025 17:20:19 +0100 Subject: [PATCH 6/6] refactor(linter): remove glob for windows --- apps/oxlint/src/lint.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index 1e3422ac4887b..8ea5c15f7bbc6 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -393,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"];