Skip to content

Commit 637c795

Browse files
kiljackenkiljacken
committed
Correctly handle multi-line fixes from cargo/clippy
1 parent 05b4fc6 commit 637c795

File tree

5 files changed

+267
-30
lines changed

5 files changed

+267
-30
lines changed

‎crates/ra_cargo_watch/src/conv.rs

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use lsp_types::{
88
Location, NumberOrString, Position, Range, TextEdit, Url, WorkspaceEdit,
99
};
1010
use std::{
11+
collections::HashMap,
1112
fmt::Write,
1213
path::{Component, Path, PathBuf, Prefix},
1314
str::FromStr,
@@ -126,44 +127,34 @@ fn map_rust_child_diagnostic(
126127
rd: &RustDiagnostic,
127128
workspace_root: &PathBuf,
128129
) -> MappedRustChildDiagnostic {
129-
let span: &DiagnosticSpan = match rd.spans.iter().find(|s| s.is_primary) {
130-
Some(span) => span,
131-
None => {
132-
// `rustc` uses these spanless children as a way to print multi-line
133-
// messages
134-
return MappedRustChildDiagnostic::MessageLine(rd.message.clone());
135-
}
136-
};
137-
138-
// If we have a primary span use its location, otherwise use the parent
139-
let location = map_span_to_location(&span, workspace_root);
140-
141-
if let Some(suggested_replacement) = &span.suggested_replacement {
142-
// Include our replacement in the title unless it's empty
143-
let title = if !suggested_replacement.is_empty() {
144-
format!("{}: '{}'", rd.message, suggested_replacement)
145-
} else {
146-
rd.message.clone()
147-
};
130+
let spans: Vec<&DiagnosticSpan> = rd.spans.iter().filter(|s| s.is_primary).collect();
131+
if spans.is_empty() {
132+
// `rustc` uses these spanless children as a way to print multi-line
133+
// messages
134+
return MappedRustChildDiagnostic::MessageLine(rd.message.clone());
135+
}
148136

149-
let edit = {
150-
let edits = vec![TextEdit::new(location.range, suggested_replacement.clone())];
151-
let mut edit_map = std::collections::HashMap::new();
152-
edit_map.insert(location.uri, edits);
153-
WorkspaceEdit::new(edit_map)
154-
};
137+
let mut edit_map: HashMap<Url, Vec<TextEdit>> = HashMap::new();
138+
for &span in &spans {
139+
if let Some(suggested_replacement) = &span.suggested_replacement {
140+
let location = map_span_to_location(span, workspace_root);
141+
let edit = TextEdit::new(location.range, suggested_replacement.clone());
142+
edit_map.entry(location.uri).or_default().push(edit);
143+
}
144+
}
155145

146+
if !edit_map.is_empty() {
156147
MappedRustChildDiagnostic::SuggestedFix(CodeAction {
157-
title,
148+
title: rd.message.clone(),
158149
kind: Some("quickfix".to_string()),
159150
diagnostics: None,
160-
edit: Some(edit),
151+
edit: Some(WorkspaceEdit::new(edit_map)),
161152
command: None,
162153
is_preferred: None,
163154
})
164155
} else {
165156
MappedRustChildDiagnostic::Related(DiagnosticRelatedInformation {
166-
location,
157+
location: map_span_to_location(spans[0], workspace_root),
167158
message: rd.message.clone(),
168159
})
169160
}

‎crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ MappedRustDiagnostic {
6363
},
6464
fixes: [
6565
CodeAction {
66-
title: "consider passing by value instead: \'self\'",
66+
title: "consider passing by value instead",
6767
kind: Some(
6868
"quickfix",
6969
),

‎crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_multi_line_fix.snap

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
---
2+
source: crates/ra_cargo_watch/src/conv/test.rs
3+
expression: diag
4+
---
5+
MappedRustDiagnostic {
6+
location: Location {
7+
uri: "file:///test/src/main.rs",
8+
range: Range {
9+
start: Position {
10+
line: 3,
11+
character: 4,
12+
},
13+
end: Position {
14+
line: 3,
15+
character: 5,
16+
},
17+
},
18+
},
19+
diagnostic: Diagnostic {
20+
range: Range {
21+
start: Position {
22+
line: 3,
23+
character: 4,
24+
},
25+
end: Position {
26+
line: 3,
27+
character: 5,
28+
},
29+
},
30+
severity: Some(
31+
Warning,
32+
),
33+
code: Some(
34+
String(
35+
"let_and_return",
36+
),
37+
),
38+
source: Some(
39+
"clippy",
40+
),
41+
message: "returning the result of a let binding from a block\n`#[warn(clippy::let_and_return)]` on by default\nfor further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return",
42+
related_information: Some(
43+
[
44+
DiagnosticRelatedInformation {
45+
location: Location {
46+
uri: "file:///test/src/main.rs",
47+
range: Range {
48+
start: Position {
49+
line: 2,
50+
character: 4,
51+
},
52+
end: Position {
53+
line: 2,
54+
character: 30,
55+
},
56+
},
57+
},
58+
message: "unnecessary let binding",
59+
},
60+
],
61+
),
62+
tags: None,
63+
},
64+
fixes: [
65+
CodeAction {
66+
title: "return the expression directly",
67+
kind: Some(
68+
"quickfix",
69+
),
70+
diagnostics: None,
71+
edit: Some(
72+
WorkspaceEdit {
73+
changes: Some(
74+
{
75+
"file:///test/src/main.rs": [
76+
TextEdit {
77+
range: Range {
78+
start: Position {
79+
line: 2,
80+
character: 4,
81+
},
82+
end: Position {
83+
line: 2,
84+
character: 30,
85+
},
86+
},
87+
new_text: "",
88+
},
89+
TextEdit {
90+
range: Range {
91+
start: Position {
92+
line: 3,
93+
character: 4,
94+
},
95+
end: Position {
96+
line: 3,
97+
character: 5,
98+
},
99+
},
100+
new_text: "(0..10).collect()",
101+
},
102+
],
103+
},
104+
),
105+
document_changes: None,
106+
},
107+
),
108+
command: None,
109+
is_preferred: None,
110+
},
111+
],
112+
}

‎crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_unused_variable.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ MappedRustDiagnostic {
4848
},
4949
fixes: [
5050
CodeAction {
51-
title: "consider prefixing with an underscore: \'_foo\'",
51+
title: "consider prefixing with an underscore",
5252
kind: Some(
5353
"quickfix",
5454
),

‎crates/ra_cargo_watch/src/conv/test.rs

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -936,3 +936,137 @@ fn snap_macro_compiler_error() {
936936
let diag = map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic");
937937
insta::assert_debug_snapshot!(diag);
938938
}
939+
940+
#[test]
941+
#[cfg(not(windows))]
942+
fn snap_multi_line_fix() {
943+
let diag = parse_diagnostic(
944+
r##"{
945+
"rendered": "warning: returning the result of a let binding from a block\n --> src/main.rs:4:5\n |\n3 | let a = (0..10).collect();\n | -------------------------- unnecessary let binding\n4 | a\n | ^\n |\n = note: `#[warn(clippy::let_and_return)]` on by default\n = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return\nhelp: return the expression directly\n |\n3 | \n4 | (0..10).collect()\n |\n\n",
946+
"children": [
947+
{
948+
"children": [],
949+
"code": null,
950+
"level": "note",
951+
"message": "`#[warn(clippy::let_and_return)]` on by default",
952+
"rendered": null,
953+
"spans": []
954+
},
955+
{
956+
"children": [],
957+
"code": null,
958+
"level": "help",
959+
"message": "for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return",
960+
"rendered": null,
961+
"spans": []
962+
},
963+
{
964+
"children": [],
965+
"code": null,
966+
"level": "help",
967+
"message": "return the expression directly",
968+
"rendered": null,
969+
"spans": [
970+
{
971+
"byte_end": 55,
972+
"byte_start": 29,
973+
"column_end": 31,
974+
"column_start": 5,
975+
"expansion": null,
976+
"file_name": "src/main.rs",
977+
"is_primary": true,
978+
"label": null,
979+
"line_end": 3,
980+
"line_start": 3,
981+
"suggested_replacement": "",
982+
"suggestion_applicability": "MachineApplicable",
983+
"text": [
984+
{
985+
"highlight_end": 31,
986+
"highlight_start": 5,
987+
"text": " let a = (0..10).collect();"
988+
}
989+
]
990+
},
991+
{
992+
"byte_end": 61,
993+
"byte_start": 60,
994+
"column_end": 6,
995+
"column_start": 5,
996+
"expansion": null,
997+
"file_name": "src/main.rs",
998+
"is_primary": true,
999+
"label": null,
1000+
"line_end": 4,
1001+
"line_start": 4,
1002+
"suggested_replacement": "(0..10).collect()",
1003+
"suggestion_applicability": "MachineApplicable",
1004+
"text": [
1005+
{
1006+
"highlight_end": 6,
1007+
"highlight_start": 5,
1008+
"text": " a"
1009+
}
1010+
]
1011+
}
1012+
]
1013+
}
1014+
],
1015+
"code": {
1016+
"code": "clippy::let_and_return",
1017+
"explanation": null
1018+
},
1019+
"level": "warning",
1020+
"message": "returning the result of a let binding from a block",
1021+
"spans": [
1022+
{
1023+
"byte_end": 55,
1024+
"byte_start": 29,
1025+
"column_end": 31,
1026+
"column_start": 5,
1027+
"expansion": null,
1028+
"file_name": "src/main.rs",
1029+
"is_primary": false,
1030+
"label": "unnecessary let binding",
1031+
"line_end": 3,
1032+
"line_start": 3,
1033+
"suggested_replacement": null,
1034+
"suggestion_applicability": null,
1035+
"text": [
1036+
{
1037+
"highlight_end": 31,
1038+
"highlight_start": 5,
1039+
"text": " let a = (0..10).collect();"
1040+
}
1041+
]
1042+
},
1043+
{
1044+
"byte_end": 61,
1045+
"byte_start": 60,
1046+
"column_end": 6,
1047+
"column_start": 5,
1048+
"expansion": null,
1049+
"file_name": "src/main.rs",
1050+
"is_primary": true,
1051+
"label": null,
1052+
"line_end": 4,
1053+
"line_start": 4,
1054+
"suggested_replacement": null,
1055+
"suggestion_applicability": null,
1056+
"text": [
1057+
{
1058+
"highlight_end": 6,
1059+
"highlight_start": 5,
1060+
"text": " a"
1061+
}
1062+
]
1063+
}
1064+
]
1065+
}
1066+
"##,
1067+
);
1068+
1069+
let workspace_root = PathBuf::from("/test/");
1070+
let diag = map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic");
1071+
insta::assert_debug_snapshot!(diag);
1072+
}

0 commit comments

Comments
 (0)