mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge #10902

Browse Source 
10902: Handle multiple cargo check quick fix spans r=Veykril a=brandondong

Resolves https://github.com/rust-analyzer/rust-analyzer/issues/10705.

**Cause:**
- For a cargo check diagnostic with multiple spans, only a single quick fix action would be created at the location of `spans[0]`. Additionally, the hover window details would only show the location of `spans[0]` next to the message.

**Fix:**
- Allow cargo check quick fix actions to be triggerable from multiple selection ranges. Specifically, if the selection intersects with any of the replacement spans, the quick fix action is shown.
- No change in behavior for the hover window details. It's pretty minor and I think showing multiple locations next to the message may be more confusing anyways.

Co-authored-by: Brandon <brandondong604@hotmail.com>
This commit is contained in:
bors[bot] 2021-12-05 10:52:54 +00:00 committed by GitHub
commit 8a084e6aca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 384 additions and 295 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
crates/rust-analyzer/src/diagnostics.rs
View File

@ -29,7 +29,8 @@ pub(crate) struct DiagnosticCollection {
#[derive(Debug, Clone)]
pub(crate) struct Fix {
pub(crate) range: lsp_types::Range,
// Fixes may be triggerable from multiple ranges.
pub(crate) ranges: Vec<lsp_types::Range>,
pub(crate) action: lsp_ext::CodeAction,
}
@ -43,7 +44,7 @@ pub(crate) fn add_check_diagnostic(
&mut self,
file_id: FileId,
diagnostic: lsp_types::Diagnostic,
fixes: Vec<lsp_ext::CodeAction>,
fix: Option<Fix>,
) {
let diagnostics = self.check.entry(file_id).or_default();
for existing_diagnostic in diagnostics.iter() {
@ -53,10 +54,7 @@ pub(crate) fn add_check_diagnostic(
}
let check_fixes = Arc::make_mut(&mut self.check_fixes);
check_fixes
.entry(file_id)
.or_default()
.extend(fixes.into_iter().map(|action| Fix { range: diagnostic.range, action }));
check_fixes.entry(file_id).or_default().extend(fix);
diagnostics.push(diagnostic);
self.changes.insert(file_id);
}

114
crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt
View File

@ -114,7 +114,7 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
MappedRustDiagnostic {
url: Url {
@ -205,7 +205,7 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
MappedRustDiagnostic {
url: Url {
@ -296,55 +296,69 @@
tags: None,
data: None,
},
fixes: [
CodeAction {
title: "consider passing by value instead: `self`",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "/test/compiler/mir/tagset.rs",
query: None,
fragment: None,
}: [
TextEdit {
range: Range {
start: Position {
line: 41,
character: 23,
},
end: Position {
line: 41,
character: 28,
},
},
new_text: "self",
},
],
},
),
document_changes: None,
change_annotations: None,
fix: Some(
Fix {
ranges: [
Range {
start: Position {
line: 41,
character: 23,
},
end: Position {
line: 41,
character: 28,
},
},
),
is_preferred: Some(
true,
),
data: None,
],
action: CodeAction {
title: "consider passing by value instead: `self`",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "/test/compiler/mir/tagset.rs",
query: None,
fragment: None,
}: [
TextEdit {
range: Range {
start: Position {
line: 41,
character: 23,
},
end: Position {
line: 41,
character: 28,
},
},
new_text: "self",
},
],
},
),
document_changes: None,
change_annotations: None,
},
),
is_preferred: Some(
true,
),
data: None,
},
},
],
),
},
]

2
crates/rust-analyzer/src/diagnostics/test_data/handles_macro_location.txt
View File

@ -59,6 +59,6 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
]

6
crates/rust-analyzer/src/diagnostics/test_data/macro_compiler_error.txt
View File

