6734: Emit additional diagnostics for hints/help/etc r=lnicola a=jonas-schievink

This makes rust-analyzer diagnostics match native rustc diagnostics in the terminal more closely. Unfortunately all of this is a bodge, since we already provide this information to the client in the form of `DiagnosticRelatedInformation`, but at least VS Code chooses such a poor UI for these that they don't help much, as evidenced [here](https://twitter.com/yaahc_/status/1335297260444250112) and in https://github.com/rust-lang/rust/issues/79741.

This PR papers over these client UI problems by taking the `DiagnosticRelatedInformation` and turning each one into its own hint-level diagnostic, which makes it show up in the source code. Quick fixes are attached to all resulting diagnostics, which makes them more discoverable.

### Example: "Consider removing this semicolon"

![screenshot-2020-12-06-01:27:29](https://user-images.githubusercontent.com/1786438/101268366-46423980-3762-11eb-9a69-1ff0b1806c2f.png)
![screenshot-2020-12-06-01:27:39](https://user-images.githubusercontent.com/1786438/101268367-46dad000-3762-11eb-81fa-afd234d44f17.png)
![screenshot-2020-12-06-01:27:46](https://user-images.githubusercontent.com/1786438/101268368-46dad000-3762-11eb-9205-4b9bd9f4406d.png)

### Example: "Value used after move"

![screenshot-2020-12-06-01:33:00](https://user-images.githubusercontent.com/1786438/101268447-22332800-3763-11eb-85ce-8c742927a2c8.png)
![screenshot-2020-12-06-01:33:07](https://user-images.githubusercontent.com/1786438/101268448-22cbbe80-3763-11eb-8f16-0590895d8bc6.png)


Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
This commit is contained in:
bors[bot] 2020-12-06 13:26:54 +00:00 committed by GitHub
commit 1403ddf029
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 1112 additions and 138 deletions

View file

@ -104,4 +104,168 @@
},
fixes: [],
},
MappedRustDiagnostic {
url: Url {
scheme: "file",
host: None,
port: None,
path: "/test/compiler/lib.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 0,
character: 8,
},
end: Position {
line: 0,
character: 19,
},
},
severity: Some(
Hint,
),
code: Some(
String(
"trivially_copy_pass_by_ref",
),
),
code_description: Some(
CodeDescription {
href: Url {
scheme: "https",
host: Some(
Domain(
"rust-lang.github.io",
),
),
port: None,
path: "/rust-clippy/master/index.html",
query: None,
fragment: Some(
"trivially_copy_pass_by_ref",
),
},
},
),
source: Some(
"clippy",
),
message: "lint level defined here",
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/compiler/mir/tagset.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 41,
character: 23,
},
end: Position {
line: 41,
character: 28,
},
},
},
message: "original diagnostic",
},
],
),
tags: None,
data: None,
},
fixes: [],
},
MappedRustDiagnostic {
url: Url {
scheme: "file",
host: None,
port: None,
path: "/test/compiler/mir/tagset.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 41,
character: 23,
},
end: Position {
line: 41,
character: 28,
},
},
severity: Some(
Hint,
),
code: Some(
String(
"trivially_copy_pass_by_ref",
),
),
code_description: Some(
CodeDescription {
href: Url {
scheme: "https",
host: Some(
Domain(
"rust-lang.github.io",
),
),
port: None,
path: "/rust-clippy/master/index.html",
query: None,
fragment: Some(
"trivially_copy_pass_by_ref",
),
},
},
),
source: Some(
"clippy",
),
message: "consider passing by value instead",
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/compiler/mir/tagset.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 41,
character: 23,
},
end: Position {
line: 41,
character: 28,
},
},
},
message: "original diagnostic",
},
],
),
tags: None,
data: None,
},
fixes: [],
},
]

View file

