project: Workaround invalid code action edits from pyright (#28354)

Ben Kunkle , Max Brunsfeld , and Anthony Eid created 8 months ago

Co-authored-by: Max Brunsfeld <[e-mail]>
Co-authored-by: Piotr Osiewicz <[e-mail]>

fixes issue where:

In a two line python file like so
```

Path()
```

If the user asks for code actions on `Path` and they select (`From
pathlib import path`)
the result they get is
```

Pathfrom pathlib import Path


Path()
```
Instead of 

```

from pathlib import Path



Path()
```

This is due to a non-lsp-spec-compliant response from pyright below

```json
{"jsonrpc":"2.0","id":40,"result":[{"title":"from pathlib import Path","edit":{"changes":{"file:///Users/neb/Zed/example-project/pyright-project/main.py":[{"range":{"start":{"line":2,"character":0},"end":{"line":2,"character":4}},"newText":"Path"},{"range":{"start":{"line":2,"character":0},"end":{"line":2,"character":0}},"newText":"from pathlib import Path\n\n\n"}]}},"kind":"quickfix"}]}
```

Release Notes:

- Fixed an issue when using auto-import code actions provided by pyright
(or basedpyright) where the import would be jumbled with the scoped
import resulting in an invalid result

Co-authored-by: Max Brunsfeld <[e-mail]>
Co-authored-by: Anthony Eid <[e-mail]>

Change summary

crates/project/src/lsp_store.rs     |  3 +
crates/project/src/project_tests.rs | 56 +++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 1 deletion(-)

Detailed changes

crates/project/src/lsp_store.rs

@@ -2635,7 +2635,8 @@ impl LocalLspStore {
                 .into_iter()
                 .map(|edit| (range_from_lsp(edit.range), edit.new_text))
                 .collect::<Vec<_>>();
-            lsp_edits.sort_by_key(|(range, _)| range.start);
+
+            lsp_edits.sort_by_key(|(range, _)| (range.start, range.end));
 
             let mut lsp_edits = lsp_edits.into_iter().peekable();
             let mut edits = Vec::new();

crates/project/src/project_tests.rs

@@ -2663,6 +2663,62 @@ async fn test_edits_from_lsp2_with_edits_on_adjacent_lines(cx: &mut gpui::TestAp
     });
 }
 
+#[gpui::test]
+async fn test_edits_from_lsp_with_replacement_followed_by_adjacent_insertion(
+    cx: &mut gpui::TestAppContext,
+) {
+    init_test(cx);
+
+    let text = "Path()";
+
+    let fs = FakeFs::new(cx.executor());
+    fs.insert_tree(
+        path!("/dir"),
+        json!({
+            "a.rs": text
+        }),
+    )
+    .await;
+
+    let project = Project::test(fs, [path!("/dir").as_ref()], cx).await;
+    let lsp_store = project.read_with(cx, |project, _| project.lsp_store());
+    let buffer = project
+        .update(cx, |project, cx| {
+            project.open_local_buffer(path!("/dir/a.rs"), cx)
+        })
+        .await
+        .unwrap();
+
+    // Simulate the language server sending us a pair of edits at the same location,
+    // with an insertion following a replacement (which violates the LSP spec).
+    let edits = lsp_store
+        .update(cx, |lsp_store, cx| {
+            lsp_store.as_local_mut().unwrap().edits_from_lsp(
+                &buffer,
+                [
+                    lsp::TextEdit {
+                        range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 4)),
+                        new_text: "Path".into(),
+                    },
+                    lsp::TextEdit {
+                        range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 0)),
+                        new_text: "from path import Path\n\n\n".into(),
+                    },
+                ],
+                LanguageServerId(0),
+                None,
+                cx,
+            )
+        })
+        .await
+        .unwrap();
+
+    buffer.update(cx, |buffer, cx| {
+        buffer.edit(edits, None, cx);
+        assert_eq!(buffer.text(), "from path import Path\n\n\nPath()")
+    });
+}
+
 #[gpui::test]
 async fn test_invalid_edits_from_lsp2(cx: &mut gpui::TestAppContext) {
     init_test(cx);