@ -64,7 +64,7 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
MappedRustDiagnostic {
url: Url {
@ -131,7 +131,7 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
MappedRustDiagnostic {
url: Url {
@ -224,6 +224,6 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
]

2
crates/rust-analyzer/src/diagnostics/test_data/rustc_incompatible_type_for_trait.txt
View File

@ -59,6 +59,6 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
]

2
crates/rust-analyzer/src/diagnostics/test_data/rustc_mismatched_type.txt
View File

@ -59,6 +59,6 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
]

112
crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable.txt
View File

@ -72,7 +72,7 @@
),
data: None,
},
fixes: [],
fix: None,
},
MappedRustDiagnostic {
url: Url {
@ -143,55 +143,69 @@
tags: None,
data: None,
},
fixes: [
CodeAction {
title: "consider prefixing with an underscore: `_foo`",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
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,
change_annotations: None,
fix: Some(
Fix {
ranges: [
Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
),
is_preferred: Some(
true,
),
data: None,
],
action: CodeAction {
title: "consider prefixing with an underscore: `_foo`",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
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,
change_annotations: None,
},
),
is_preferred: Some(
true,
),
data: None,
},
},
],
),
},
]

112
crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_hint.txt
View File

@ -72,7 +72,7 @@
),
data: None,
},
fixes: [],
fix: None,
},
MappedRustDiagnostic {
url: Url {
@ -143,55 +143,69 @@
tags: None,
data: None,
},
fixes: [
CodeAction {
title: "consider prefixing with an underscore: `_foo`",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
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,
change_annotations: None,
fix: Some(
Fix {
ranges: [
Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
),
is_preferred: Some(
true,
),
data: None,
],
action: CodeAction {
title: "consider prefixing with an underscore: `_foo`",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
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,
change_annotations: None,
},
),
is_preferred: Some(
true,
),
data: None,
},
},
],
),
},
]

112
crates/rust-analyzer/src/diagnostics/test_data/rustc_unused_variable_as_info.txt
View File

@ -72,7 +72,7 @@
),
data: None,
},
fixes: [],
fix: None,
},
MappedRustDiagnostic {
url: Url {
@ -143,55 +143,69 @@
tags: None,
data: None,
},
fixes: [
CodeAction {
title: "consider prefixing with an underscore: `_foo`",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
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,
change_annotations: None,
fix: Some(
Fix {
ranges: [
Range {
start: Position {
line: 290,
character: 8,
},
end: Position {
line: 290,
character: 11,
},
},
),
is_preferred: Some(
true,
),
data: None,
],
action: CodeAction {
title: "consider prefixing with an underscore: `_foo`",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
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,
change_annotations: None,
},
),
is_preferred: Some(
true,
),
data: None,
},
},
],
),
},
]

4
crates/rust-analyzer/src/diagnostics/test_data/rustc_wrong_number_of_parameters.txt
View File

