From 493082c477cb4bb79d9f7d1901a99d2ebc319098 Mon Sep 17 00:00:00 2001 From: Sysix <3897725+Sysix@users.noreply.github.com> Date: Wed, 8 Oct 2025 10:11:21 +0000 Subject: [PATCH] fix(language_server): use the first Span of the message as the primary Diagnostic range (#14057) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR aligns the main label/span with `oxlint`. `oxlint` will use the first available span as the error. The LSP should use this span for the red line. ``` let a: { b?: { c: number; }; } = {}; for(var i = 0; i < 10; i--){} console.log(a?.b?.c!); ``` Note that `file.ts:7:16` means the line/column number and is linkable. ``` ⚠ eslint(for-direction): The update clause in this loop moves the variable in the wrong direction ╭─[for_direction.ts:7:16] 6 │ 7 │ for(var i = 0; i < 10; i--){} · ───┬── ─┬─ · │ ╰── with this update · ╰── This test moves in the wrong direction 8 │ console.log(a?.b?.c!); ╰──── help: Use while loop for intended infinite loop ⚠ typescript-eslint(no-non-null-asserted-optional-chain): Optional chain expressions can return undefined by design: using a non-null assertion is unsafe and wrong. ╭─[for_direction.ts:8:20] 7 │ for(var i = 0; i < 10; i--){} 8 │ console.log(a?.b?.c!); · ┬ ┬ · │ ╰── non-null assertion made after optional chain · ╰── optional chain used 9 │ ╰──── help: Remove the non-null assertion. ``` https://github.com/oxc-project/oxc/blob/main/crates/oxc_language_server/fixtures/linter/issue_9958/issue.ts Main: grafik This PR: grafik --- .../src/linter/error_with_position.rs | 37 +++++-------------- crates/oxc_language_server/src/main.rs | 4 -- .../fixtures_linter_issue_9958@issue.ts.snap | 8 ++-- crates/oxc_linter/src/lib.rs | 2 +- crates/oxc_linter/src/lsp.rs | 3 +- 5 files changed, 17 insertions(+), 37 deletions(-) diff --git a/crates/oxc_language_server/src/linter/error_with_position.rs b/crates/oxc_language_server/src/linter/error_with_position.rs index 73114a46a4a8e..5285783bf797f 100644 --- a/crates/oxc_language_server/src/linter/error_with_position.rs +++ b/crates/oxc_language_server/src/linter/error_with_position.rs @@ -1,6 +1,8 @@ use std::{borrow::Cow, str::FromStr}; -use oxc_linter::{FixWithPosition, MessageWithPosition, PossibleFixesWithPosition}; +use oxc_linter::{ + FixWithPosition, MessageWithPosition, PossibleFixesWithPosition, SpanPositionMessage, +}; use tower_lsp_server::lsp_types::{ self, CodeDescription, DiagnosticRelatedInformation, DiagnosticSeverity, NumberOrString, Position, Range, Uri, @@ -8,8 +10,6 @@ use tower_lsp_server::lsp_types::{ use oxc_diagnostics::Severity; -use crate::LSP_MAX_INT; - #[derive(Debug, Clone, Default)] pub struct DiagnosticReport { pub diagnostic: lsp_types::Diagnostic, @@ -31,13 +31,6 @@ pub enum PossibleFixContent { Multiple(Vec), } -fn cmp_range(first: &Range, other: &Range) -> std::cmp::Ordering { - match first.start.cmp(&other.start) { - std::cmp::Ordering::Equal => first.end.cmp(&other.end), - o => o, - } -} - fn message_with_position_to_lsp_diagnostic( message: &MessageWithPosition<'_>, uri: &Uri, @@ -71,24 +64,14 @@ fn message_with_position_to_lsp_diagnostic( .collect() }); - let range = related_information.as_ref().map_or( + let range = message.labels.as_ref().map_or(Range::default(), |labels| { + let start = labels.first().map(SpanPositionMessage::start).cloned().unwrap_or_default(); + let end = labels.first().map(SpanPositionMessage::end).cloned().unwrap_or_default(); Range { - start: Position { line: LSP_MAX_INT, character: LSP_MAX_INT }, - end: Position { line: LSP_MAX_INT, character: LSP_MAX_INT }, - }, - |infos: &Vec| { - let mut ret_range = Range { - start: Position { line: LSP_MAX_INT, character: LSP_MAX_INT }, - end: Position { line: LSP_MAX_INT, character: LSP_MAX_INT }, - }; - for info in infos { - if cmp_range(&ret_range, &info.location.range) == std::cmp::Ordering::Greater { - ret_range = info.location.range; - } - } - ret_range - }, - ); + start: Position::new(start.line, start.character), + end: Position::new(end.line, end.character), + } + }); let code = message.code.to_string(); let code_description = message.url.as_ref().map(|url| CodeDescription { href: Uri::from_str(url).ok().unwrap() }); diff --git a/crates/oxc_language_server/src/main.rs b/crates/oxc_language_server/src/main.rs index 1f60cc0dc0a9a..cb73703ff78b7 100644 --- a/crates/oxc_language_server/src/main.rs +++ b/crates/oxc_language_server/src/main.rs @@ -19,10 +19,6 @@ type ConcurrentHashMap = papaya::HashMap; const OXC_CONFIG_FILE: &str = ".oxlintrc.json"; -// max range for LSP integer is 2^31 - 1 -// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#baseTypes -const LSP_MAX_INT: u32 = 2u32.pow(31) - 1; - #[tokio::main] async fn main() { env_logger::init(); diff --git a/crates/oxc_language_server/src/snapshots/fixtures_linter_issue_9958@issue.ts.snap b/crates/oxc_language_server/src/snapshots/fixtures_linter_issue_9958@issue.ts.snap index 4f52f67a1b7b9..224894b703ada 100644 --- a/crates/oxc_language_server/src/snapshots/fixtures_linter_issue_9958@issue.ts.snap +++ b/crates/oxc_language_server/src/snapshots/fixtures_linter_issue_9958@issue.ts.snap @@ -56,7 +56,7 @@ fixed: Multiple( code: "typescript-eslint(no-non-null-asserted-optional-chain)" code_description.href: "https://oxc.rs/docs/guide/usage/linter/rules/typescript/no-non-null-asserted-optional-chain.html" message: "Optional chain expressions can return undefined by design: using a non-null assertion is unsafe and wrong.\nhelp: Remove the non-null assertion." -range: Range { start: Position { line: 11, character: 18 }, end: Position { line: 11, character: 19 } } +range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } } related_information[0].message: "non-null assertion made after optional chain" related_information[0].location.uri: "file:///fixtures/linter/issue_9958/issue.ts" related_information[0].location.range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } } @@ -106,11 +106,11 @@ fixed: Multiple( code: "None" code_description.href: "None" -message: "non-null assertion made after optional chain" -range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } } +message: "optional chain used" +range: Range { start: Position { line: 11, character: 18 }, end: Position { line: 11, character: 19 } } related_information[0].message: "original diagnostic" related_information[0].location.uri: "file:///fixtures/linter/issue_9958/issue.ts" -related_information[0].location.range: Range { start: Position { line: 11, character: 18 }, end: Position { line: 11, character: 19 } } +related_information[0].location.range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } } severity: Some(Hint) source: Some("oxc") tags: None diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index c723f41591401..9d0a7f8a39e8e 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -88,7 +88,7 @@ use crate::{ #[cfg(feature = "language_server")] pub use crate::lsp::{ - FixWithPosition, MessageWithPosition, PossibleFixesWithPosition, + FixWithPosition, MessageWithPosition, PossibleFixesWithPosition, SpanPositionMessage, oxc_diagnostic_to_message_with_position, }; diff --git a/crates/oxc_linter/src/lsp.rs b/crates/oxc_linter/src/lsp.rs index f4b743efa33c4..399511d61cf97 100644 --- a/crates/oxc_linter/src/lsp.rs +++ b/crates/oxc_linter/src/lsp.rs @@ -20,6 +20,7 @@ impl<'a> SpanPositionMessage<'a> { Self { start, end, message: None } } + #[must_use] pub fn with_message(mut self, message: Option>) -> Self { self.message = message; self @@ -38,7 +39,7 @@ impl<'a> SpanPositionMessage<'a> { } } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct SpanPosition { pub line: u32, pub character: u32,