@ -4,19 +4,19 @@
scheme: "file",
host: None,
port: None,
path: "/test/crates/hir_def/src/data.rs",
path: "/test/crates/hir_def/src/path.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 79,
character: 15,
line: 264,
character: 8,
},
end: Position {
line: 79,
character: 41,
line: 264,
character: 76,
},
},
severity: Some(
@ -36,22 +36,22 @@
scheme: "file",
host: None,
port: None,
path: "/test/crates/hir_def/src/path.rs",
path: "/test/crates/hir_def/src/data.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 264,
character: 8,
line: 79,
character: 15,
},
end: Position {
line: 264,
character: 76,
line: 79,
character: 41,
},
},
},
message: "Error originated from macro here",
message: "Exact error occurred here",
},
],
),
@ -65,19 +65,19 @@
scheme: "file",
host: None,
port: None,
path: "/test/crates/hir_def/src/path.rs",
path: "/test/crates/hir_def/src/data.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 264,
character: 8,
line: 79,
character: 15,
},
end: Position {
line: 264,
character: 76,
line: 79,
character: 41,
},
},
severity: Some(
@ -89,33 +89,7 @@
"rustc",
),
message: "Please register your known path in the path module",
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/crates/hir_def/src/data.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 79,
character: 15,
},
end: Position {
line: 79,
character: 41,
},
},
},
message: "Exact error occured here",
},
],
),
related_information: None,
tags: None,
data: None,
},

View file

@ -32,7 +32,33 @@
"rustc",
),
message: "unused variable: `foo`\n#[warn(unused_variables)] on by default",
related_information: None,
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
},
message: "consider prefixing with an underscore",
},
],
),
tags: Some(
[
Unnecessary,
@ -87,4 +113,114 @@
},
],
},
MappedRustDiagnostic {
url: Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
severity: Some(
Hint,
),
code: Some(
String(
"unused_variables",
),
),
code_description: None,
source: Some(
"rustc",
),
message: "consider prefixing with an underscore",
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
},
message: "original diagnostic",
},
],
),
tags: None,
data: None,
},
fixes: [
CodeAction {
title: "consider prefixing with an underscore",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
}: [
TextEdit {
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
new_text: "_foo",
},
],
},
),
document_changes: None,
},
),
is_preferred: Some(
true,
),
data: None,
},
],
},
]

View file

@ -32,7 +32,33 @@
"rustc",
),
message: "unused variable: `foo`\n#[warn(unused_variables)] on by default",
related_information: None,
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
},
message: "consider prefixing with an underscore",
},
],
),
tags: Some(
[
Unnecessary,
@ -87,4 +113,114 @@
},
],
},
MappedRustDiagnostic {
url: Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
severity: Some(
Hint,
),
code: Some(
String(
"unused_variables",
),
),
code_description: None,
source: Some(
"rustc",
),
message: "consider prefixing with an underscore",
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
},
message: "original diagnostic",
},
],
),
tags: None,
data: None,
},
fixes: [
CodeAction {
title: "consider prefixing with an underscore",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
}: [
TextEdit {
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
new_text: "_foo",
},
],
},
),
document_changes: None,
},
),
is_preferred: Some(
true,
),
data: None,
},
],
},
]

View file

@ -32,7 +32,33 @@
"rustc",
),
message: "unused variable: `foo`\n#[warn(unused_variables)] on by default",
related_information: None,
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
},
message: "consider prefixing with an underscore",
},
],
),
tags: Some(
[
Unnecessary,
@ -87,4 +113,114 @@
},
],
},
MappedRustDiagnostic {
url: Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
severity: Some(
Hint,
),
code: Some(
String(
"unused_variables",
),
),
code_description: None,
source: Some(
"rustc",
),
message: "consider prefixing with an underscore",
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
},
message: "original diagnostic",
},
],
),
tags: None,
data: None,
},
fixes: [
CodeAction {
title: "consider prefixing with an underscore",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
host: None,
port: None,
path: "/test/driver/subcommand/repl.rs",
query: None,
fragment: None,
}: [
TextEdit {
range: Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
new_text: "_foo",
},
],
},
),
document_changes: None,
},
),
is_preferred: Some(
true,
),
data: None,
},
],
},
]

View file

@ -81,4 +81,86 @@
},
fixes: [],
},
MappedRustDiagnostic {
url: Url {
scheme: "file",
host: None,
port: None,
path: "/test/compiler/ty/select.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 218,
character: 4,
},
end: Position {
line: 230,
character: 5,
},
},
severity: Some(
Hint,
),
code: Some(
String(
"E0061",
),
),
code_description: Some(
CodeDescription {
href: Url {
scheme: "https",
host: Some(
Domain(
"doc.rust-lang.org",
),
),
port: None,
path: "/error-index.html",
query: None,
fragment: Some(
"E0061",
),
},
},
),
source: Some(
"rustc",
),
message: "defined here",
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/compiler/ty/select.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 103,
character: 17,
},
end: Position {
line: 103,
character: 29,
},
},
},
message: "original diagnostic",
},
],
),
tags: None,
data: None,
},
fixes: [],
},
]

View file

