Skip to content

Commit 493082c

Browse files
committed
fix(language_server): use the first Span of the message as the primary Diagnostic range (#14057)
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: <img width="258" height="88" alt="grafik" src="https://github.com/user-attachments/assets/7da86f38-9789-4e04-b08b-18aa783a93ed" /> This PR: <img width="265" height="108" alt="grafik" src="https://github.com/user-attachments/assets/2a031f84-04c3-49a8-8409-a3b1822c55a5" />
1 parent 576be20 commit 493082c

File tree

5 files changed

+17
-37
lines changed

5 files changed

+17
-37
lines changed

crates/oxc_language_server/src/linter/error_with_position.rs

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
use std::{borrow::Cow, str::FromStr};
22

3-
use oxc_linter::{FixWithPosition, MessageWithPosition, PossibleFixesWithPosition};
3+
use oxc_linter::{
4+
FixWithPosition, MessageWithPosition, PossibleFixesWithPosition, SpanPositionMessage,
5+
};
46
use tower_lsp_server::lsp_types::{
57
self, CodeDescription, DiagnosticRelatedInformation, DiagnosticSeverity, NumberOrString,
68
Position, Range, Uri,
79
};
810

911
use oxc_diagnostics::Severity;
1012

11-
use crate::LSP_MAX_INT;
12-
1313
#[derive(Debug, Clone, Default)]
1414
pub struct DiagnosticReport {
1515
pub diagnostic: lsp_types::Diagnostic,
@@ -31,13 +31,6 @@ pub enum PossibleFixContent {
3131
Multiple(Vec<FixedContent>),
3232
}
3333

34-
fn cmp_range(first: &Range, other: &Range) -> std::cmp::Ordering {
35-
match first.start.cmp(&other.start) {
36-
std::cmp::Ordering::Equal => first.end.cmp(&other.end),
37-
o => o,
38-
}
39-
}
40-
4134
fn message_with_position_to_lsp_diagnostic(
4235
message: &MessageWithPosition<'_>,
4336
uri: &Uri,
@@ -71,24 +64,14 @@ fn message_with_position_to_lsp_diagnostic(
7164
.collect()
7265
});
7366

74-
let range = related_information.as_ref().map_or(
67+
let range = message.labels.as_ref().map_or(Range::default(), |labels| {
68+
let start = labels.first().map(SpanPositionMessage::start).cloned().unwrap_or_default();
69+
let end = labels.first().map(SpanPositionMessage::end).cloned().unwrap_or_default();
7570
Range {
76-
start: Position { line: LSP_MAX_INT, character: LSP_MAX_INT },
77-
end: Position { line: LSP_MAX_INT, character: LSP_MAX_INT },
78-
},
79-
|infos: &Vec<DiagnosticRelatedInformation>| {
80-
let mut ret_range = Range {
81-
start: Position { line: LSP_MAX_INT, character: LSP_MAX_INT },
82-
end: Position { line: LSP_MAX_INT, character: LSP_MAX_INT },
83-
};
84-
for info in infos {
85-
if cmp_range(&ret_range, &info.location.range) == std::cmp::Ordering::Greater {
86-
ret_range = info.location.range;
87-
}
88-
}
89-
ret_range
90-
},
91-
);
71+
start: Position::new(start.line, start.character),
72+
end: Position::new(end.line, end.character),
73+
}
74+
});
9275
let code = message.code.to_string();
9376
let code_description =
9477
message.url.as_ref().map(|url| CodeDescription { href: Uri::from_str(url).ok().unwrap() });

crates/oxc_language_server/src/main.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,6 @@ type ConcurrentHashMap<K, V> = papaya::HashMap<K, V, FxBuildHasher>;
1919

2020
const OXC_CONFIG_FILE: &str = ".oxlintrc.json";
2121

22-
// max range for LSP integer is 2^31 - 1
23-
// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#baseTypes
24-
const LSP_MAX_INT: u32 = 2u32.pow(31) - 1;
25-
2622
#[tokio::main]
2723
async fn main() {
2824
env_logger::init();

crates/oxc_language_server/src/snapshots/[email protected]

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ fixed: Multiple(
5656
code: "typescript-eslint(no-non-null-asserted-optional-chain)"
5757
code_description.href: "https://oxc.rs/docs/guide/usage/linter/rules/typescript/no-non-null-asserted-optional-chain.html"
5858
message: "Optional chain expressions can return undefined by design: using a non-null assertion is unsafe and wrong.\nhelp: Remove the non-null assertion."
59-
range: Range { start: Position { line: 11, character: 18 }, end: Position { line: 11, character: 19 } }
59+
range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } }
6060
related_information[0].message: "non-null assertion made after optional chain"
6161
related_information[0].location.uri: "file://<variable>/fixtures/linter/issue_9958/issue.ts"
6262
related_information[0].location.range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } }
@@ -106,11 +106,11 @@ fixed: Multiple(
106106

107107
code: "None"
108108
code_description.href: "None"
109-
message: "non-null assertion made after optional chain"
110-
range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } }
109+
message: "optional chain used"
110+
range: Range { start: Position { line: 11, character: 18 }, end: Position { line: 11, character: 19 } }
111111
related_information[0].message: "original diagnostic"
112112
related_information[0].location.uri: "file://<variable>/fixtures/linter/issue_9958/issue.ts"
113-
related_information[0].location.range: Range { start: Position { line: 11, character: 18 }, end: Position { line: 11, character: 19 } }
113+
related_information[0].location.range: Range { start: Position { line: 11, character: 21 }, end: Position { line: 11, character: 22 } }
114114
severity: Some(Hint)
115115
source: Some("oxc")
116116
tags: None

crates/oxc_linter/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ use crate::{
8888

8989
#[cfg(feature = "language_server")]
9090
pub use crate::lsp::{
91-
FixWithPosition, MessageWithPosition, PossibleFixesWithPosition,
91+
FixWithPosition, MessageWithPosition, PossibleFixesWithPosition, SpanPositionMessage,
9292
oxc_diagnostic_to_message_with_position,
9393
};
9494

crates/oxc_linter/src/lsp.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ impl<'a> SpanPositionMessage<'a> {
2020
Self { start, end, message: None }
2121
}
2222

23+
#[must_use]
2324
pub fn with_message(mut self, message: Option<Cow<'a, str>>) -> Self {
2425
self.message = message;
2526
self
@@ -38,7 +39,7 @@ impl<'a> SpanPositionMessage<'a> {
3839
}
3940
}
4041

41-
#[derive(Clone, Debug)]
42+
#[derive(Clone, Debug, Default)]
4243
pub struct SpanPosition {
4344
pub line: u32,
4445
pub character: u32,

0 commit comments

Comments
 (0)