Skip to content

Commit ea8feca

Browse files
bors[bot]jonas-schievinkJonas Schievink
authored
Merge #8265
8265: Improve rustc diagnostic mapping r=jonas-schievink a=jonas-schievink Try to mirror rustc diagnostics more closely by: * Emitting hint-level diagnostics at *all* macro invocation sites that caused the diagnostic * Previously we emitted a copy of the diagnostic (not at hint level) at the last macro invocation site only * Emitting the original diagnostic inside the macro, if it was caused by a macro * Always including related information pointing to the invocation site or the macro, respectively (the old code contained a bug that would sometimes omit it) Fixes #8260 ![screenshot-2021-03-30-19:34:56](https://user-images.githubusercontent.com/1786438/113031484-1266a600-918f-11eb-9164-fed01c8ba37e.png) ![screenshot-2021-03-30-19:35:10](https://user-images.githubusercontent.com/1786438/113031486-12ff3c80-918f-11eb-8f15-9d7f23b69653.png) Co-authored-by: Jonas Schievink <[email protected]> Co-authored-by: Jonas Schievink <[email protected]>
2 parents 5ef0c7a + 608a465 commit ea8feca

File tree

2 files changed

+192
-47
lines changed

2 files changed

+192
-47
lines changed

‎crates/rust-analyzer/src/diagnostics/test_data/macro_compiler_error.txt‎

Lines changed: 128 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,16 @@
1313
diagnostic: Diagnostic {
1414
range: Range {
1515
start: Position {
16-
line: 264,
16+
line: 271,
1717
character: 8,
1818
},
1919
end: Position {
20-
line: 264,
21-
character: 76,
20+
line: 271,
21+
character: 50,
2222
},
2323
},
2424
severity: Some(
25-
Error,
25+
Hint,
2626
),
2727
code: None,
2828
code_description: None,
@@ -40,18 +40,18 @@
4040
password: None,
4141
host: None,
4242
port: None,
43-
path: "/test/crates/hir_def/src/data.rs",
43+
path: "/test/crates/hir_def/src/path.rs",
4444
query: None,
4545
fragment: None,
4646
},
4747
range: Range {
4848
start: Position {
49-
line: 79,
50-
character: 15,
49+
line: 264,
50+
character: 8,
5151
},
5252
end: Position {
53-
line: 79,
54-
character: 41,
53+
line: 264,
54+
character: 76,
5555
},
5656
},
5757
},
@@ -86,6 +86,71 @@
8686
character: 41,
8787
},
8888
},
89+
severity: Some(
90+
Hint,
91+
),
92+
code: None,
93+
code_description: None,
94+
source: Some(
95+
"rustc",
96+
),
97+
message: "Please register your known path in the path module",
98+
related_information: Some(
99+
[
100+
DiagnosticRelatedInformation {
101+
location: Location {
102+
uri: Url {
103+
scheme: "file",
104+
username: "",
105+
password: None,
106+
host: None,
107+
port: None,
108+
path: "/test/crates/hir_def/src/path.rs",
109+
query: None,
110+
fragment: None,
111+
},
112+
range: Range {
113+
start: Position {
114+
line: 264,
115+
character: 8,
116+
},
117+
end: Position {
118+
line: 264,
119+
character: 76,
120+
},
121+
},
122+
},
123+
message: "Exact error occurred here",
124+
},
125+
],
126+
),
127+
tags: None,
128+
data: None,
129+
},
130+
fixes: [],
131+
},
132+
MappedRustDiagnostic {
133+
url: Url {
134+
scheme: "file",
135+
username: "",
136+
password: None,
137+
host: None,
138+
port: None,
139+
path: "/test/crates/hir_def/src/path.rs",
140+
query: None,
141+
fragment: None,
142+
},
143+
diagnostic: Diagnostic {
144+
range: Range {
145+
start: Position {
146+
line: 264,
147+
character: 8,
148+
},
149+
end: Position {
150+
line: 264,
151+
character: 76,
152+
},
153+
},
89154
severity: Some(
90155
Error,
91156
),
@@ -95,7 +160,60 @@
95160
"rustc",
96161
),
97162
message: "Please register your known path in the path module",
98-
related_information: None,
163+
related_information: Some(
164+
[
165+
DiagnosticRelatedInformation {
166+
location: Location {
167+
uri: Url {
168+
scheme: "file",
169+
username: "",
170+
password: None,
171+
host: None,
172+
port: None,
173+
path: "/test/crates/hir_def/src/path.rs",
174+
query: None,
175+
fragment: None,
176+
},
177+
range: Range {
178+
start: Position {
179+
line: 271,
180+
character: 8,
181+
},
182+
end: Position {
183+
line: 271,
184+
character: 50,
185+
},
186+
},
187+
},
188+
message: "Error originated from macro call here",
189+
},
190+
DiagnosticRelatedInformation {
191+
location: Location {
192+
uri: Url {
193+
scheme: "file",
194+
username: "",
195+
password: None,
196+
host: None,
197+
port: None,
198+
path: "/test/crates/hir_def/src/data.rs",
199+
query: None,
200+
fragment: None,
201+
},
202+
range: Range {
203+
start: Position {
204+
line: 79,
205+
character: 15,
206+
},
207+
end: Position {
208+
line: 79,
209+
character: 41,
210+
},
211+
},
212+
},
213+
message: "Error originated from macro call here",
214+
},
215+
],
216+
),
99217
tags: None,
100218
data: None,
101219
},

