Finish implementing Buffer::edits_from_lsp

Max Brunsfeld created

Change summary

crates/language/src/buffer.rs | 156 +++++++++++++++++--------
crates/language/src/tests.rs  | 222 ++++++++++++++++++++++++++++++++++++
crates/project/src/project.rs |   8 
3 files changed, 325 insertions(+), 61 deletions(-)

Detailed changes

crates/language/src/buffer.rs 🔗

@@ -233,6 +233,15 @@ pub struct FakeFile {
     pub path: Arc<Path>,
 }
 
+#[cfg(any(test, feature = "test-support"))]
+impl FakeFile {
+    pub fn new(path: impl AsRef<Path>) -> Self {
+        Self {
+            path: path.as_ref().into(),
+        }
+    }
+}
+
 #[cfg(any(test, feature = "test-support"))]
 impl File for FakeFile {
     fn as_local(&self) -> Option<&dyn LocalFile> {
@@ -349,7 +358,7 @@ pub struct Chunk<'a> {
     pub diagnostic: Option<DiagnosticSeverity>,
 }
 
-pub struct Diff {
+pub(crate) struct Diff {
     base_version: clock::Global,
     new_text: Arc<str>,
     changes: Vec<(ChangeTag, usize)>,
@@ -1236,7 +1245,7 @@ impl Buffer {
         })
     }
 
-    pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext<Self>) -> bool {
+    pub(crate) fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext<Self>) -> bool {
         if self.version == diff.base_version {
             self.start_transaction();
             let mut offset = diff.start_offset;
@@ -1496,71 +1505,112 @@ impl Buffer {
         Some(edit_id)
     }
 
-    pub fn diff_for_lsp_edits(
+    pub fn edits_from_lsp(
         &mut self,
         lsp_edits: impl 'static + Send + IntoIterator<Item = lsp::TextEdit>,
         version: Option<i32>,
         cx: &mut ModelContext<Self>,
-    ) -> Task<Result<Diff>> {
-        let snapshot = TextBuffer::deref(self).clone();
-        let lsp_snapshot =
-            if let Some((version, language_server)) = version.zip(self.language_server.as_mut()) {
-                language_server
-                    .snapshot_for_version(version as usize)
-                    .map(|s| s.clone())
-            } else {
-                Ok(snapshot.clone())
-            };
+    ) -> Task<Result<Vec<(Range<Anchor>, String)>>> {
+        let snapshot = if let Some((version, state)) = version.zip(self.language_server.as_mut()) {
+            state
+                .snapshot_for_version(version as usize)
+                .map(Clone::clone)
+        } else {
+            Ok(TextBuffer::deref(self).clone())
+        };
 
         cx.background().spawn(async move {
-            let lsp_snapshot = lsp_snapshot?;
+            let snapshot = snapshot?;
+            let mut lsp_edits = lsp_edits
+                .into_iter()
+                .map(|edit| (range_from_lsp(edit.range), edit.new_text))
+                .peekable();
 
-            // Convert LSP ranges into offsets in the current snapshot.
             let mut edits = Vec::new();
-            for edit in lsp_edits.into_iter() {
-                let range = range_from_lsp(edit.range);
-                if lsp_snapshot.clip_point_utf16(range.start, Bias::Left) != range.start
-                    || lsp_snapshot.clip_point_utf16(range.end, Bias::Left) != range.end
+            while let Some((mut range, mut new_text)) = lsp_edits.next() {
+                // Combine any LSP edits that are adjacent.
+                //
+                // Also, combine LSP edits that are separated from each other by only
+                // a newline. This is important because for some code actions,
+                // Rust-analyzer rewrites the entire buffer via a series of edits that
+                // are separated by unchanged newline characters.
+                //
+                // In order for the diffing logic below to work properly, any edits that
+                // cancel each other out must be combined into one.
+                while let Some((next_range, next_text)) = lsp_edits.peek() {
+                    if next_range.start > range.end {
+                        if next_range.start.row > range.end.row + 1
+                            || next_range.start.column > 0
+                            || snapshot.clip_point_utf16(
+                                PointUtf16::new(range.end.row, u32::MAX),
+                                Bias::Left,
+                            ) > range.end
+                        {
+                            break;
+                        }
+                        new_text.push('\n');
+                    }
+                    range.end = next_range.end;
+                    new_text.push_str(&next_text);
+                    lsp_edits.next();
+                }
+
+                if snapshot.clip_point_utf16(range.start, Bias::Left) != range.start
+                    || snapshot.clip_point_utf16(range.end, Bias::Left) != range.end
                 {
                     return Err(anyhow!("invalid edits received from language server"));
                 }
-                let edit_start = lsp_snapshot.anchor_before(range.start).to_offset(&snapshot);
-                let edit_end = lsp_snapshot.anchor_before(range.end).to_offset(&snapshot);
-                edits.push((edit_start..edit_end, edit.new_text));
-            }
 
-            if edits.is_empty() {
-                return Ok(Diff {
-                    base_version: snapshot.version().clone(),
-                    new_text: "".into(),
-                    changes: Default::default(),
-                    start_offset: Default::default(),
-                });
-            }
-
-            let slice_start = edits.first().unwrap().0.start;
-            let slice_end = edits.last().unwrap().0.end;
-            let mut cursor = snapshot.as_rope().cursor(slice_start);
-            let mut slice = cursor.slice(slice_end);
-            let old_text = slice.to_string();
-            for (range, new_text) in edits.into_iter().rev() {
-                slice.replace(
-                    range.start - slice_start..range.end - slice_start,
-                    &new_text,
-                );
+                // For multiline edits, perform a diff of the old and new text so that
+                // we can identify the changes more precisely, preserving the locations
+                // of any anchors positioned in the unchanged regions.
+                if range.end.row > range.start.row {
+                    let mut offset = range.start.to_offset(&snapshot);
+                    let old_text = snapshot.text_for_range(range).collect::<String>();
+
+                    let diff = TextDiff::from_lines(old_text.as_str(), &new_text);
+                    let mut moved_since_edit = true;
+                    for change in diff.iter_all_changes() {
+                        let tag = change.tag();
+                        let value = change.value();
+                        match tag {
+                            ChangeTag::Equal => {
+                                offset += value.len();
+                                moved_since_edit = true;
+                            }
+                            ChangeTag::Delete => {
+                                let start = snapshot.anchor_after(offset);
+                                let end = snapshot.anchor_before(offset + value.len());
+                                if moved_since_edit {
+                                    edits.push((start..end, String::new()));
+                                } else {
+                                    edits.last_mut().unwrap().0.end = end;
+                                }
+                                offset += value.len();
+                                moved_since_edit = false;
+                            }
+                            ChangeTag::Insert => {
+                                if moved_since_edit {
+                                    let anchor = snapshot.anchor_after(offset);
+                                    edits.push((anchor.clone()..anchor, value.to_string()));
+                                } else {
+                                    edits.last_mut().unwrap().1.push_str(value);
+                                }
+                                moved_since_edit = false;
+                            }
+                        }
+                    }
+                } else if range.end == range.start {
+                    let anchor = snapshot.anchor_after(range.start);
+                    edits.push((anchor.clone()..anchor, new_text));
+                } else {
+                    let edit_start = snapshot.anchor_after(range.start);
+                    let edit_end = snapshot.anchor_before(range.end);
+                    edits.push((edit_start..edit_end, new_text));
+                }
             }
 
-            let new_text = Arc::from(slice.to_string());
-            let changes = TextDiff::from_words(old_text.as_str(), &new_text)
-                .iter_all_changes()
-                .map(|c| (c.tag(), c.value().len()))
-                .collect::<Vec<_>>();
-            Ok(Diff {
-                base_version: snapshot.version().clone(),
-                new_text,
-                changes,
-                start_offset: slice_start,
-            })
+            Ok(edits)
         })
     }
 

crates/language/src/tests.rs 🔗

@@ -571,11 +571,8 @@ async fn test_diagnostics(mut cx: gpui::TestAppContext) {
     "
     .unindent();
 
-    let file = Box::new(FakeFile {
-        path: Path::new("/some/path").into(),
-    }) as Box<dyn File>;
     let buffer = cx.add_model(|cx| {
-        Buffer::from_file(0, text, file, cx)
+        Buffer::from_file(0, text, Box::new(FakeFile::new("/some/path")), cx)
             .with_language(Arc::new(rust_lang), cx)
             .with_language_server(language_server, cx)
     });
@@ -841,6 +838,223 @@ async fn test_diagnostics(mut cx: gpui::TestAppContext) {
     });
 }
 
+#[gpui::test]
+async fn test_edits_from_lsp_with_past_version(mut cx: gpui::TestAppContext) {
+    let (language_server, mut fake) = lsp::LanguageServer::fake(&cx).await;
+
+    let text = "
+        fn a() {
+            f1();
+        }
+        fn b() {
+            f2();
+        }
+        fn c() {
+            f3();
+        }
+    "
+    .unindent();
+
+    let buffer = cx.add_model(|cx| {
+        Buffer::from_file(0, text, Box::new(FakeFile::new("/some/path")), cx)
+            .with_language(Arc::new(rust_lang()), cx)
+            .with_language_server(language_server, cx)
+    });
+
+    let lsp_document_version = fake
+        .receive_notification::<lsp::notification::DidOpenTextDocument>()
+        .await
+        .text_document
+        .version;
+
+    // Simulate editing the buffer after the language server computes some edits.
+    buffer.update(&mut cx, |buffer, cx| {
+        buffer.edit(
+            [Point::new(0, 0)..Point::new(0, 0)],
+            "// above first function\n",
+            cx,
+        );
+        buffer.edit(
+            [Point::new(2, 0)..Point::new(2, 0)],
+            "    // inside first function\n",
+            cx,
+        );
+        buffer.edit(
+            [Point::new(6, 4)..Point::new(6, 4)],
+            "// inside second function ",
+            cx,
+        );
+
+        assert_eq!(
+            buffer.text(),
+            "
+                // above first function
+                fn a() {
+                    // inside first function
+                    f1();
+                }
+                fn b() {
+                    // inside second function f2();
+                }
+                fn c() {
+                    f3();
+                }
+            "
+            .unindent()
+        );
+    });
+
+    let edits = buffer
+        .update(&mut cx, |buffer, cx| {
+            buffer.edits_from_lsp(
+                vec![
+                    // replace body of first function
+                    lsp::TextEdit {
+                        range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(3, 0)),
+                        new_text: "
+                            fn a() {
+                                f10();
+                            }
+                        "
+                        .unindent(),
+                    },
+                    // edit inside second function
+                    lsp::TextEdit {
+                        range: lsp::Range::new(lsp::Position::new(4, 6), lsp::Position::new(4, 6)),
+                        new_text: "00".into(),
+                    },
+                    // edit inside third function via two distinct edits
+                    lsp::TextEdit {
+                        range: lsp::Range::new(lsp::Position::new(7, 5), lsp::Position::new(7, 5)),
+                        new_text: "4000".into(),
+                    },
+                    lsp::TextEdit {
+                        range: lsp::Range::new(lsp::Position::new(7, 5), lsp::Position::new(7, 6)),
+                        new_text: "".into(),
+                    },
+                ],
+                Some(lsp_document_version),
+                cx,
+            )
+        })
+        .await
+        .unwrap();
+
+    buffer.update(&mut cx, |buffer, cx| {
+        for (range, new_text) in edits {
+            buffer.edit([range], new_text, cx);
+        }
+        assert_eq!(
+            buffer.text(),
+            "
+                // above first function
+                fn a() {
+                    // inside first function
+                    f10();
+                }
+                fn b() {
+                    // inside second function f200();
+                }
+                fn c() {
+                    f4000();
+                }
+            "
+            .unindent()
+        );
+    });
+}
+
+#[gpui::test]
+async fn test_edits_from_lsp_with_edits_on_adjacent_lines(mut cx: gpui::TestAppContext) {
+    let text = "
+        use a::b;
+        use a::c;
+
+        fn f() {
+            b();
+            c();
+        }
+    "
+    .unindent();
+
+    let buffer = cx.add_model(|cx| Buffer::new(0, text, cx));
+
+    // Simulate the language server sending us a small edit in the form of a very large diff.
+    // Rust-analyzer does this when performing a merge-imports code action.
+    let edits = buffer
+        .update(&mut cx, |buffer, cx| {
+            buffer.edits_from_lsp(
+                [
+                    // Replace the first use statement without editing the semicolon.
+                    lsp::TextEdit {
+                        range: lsp::Range::new(lsp::Position::new(0, 4), lsp::Position::new(0, 8)),
+                        new_text: "a::{b, c}".into(),
+                    },
+                    // Reinsert the remainder of the file between the semicolon and the final
+                    // newline of the file.
+                    lsp::TextEdit {
+                        range: lsp::Range::new(lsp::Position::new(0, 9), lsp::Position::new(0, 9)),
+                        new_text: "\n\n".into(),
+                    },
+                    lsp::TextEdit {
+                        range: lsp::Range::new(lsp::Position::new(0, 9), lsp::Position::new(0, 9)),
+                        new_text: "
+                            fn f() {
+                                b();
+                                c();
+                            }"
+                        .unindent(),
+                    },
+                    // Delete everything after the first newline of the file.
+                    lsp::TextEdit {
+                        range: lsp::Range::new(lsp::Position::new(1, 0), lsp::Position::new(7, 0)),
+                        new_text: "".into(),
+                    },
+                ],
+                None,
+                cx,
+            )
+        })
+        .await
+        .unwrap();
+
+    buffer.update(&mut cx, |buffer, cx| {
+        let edits = edits
+            .into_iter()
+            .map(|(range, text)| {
+                (
+                    range.start.to_point(&buffer)..range.end.to_point(&buffer),
+                    text,
+                )
+            })
+            .collect::<Vec<_>>();
+
+        assert_eq!(
+            edits,
+            [
+                (Point::new(0, 4)..Point::new(0, 8), "a::{b, c}".into()),
+                (Point::new(1, 0)..Point::new(2, 0), "".into())
+            ]
+        );
+
+        for (range, new_text) in edits {
+            buffer.edit([range], new_text, cx);
+        }
+        assert_eq!(
+            dbg!(buffer.text()),
+            "
+                use a::{b, c};
+        
+                fn f() {
+                    b();
+                    c();
+                }
+            "
+            .unindent()
+        );
+    });
+}
+
 #[gpui::test]
 async fn test_empty_diagnostic_ranges(mut cx: gpui::TestAppContext) {
     cx.add_model(|cx| {

crates/project/src/project.rs 🔗

@@ -1586,21 +1586,21 @@ impl Project {
                                 })
                                 .await?;
 
-                            let diff = buffer_to_edit
+                            let edits = buffer_to_edit
                                 .update(&mut cx, |buffer, cx| {
                                     let edits = op.edits.into_iter().map(|edit| match edit {
                                         lsp::OneOf::Left(edit) => edit,
                                         lsp::OneOf::Right(edit) => edit.text_edit,
                                     });
-                                    buffer.diff_for_lsp_edits(edits, op.text_document.version, cx)
+                                    buffer.edits_from_lsp(edits, op.text_document.version, cx)
                                 })
                                 .await?;
 
                             let transaction = buffer_to_edit.update(&mut cx, |buffer, cx| {
                                 buffer.finalize_last_transaction();
                                 buffer.start_transaction();
-                                if !buffer.apply_diff(diff, cx) {
-                                    log::info!("buffer has changed since computing code action");
+                                for (range, text) in edits {
+                                    buffer.edit([range], text, cx);
                                 }
                                 let transaction = if buffer.end_transaction(cx).is_some() {
                                     let transaction =