From 992b81ecfdae4e45abb424ae896511849d6f212f Mon Sep 17 00:00:00 2001 From: Sysix Date: Sun, 29 Dec 2024 15:43:09 +0100 Subject: [PATCH 1/2] 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 e14cfd6ada8b6bd97359d0da54ba07ef096feac6 Mon Sep 17 00:00:00 2001 From: Sysix Date: Thu, 30 Jan 2025 19:02:19 +0100 Subject: [PATCH 2/2] test(linter): fix InvalidOptionTsConfig tests for windows --- Cargo.lock | 1 + apps/oxlint/Cargo.toml | 1 + apps/oxlint/src/lint.rs | 4 +++- apps/oxlint/src/tester.rs | 11 +++++------ 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 531d9b18ca402..b8ccdda96a7ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2271,6 +2271,7 @@ name = "oxlint" version = "0.15.8" dependencies = [ "bpaf", + "cow-utils", "ignore", "insta", "jemallocator", diff --git a/apps/oxlint/Cargo.toml b/apps/oxlint/Cargo.toml index 19bc43ea2be6a..fbe2544ec89bb 100644 --- a/apps/oxlint/Cargo.toml +++ b/apps/oxlint/Cargo.toml @@ -36,6 +36,7 @@ oxc_linter = { workspace = true } oxc_span = { workspace = true } bpaf = { workspace = true, features = ["autocomplete", "bright-color", "derive"] } +cow-utils = { workspace = true } ignore = { workspace = true, features = ["simd-accel"] } miette = { workspace = true } rayon = { workspace = true } diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index 4ed57ca79c3ac..1f7649b52c5fb 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -5,6 +5,7 @@ use std::{ time::Instant, }; +use cow_utils::CowUtils; use ignore::gitignore::Gitignore; use oxc_diagnostics::{DiagnosticService, GraphicalReportHandler}; use oxc_linter::{ @@ -209,7 +210,8 @@ impl Runner for LintRunner { } else { let path = if path.is_relative() { options.cwd().join(path) } else { path.clone() }; stdout.write_all(format!( - "The tsconfig file {path:?} does not exist, Please provide a valid tsconfig file.\n", + "The tsconfig file {:?} does not exist, Please provide a valid tsconfig file.\n", + path.to_string_lossy().cow_replace('\\', "/") ).as_bytes()).or_else(Self::check_for_writer_error).unwrap(); stdout.flush().unwrap(); diff --git a/apps/oxlint/src/tester.rs b/apps/oxlint/src/tester.rs index 6ebae30de1452..b4a252b23400f 100644 --- a/apps/oxlint/src/tester.rs +++ b/apps/oxlint/src/tester.rs @@ -3,6 +3,8 @@ use crate::cli::{lint_command, LintRunner}; #[cfg(test)] use crate::runner::Runner; #[cfg(test)] +use cow_utils::CowUtils; +#[cfg(test)] use regex::Regex; #[cfg(test)] use std::{env, path::PathBuf}; @@ -78,10 +80,8 @@ impl Tester { // do not output the current working directory, each machine has a different one let cwd_string = current_cwd.to_str().unwrap(); - #[allow(clippy::disallowed_methods)] - let cwd_string = cwd_string.replace('\\', "/"); - #[allow(clippy::disallowed_methods)] - let output_string = output_string.replace(&cwd_string, ""); + let cwd_string = cwd_string.cow_replace('\\', "/").to_string(); // for windows + let output_string = output_string.cow_replace(&cwd_string, ""); let full_args_list = multiple_args.iter().map(|args| args.join(" ")).collect::>().join(" "); @@ -90,8 +90,7 @@ impl Tester { // windows can not handle filenames with * // allow replace instead of cow_replace. It only test - #[allow(clippy::disallowed_methods)] - let snapshot_file_name = snapshot_file_name.replace('*', "_"); + let snapshot_file_name = snapshot_file_name.cow_replace('*', "_").to_string(); settings.bind(|| { insta::assert_snapshot!(snapshot_file_name, output_string); });