‎crates/rust-analyzer/src/diagnostics/to_proto.rs‎

Lines changed: 64 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,14 @@ fn diagnostic_severity(
3434
Some(res)
3535
}
3636

37-
/// Check whether a file name is from macro invocation
38-
fn is_from_macro(file_name: &str) -> bool {
37+
/// Checks whether a file name is from macro invocation and does not refer to an actual file.
38+
fn is_dummy_macro_file(file_name: &str) -> bool {
39+
// FIXME: current rustc does not seem to emit `<macro file>` files anymore?
3940
file_name.starts_with('<') && file_name.ends_with('>')
4041
}
4142

42-
/// Converts a Rust span to a LSP location, resolving macro expansion site if neccesary
43-
fn location(workspace_root: &Path, span: &DiagnosticSpan) -> lsp_types::Location {
44-
let mut span = span.clone();
45-
while let Some(expansion) = span.expansion {
46-
span = expansion.span;
47-
}
48-
return location_naive(workspace_root, &span);
49-
}
50-
5143
/// Converts a Rust span to a LSP location
52-
fn location_naive(workspace_root: &Path, span: &DiagnosticSpan) -> lsp_types::Location {
44+
fn location(workspace_root: &Path, span: &DiagnosticSpan) -> lsp_types::Location {
5345
let file_name = workspace_root.join(&span.file_name);
5446
let uri = url_from_abs_path(&file_name);
5547

@@ -62,7 +54,25 @@ fn location_naive(workspace_root: &Path, span: &DiagnosticSpan) -> lsp_types::Lo
6254
lsp_types::Location { uri, range }
6355
}
6456

65-
/// Converts a secondary Rust span to a LSP related inflocation(ormation
57+
/// Extracts a suitable "primary" location from a rustc diagnostic.
58+
///
59+
/// This takes locations pointing into the standard library, or generally outside the current
60+
/// workspace into account and tries to avoid those, in case macros are involved.
61+
fn primary_location(workspace_root: &Path, span: &DiagnosticSpan) -> lsp_types::Location {
62+
let span_stack = std::iter::successors(Some(span), |span| Some(&span.expansion.as_ref()?.span));
63+
for span in span_stack.clone() {
64+
let abs_path = workspace_root.join(&span.file_name);
65+
if !is_dummy_macro_file(&span.file_name) && abs_path.starts_with(workspace_root) {
66+
return location(workspace_root, span);
67+
}
68+
}
69+
70+
// Fall back to the outermost macro invocation if no suitable span comes up.
71+
let last_span = span_stack.last().unwrap();
72+
location(workspace_root, last_span)
73+
}
74+
75+
/// Converts a secondary Rust span to a LSP related information
6676
///
6777
/// If the span is unlabelled this will return `None`.
6878
fn diagnostic_related_information(
@@ -231,7 +241,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
231241
primary_spans
232242
.iter()
233243
.flat_map(|primary_span| {
234-
let location = location(workspace_root, &primary_span);
244+
let primary_location = primary_location(workspace_root, &primary_span);
235245

236246
let mut message = message.clone();
237247
if needs_primary_span_label {
@@ -243,31 +253,47 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
243253
// Each primary diagnostic span may result in multiple LSP diagnostics.
244254
let mut diagnostics = Vec::new();
245255

246-
let mut related_macro_info = None;
256+
let mut related_info_macro_calls = vec![];
247257

248258
// If error occurs from macro expansion, add related info pointing to
249259
// where the error originated
250260
// Also, we would generate an additional diagnostic, so that exact place of macro
251261
// will be highlighted in the error origin place.
252-
if !is_from_macro(&primary_span.file_name) && primary_span.expansion.is_some() {
253-
let in_macro_location = location_naive(workspace_root, &primary_span);
262+
let span_stack = std::iter::successors(Some(*primary_span), |span| {
263+
Some(&span.expansion.as_ref()?.span)
264+
});
265+
for (i, span) in span_stack.enumerate() {
266+
if is_dummy_macro_file(&span.file_name) {
267+
continue;
268+
}
254269

255-
// Add related information for the main disagnostic.
256-
related_macro_info = Some(lsp_types::DiagnosticRelatedInformation {
257-
location: in_macro_location.clone(),
258-
message: "Error originated from macro here".to_string(),
259-
});
270+
// First span is the original diagnostic, others are macro call locations that
271+
// generated that code.
272+
let is_in_macro_call = i != 0;
260273

274+
let secondary_location = location(workspace_root, &span);
275+
if secondary_location == primary_location {
276+
continue;
277+
}
278+
related_info_macro_calls.push(lsp_types::DiagnosticRelatedInformation {
279+
location: secondary_location.clone(),
280+
message: if is_in_macro_call {
281+
"Error originated from macro call here".to_string()
282+
} else {
283+
"Actual error occurred here".to_string()
284+
},
285+
});
261286
// For the additional in-macro diagnostic we add the inverse message pointing to the error location in code.
262287
let information_for_additional_diagnostic =
263288
vec![lsp_types::DiagnosticRelatedInformation {
264-
location: location.clone(),
289+
location: primary_location.clone(),
265290
message: "Exact error occurred here".to_string(),
266291
}];
267292

268293
let diagnostic = lsp_types::Diagnostic {
269-
range: in_macro_location.range,
270-
severity,
294+
range: secondary_location.range,
295+
// downgrade to hint if we're pointing at the macro
296+
severity: Some(lsp_types::DiagnosticSeverity::Hint),
271297
code: code.clone().map(lsp_types::NumberOrString::String),
272298
code_description: code_description.clone(),
273299
source: Some(source.clone()),
@@ -276,33 +302,34 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
276302
tags: if tags.is_empty() { None } else { Some(tags.clone()) },
277303
data: None,
278304
};
279-
280305
diagnostics.push(MappedRustDiagnostic {
281-
url: in_macro_location.uri,
306+
url: secondary_location.uri,
282307
diagnostic,
283308
fixes: Vec::new(),
284309
});
285310
}
286311

287312
// Emit the primary diagnostic.
288313
diagnostics.push(MappedRustDiagnostic {
289-
url: location.uri.clone(),
314+
url: primary_location.uri.clone(),
290315
diagnostic: lsp_types::Diagnostic {
291-
range: location.range,
316+
range: primary_location.range,
292317
severity,
293318
code: code.clone().map(lsp_types::NumberOrString::String),
294319
code_description: code_description.clone(),
295320
source: Some(source.clone()),
296321
message,
297-
related_information: if subdiagnostics.is_empty() {
298-
None
299-
} else {
300-
let mut related = subdiagnostics
322+
related_information: {
323+
let info = related_info_macro_calls
301324
.iter()
302-
.map(|sub| sub.related.clone())
325+
.cloned()
326+
.chain(subdiagnostics.iter().map(|sub| sub.related.clone()))
303327
.collect::<Vec<_>>();
304-
related.extend(related_macro_info);
305-
Some(related)
328+
if info.is_empty() {
329+
None
330+
} else {
331+
Some(info)
332+
}
306333
},
307334
tags: if tags.is_empty() { None } else { Some(tags.clone()) },
308335
data: None,
@@ -314,7 +341,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
314341
// This is useful because they will show up in the user's editor, unlike
315342
// `related_information`, which just produces hard-to-read links, at least in VS Code.
316343
let back_ref = lsp_types::DiagnosticRelatedInformation {
317-
location,
344+
location: primary_location,
318345
message: "original diagnostic".to_string(),
319346
};
320347
for sub in &subdiagnostics {

0 commit comments

Comments
 (0)