From 645df73a37d0944a369950f4375aa1275f0b5975 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 10 Feb 2022 18:01:18 -0800 Subject: [PATCH] Finish implementing Buffer::edits_from_lsp --- 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(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 2cc8542dd747f0b4f5a13602f2f698270a48ff88..7664529272c94221ad225afe81ec8310da26eb1c 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -233,6 +233,15 @@ pub struct FakeFile { pub path: Arc, } +#[cfg(any(test, feature = "test-support"))] +impl FakeFile { + pub fn new(path: impl AsRef) -> 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, } -pub struct Diff { +pub(crate) struct Diff { base_version: clock::Global, new_text: Arc, changes: Vec<(ChangeTag, usize)>, @@ -1236,7 +1245,7 @@ impl Buffer { }) } - pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> bool { + pub(crate) fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> 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, version: Option, cx: &mut ModelContext, - ) -> Task> { - 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, 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::(); + + 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::>(); - Ok(Diff { - base_version: snapshot.version().clone(), - new_text, - changes, - start_offset: slice_start, - }) + Ok(edits) }) } diff --git a/crates/language/src/tests.rs b/crates/language/src/tests.rs index 6e2ebd9015abfaefcc8dea344a500f62298f8c68..024d63f0ebe1abbc81867717d50f34d1f86b0a73 100644 --- a/crates/language/src/tests.rs +++ b/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; 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::() + .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::>(); + + 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| { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 9278d2828dc0e4d25da2b2630d029ebcf5dd0827..85e5303dfd81677a80c90f8d760bbf9a079276d1 100644 --- a/crates/project/src/project.rs +++ b/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 =