From d24b74e8a0fb4393b7d36be32d33a1f2cb6bfbba Mon Sep 17 00:00:00 2001 From: Sysix <3897725+Sysix@users.noreply.github.com> Date: Wed, 8 Oct 2025 11:11:28 +0000 Subject: [PATCH] refactor(linter/language_server): `oxc_linter::TsgoLinter::run_source` returns `Message` (#14429) --- .../src/linter/tsgo_linter.rs | 17 ++- crates/oxc_linter/src/tsgolint.rs | 106 ++++++++---------- 2 files changed, 60 insertions(+), 63 deletions(-) diff --git a/crates/oxc_language_server/src/linter/tsgo_linter.rs b/crates/oxc_language_server/src/linter/tsgo_linter.rs index 45e59edb6253e..913168bfb4a03 100644 --- a/crates/oxc_language_server/src/linter/tsgo_linter.rs +++ b/crates/oxc_language_server/src/linter/tsgo_linter.rs @@ -3,9 +3,10 @@ use std::{ sync::{Arc, OnceLock}, }; +use oxc_data_structures::rope::Rope; use oxc_linter::{ ConfigStore, LINTABLE_EXTENSIONS, TsGoLintState, loader::LINT_PARTIAL_LOADER_EXTENSIONS, - read_to_string, + message_to_message_with_position, read_to_string, }; use rustc_hash::FxHashSet; use tower_lsp_server::{UriExt, lsp_types::Uri}; @@ -32,12 +33,20 @@ impl TsgoLinter { } let source_text = content.or_else(|| read_to_string(&path).ok())?; + let rope = Rope::from_str(&source_text); - let messages = self.state.lint_source(&Arc::from(path.as_os_str()), source_text).ok()?; + // TODO: Avoid cloning the source text + let messages = + self.state.lint_source(&Arc::from(path.as_os_str()), source_text.clone()).ok()?; let mut diagnostics: Vec = messages - .iter() - .map(|e| message_with_position_to_lsp_diagnostic_report(e, uri)) + .into_iter() + .map(|e| { + message_with_position_to_lsp_diagnostic_report( + &message_to_message_with_position(e, &source_text, &rope), + uri, + ) + }) .collect(); let mut inverted_diagnostics = generate_inverted_diagnostics(&diagnostics, uri); diff --git a/crates/oxc_linter/src/tsgolint.rs b/crates/oxc_linter/src/tsgolint.rs index 1b5ff42a22465..37d44da5560c7 100644 --- a/crates/oxc_linter/src/tsgolint.rs +++ b/crates/oxc_linter/src/tsgolint.rs @@ -15,10 +15,7 @@ use oxc_span::{SourceType, Span}; use super::{AllowWarnDeny, ConfigStore, DisableDirectives, ResolvedLinterState, read_to_string}; #[cfg(feature = "language_server")] -use crate::{ - fixer::{CompositeFix, Message, PossibleFixes}, - lsp::{MessageWithPosition, message_to_message_with_position}, -}; +use crate::fixer::{CompositeFix, Message, PossibleFixes}; /// State required to initialize the `tsgolint` linter. #[derive(Debug, Clone)] @@ -294,9 +291,7 @@ impl TsGoLintState { &self, path: &Arc, source_text: String, - ) -> Result>, String> { - use oxc_data_structures::rope::Rope; - + ) -> Result>, String> { let mut resolved_configs: FxHashMap = FxHashMap::default(); let json_input = self.json_input(std::slice::from_ref(path), &mut resolved_configs); @@ -336,65 +331,58 @@ impl TsGoLintState { // Stream diagnostics as they are emitted, rather than waiting for all output let stdout = child.stdout.take().expect("Failed to open tsgolint stdout"); - let stdout_handler = - std::thread::spawn(move || -> Result>, String> { - let msg_iter = TsGoLintMessageStream::new(stdout); + let stdout_handler = std::thread::spawn(move || -> Result>, String> { + let msg_iter = TsGoLintMessageStream::new(stdout); - let mut result = vec![]; + let mut result = vec![]; - for msg in msg_iter { - match msg { - Ok(TsGoLintMessage::Error(err)) => { - return Err(err.error); - } - Ok(TsGoLintMessage::Diagnostic(tsgolint_diagnostic)) => { - let path = tsgolint_diagnostic.file_path.clone(); - let Some(resolved_config) = resolved_configs.get(&path) else { - // If we don't have a resolved config for this path, skip it. We should always - // have a resolved config though, since we processed them already above. - continue; - }; - - let severity = - resolved_config.rules.iter().find_map(|(rule, status)| { - if rule.name() == tsgolint_diagnostic.rule { - Some(*status) - } else { - None - } - }); - let Some(severity) = severity else { - // If the severity is not found, we should not report the diagnostic - continue; - }; - - let mut message_with_position: MessageWithPosition<'_> = - message_to_message_with_position( - Message::from_tsgo_lint_diagnostic( - tsgolint_diagnostic, - &source_text, - ), - &source_text, - &Rope::from_str(&source_text), - ); + for msg in msg_iter { + match msg { + Ok(TsGoLintMessage::Error(err)) => { + return Err(err.error); + } + Ok(TsGoLintMessage::Diagnostic(tsgolint_diagnostic)) => { + let path = tsgolint_diagnostic.file_path.clone(); + let Some(resolved_config) = resolved_configs.get(&path) else { + // If we don't have a resolved config for this path, skip it. We should always + // have a resolved config though, since we processed them already above. + continue; + }; - message_with_position.severity = if severity == AllowWarnDeny::Deny - { - Severity::Error - } else { - Severity::Warning - }; + let severity = + resolved_config.rules.iter().find_map(|(rule, status)| { + if rule.name() == tsgolint_diagnostic.rule { + Some(*status) + } else { + None + } + }); + let Some(severity) = severity else { + // If the severity is not found, we should not report the diagnostic + continue; + }; - result.push(message_with_position); - } - Err(e) => { - return Err(e); - } + let mut message = Message::from_tsgo_lint_diagnostic( + tsgolint_diagnostic, + &source_text, + ); + + message.error.severity = if severity == AllowWarnDeny::Deny { + Severity::Error + } else { + Severity::Warning + }; + + result.push(message); + } + Err(e) => { + return Err(e); } } + } - Ok(result) - }); + Ok(result) + }); // Wait for process to complete and stdout processing to finish let exit_status = child.wait().expect("Failed to wait for tsgolint process");