From 381cbc6088b6c4238b0f82df8ef37b552257ae59 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 5 May 2020 22:14:01 +0200 Subject: [PATCH 1/4] Minor cleanups --- crates/ra_assists/src/assist_ctx.rs | 1 - crates/ra_assists/src/lib.rs | 14 +++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index 82f61bc8f7..89547ce035 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -42,7 +42,6 @@ impl AssistInfo { } } -pub(crate) type AssistHandler = fn(AssistCtx) -> Option; /// `AssistCtx` allows to apply an assist or check if it could be applied. /// diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index 5cec10088c..ad85f5553b 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -23,7 +23,7 @@ use ra_ide_db::RootDatabase; use ra_syntax::{TextRange, TextSize}; use ra_text_edit::TextEdit; -pub(crate) use crate::assist_ctx::{Assist, AssistCtx, AssistHandler}; +pub(crate) use crate::assist_ctx::{Assist, AssistCtx}; /// Unique identifier of the assist, should not be shown to the user /// directly. @@ -109,7 +109,9 @@ pub fn resolved_assists(db: &RootDatabase, range: FileRange) -> Vec Option; mod add_custom_impl; mod add_derive; @@ -145,12 +147,13 @@ mod handlers { mod reorder_fields; mod unwrap_block; - pub(crate) fn all() -> &'static [AssistHandler] { + pub(crate) fn all() -> &'static [Handler] { &[ // These are alphabetic for the foolish consistency add_custom_impl::add_custom_impl, add_derive::add_derive, add_explicit_type::add_explicit_type, + add_from_impl_for_enum::add_from_impl_for_enum, add_function::add_function, add_impl::add_impl, add_new::add_new, @@ -176,17 +179,18 @@ mod handlers { raw_string::remove_hash, remove_dbg::remove_dbg, remove_mut::remove_mut, + reorder_fields::reorder_fields, replace_if_let_with_match::replace_if_let_with_match, replace_let_with_if_let::replace_let_with_if_let, replace_qualified_name_with_use::replace_qualified_name_with_use, replace_unwrap_with_match::replace_unwrap_with_match, split_import::split_import, - add_from_impl_for_enum::add_from_impl_for_enum, unwrap_block::unwrap_block, // These are manually sorted for better priorities add_missing_impl_members::add_missing_impl_members, add_missing_impl_members::add_missing_default_members, - reorder_fields::reorder_fields, + // Are you sure you want to add new assist here, and not to the + // sorted list above? ] } } From 4a6fa8f0dfcebbb4ea80394e5e4ca21f076f58f2 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 5 May 2020 23:15:49 +0200 Subject: [PATCH 2/4] Rename AtomTextEdit -> Indel --- crates/ra_assists/src/assist_ctx.rs | 4 +- .../src/completion/completion_context.rs | 4 +- .../ra_ide/src/completion/completion_item.rs | 4 +- crates/ra_ide_db/src/line_index_utils.rs | 6 +- crates/ra_syntax/src/fuzz.rs | 6 +- crates/ra_syntax/src/lib.rs | 14 +- crates/ra_syntax/src/parsing/reparsing.rs | 16 +-- crates/ra_text_edit/src/lib.rs | 131 +++++++++++++++--- crates/ra_text_edit/src/text_edit.rs | 102 -------------- crates/rust-analyzer/src/conv.rs | 29 ++-- xtask/tests/tidy-tests/main.rs | 1 - 11 files changed, 154 insertions(+), 163 deletions(-) delete mode 100644 crates/ra_text_edit/src/text_edit.rs diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index 89547ce035..83dd270c6c 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -4,14 +4,13 @@ use ra_db::FileRange; use ra_fmt::{leading_indent, reindent}; use ra_ide_db::RootDatabase; use ra_syntax::{ - algo::{self, find_covering_element, find_node_at_offset}, + algo::{self, find_covering_element, find_node_at_offset, SyntaxRewriter}, AstNode, SourceFile, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize, TokenAtOffset, }; use ra_text_edit::TextEditBuilder; use crate::{AssistAction, AssistFile, AssistId, AssistLabel, GroupLabel, ResolvedAssist}; -use algo::SyntaxRewriter; #[derive(Clone, Debug)] pub(crate) struct Assist(pub(crate) Vec); @@ -42,7 +41,6 @@ impl AssistInfo { } } - /// `AssistCtx` allows to apply an assist or check if it could be applied. /// /// Assists use a somewhat over-engineered approach, given the current needs. The diff --git a/crates/ra_ide/src/completion/completion_context.rs b/crates/ra_ide/src/completion/completion_context.rs index dd87bd119e..b6b9627dea 100644 --- a/crates/ra_ide/src/completion/completion_context.rs +++ b/crates/ra_ide/src/completion/completion_context.rs @@ -9,7 +9,7 @@ use ra_syntax::{ SyntaxKind::*, SyntaxNode, SyntaxToken, TextRange, TextSize, }; -use ra_text_edit::AtomTextEdit; +use ra_text_edit::Indel; use crate::{call_info::ActiveParameter, completion::CompletionConfig, FilePosition}; @@ -76,7 +76,7 @@ impl<'a> CompletionContext<'a> { // actual completion. let file_with_fake_ident = { let parse = db.parse(position.file_id); - let edit = AtomTextEdit::insert(position.offset, "intellijRulezz".to_string()); + let edit = Indel::insert(position.offset, "intellijRulezz".to_string()); parse.reparse(&edit).tree() }; let fake_ident_token = diff --git a/crates/ra_ide/src/completion/completion_item.rs b/crates/ra_ide/src/completion/completion_item.rs index 5936fb8f7c..383b23ac44 100644 --- a/crates/ra_ide/src/completion/completion_item.rs +++ b/crates/ra_ide/src/completion/completion_item.rs @@ -62,8 +62,8 @@ impl fmt::Debug for CompletionItem { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let mut s = f.debug_struct("CompletionItem"); s.field("label", &self.label()).field("source_range", &self.source_range()); - if self.text_edit().as_atoms().len() == 1 { - let atom = &self.text_edit().as_atoms()[0]; + if self.text_edit().as_indels().len() == 1 { + let atom = &self.text_edit().as_indels()[0]; s.field("delete", &atom.delete); s.field("insert", &atom.insert); } else { diff --git a/crates/ra_ide_db/src/line_index_utils.rs b/crates/ra_ide_db/src/line_index_utils.rs index 039a12c0d8..7fa6fc448e 100644 --- a/crates/ra_ide_db/src/line_index_utils.rs +++ b/crates/ra_ide_db/src/line_index_utils.rs @@ -10,7 +10,7 @@ use std::convert::TryInto; use ra_syntax::{TextRange, TextSize}; -use ra_text_edit::{AtomTextEdit, TextEdit}; +use ra_text_edit::{Indel, TextEdit}; use crate::line_index::{LineCol, LineIndex, Utf16Char}; @@ -182,14 +182,14 @@ struct TranslatedEdit<'a> { } struct Edits<'a> { - edits: &'a [AtomTextEdit], + edits: &'a [Indel], current: Option>, acc_diff: i64, } impl<'a> Edits<'a> { fn from_text_edit(text_edit: &'a TextEdit) -> Edits<'a> { - let mut x = Edits { edits: text_edit.as_atoms(), current: None, acc_diff: 0 }; + let mut x = Edits { edits: text_edit.as_indels(), current: None, acc_diff: 0 }; x.advance_edit(); x } diff --git a/crates/ra_syntax/src/fuzz.rs b/crates/ra_syntax/src/fuzz.rs index 10fbe31768..39f9b12ab2 100644 --- a/crates/ra_syntax/src/fuzz.rs +++ b/crates/ra_syntax/src/fuzz.rs @@ -5,7 +5,7 @@ use std::{ str::{self, FromStr}, }; -use ra_text_edit::AtomTextEdit; +use ra_text_edit::Indel; use crate::{validation, AstNode, SourceFile, TextRange}; @@ -22,7 +22,7 @@ pub fn check_parser(text: &str) { #[derive(Debug, Clone)] pub struct CheckReparse { text: String, - edit: AtomTextEdit, + edit: Indel, edited_text: String, } @@ -43,7 +43,7 @@ impl CheckReparse { TextRange::at(delete_start.try_into().unwrap(), delete_len.try_into().unwrap()); let edited_text = format!("{}{}{}", &text[..delete_start], &insert, &text[delete_start + delete_len..]); - let edit = AtomTextEdit { delete, insert }; + let edit = Indel { delete, insert }; Some(CheckReparse { text, edit, edited_text }) } diff --git a/crates/ra_syntax/src/lib.rs b/crates/ra_syntax/src/lib.rs index d0234cadaf..1a7348dacd 100644 --- a/crates/ra_syntax/src/lib.rs +++ b/crates/ra_syntax/src/lib.rs @@ -39,7 +39,7 @@ pub mod fuzz; use std::{marker::PhantomData, sync::Arc}; -use ra_text_edit::AtomTextEdit; +use ra_text_edit::Indel; use stdx::format_to; use crate::syntax_node::GreenNode; @@ -126,13 +126,13 @@ impl Parse { buf } - pub fn reparse(&self, edit: &AtomTextEdit) -> Parse { - self.incremental_reparse(edit).unwrap_or_else(|| self.full_reparse(edit)) + pub fn reparse(&self, indel: &Indel) -> Parse { + self.incremental_reparse(indel).unwrap_or_else(|| self.full_reparse(indel)) } - fn incremental_reparse(&self, edit: &AtomTextEdit) -> Option> { + fn incremental_reparse(&self, indel: &Indel) -> Option> { // FIXME: validation errors are not handled here - parsing::incremental_reparse(self.tree().syntax(), edit, self.errors.to_vec()).map( + parsing::incremental_reparse(self.tree().syntax(), indel, self.errors.to_vec()).map( |(green_node, errors, _reparsed_range)| Parse { green: green_node, errors: Arc::new(errors), @@ -141,8 +141,8 @@ impl Parse { ) } - fn full_reparse(&self, edit: &AtomTextEdit) -> Parse { - let text = edit.apply(self.tree().syntax().text().to_string()); + fn full_reparse(&self, indel: &Indel) -> Parse { + let text = indel.apply(self.tree().syntax().text().to_string()); SourceFile::parse(&text) } } diff --git a/crates/ra_syntax/src/parsing/reparsing.rs b/crates/ra_syntax/src/parsing/reparsing.rs index ffff0a7b20..6257e3f338 100644 --- a/crates/ra_syntax/src/parsing/reparsing.rs +++ b/crates/ra_syntax/src/parsing/reparsing.rs @@ -7,7 +7,7 @@ //! and try to parse only this block. use ra_parser::Reparser; -use ra_text_edit::AtomTextEdit; +use ra_text_edit::Indel; use crate::{ algo, @@ -24,7 +24,7 @@ use crate::{ pub(crate) fn incremental_reparse( node: &SyntaxNode, - edit: &AtomTextEdit, + edit: &Indel, errors: Vec, ) -> Option<(GreenNode, Vec, TextRange)> { if let Some((green, new_errors, old_range)) = reparse_token(node, &edit) { @@ -39,7 +39,7 @@ pub(crate) fn incremental_reparse( fn reparse_token<'node>( root: &'node SyntaxNode, - edit: &AtomTextEdit, + edit: &Indel, ) -> Option<(GreenNode, Vec, TextRange)> { let prev_token = algo::find_covering_element(root, edit.delete).as_token()?.clone(); let prev_token_kind = prev_token.kind(); @@ -88,7 +88,7 @@ fn reparse_token<'node>( fn reparse_block<'node>( root: &'node SyntaxNode, - edit: &AtomTextEdit, + edit: &Indel, ) -> Option<(GreenNode, Vec, TextRange)> { let (node, reparser) = find_reparsable_node(root, edit.delete)?; let text = get_text_after_edit(node.clone().into(), edit); @@ -108,9 +108,9 @@ fn reparse_block<'node>( Some((node.replace_with(green), new_parser_errors, node.text_range())) } -fn get_text_after_edit(element: SyntaxElement, edit: &AtomTextEdit) -> String { +fn get_text_after_edit(element: SyntaxElement, edit: &Indel) -> String { let edit = - AtomTextEdit::replace(edit.delete - element.text_range().start(), edit.insert.clone()); + Indel::replace(edit.delete - element.text_range().start(), edit.insert.clone()); let text = match element { NodeOrToken::Token(token) => token.text().to_string(), @@ -167,7 +167,7 @@ fn merge_errors( old_errors: Vec, new_errors: Vec, range_before_reparse: TextRange, - edit: &AtomTextEdit, + edit: &Indel, ) -> Vec { let mut res = Vec::new(); @@ -198,7 +198,7 @@ mod tests { fn do_check(before: &str, replace_with: &str, reparsed_len: u32) { let (range, before) = extract_range(before); - let edit = AtomTextEdit::replace(range, replace_with.to_owned()); + let edit = Indel::replace(range, replace_with.to_owned()); let after = edit.apply(before.clone()); let fully_reparsed = SourceFile::parse(&after); diff --git a/crates/ra_text_edit/src/lib.rs b/crates/ra_text_edit/src/lib.rs index e656260c73..c41bf324b6 100644 --- a/crates/ra_text_edit/src/lib.rs +++ b/crates/ra_text_edit/src/lib.rs @@ -1,30 +1,40 @@ -//! FIXME: write short doc here - -mod text_edit; +//! Representation of a `TextEdit`. +//! +//! `rust-analyzer` never mutates text itself and only sends diffs to clients, +//! so `TextEdit` is the ultimate representation of the work done by +//! rust-analyzer. use text_size::{TextRange, TextSize}; -pub use crate::text_edit::{TextEdit, TextEditBuilder}; - -/// Must not overlap with other `AtomTextEdit`s +/// `InsertDelete` -- a single "atomic" change to text +/// +/// Must not overlap with other `InDel`s #[derive(Debug, Clone)] -pub struct AtomTextEdit { +pub struct Indel { + pub insert: String, /// Refers to offsets in the original text pub delete: TextRange, - pub insert: String, } -impl AtomTextEdit { - pub fn replace(range: TextRange, replace_with: String) -> AtomTextEdit { - AtomTextEdit { delete: range, insert: replace_with } - } +#[derive(Debug, Clone)] +pub struct TextEdit { + indels: Vec, +} - pub fn delete(range: TextRange) -> AtomTextEdit { - AtomTextEdit::replace(range, String::new()) - } +#[derive(Debug, Default)] +pub struct TextEditBuilder { + indels: Vec, +} - pub fn insert(offset: TextSize, text: String) -> AtomTextEdit { - AtomTextEdit::replace(TextRange::empty(offset), text) +impl Indel { + pub fn insert(offset: TextSize, text: String) -> Indel { + Indel::replace(TextRange::empty(offset), text) + } + pub fn delete(range: TextRange) -> Indel { + Indel::replace(range, String::new()) + } + pub fn replace(range: TextRange, replace_with: String) -> Indel { + Indel { delete: range, insert: replace_with } } pub fn apply(&self, mut text: String) -> String { @@ -34,3 +44,90 @@ impl AtomTextEdit { text } } + +impl TextEdit { + pub fn insert(offset: TextSize, text: String) -> TextEdit { + let mut builder = TextEditBuilder::default(); + builder.insert(offset, text); + builder.finish() + } + + pub fn delete(range: TextRange) -> TextEdit { + let mut builder = TextEditBuilder::default(); + builder.delete(range); + builder.finish() + } + + pub fn replace(range: TextRange, replace_with: String) -> TextEdit { + let mut builder = TextEditBuilder::default(); + builder.replace(range, replace_with); + builder.finish() + } + + pub(crate) fn from_indels(mut indels: Vec) -> TextEdit { + indels.sort_by_key(|a| (a.delete.start(), a.delete.end())); + for (a1, a2) in indels.iter().zip(indels.iter().skip(1)) { + assert!(a1.delete.end() <= a2.delete.start()) + } + TextEdit { indels } + } + + pub fn as_indels(&self) -> &[Indel] { + &self.indels + } + + pub fn apply(&self, text: &str) -> String { + let mut total_len = TextSize::of(text); + for indel in self.indels.iter() { + total_len += TextSize::of(&indel.insert); + total_len -= indel.delete.end() - indel.delete.start(); + } + let mut buf = String::with_capacity(total_len.into()); + let mut prev = 0; + for indel in self.indels.iter() { + let start: usize = indel.delete.start().into(); + let end: usize = indel.delete.end().into(); + if start > prev { + buf.push_str(&text[prev..start]); + } + buf.push_str(&indel.insert); + prev = end; + } + buf.push_str(&text[prev..text.len()]); + assert_eq!(TextSize::of(&buf), total_len); + buf + } + + pub fn apply_to_offset(&self, offset: TextSize) -> Option { + let mut res = offset; + for indel in self.indels.iter() { + if indel.delete.start() >= offset { + break; + } + if offset < indel.delete.end() { + return None; + } + res += TextSize::of(&indel.insert); + res -= indel.delete.len(); + } + Some(res) + } +} + +impl TextEditBuilder { + pub fn replace(&mut self, range: TextRange, replace_with: String) { + self.indels.push(Indel::replace(range, replace_with)) + } + pub fn delete(&mut self, range: TextRange) { + self.indels.push(Indel::delete(range)) + } + pub fn insert(&mut self, offset: TextSize, text: String) { + self.indels.push(Indel::insert(offset, text)) + } + pub fn finish(self) -> TextEdit { + TextEdit::from_indels(self.indels) + } + pub fn invalidates_offset(&self, offset: TextSize) -> bool { + self.indels.iter().any(|indel| indel.delete.contains_inclusive(offset)) + } +} diff --git a/crates/ra_text_edit/src/text_edit.rs b/crates/ra_text_edit/src/text_edit.rs deleted file mode 100644 index eabab4b4d1..0000000000 --- a/crates/ra_text_edit/src/text_edit.rs +++ /dev/null @@ -1,102 +0,0 @@ -//! FIXME: write short doc here - -use crate::AtomTextEdit; - -use text_size::{TextRange, TextSize}; - -#[derive(Debug, Clone)] -pub struct TextEdit { - atoms: Vec, -} - -#[derive(Debug, Default)] -pub struct TextEditBuilder { - atoms: Vec, -} - -impl TextEditBuilder { - pub fn replace(&mut self, range: TextRange, replace_with: String) { - self.atoms.push(AtomTextEdit::replace(range, replace_with)) - } - pub fn delete(&mut self, range: TextRange) { - self.atoms.push(AtomTextEdit::delete(range)) - } - pub fn insert(&mut self, offset: TextSize, text: String) { - self.atoms.push(AtomTextEdit::insert(offset, text)) - } - pub fn finish(self) -> TextEdit { - TextEdit::from_atoms(self.atoms) - } - pub fn invalidates_offset(&self, offset: TextSize) -> bool { - self.atoms.iter().any(|atom| atom.delete.contains_inclusive(offset)) - } -} - -impl TextEdit { - pub fn insert(offset: TextSize, text: String) -> TextEdit { - let mut builder = TextEditBuilder::default(); - builder.insert(offset, text); - builder.finish() - } - - pub fn delete(range: TextRange) -> TextEdit { - let mut builder = TextEditBuilder::default(); - builder.delete(range); - builder.finish() - } - - pub fn replace(range: TextRange, replace_with: String) -> TextEdit { - let mut builder = TextEditBuilder::default(); - builder.replace(range, replace_with); - builder.finish() - } - - pub(crate) fn from_atoms(mut atoms: Vec) -> TextEdit { - atoms.sort_by_key(|a| (a.delete.start(), a.delete.end())); - for (a1, a2) in atoms.iter().zip(atoms.iter().skip(1)) { - assert!(a1.delete.end() <= a2.delete.start()) - } - TextEdit { atoms } - } - - pub fn as_atoms(&self) -> &[AtomTextEdit] { - &self.atoms - } - - pub fn apply(&self, text: &str) -> String { - let mut total_len = TextSize::of(text); - for atom in self.atoms.iter() { - total_len += TextSize::of(&atom.insert); - total_len -= atom.delete.end() - atom.delete.start(); - } - let mut buf = String::with_capacity(total_len.into()); - let mut prev = 0; - for atom in self.atoms.iter() { - let start: usize = atom.delete.start().into(); - let end: usize = atom.delete.end().into(); - if start > prev { - buf.push_str(&text[prev..start]); - } - buf.push_str(&atom.insert); - prev = end; - } - buf.push_str(&text[prev..text.len()]); - assert_eq!(TextSize::of(&buf), total_len); - buf - } - - pub fn apply_to_offset(&self, offset: TextSize) -> Option { - let mut res = offset; - for atom in self.atoms.iter() { - if atom.delete.start() >= offset { - break; - } - if offset < atom.delete.end() { - return None; - } - res += TextSize::of(&atom.insert); - res -= atom.delete.len(); - } - Some(res) - } -} diff --git a/crates/rust-analyzer/src/conv.rs b/crates/rust-analyzer/src/conv.rs index 7be5ebcdb5..f64c90b5b1 100644 --- a/crates/rust-analyzer/src/conv.rs +++ b/crates/rust-analyzer/src/conv.rs @@ -15,7 +15,7 @@ use ra_ide::{ ReferenceAccess, Severity, SourceChange, SourceFileEdit, }; use ra_syntax::{SyntaxKind, TextRange, TextSize}; -use ra_text_edit::{AtomTextEdit, TextEdit}; +use ra_text_edit::{Indel, TextEdit}; use ra_vfs::LineEndings; use crate::{ @@ -124,23 +124,22 @@ impl ConvWith<(&LineIndex, LineEndings)> for CompletionItem { let mut text_edit = None; // LSP does not allow arbitrary edits in completion, so we have to do a // non-trivial mapping here. - for atom_edit in self.text_edit().as_atoms() { - if atom_edit.delete.contains_range(self.source_range()) { - text_edit = Some(if atom_edit.delete == self.source_range() { - atom_edit.conv_with((ctx.0, ctx.1)) + for indel in self.text_edit().as_indels() { + if indel.delete.contains_range(self.source_range()) { + text_edit = Some(if indel.delete == self.source_range() { + indel.conv_with((ctx.0, ctx.1)) } else { - assert!(self.source_range().end() == atom_edit.delete.end()); - let range1 = - TextRange::new(atom_edit.delete.start(), self.source_range().start()); + assert!(self.source_range().end() == indel.delete.end()); + let range1 = TextRange::new(indel.delete.start(), self.source_range().start()); let range2 = self.source_range(); - let edit1 = AtomTextEdit::replace(range1, String::new()); - let edit2 = AtomTextEdit::replace(range2, atom_edit.insert.clone()); + let edit1 = Indel::replace(range1, String::new()); + let edit2 = Indel::replace(range2, indel.insert.clone()); additional_text_edits.push(edit1.conv_with((ctx.0, ctx.1))); edit2.conv_with((ctx.0, ctx.1)) }) } else { - assert!(self.source_range().intersect(atom_edit.delete).is_none()); - additional_text_edits.push(atom_edit.conv_with((ctx.0, ctx.1))); + assert!(self.source_range().intersect(indel.delete).is_none()); + additional_text_edits.push(indel.conv_with((ctx.0, ctx.1))); } } let text_edit = text_edit.unwrap(); @@ -257,11 +256,11 @@ impl ConvWith<(&LineIndex, LineEndings)> for TextEdit { type Output = Vec; fn conv_with(self, ctx: (&LineIndex, LineEndings)) -> Vec { - self.as_atoms().iter().map_conv_with(ctx).collect() + self.as_indels().iter().map_conv_with(ctx).collect() } } -impl ConvWith<(&LineIndex, LineEndings)> for &AtomTextEdit { +impl ConvWith<(&LineIndex, LineEndings)> for &Indel { type Output = lsp_types::TextEdit; fn conv_with( @@ -522,7 +521,7 @@ impl TryConvWith<&WorldSnapshot> for SourceFileEdit { let line_index = world.analysis().file_line_index(self.file_id)?; let line_endings = world.file_line_endings(self.file_id); let edits = - self.edit.as_atoms().iter().map_conv_with((&line_index, line_endings)).collect(); + self.edit.as_indels().iter().map_conv_with((&line_index, line_endings)).collect(); Ok(TextDocumentEdit { text_document, edits }) } } diff --git a/xtask/tests/tidy-tests/main.rs b/xtask/tests/tidy-tests/main.rs index ead642acc6..3213c4dfa3 100644 --- a/xtask/tests/tidy-tests/main.rs +++ b/xtask/tests/tidy-tests/main.rs @@ -115,7 +115,6 @@ impl TidyDocs { "ra_prof", "ra_project_model", "ra_syntax", - "ra_text_edit", "ra_tt", "ra_hir_ty", ]; From 27c7ef6d65ffa6a642768377d3f0ba85ac8564bf Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 5 May 2020 23:23:29 +0200 Subject: [PATCH 3/4] Use more natural signature for Edit::apply --- crates/ra_ide/src/ssr.rs | 24 ++++++++++++++--------- crates/ra_syntax/src/lib.rs | 3 ++- crates/ra_syntax/src/parsing/reparsing.rs | 14 ++++++++----- crates/ra_text_edit/src/lib.rs | 21 +++++++++++++++----- 4 files changed, 42 insertions(+), 20 deletions(-) diff --git a/crates/ra_ide/src/ssr.rs b/crates/ra_ide/src/ssr.rs index 7b93ff2d28..e213da6068 100644 --- a/crates/ra_ide/src/ssr.rs +++ b/crates/ra_ide/src/ssr.rs @@ -401,16 +401,22 @@ fn render_replace( ignored_comments: &Vec, template: &SsrTemplate, ) -> String { - let mut builder = TextEditBuilder::default(); - for element in template.template.descendants() { - if let Some(var) = template.placeholders.get(&element) { - builder.replace(element.text_range(), binding[var].to_string()) + let edit = { + let mut builder = TextEditBuilder::default(); + for element in template.template.descendants() { + if let Some(var) = template.placeholders.get(&element) { + builder.replace(element.text_range(), binding[var].to_string()) + } } - } - for comment in ignored_comments { - builder.insert(template.template.text_range().end(), comment.syntax().to_string()) - } - builder.finish().apply(&template.template.text().to_string()) + for comment in ignored_comments { + builder.insert(template.template.text_range().end(), comment.syntax().to_string()) + } + builder.finish() + }; + + let mut text = template.template.text().to_string(); + edit.apply(&mut text); + text } #[cfg(test)] diff --git a/crates/ra_syntax/src/lib.rs b/crates/ra_syntax/src/lib.rs index 1a7348dacd..61e686da5e 100644 --- a/crates/ra_syntax/src/lib.rs +++ b/crates/ra_syntax/src/lib.rs @@ -142,7 +142,8 @@ impl Parse { } fn full_reparse(&self, indel: &Indel) -> Parse { - let text = indel.apply(self.tree().syntax().text().to_string()); + let mut text = self.tree().syntax().text().to_string(); + indel.apply(&mut text); SourceFile::parse(&text) } } diff --git a/crates/ra_syntax/src/parsing/reparsing.rs b/crates/ra_syntax/src/parsing/reparsing.rs index 6257e3f338..edbc190f85 100644 --- a/crates/ra_syntax/src/parsing/reparsing.rs +++ b/crates/ra_syntax/src/parsing/reparsing.rs @@ -109,14 +109,14 @@ fn reparse_block<'node>( } fn get_text_after_edit(element: SyntaxElement, edit: &Indel) -> String { - let edit = - Indel::replace(edit.delete - element.text_range().start(), edit.insert.clone()); + let edit = Indel::replace(edit.delete - element.text_range().start(), edit.insert.clone()); - let text = match element { + let mut text = match element { NodeOrToken::Token(token) => token.text().to_string(), NodeOrToken::Node(node) => node.text().to_string(), }; - edit.apply(text) + edit.apply(&mut text); + text } fn is_contextual_kw(text: &str) -> bool { @@ -199,7 +199,11 @@ mod tests { fn do_check(before: &str, replace_with: &str, reparsed_len: u32) { let (range, before) = extract_range(before); let edit = Indel::replace(range, replace_with.to_owned()); - let after = edit.apply(before.clone()); + let after = { + let mut after = before.clone(); + edit.apply(&mut after); + after + }; let fully_reparsed = SourceFile::parse(&after); let incrementally_reparsed: Parse = { diff --git a/crates/ra_text_edit/src/lib.rs b/crates/ra_text_edit/src/lib.rs index c41bf324b6..7138bbc655 100644 --- a/crates/ra_text_edit/src/lib.rs +++ b/crates/ra_text_edit/src/lib.rs @@ -37,11 +37,10 @@ impl Indel { Indel { delete: range, insert: replace_with } } - pub fn apply(&self, mut text: String) -> String { + pub fn apply(&self, text: &mut String) { let start: usize = self.delete.start().into(); let end: usize = self.delete.end().into(); text.replace_range(start..end, &self.insert); - text } } @@ -76,8 +75,17 @@ impl TextEdit { &self.indels } - pub fn apply(&self, text: &str) -> String { - let mut total_len = TextSize::of(text); + pub fn apply(&self, text: &mut String) { + match self.indels.len() { + 0 => return, + 1 => { + self.indels[0].apply(text); + return; + } + _ => (), + } + + let mut total_len = TextSize::of(&*text); for indel in self.indels.iter() { total_len += TextSize::of(&indel.insert); total_len -= indel.delete.end() - indel.delete.start(); @@ -95,7 +103,10 @@ impl TextEdit { } buf.push_str(&text[prev..text.len()]); assert_eq!(TextSize::of(&buf), total_len); - buf + + // FIXME: figure out a way to mutate the text in-place or reuse the + // memory in some other way + *text = buf } pub fn apply_to_offset(&self, offset: TextSize) -> Option { From ca9e0f5fe9ad29ab0c5a0771a0d0eaec97e4104b Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 5 May 2020 23:48:26 +0200 Subject: [PATCH 4/4] Fixup tests --- crates/ra_assists/src/doc_tests.rs | 6 ++++- crates/ra_assists/src/lib.rs | 20 +++++++--------- crates/ra_ide/src/diagnostics.rs | 24 ++++++++++++++----- crates/ra_ide/src/join_lines.rs | 6 ++++- crates/ra_ide/src/references/rename.rs | 32 +++++++++++++------------- crates/ra_ide/src/ssr.rs | 8 +++++-- crates/ra_ide/src/test_utils.rs | 6 ++++- crates/ra_ide/src/typing.rs | 9 +++++--- crates/ra_ide/src/typing/on_enter.rs | 3 ++- 9 files changed, 71 insertions(+), 43 deletions(-) diff --git a/crates/ra_assists/src/doc_tests.rs b/crates/ra_assists/src/doc_tests.rs index c0f9bc1fbe..f627f31dcd 100644 --- a/crates/ra_assists/src/doc_tests.rs +++ b/crates/ra_assists/src/doc_tests.rs @@ -30,6 +30,10 @@ fn check(assist_id: &str, before: &str, after: &str) { ) }); - let actual = assist.action.edit.apply(&before); + let actual = { + let mut actual = before.clone(); + assist.action.edit.apply(&mut actual); + actual + }; assert_eq_text!(after, &actual); } diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index ad85f5553b..0f94f5ee89 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -199,12 +199,12 @@ mod handlers { mod helpers { use std::sync::Arc; + use hir::Semantics; use ra_db::{fixture::WithFixture, FileId, FileRange, SourceDatabaseExt}; use ra_ide_db::{symbol_index::SymbolsDatabase, RootDatabase}; use test_utils::{add_cursor, assert_eq_text, extract_range_or_offset, RangeOrOffset}; - use crate::{AssistCtx, AssistFile, AssistHandler}; - use hir::Semantics; + use crate::{handlers::Handler, AssistCtx, AssistFile}; pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) { let (mut db, file_id) = RootDatabase::with_single_file(text); @@ -214,22 +214,18 @@ mod helpers { (db, file_id) } - pub(crate) fn check_assist( - assist: AssistHandler, - ra_fixture_before: &str, - ra_fixture_after: &str, - ) { + pub(crate) fn check_assist(assist: Handler, ra_fixture_before: &str, ra_fixture_after: &str) { check(assist, ra_fixture_before, ExpectedResult::After(ra_fixture_after)); } // FIXME: instead of having a separate function here, maybe use // `extract_ranges` and mark the target as ` ` in the // fixuture? - pub(crate) fn check_assist_target(assist: AssistHandler, ra_fixture: &str, target: &str) { + pub(crate) fn check_assist_target(assist: Handler, ra_fixture: &str, target: &str) { check(assist, ra_fixture, ExpectedResult::Target(target)); } - pub(crate) fn check_assist_not_applicable(assist: AssistHandler, ra_fixture: &str) { + pub(crate) fn check_assist_not_applicable(assist: Handler, ra_fixture: &str) { check(assist, ra_fixture, ExpectedResult::NotApplicable); } @@ -239,7 +235,7 @@ mod helpers { Target(&'a str), } - fn check(assist: AssistHandler, before: &str, expected: ExpectedResult) { + fn check(assist: Handler, before: &str, expected: ExpectedResult) { let (text_without_caret, file_with_caret_id, range_or_offset, db) = if before.contains("//-") { let (mut db, position) = RootDatabase::with_position(before); @@ -265,13 +261,13 @@ mod helpers { (Some(assist), ExpectedResult::After(after)) => { let action = assist.0[0].action.clone().unwrap(); - let assisted_file_text = if let AssistFile::TargetFile(file_id) = action.file { + let mut actual = if let AssistFile::TargetFile(file_id) = action.file { db.file_text(file_id).as_ref().to_owned() } else { text_without_caret }; + action.edit.apply(&mut actual); - let mut actual = action.edit.apply(&assisted_file_text); match action.cursor_position { None => { if let RangeOrOffset::Offset(before_cursor_pos) = range_or_offset { diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 4c04cee078..87a0b80f13 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -241,7 +241,11 @@ mod tests { diagnostics.pop().unwrap_or_else(|| panic!("no diagnostics for:\n{}\n", before)); let mut fix = diagnostic.fix.unwrap(); let edit = fix.source_file_edits.pop().unwrap().edit; - let actual = edit.apply(&before); + let actual = { + let mut actual = before.to_string(); + edit.apply(&mut actual); + actual + }; assert_eq_text!(after, &actual); } @@ -256,7 +260,11 @@ mod tests { let mut fix = diagnostic.fix.unwrap(); let edit = fix.source_file_edits.pop().unwrap().edit; let target_file_contents = analysis.file_text(file_position.file_id).unwrap(); - let actual = edit.apply(&target_file_contents); + let actual = { + let mut actual = target_file_contents.to_string(); + edit.apply(&mut actual); + actual + }; // Strip indent and empty lines from `after`, to match the behaviour of // `parse_fixture` called from `analysis_and_position`. @@ -288,7 +296,11 @@ mod tests { let diagnostic = analysis.diagnostics(file_id).unwrap().pop().unwrap(); let mut fix = diagnostic.fix.unwrap(); let edit = fix.source_file_edits.pop().unwrap().edit; - let actual = edit.apply(&before); + let actual = { + let mut actual = before.to_string(); + edit.apply(&mut actual); + actual + }; assert_eq_text!(after, &actual); } @@ -662,10 +674,10 @@ mod tests { 1, ), edit: TextEdit { - atoms: [ - AtomTextEdit { - delete: 3..9, + indels: [ + Indel { insert: "{a:42, b: ()}", + delete: 3..9, }, ], }, diff --git a/crates/ra_ide/src/join_lines.rs b/crates/ra_ide/src/join_lines.rs index 63fd6b3e45..d3af780c45 100644 --- a/crates/ra_ide/src/join_lines.rs +++ b/crates/ra_ide/src/join_lines.rs @@ -569,7 +569,11 @@ fn foo() { let (sel, before) = extract_range(before); let parse = SourceFile::parse(&before); let result = join_lines(&parse.tree(), sel); - let actual = result.apply(&before); + let actual = { + let mut actual = before.to_string(); + result.apply(&mut actual); + actual + }; assert_eq_text!(after, &actual); } diff --git a/crates/ra_ide/src/references/rename.rs b/crates/ra_ide/src/references/rename.rs index 52e55b0a08..0398d53bc9 100644 --- a/crates/ra_ide/src/references/rename.rs +++ b/crates/ra_ide/src/references/rename.rs @@ -537,10 +537,10 @@ mod tests { 2, ), edit: TextEdit { - atoms: [ - AtomTextEdit { - delete: 4..7, + indels: [ + Indel { insert: "foo2", + delete: 4..7, }, ], }, @@ -589,10 +589,10 @@ mod tests { 1, ), edit: TextEdit { - atoms: [ - AtomTextEdit { - delete: 4..7, + indels: [ + Indel { insert: "foo2", + delete: 4..7, }, ], }, @@ -672,10 +672,10 @@ mod tests { 2, ), edit: TextEdit { - atoms: [ - AtomTextEdit { - delete: 8..11, + indels: [ + Indel { insert: "foo2", + delete: 8..11, }, ], }, @@ -685,10 +685,10 @@ mod tests { 1, ), edit: TextEdit { - atoms: [ - AtomTextEdit { - delete: 27..30, + indels: [ + Indel { insert: "foo2", + delete: 27..30, }, ], }, @@ -720,13 +720,13 @@ mod tests { if let Some(change) = source_change { for edit in change.info.source_file_edits { file_id = Some(edit.file_id); - for atom in edit.edit.as_atoms() { - text_edit_builder.replace(atom.delete, atom.insert.clone()); + for indel in edit.edit.as_indels() { + text_edit_builder.replace(indel.delete, indel.insert.clone()); } } } - let result = - text_edit_builder.finish().apply(&*analysis.file_text(file_id.unwrap()).unwrap()); + let mut result = analysis.file_text(file_id.unwrap()).unwrap().to_string(); + text_edit_builder.finish().apply(&mut result); assert_eq_text!(expected, &*result); } } diff --git a/crates/ra_ide/src/ssr.rs b/crates/ra_ide/src/ssr.rs index e213da6068..8bf52d0fa8 100644 --- a/crates/ra_ide/src/ssr.rs +++ b/crates/ra_ide/src/ssr.rs @@ -511,7 +511,9 @@ mod tests { ); let edit = replace(&matches, &query.template); - assert_eq!(edit.apply(input), "fn main() { bar(1+2); }"); + let mut after = input.to_string(); + edit.apply(&mut after); + assert_eq!(after, "fn main() { bar(1+2); }"); } fn assert_ssr_transform(query: &str, input: &str, result: &str) { @@ -519,7 +521,9 @@ mod tests { let code = SourceFile::parse(input).tree(); let matches = find(&query.pattern, code.syntax()); let edit = replace(&matches, &query.template); - assert_eq!(edit.apply(input), result); + let mut after = input.to_string(); + edit.apply(&mut after); + assert_eq!(after, result); } #[test] diff --git a/crates/ra_ide/src/test_utils.rs b/crates/ra_ide/src/test_utils.rs index f14533e14b..48c8fd1f46 100644 --- a/crates/ra_ide/src/test_utils.rs +++ b/crates/ra_ide/src/test_utils.rs @@ -13,7 +13,11 @@ pub fn check_action Option>( let (before_cursor_pos, before) = extract_offset(before); let file = SourceFile::parse(&before).ok().unwrap(); let result = f(&file, before_cursor_pos).expect("code action is not applicable"); - let actual = result.apply(&before); + let actual = { + let mut actual = before.to_string(); + result.apply(&mut actual); + actual + }; let actual_cursor_pos = result.apply_to_offset(before_cursor_pos).expect("cursor position is affected by the edit"); let actual = add_cursor(&actual, actual_cursor_pos); diff --git a/crates/ra_ide/src/typing.rs b/crates/ra_ide/src/typing.rs index 2a8b4327f9..a03da46938 100644 --- a/crates/ra_ide/src/typing.rs +++ b/crates/ra_ide/src/typing.rs @@ -142,10 +142,13 @@ mod tests { fn do_type_char(char_typed: char, before: &str) -> Option<(String, SingleFileChange)> { let (offset, before) = extract_offset(before); let edit = TextEdit::insert(offset, char_typed.to_string()); - let before = edit.apply(&before); + let mut before = before.to_string(); + edit.apply(&mut before); let parse = SourceFile::parse(&before); - on_char_typed_inner(&parse.tree(), offset, char_typed) - .map(|it| (it.edit.apply(&before), it)) + on_char_typed_inner(&parse.tree(), offset, char_typed).map(|it| { + it.edit.apply(&mut before); + (before.to_string(), it) + }) } fn type_char(char_typed: char, before: &str, after: &str) { diff --git a/crates/ra_ide/src/typing/on_enter.rs b/crates/ra_ide/src/typing/on_enter.rs index 7252374641..78a40cc94c 100644 --- a/crates/ra_ide/src/typing/on_enter.rs +++ b/crates/ra_ide/src/typing/on_enter.rs @@ -96,7 +96,8 @@ mod tests { let result = analysis.on_enter(FilePosition { offset, file_id }).unwrap()?; assert_eq!(result.source_file_edits.len(), 1); - let actual = result.source_file_edits[0].edit.apply(&before); + let mut actual = before.to_string(); + result.source_file_edits[0].edit.apply(&mut actual); let actual = add_cursor(&actual, result.cursor_position.unwrap().offset); Some(actual) }