From 38ec45008c69bc1d8042318708905f9baae311ed Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Tue, 8 Apr 2025 16:13:44 -0400 Subject: [PATCH] project: Workaround invalid code action edits from pyright (#28354) Co-authored-by: Max Brunsfeld Co-authored-by: Piotr Osiewicz 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 Co-authored-by: Anthony Eid --- crates/project/src/lsp_store.rs | 3 +- crates/project/src/project_tests.rs | 56 +++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 7d46754a30700e36aa09acbd08e8f4c407d654bb..4df32f726701b822152a57e68f87659c09a77b81 100644 --- a/crates/project/src/lsp_store.rs +++ b/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::>(); - 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(); diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 45fcf678e069966632fa99519fa3372fda10e443..24325e5dd9f3020f1c10af96185f6f5dfce040c2 100644 --- a/crates/project/src/project_tests.rs +++ b/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);