@ -74,6 +74,309 @@
},
message: "unnecessary let binding",
},
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/src/main.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 2,
character: 4,
},
end: Position {
line: 2,
character: 30,
},
},
},
message: "return the expression directly",
},
],
),
tags: None,
data: None,
},
fixes: [
CodeAction {
title: "return the expression directly",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
host: None,
port: None,
path: "/test/src/main.rs",
query: None,
fragment: None,
}: [
TextEdit {
range: Range {
start: Position {
line: 2,
character: 4,
},
end: Position {
line: 2,
character: 30,
},
},
new_text: "",
},
TextEdit {
range: Range {
start: Position {
line: 3,
character: 4,
},
end: Position {
line: 3,
character: 5,
},
},
new_text: "(0..10).collect()",
},
],
},
),
document_changes: None,
},
),
is_preferred: Some(
true,
),
data: None,
},
],
},
MappedRustDiagnostic {
url: Url {
scheme: "file",
host: None,
port: None,
path: "/test/src/main.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 2,
character: 4,
},
end: Position {
line: 2,
character: 30,
},
},
severity: Some(
Hint,
),
code: Some(
String(
"let_and_return",
),
),
code_description: Some(
CodeDescription {
href: Url {
scheme: "https",
host: Some(
Domain(
"rust-lang.github.io",
),
),
port: None,
path: "/rust-clippy/master/index.html",
query: None,
fragment: Some(
"let_and_return",
),
},
},
),
source: Some(
"clippy",
),
message: "unnecessary let binding",
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/src/main.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 3,
character: 4,
},
end: Position {
line: 3,
character: 5,
},
},
},
message: "original diagnostic",
},
],
),
tags: None,
data: None,
},
fixes: [
CodeAction {
title: "return the expression directly",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
host: None,
port: None,
path: "/test/src/main.rs",
query: None,
fragment: None,
}: [
TextEdit {
range: Range {
start: Position {
line: 2,
character: 4,
},
end: Position {
line: 2,
character: 30,
},
},
new_text: "",
},
TextEdit {
range: Range {
start: Position {
line: 3,
character: 4,
},
end: Position {
line: 3,
character: 5,
},
},
new_text: "(0..10).collect()",
},
],
},
),
document_changes: None,
},
),
is_preferred: Some(
true,
),
data: None,
},
],
},
MappedRustDiagnostic {
url: Url {
scheme: "file",
host: None,
port: None,
path: "/test/src/main.rs",
query: None,
fragment: None,
},
diagnostic: Diagnostic {
range: Range {
start: Position {
line: 2,
character: 4,
},
end: Position {
line: 2,
character: 30,
},
},
severity: Some(
Hint,
),
code: Some(
String(
"let_and_return",
),
),
code_description: Some(
CodeDescription {
href: Url {
scheme: "https",
host: Some(
Domain(
"rust-lang.github.io",
),
),
port: None,
path: "/rust-clippy/master/index.html",
query: None,
fragment: Some(
"let_and_return",
),
},
},
),
source: Some(
"clippy",
),
message: "return the expression directly",
related_information: Some(
[
DiagnosticRelatedInformation {
location: Location {
uri: Url {
scheme: "file",
host: None,
port: None,
path: "/test/src/main.rs",
query: None,
fragment: None,
},
range: Range {
start: Position {
line: 3,
character: 4,
},
end: Position {
line: 3,
character: 5,
},
},
},
message: "original diagnostic",
},
],
),
tags: None,

View file

@ -75,8 +75,10 @@ fn diagnostic_related_information(
}
enum MappedRustChildDiagnostic {
Related(lsp_types::DiagnosticRelatedInformation),
SuggestedFix(lsp_ext::CodeAction),
Related {
related: lsp_types::DiagnosticRelatedInformation,
suggested_fix: Option<lsp_ext::CodeAction>,
},
MessageLine(String),
}
@ -103,12 +105,20 @@ fn map_rust_child_diagnostic(
}
if edit_map.is_empty() {
MappedRustChildDiagnostic::Related(lsp_types::DiagnosticRelatedInformation {
MappedRustChildDiagnostic::Related {
related: lsp_types::DiagnosticRelatedInformation {
location: location(workspace_root, spans[0]),
message: rd.message.clone(),
})
},
suggested_fix: None,
}
} else {
MappedRustChildDiagnostic::SuggestedFix(lsp_ext::CodeAction {
MappedRustChildDiagnostic::Related {
related: lsp_types::DiagnosticRelatedInformation {
location: location(workspace_root, spans[0]),
message: rd.message.clone(),
},
suggested_fix: Some(lsp_ext::CodeAction {
title: rd.message.clone(),
group: None,
kind: Some(lsp_types::CodeActionKind::QUICKFIX),
@ -119,7 +129,8 @@ fn map_rust_child_diagnostic(
}),
is_preferred: Some(true),
data: None,
})
}),
}
}
}
@ -179,8 +190,12 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
for child in &rd.children {
let child = map_rust_child_diagnostic(workspace_root, &child);
match child {
MappedRustChildDiagnostic::Related(related) => related_information.push(related),
MappedRustChildDiagnostic::SuggestedFix(code_action) => fixes.push(code_action),
MappedRustChildDiagnostic::Related { related, suggested_fix } => {
related_information.push(related);
if let Some(code_action) = suggested_fix {
fixes.push(code_action);
}
}
MappedRustChildDiagnostic::MessageLine(message_line) => {
format_to!(message, "\n{}", message_line);
@ -219,7 +234,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
primary_spans
.iter()
.map(|primary_span| {
.flat_map(|primary_span| {
let location = location(workspace_root, &primary_span);
let mut message = message.clone();
@ -229,16 +244,20 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
}
}
// Each primary diagnostic span may result in multiple LSP diagnostics.
let mut diagnostics = Vec::new();
let mut related_macro_info = None;
// If error occurs from macro expansion, add related info pointing to
// where the error originated
// Also, we would generate an additional diagnostic, so that exact place of macro
// will be highlighted in the error origin place.
let additional_diagnostic =
if !is_from_macro(&primary_span.file_name) && primary_span.expansion.is_some() {
let in_macro_location = location_naive(workspace_root, &primary_span);
// Add related information for the main disagnostic.
related_information.push(lsp_types::DiagnosticRelatedInformation {
related_macro_info = Some(lsp_types::DiagnosticRelatedInformation {
location: in_macro_location.clone(),
message: "Error originated from macro here".to_string(),
});
@ -247,7 +266,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
let information_for_additional_diagnostic =
vec![lsp_types::DiagnosticRelatedInformation {
location: location.clone(),
message: "Exact error occured here".to_string(),
message: "Exact error occurred here".to_string(),
}];
let diagnostic = lsp_types::Diagnostic {
@ -262,16 +281,17 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
data: None,
};
Some(MappedRustDiagnostic {
diagnostics.push(MappedRustDiagnostic {
url: in_macro_location.uri,
diagnostic,
fixes: fixes.clone(),
})
} else {
None
};
});
}
let diagnostic = lsp_types::Diagnostic {
// Emit the primary diagnostic.
diagnostics.push(MappedRustDiagnostic {
url: location.uri.clone(),
diagnostic: lsp_types::Diagnostic {
range: location.range,
severity,
code: code.clone().map(lsp_types::NumberOrString::String),
@ -281,20 +301,43 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
related_information: if related_information.is_empty() {
None
} else {
Some(related_information.clone())
let mut related = related_information.clone();
related.extend(related_macro_info);
Some(related)
},
tags: if tags.is_empty() { None } else { Some(tags.clone()) },
data: None,
};
},
fixes: fixes.clone(),
});
let main_diagnostic =
MappedRustDiagnostic { url: location.uri, diagnostic, fixes: fixes.clone() };
match additional_diagnostic {
None => vec![main_diagnostic],
Some(additional_diagnostic) => vec![main_diagnostic, additional_diagnostic],
// Emit hint-level diagnostics for all `related_information` entries such as "help"s.
// This is useful because they will show up in the user's editor, unlike
// `related_information`, which just produces hard-to-read links, at least in VS Code.
let back_ref = lsp_types::DiagnosticRelatedInformation {
location,
message: "original diagnostic".to_string(),
};
for info in &related_information {
diagnostics.push(MappedRustDiagnostic {
url: info.location.uri.clone(),
fixes: fixes.clone(), // share fixes to make them easier to apply
diagnostic: lsp_types::Diagnostic {
range: info.location.range,
severity: Some(lsp_types::DiagnosticSeverity::Hint),
code: code.clone().map(lsp_types::NumberOrString::String),
code_description: code_description.clone(),
source: Some(source.clone()),
message: info.message.clone(),
related_information: Some(vec![back_ref.clone()]),
tags: None, // don't apply modifiers again
data: None,
},
});
}
diagnostics
})
.flatten()
.collect()
}