@ -88,7 +88,7 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
MappedRustDiagnostic {
url: Url {
@ -179,6 +179,6 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
]

150
crates/rust-analyzer/src/diagnostics/test_data/snap_multi_line_fix.txt
View File

@ -114,7 +114,7 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
MappedRustDiagnostic {
url: Url {
@ -205,7 +205,7 @@
tags: None,
data: None,
},
fixes: [],
fix: None,
},
MappedRustDiagnostic {
url: Url {
@ -296,68 +296,92 @@
tags: None,
data: None,
},
fixes: [
CodeAction {
title: "return the expression directly: `(0..10).collect()`",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
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,
change_annotations: None,
fix: Some(
Fix {
ranges: [
Range {
start: Position {
line: 2,
character: 4,
},
end: Position {
line: 2,
character: 30,
},
},
),
is_preferred: Some(
true,
),
data: None,
Range {
start: Position {
line: 3,
character: 4,
},
end: Position {
line: 3,
character: 5,
},
},
],
action: CodeAction {
title: "return the expression directly: `(0..10).collect()`",
group: None,
kind: Some(
CodeActionKind(
"quickfix",
),
),
edit: Some(
SnippetWorkspaceEdit {
changes: Some(
{
Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
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,
change_annotations: None,
},
),
is_preferred: Some(
true,
),
data: None,
},
},
],
),
},
]

42
crates/rust-analyzer/src/diagnostics/to_proto.rs
View File

@ -9,7 +9,7 @@
use crate::{lsp_ext, to_proto::url_from_abs_path};
use super::DiagnosticsMapConfig;
use super::{DiagnosticsMapConfig, Fix};
/// Determines the LSP severity from a diagnostic
fn diagnostic_severity(
@ -124,7 +124,7 @@ fn resolve_path(
struct SubDiagnostic {
related: lsp_types::DiagnosticRelatedInformation,
suggested_fix: Option<lsp_ext::CodeAction>,
suggested_fix: Option<Fix>,
}
enum MappedRustChildDiagnostic {
@ -181,18 +181,24 @@ fn map_rust_child_diagnostic(
location: location(config, workspace_root, spans[0]),
message: message.clone(),
},
suggested_fix: Some(lsp_ext::CodeAction {
title: message,
group: None,
kind: Some(lsp_types::CodeActionKind::QUICKFIX),
edit: Some(lsp_ext::SnippetWorkspaceEdit {
// FIXME: there's no good reason to use edit_map here....
changes: Some(edit_map),
document_changes: None,
change_annotations: None,
}),
is_preferred: Some(true),
data: None,
suggested_fix: Some(Fix {
ranges: spans
.iter()
.map(|&span| location(config, workspace_root, span).range)
.collect(),
action: lsp_ext::CodeAction {
title: message,
group: None,
kind: Some(lsp_types::CodeActionKind::QUICKFIX),
edit: Some(lsp_ext::SnippetWorkspaceEdit {
// FIXME: there's no good reason to use edit_map here....
changes: Some(edit_map),
document_changes: None,
change_annotations: None,
}),
is_preferred: Some(true),
data: None,
},
}),
})
}
@ -202,7 +208,7 @@ fn map_rust_child_diagnostic(
pub(crate) struct MappedRustDiagnostic {
pub(crate) url: lsp_types::Url,
pub(crate) diagnostic: lsp_types::Diagnostic,
pub(crate) fixes: Vec<lsp_ext::CodeAction>,
pub(crate) fix: Option<Fix>,
}
/// Converts a Rust root diagnostic to LSP form
@ -359,7 +365,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
diagnostics.push(MappedRustDiagnostic {
url: secondary_location.uri,
diagnostic,
fixes: Vec::new(),
fix: None,
});
}
@ -388,7 +394,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
tags: if tags.is_empty() { None } else { Some(tags.clone()) },
data: None,
},
fixes: Vec::new(),
fix: None,
});
// Emit hint-level diagnostics for all `related_information` entries such as "help"s.
@ -405,7 +411,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp(
}
diagnostics.push(MappedRustDiagnostic {
url: sub.related.location.uri.clone(),
fixes: sub.suggested_fix.iter().cloned().collect(),
fix: sub.suggested_fix.clone(),
diagnostic: lsp_types::Diagnostic {
range: sub.related.location.range,
severity: Some(lsp_types::DiagnosticSeverity::HINT),

9
crates/rust-analyzer/src/handlers.rs
View File

@ -1081,8 +1081,13 @@ pub(crate) fn handle_code_action(
for fix in snap.check_fixes.get(&frange.file_id).into_iter().flatten() {
// FIXME: this mapping is awkward and shouldn't exist. Refactor
// `snap.check_fixes` to not convert to LSP prematurely.
let fix_range = from_proto::text_range(&line_index, fix.range);
if fix_range.intersect(frange.range).is_some() {
let intersect_fix_range = fix
.ranges
.iter()
.copied()
.map(|range| from_proto::text_range(&line_index, range))
.any(|fix_range| fix_range.intersect(frange.range).is_some());
if intersect_fix_range {
res.push(fix.action.clone());
}
}

2
crates/rust-analyzer/src/main_loop.rs
View File

@ -370,7 +370,7 @@ fn handle_event(&mut self, event: Event) -> Result<()> {
Ok(file_id) => self.diagnostics.add_check_diagnostic(
file_id,
diag.diagnostic,
diag.fixes,
diag.fix,
),
Err(err) => {
tracing::error!(