Correctly handle multi-line fixes from cargo/clippy

5 files changed, 267 insertions, 30 deletions

diff --git a/crates/ra_cargo_watch/src/conv.rs b/crates/ra_cargo_watch/src/conv.rs
index 0246adfb5..a3f05bede 100644
--- a/crates/ra_cargo_watch/src/conv.rs
+++ b/crates/ra_cargo_watch/src/conv.rs
@@ -8,6 +8,7 @@ use lsp_types::{
     Location, NumberOrString, Position, Range, TextEdit, Url, WorkspaceEdit,
 };
 use std::{
+    collections::HashMap,
     fmt::Write,
     path::{Component, Path, PathBuf, Prefix},
     str::FromStr,
@@ -126,44 +127,34 @@ fn map_rust_child_diagnostic(
     rd: &RustDiagnostic,
     workspace_root: &PathBuf,
 ) -> MappedRustChildDiagnostic {
-    let span: &DiagnosticSpan = match rd.spans.iter().find(|s| s.is_primary) {
-        Some(span) => span,
-        None => {
-            // `rustc` uses these spanless children as a way to print multi-line
-            // messages
-            return MappedRustChildDiagnostic::MessageLine(rd.message.clone());
-        }
-    };
-
-    // If we have a primary span use its location, otherwise use the parent
-    let location = map_span_to_location(&span, workspace_root);
-
-    if let Some(suggested_replacement) = &span.suggested_replacement {
-        // Include our replacement in the title unless it's empty
-        let title = if !suggested_replacement.is_empty() {
-            format!("{}: '{}'", rd.message, suggested_replacement)
-        } else {
-            rd.message.clone()
-        };
+    let spans: Vec<&DiagnosticSpan> = rd.spans.iter().filter(|s| s.is_primary).collect();
+    if spans.is_empty() {
+        // `rustc` uses these spanless children as a way to print multi-line
+        // messages
+        return MappedRustChildDiagnostic::MessageLine(rd.message.clone());
+    }
 
-        let edit = {
-            let edits = vec![TextEdit::new(location.range, suggested_replacement.clone())];
-            let mut edit_map = std::collections::HashMap::new();
-            edit_map.insert(location.uri, edits);
-            WorkspaceEdit::new(edit_map)
-        };
+    let mut edit_map: HashMap<Url, Vec<TextEdit>> = HashMap::new();
+    for &span in &spans {
+        if let Some(suggested_replacement) = &span.suggested_replacement {
+            let location = map_span_to_location(span, workspace_root);
+            let edit = TextEdit::new(location.range, suggested_replacement.clone());
+            edit_map.entry(location.uri).or_default().push(edit);
+        }
+    }
 
+    if !edit_map.is_empty() {
         MappedRustChildDiagnostic::SuggestedFix(CodeAction {
-            title,
+            title: rd.message.clone(),
             kind: Some("quickfix".to_string()),
             diagnostics: None,
-            edit: Some(edit),
+            edit: Some(WorkspaceEdit::new(edit_map)),
             command: None,
             is_preferred: None,
         })
     } else {
         MappedRustChildDiagnostic::Related(DiagnosticRelatedInformation {
-            location,
+            location: map_span_to_location(spans[0], workspace_root),
             message: rd.message.clone(),
         })
     }
diff --git a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap
index 95ca163dc..47801ae79 100644
--- a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap
+++ b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_clippy_pass_by_ref.snap
@@ -63,7 +63,7 @@ MappedRustDiagnostic {
     },
     fixes: [
         CodeAction {
-            title: "consider passing by value instead: \'self\'",
+            title: "consider passing by value instead",
             kind: Some(
                 "quickfix",
             ),
diff --git a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_multi_line_fix.snap b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_multi_line_fix.snap
new file mode 100644
index 000000000..23c4f5a2c
--- /dev/null
+++ b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_multi_line_fix.snap
@@ -0,0 +1,112 @@
+---
+source: crates/ra_cargo_watch/src/conv/test.rs
+expression: diag
+---
+MappedRustDiagnostic {
+    location: Location {
+        uri: "file:///test/src/main.rs",
+        range: Range {
+            start: Position {
+                line: 3,
+                character: 4,
+            },
+            end: Position {
+                line: 3,
+                character: 5,
+            },
+        },
+    },
+    diagnostic: Diagnostic {
+        range: Range {
+            start: Position {
+                line: 3,
+                character: 4,
+            },
+            end: Position {
+                line: 3,
+                character: 5,
+            },
+        },
+        severity: Some(
+            Warning,
+        ),
+        code: Some(
+            String(
+                "let_and_return",
+            ),
+        ),
+        source: Some(
+            "clippy",
+        ),
+        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",
+        related_information: Some(
+            [
+                DiagnosticRelatedInformation {
+                    location: Location {
+                        uri: "file:///test/src/main.rs",
+                        range: Range {
+                            start: Position {
+                                line: 2,
+                                character: 4,
+                            },
+                            end: Position {
+                                line: 2,
+                                character: 30,
+                            },
+                        },
+                    },
+                    message: "unnecessary let binding",
+                },
+            ],
+        ),
+        tags: None,
+    },
+    fixes: [
+        CodeAction {
+            title: "return the expression directly",
+            kind: Some(
+                "quickfix",
+            ),
+            diagnostics: None,
+            edit: Some(
+                WorkspaceEdit {
+                    changes: Some(
+                        {
+                            "file:///test/src/main.rs": [
+                                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,
+                },
+            ),
+            command: None,
+            is_preferred: None,
+        },
+    ],
+}
diff --git a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_unused_variable.snap b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_unused_variable.snap
index 3e1fe736c..8bab09540 100644
--- a/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_unused_variable.snap
+++ b/crates/ra_cargo_watch/src/conv/snapshots/ra_cargo_watch__conv__test__snap_rustc_unused_variable.snap
@@ -48,7 +48,7 @@ MappedRustDiagnostic {
     },
     fixes: [
         CodeAction {
-            title: "consider prefixing with an underscore: \'_foo\'",
+            title: "consider prefixing with an underscore",
             kind: Some(
                 "quickfix",
             ),
diff --git a/crates/ra_cargo_watch/src/conv/test.rs b/crates/ra_cargo_watch/src/conv/test.rs
index 6b86525b8..c880dcdc3 100644
--- a/crates/ra_cargo_watch/src/conv/test.rs
+++ b/crates/ra_cargo_watch/src/conv/test.rs
@@ -936,3 +936,137 @@ fn snap_macro_compiler_error() {
     let diag = map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic");
     insta::assert_debug_snapshot!(diag);
 }
+
+#[test]
+#[cfg(not(windows))]
+fn snap_multi_line_fix() {
+    let diag = parse_diagnostic(
+        r##"{
+            "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",
+            "children": [
+                {
+                "children": [],
+                "code": null,
+                "level": "note",
+                "message": "`#[warn(clippy::let_and_return)]` on by default",
+                "rendered": null,
+                "spans": []
+                },
+                {
+                "children": [],
+                "code": null,
+                "level": "help",
+                "message": "for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return",
+                "rendered": null,
+                "spans": []
+                },
+                {
+                "children": [],
+                "code": null,
+                "level": "help",
+                "message": "return the expression directly",
+                "rendered": null,
+                "spans": [
+                    {
+                    "byte_end": 55,
+                    "byte_start": 29,
+                    "column_end": 31,
+                    "column_start": 5,
+                    "expansion": null,
+                    "file_name": "src/main.rs",
+                    "is_primary": true,
+                    "label": null,
+                    "line_end": 3,
+                    "line_start": 3,
+                    "suggested_replacement": "",
+                    "suggestion_applicability": "MachineApplicable",
+                    "text": [
+                        {
+                        "highlight_end": 31,
+                        "highlight_start": 5,
+                        "text": "    let a = (0..10).collect();"
+                        }
+                    ]
+                    },
+                    {
+                    "byte_end": 61,
+                    "byte_start": 60,
+                    "column_end": 6,
+                    "column_start": 5,
+                    "expansion": null,
+                    "file_name": "src/main.rs",
+                    "is_primary": true,
+                    "label": null,
+                    "line_end": 4,
+                    "line_start": 4,
+                    "suggested_replacement": "(0..10).collect()",
+                    "suggestion_applicability": "MachineApplicable",
+                    "text": [
+                        {
+                        "highlight_end": 6,
+                        "highlight_start": 5,
+                        "text": "    a"
+                        }
+                    ]
+                    }
+                ]
+                }
+            ],
+            "code": {
+                "code": "clippy::let_and_return",
+                "explanation": null
+            },
+            "level": "warning",
+            "message": "returning the result of a let binding from a block",
+            "spans": [
+                {
+                "byte_end": 55,
+                "byte_start": 29,
+                "column_end": 31,
+                "column_start": 5,
+                "expansion": null,
+                "file_name": "src/main.rs",
+                "is_primary": false,
+                "label": "unnecessary let binding",
+                "line_end": 3,
+                "line_start": 3,
+                "suggested_replacement": null,
+                "suggestion_applicability": null,
+                "text": [
+                    {
+                    "highlight_end": 31,
+                    "highlight_start": 5,
+                    "text": "    let a = (0..10).collect();"
+                    }
+                ]
+                },
+                {
+                "byte_end": 61,
+                "byte_start": 60,
+                "column_end": 6,
+                "column_start": 5,
+                "expansion": null,
+                "file_name": "src/main.rs",
+                "is_primary": true,
+                "label": null,
+                "line_end": 4,
+                "line_start": 4,
+                "suggested_replacement": null,
+                "suggestion_applicability": null,
+                "text": [
+                    {
+                    "highlight_end": 6,
+                    "highlight_start": 5,
+                    "text": "    a"
+                    }
+                ]
+                }
+            ]
+        }
+        "##,
+    );
+
+    let workspace_root = PathBuf::from("/test/");
+    let diag = map_rust_diagnostic_to_lsp(&diag, &workspace_root).expect("couldn't map diagnostic");
+    insta::assert_debug_snapshot!(diag);
+}