From b298fdfc1d714c82d358e82180b659ab5112eb0c Mon Sep 17 00:00:00 2001 From: Tarek Date: Mon, 14 Oct 2024 02:24:15 +0300 Subject: [PATCH 1/5] feat: migrate merge_import to use SyntaxFactory Signed-off-by: Tarek --- .../ide-assists/src/handlers/merge_imports.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/crates/ide-assists/src/handlers/merge_imports.rs b/crates/ide-assists/src/handlers/merge_imports.rs index 417123083690..804702b24e68 100644 --- a/crates/ide-assists/src/handlers/merge_imports.rs +++ b/crates/ide-assists/src/handlers/merge_imports.rs @@ -6,8 +6,8 @@ use ide_db::imports::{ use itertools::Itertools; use syntax::{ algo::neighbor, - ast::{self, edit_in_place::Removable}, - match_ast, ted, AstNode, SyntaxElement, SyntaxNode, + ast::{self, edit_in_place::Removable, syntax_factory::SyntaxFactory}, + match_ast, AstNode, SyntaxElement, SyntaxNode, }; use crate::{ @@ -68,11 +68,19 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio (selection_range, edits?) }; + // FIXME: Is this the best to get a `SyntaxNode` object? We need one for `SourceChangeBuilder::make_editor`. + let parent_node = match ctx.covering_element() { + SyntaxElement::Node(n) => n, + SyntaxElement::Token(t) => t.parent()?, + }; + let make = SyntaxFactory::new(); + acc.add( AssistId("merge_imports", AssistKind::RefactorRewrite), "Merge imports", target, |builder| { + let mut editor = builder.make_editor(&parent_node); let edits_mut: Vec = edits .into_iter() .map(|it| match it { @@ -85,7 +93,7 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio match edit { Remove(it) => it.as_ref().either(Removable::remove, Removable::remove), Replace(old, new) => { - ted::replace(old, &new); + editor.replace(old, &new); // If there's a selection and we're replacing a use tree in a tree list, // normalize the parent use tree if it only contains the merged subtree. @@ -109,12 +117,14 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio }); if let Some((old_tree, new_tree)) = normalized_use_tree { cov_mark::hit!(replace_parent_with_normalized_use_tree); - ted::replace(old_tree.syntax(), new_tree.syntax()); + editor.replace(old_tree.syntax(), new_tree.syntax()); } } } } } + editor.add_mappings(make.finish_with_mappings()); + builder.add_file_edits(ctx.file_id(), editor); }, ) } From e511e182f0050d92110900315c565ed0876575e9 Mon Sep 17 00:00:00 2001 From: Tarek Date: Wed, 30 Oct 2024 16:34:41 +0300 Subject: [PATCH 2/5] feat: define EditorRemovable trait Signed-off-by: Tarek --- .../ide-assists/src/handlers/merge_imports.rs | 21 +++--- crates/syntax/src/ast/edit_in_place.rs | 72 ++++++++++++++++++- 2 files changed, 79 insertions(+), 14 deletions(-) diff --git a/crates/ide-assists/src/handlers/merge_imports.rs b/crates/ide-assists/src/handlers/merge_imports.rs index 804702b24e68..b65be5bb0766 100644 --- a/crates/ide-assists/src/handlers/merge_imports.rs +++ b/crates/ide-assists/src/handlers/merge_imports.rs @@ -6,7 +6,7 @@ use ide_db::imports::{ use itertools::Itertools; use syntax::{ algo::neighbor, - ast::{self, edit_in_place::Removable, syntax_factory::SyntaxFactory}, + ast::{self, edit_in_place::EditorRemovable, syntax_factory::SyntaxFactory}, match_ast, AstNode, SyntaxElement, SyntaxNode, }; @@ -81,17 +81,16 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio target, |builder| { let mut editor = builder.make_editor(&parent_node); - let edits_mut: Vec = edits - .into_iter() - .map(|it| match it { - Remove(Either::Left(it)) => Remove(Either::Left(builder.make_mut(it))), - Remove(Either::Right(it)) => Remove(Either::Right(builder.make_mut(it))), - Replace(old, new) => Replace(builder.make_syntax_mut(old), new), - }) - .collect(); - for edit in edits_mut { + for edit in edits { match edit { - Remove(it) => it.as_ref().either(Removable::remove, Removable::remove), + Remove(it) => { + let node = it.as_ref(); + if let Some(left) = node.left() { + left.remove(&mut editor); + } else if let Some(right) = node.right() { + right.remove(&mut editor); + } + } Replace(old, new) => { editor.replace(old, &new); diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index 291fc646e216..01abb28b0d74 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -6,7 +6,11 @@ use parser::{SyntaxKind, T}; use crate::{ algo::{self, neighbor}, - ast::{self, edit::IndentLevel, make, HasGenericArgs, HasGenericParams}, + ast::{ + self, edit::IndentLevel, make, syntax_factory::SyntaxFactory, HasGenericArgs, + HasGenericParams, + }, + syntax_editor::SyntaxEditor, ted::{self, Position}, AstNode, AstToken, Direction, SyntaxElement, SyntaxKind::{ATTR, COMMENT, WHITESPACE}, @@ -385,6 +389,10 @@ pub trait Removable: AstNode { fn remove(&self); } +pub trait EditorRemovable: AstNode { + fn remove(&self, editor: &mut SyntaxEditor); +} + impl Removable for ast::TypeBoundList { fn remove(&self) { match self.syntax().siblings_with_tokens(Direction::Prev).find(|it| it.kind() == T![:]) { @@ -439,16 +447,35 @@ impl Removable for ast::UseTree { } } +impl EditorRemovable for ast::UseTree { + fn remove(&self, editor: &mut SyntaxEditor) { + for dir in [Direction::Next, Direction::Prev] { + if let Some(next_use_tree) = neighbor(self, dir) { + let separators = self + .syntax() + .siblings_with_tokens(dir) + .skip(1) + .take_while(|it| it.as_node() != Some(next_use_tree.syntax())); + for sep in separators { + editor.delete(sep); + } + break; + } + } + editor.delete(self.syntax()); + } +} + impl ast::UseTree { /// Deletes the usetree node represented by the input. Recursively removes parents, including use nodes that become empty. pub fn remove_recursive(self) { let parent = self.syntax().parent(); - self.remove(); + Removable::remove(&self); if let Some(u) = parent.clone().and_then(ast::Use::cast) { if u.use_tree().is_none() { - u.remove(); + Removable::remove(&u); } } else if let Some(u) = parent.and_then(ast::UseTreeList::cast) { if u.use_trees().next().is_none() { @@ -616,6 +643,45 @@ impl Removable for ast::Use { } } +impl EditorRemovable for ast::Use { + fn remove(&self, editor: &mut SyntaxEditor) { + let make = SyntaxFactory::new(); + + let next_ws = self + .syntax() + .next_sibling_or_token() + .and_then(|it| it.into_token()) + .and_then(ast::Whitespace::cast); + if let Some(next_ws) = next_ws { + let ws_text = next_ws.syntax().text(); + if let Some(rest) = ws_text.strip_prefix('\n') { + if rest.is_empty() { + editor.delete(next_ws.syntax()); + } else { + editor.replace(next_ws.syntax(), make.whitespace(rest)); + } + } + } + let prev_ws = self + .syntax() + .prev_sibling_or_token() + .and_then(|it| it.into_token()) + .and_then(ast::Whitespace::cast); + if let Some(prev_ws) = prev_ws { + let ws_text = prev_ws.syntax().text(); + let prev_newline = ws_text.rfind('\n').map(|x| x + 1).unwrap_or(0); + let rest = &ws_text[0..prev_newline]; + if rest.is_empty() { + editor.delete(prev_ws.syntax()); + } else { + editor.replace(prev_ws.syntax(), make.whitespace(rest)); + } + } + + editor.delete(self.syntax()); + } +} + impl ast::Impl { pub fn get_or_create_assoc_item_list(&self) -> ast::AssocItemList { if self.assoc_item_list().is_none() { From 59cfb84379727413e15656b65c0c8564b6c3d213 Mon Sep 17 00:00:00 2001 From: Tarek Date: Sat, 7 Dec 2024 22:51:39 +0200 Subject: [PATCH 3/5] refactor: move EditorRemovable trait to syntax_editor Signed-off-by: Tarek --- .../ide-assists/src/handlers/merge_imports.rs | 7 +- crates/syntax/src/ast/edit_in_place.rs | 72 +------------------ crates/syntax/src/syntax_editor.rs | 2 +- 3 files changed, 8 insertions(+), 73 deletions(-) diff --git a/crates/ide-assists/src/handlers/merge_imports.rs b/crates/ide-assists/src/handlers/merge_imports.rs index b65be5bb0766..7eae4359e967 100644 --- a/crates/ide-assists/src/handlers/merge_imports.rs +++ b/crates/ide-assists/src/handlers/merge_imports.rs @@ -6,8 +6,10 @@ use ide_db::imports::{ use itertools::Itertools; use syntax::{ algo::neighbor, - ast::{self, edit_in_place::EditorRemovable, syntax_factory::SyntaxFactory}, - match_ast, AstNode, SyntaxElement, SyntaxNode, + ast::{self, syntax_factory::SyntaxFactory}, + match_ast, + syntax_editor::edits::Removable, + AstNode, SyntaxElement, SyntaxNode, }; use crate::{ @@ -68,7 +70,6 @@ pub(crate) fn merge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Optio (selection_range, edits?) }; - // FIXME: Is this the best to get a `SyntaxNode` object? We need one for `SourceChangeBuilder::make_editor`. let parent_node = match ctx.covering_element() { SyntaxElement::Node(n) => n, SyntaxElement::Token(t) => t.parent()?, diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index 01abb28b0d74..7b09662022e3 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -6,11 +6,7 @@ use parser::{SyntaxKind, T}; use crate::{ algo::{self, neighbor}, - ast::{ - self, edit::IndentLevel, make, syntax_factory::SyntaxFactory, HasGenericArgs, - HasGenericParams, - }, - syntax_editor::SyntaxEditor, + ast::{self, edit::IndentLevel, make, HasGenericArgs, HasGenericParams}, ted::{self, Position}, AstNode, AstToken, Direction, SyntaxElement, SyntaxKind::{ATTR, COMMENT, WHITESPACE}, @@ -389,10 +385,6 @@ pub trait Removable: AstNode { fn remove(&self); } -pub trait EditorRemovable: AstNode { - fn remove(&self, editor: &mut SyntaxEditor); -} - impl Removable for ast::TypeBoundList { fn remove(&self) { match self.syntax().siblings_with_tokens(Direction::Prev).find(|it| it.kind() == T![:]) { @@ -447,35 +439,16 @@ impl Removable for ast::UseTree { } } -impl EditorRemovable for ast::UseTree { - fn remove(&self, editor: &mut SyntaxEditor) { - for dir in [Direction::Next, Direction::Prev] { - if let Some(next_use_tree) = neighbor(self, dir) { - let separators = self - .syntax() - .siblings_with_tokens(dir) - .skip(1) - .take_while(|it| it.as_node() != Some(next_use_tree.syntax())); - for sep in separators { - editor.delete(sep); - } - break; - } - } - editor.delete(self.syntax()); - } -} - impl ast::UseTree { /// Deletes the usetree node represented by the input. Recursively removes parents, including use nodes that become empty. pub fn remove_recursive(self) { let parent = self.syntax().parent(); - Removable::remove(&self); + self.remove(); if let Some(u) = parent.clone().and_then(ast::Use::cast) { if u.use_tree().is_none() { - Removable::remove(&u); + u.remove() } } else if let Some(u) = parent.and_then(ast::UseTreeList::cast) { if u.use_trees().next().is_none() { @@ -643,45 +616,6 @@ impl Removable for ast::Use { } } -impl EditorRemovable for ast::Use { - fn remove(&self, editor: &mut SyntaxEditor) { - let make = SyntaxFactory::new(); - - let next_ws = self - .syntax() - .next_sibling_or_token() - .and_then(|it| it.into_token()) - .and_then(ast::Whitespace::cast); - if let Some(next_ws) = next_ws { - let ws_text = next_ws.syntax().text(); - if let Some(rest) = ws_text.strip_prefix('\n') { - if rest.is_empty() { - editor.delete(next_ws.syntax()); - } else { - editor.replace(next_ws.syntax(), make.whitespace(rest)); - } - } - } - let prev_ws = self - .syntax() - .prev_sibling_or_token() - .and_then(|it| it.into_token()) - .and_then(ast::Whitespace::cast); - if let Some(prev_ws) = prev_ws { - let ws_text = prev_ws.syntax().text(); - let prev_newline = ws_text.rfind('\n').map(|x| x + 1).unwrap_or(0); - let rest = &ws_text[0..prev_newline]; - if rest.is_empty() { - editor.delete(prev_ws.syntax()); - } else { - editor.replace(prev_ws.syntax(), make.whitespace(rest)); - } - } - - editor.delete(self.syntax()); - } -} - impl ast::Impl { pub fn get_or_create_assoc_item_list(&self) -> ast::AssocItemList { if self.assoc_item_list().is_none() { diff --git a/crates/syntax/src/syntax_editor.rs b/crates/syntax/src/syntax_editor.rs index b82181ae13ad..95a5f76e749c 100644 --- a/crates/syntax/src/syntax_editor.rs +++ b/crates/syntax/src/syntax_editor.rs @@ -16,7 +16,7 @@ use rustc_hash::FxHashMap; use crate::{SyntaxElement, SyntaxNode, SyntaxToken}; mod edit_algo; -mod edits; +pub mod edits; mod mapping; pub use mapping::{SyntaxMapping, SyntaxMappingBuilder}; From 52dd11ee03a4141bd3a99b81fc4d042bc59b5992 Mon Sep 17 00:00:00 2001 From: Tarek Date: Sat, 7 Dec 2024 22:52:54 +0200 Subject: [PATCH 4/5] fix: add missing semicolon in remove method call Signed-off-by: Tarek --- crates/syntax/src/ast/edit_in_place.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index 7b09662022e3..291fc646e216 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -448,7 +448,7 @@ impl ast::UseTree { if let Some(u) = parent.clone().and_then(ast::Use::cast) { if u.use_tree().is_none() { - u.remove() + u.remove(); } } else if let Some(u) = parent.and_then(ast::UseTreeList::cast) { if u.use_trees().next().is_none() { From b9275f1f318e19cc927e06d13acc8ec6536a8591 Mon Sep 17 00:00:00 2001 From: Tarek Date: Sat, 11 Jan 2025 09:33:40 -0800 Subject: [PATCH 5/5] feat: implement Removable trait for ast::Use and ast::UseTree Signed-off-by: Tarek --- crates/syntax/src/syntax_editor/edits.rs | 67 +++++++++++++++++++++++- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/crates/syntax/src/syntax_editor/edits.rs b/crates/syntax/src/syntax_editor/edits.rs index 450d601615ee..62479fb990ad 100644 --- a/crates/syntax/src/syntax_editor/edits.rs +++ b/crates/syntax/src/syntax_editor/edits.rs @@ -1,9 +1,10 @@ //! Structural editing for ast using `SyntaxEditor` use crate::{ + algo::neighbor, ast::{ - self, edit::IndentLevel, make, syntax_factory::SyntaxFactory, AstNode, Fn, GenericParam, - HasGenericParams, HasName, + self, edit::IndentLevel, make, syntax_factory::SyntaxFactory, AstNode, AstToken, Fn, + GenericParam, HasGenericParams, HasName, }, syntax_editor::{Position, SyntaxEditor}, Direction, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, T, @@ -143,6 +144,68 @@ fn normalize_ws_between_braces(editor: &mut SyntaxEditor, node: &SyntaxNode) -> Some(()) } +pub trait Removable: AstNode { + fn remove(&self, editor: &mut SyntaxEditor); +} + +impl Removable for ast::Use { + fn remove(&self, editor: &mut SyntaxEditor) { + let make = SyntaxFactory::new(); + + let next_ws = self + .syntax() + .next_sibling_or_token() + .and_then(|it| it.into_token()) + .and_then(ast::Whitespace::cast); + if let Some(next_ws) = next_ws { + let ws_text = next_ws.syntax().text(); + if let Some(rest) = ws_text.strip_prefix('\n') { + if rest.is_empty() { + editor.delete(next_ws.syntax()); + } else { + editor.replace(next_ws.syntax(), make.whitespace(rest)); + } + } + } + let prev_ws = self + .syntax() + .prev_sibling_or_token() + .and_then(|it| it.into_token()) + .and_then(ast::Whitespace::cast); + if let Some(prev_ws) = prev_ws { + let ws_text = prev_ws.syntax().text(); + let prev_newline = ws_text.rfind('\n').map(|x| x + 1).unwrap_or(0); + let rest = &ws_text[0..prev_newline]; + if rest.is_empty() { + editor.delete(prev_ws.syntax()); + } else { + editor.replace(prev_ws.syntax(), make.whitespace(rest)); + } + } + + editor.delete(self.syntax()); + } +} + +impl Removable for ast::UseTree { + fn remove(&self, editor: &mut SyntaxEditor) { + for dir in [Direction::Next, Direction::Prev] { + if let Some(next_use_tree) = neighbor(self, dir) { + let separators = self + .syntax() + .siblings_with_tokens(dir) + .skip(1) + .take_while(|it| it.as_node() != Some(next_use_tree.syntax())); + for sep in separators { + editor.delete(sep); + } + break; + } + } + editor.delete(self.syntax()); + } +} + #[cfg(test)] mod tests { use parser::Edition;