From d3681726b46641ae716517be5844dff60c49eb20 Mon Sep 17 00:00:00 2001 From: Prajwal S N Date: Fri, 28 Mar 2025 03:15:10 +0530 Subject: [PATCH 1/2] fix: migrate `unmerge_use` to syntax editor Also ensures that attributes on the use item are applied to the new use item when unmerging. Signed-off-by: Prajwal S N --- .../ide-assists/src/handlers/unmerge_use.rs | 59 ++++++++++++++----- .../src/ast/syntax_factory/constructors.rs | 14 +++++ crates/syntax/src/syntax_editor.rs | 1 + crates/syntax/src/syntax_editor/edits.rs | 50 +++++++++++++++- 4 files changed, 108 insertions(+), 16 deletions(-) diff --git a/crates/ide-assists/src/handlers/unmerge_use.rs b/crates/ide-assists/src/handlers/unmerge_use.rs index 805a7344494a..fc26efb27867 100644 --- a/crates/ide-assists/src/handlers/unmerge_use.rs +++ b/crates/ide-assists/src/handlers/unmerge_use.rs @@ -1,7 +1,10 @@ use syntax::{ AstNode, SyntaxKind, - ast::{self, HasVisibility, edit_in_place::Removable, make}, - ted::{self, Position}, + ast::{ + self, HasAttrs, HasVisibility, edit::IndentLevel, edit_in_place::AttrsOwnerEdit, make, + syntax_factory::SyntaxFactory, + }, + syntax_editor::{Element, Position, Removable}, }; use crate::{ @@ -22,7 +25,7 @@ use crate::{ // use std::fmt::Display; // ``` pub(crate) fn unmerge_use(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { - let tree: ast::UseTree = ctx.find_node_at_offset::()?.clone_for_update(); + let tree = ctx.find_node_at_offset::()?; let tree_list = tree.syntax().parent().and_then(ast::UseTreeList::cast)?; if tree_list.use_trees().count() < 2 { @@ -30,12 +33,9 @@ pub(crate) fn unmerge_use(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option< return None; } - let use_: ast::Use = tree_list.syntax().ancestors().find_map(ast::Use::cast)?; + let use_ = tree_list.syntax().ancestors().find_map(ast::Use::cast)?; let path = resolve_full_path(&tree)?; - let old_parent_range = use_.syntax().parent()?.text_range(); - let new_parent = use_.syntax().parent()?; - // If possible, explain what is going to be done. let label = match tree.path().and_then(|path| path.first_segment()) { Some(name) => format!("Unmerge use of `{name}`"), @@ -44,16 +44,30 @@ pub(crate) fn unmerge_use(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option< let target = tree.syntax().text_range(); acc.add(AssistId::refactor_rewrite("unmerge_use"), label, target, |builder| { - let new_use = make::use_( + let make = SyntaxFactory::new(); + let new_use = make.use_( use_.visibility(), - make::use_tree(path, tree.use_tree_list(), tree.rename(), tree.star_token().is_some()), - ) - .clone_for_update(); - - tree.remove(); - ted::insert(Position::after(use_.syntax()), new_use.syntax()); + make.use_tree(path, tree.use_tree_list(), tree.rename(), tree.star_token().is_some()), + ); + // Add any attributes that are present on the use tree + use_.attrs().for_each(|attr| { + new_use.add_attr(attr.clone_for_update()); + }); - builder.replace(old_parent_range, new_parent.to_string()); + let mut editor = builder.make_editor(use_.syntax()); + // Remove the use tree from the current use item + tree.remove(&mut editor); + // Insert a newline and indentation, followed by the new use item + editor.insert_all( + Position::after(use_.syntax()), + vec![ + make.whitespace(&format!("\n{}", IndentLevel::from_node(use_.syntax()))) + .syntax_element(), + new_use.syntax().syntax_element(), + ], + ); + editor.add_mappings(make.finish_with_mappings()); + builder.add_file_edits(ctx.file_id(), editor); }) } @@ -230,4 +244,19 @@ pub use std::fmt::Display; use std::process;", ); } + + #[test] + fn unmerge_use_item_with_attributes() { + check_assist( + unmerge_use, + r" +#[allow(deprecated)] +use foo::{bar, baz$0};", + r" +#[allow(deprecated)] +use foo::{bar}; +#[allow(deprecated)] +use foo::baz;", + ); + } } diff --git a/crates/syntax/src/ast/syntax_factory/constructors.rs b/crates/syntax/src/ast/syntax_factory/constructors.rs index f9dadf4b2c6a..a5c15881c557 100644 --- a/crates/syntax/src/ast/syntax_factory/constructors.rs +++ b/crates/syntax/src/ast/syntax_factory/constructors.rs @@ -107,6 +107,20 @@ impl SyntaxFactory { ast } + pub fn use_(&self, visibility: Option, use_tree: ast::UseTree) -> ast::Use { + make::use_(visibility, use_tree).clone_for_update() + } + + pub fn use_tree( + &self, + path: ast::Path, + use_tree_list: Option, + alias: Option, + add_star: bool, + ) -> ast::UseTree { + make::use_tree(path, use_tree_list, alias, add_star).clone_for_update() + } + pub fn path_unqualified(&self, segment: ast::PathSegment) -> ast::Path { let ast = make::path_unqualified(segment.clone()).clone_for_update(); diff --git a/crates/syntax/src/syntax_editor.rs b/crates/syntax/src/syntax_editor.rs index 15515dd1fe87..648bda955783 100644 --- a/crates/syntax/src/syntax_editor.rs +++ b/crates/syntax/src/syntax_editor.rs @@ -20,6 +20,7 @@ mod edit_algo; mod edits; mod mapping; +pub use edits::Removable; pub use mapping::{SyntaxMapping, SyntaxMappingBuilder}; #[derive(Debug)] diff --git a/crates/syntax/src/syntax_editor/edits.rs b/crates/syntax/src/syntax_editor/edits.rs index 350cb3e2544f..c88e174eec2f 100644 --- a/crates/syntax/src/syntax_editor/edits.rs +++ b/crates/syntax/src/syntax_editor/edits.rs @@ -1,7 +1,8 @@ //! Structural editing for ast using `SyntaxEditor` use crate::{ - Direction, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, T, + AstToken, Direction, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, T, + algo::neighbor, ast::{ self, AstNode, Fn, GenericParam, HasGenericParams, HasName, edit::IndentLevel, make, syntax_factory::SyntaxFactory, @@ -143,6 +144,53 @@ 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)); + } + } + } + + 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; From aacec6cd05aafa0d8d846560eab4d425c5926e50 Mon Sep 17 00:00:00 2001 From: Prajwal S N Date: Fri, 28 Mar 2025 15:12:21 +0530 Subject: [PATCH 2/2] chore: rename `unmerge_use` to `unmerge_imports` Signed-off-by: Prajwal S N --- .../{unmerge_use.rs => unmerge_imports.rs} | 56 +++++++++---------- crates/ide-assists/src/lib.rs | 4 +- crates/ide-assists/src/tests/generated.rs | 28 +++++----- 3 files changed, 44 insertions(+), 44 deletions(-) rename crates/ide-assists/src/handlers/{unmerge_use.rs => unmerge_imports.rs} (82%) diff --git a/crates/ide-assists/src/handlers/unmerge_use.rs b/crates/ide-assists/src/handlers/unmerge_imports.rs similarity index 82% rename from crates/ide-assists/src/handlers/unmerge_use.rs rename to crates/ide-assists/src/handlers/unmerge_imports.rs index fc26efb27867..112b94e8fc81 100644 --- a/crates/ide-assists/src/handlers/unmerge_use.rs +++ b/crates/ide-assists/src/handlers/unmerge_imports.rs @@ -12,9 +12,9 @@ use crate::{ assist_context::{AssistContext, Assists}, }; -// Assist: unmerge_use +// Assist: unmerge_imports // -// Extracts single use item from use list. +// Extracts a use item from a use list into a standalone use list. // // ``` // use std::fmt::{Debug, Display$0}; @@ -24,12 +24,12 @@ use crate::{ // use std::fmt::{Debug}; // use std::fmt::Display; // ``` -pub(crate) fn unmerge_use(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { +pub(crate) fn unmerge_imports(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option<()> { let tree = ctx.find_node_at_offset::()?; let tree_list = tree.syntax().parent().and_then(ast::UseTreeList::cast)?; if tree_list.use_trees().count() < 2 { - cov_mark::hit!(skip_single_use_item); + cov_mark::hit!(skip_single_import); return None; } @@ -43,7 +43,7 @@ pub(crate) fn unmerge_use(acc: &mut Assists, ctx: &AssistContext<'_>) -> Option< }; let target = tree.syntax().text_range(); - acc.add(AssistId::refactor_rewrite("unmerge_use"), label, target, |builder| { + acc.add(AssistId::refactor_rewrite("unmerge_imports"), label, target, |builder| { let make = SyntaxFactory::new(); let new_use = make.use_( use_.visibility(), @@ -94,22 +94,22 @@ mod tests { use super::*; #[test] - fn skip_single_use_item() { - cov_mark::check!(skip_single_use_item); + fn skip_single_import() { + cov_mark::check!(skip_single_import); check_assist_not_applicable( - unmerge_use, + unmerge_imports, r" use std::fmt::Debug$0; ", ); check_assist_not_applicable( - unmerge_use, + unmerge_imports, r" use std::fmt::{Debug$0}; ", ); check_assist_not_applicable( - unmerge_use, + unmerge_imports, r" use std::fmt::Debug as Dbg$0; ", @@ -119,7 +119,7 @@ use std::fmt::Debug as Dbg$0; #[test] fn skip_single_glob_import() { check_assist_not_applicable( - unmerge_use, + unmerge_imports, r" use std::fmt::*$0; ", @@ -127,9 +127,9 @@ use std::fmt::*$0; } #[test] - fn unmerge_use_item() { + fn unmerge_import() { check_assist( - unmerge_use, + unmerge_imports, r" use std::fmt::{Debug, Display$0}; ", @@ -140,7 +140,7 @@ use std::fmt::Display; ); check_assist( - unmerge_use, + unmerge_imports, r" use std::fmt::{Debug, format$0, Display}; ", @@ -154,7 +154,7 @@ use std::fmt::format; #[test] fn unmerge_glob_import() { check_assist( - unmerge_use, + unmerge_imports, r" use std::fmt::{*$0, Display}; ", @@ -166,9 +166,9 @@ use std::fmt::*; } #[test] - fn unmerge_renamed_use_item() { + fn unmerge_renamed_import() { check_assist( - unmerge_use, + unmerge_imports, r" use std::fmt::{Debug, Display as Disp$0}; ", @@ -180,9 +180,9 @@ use std::fmt::Display as Disp; } #[test] - fn unmerge_indented_use_item() { + fn unmerge_indented_import() { check_assist( - unmerge_use, + unmerge_imports, r" mod format { use std::fmt::{Debug, Display$0 as Disp, format}; @@ -198,9 +198,9 @@ mod format { } #[test] - fn unmerge_nested_use_item() { + fn unmerge_nested_import() { check_assist( - unmerge_use, + unmerge_imports, r" use foo::bar::{baz::{qux$0, foobar}, barbaz}; ", @@ -210,7 +210,7 @@ use foo::bar::baz::qux; ", ); check_assist( - unmerge_use, + unmerge_imports, r" use foo::bar::{baz$0::{qux, foobar}, barbaz}; ", @@ -222,9 +222,9 @@ use foo::bar::baz::{qux, foobar}; } #[test] - fn unmerge_use_item_with_visibility() { + fn unmerge_import_with_visibility() { check_assist( - unmerge_use, + unmerge_imports, r" pub use std::fmt::{Debug, Display$0}; ", @@ -236,9 +236,9 @@ pub use std::fmt::Display; } #[test] - fn unmerge_use_item_on_self() { + fn unmerge_import_on_self() { check_assist( - unmerge_use, + unmerge_imports, r"use std::process::{Command, self$0};", r"use std::process::{Command}; use std::process;", @@ -246,9 +246,9 @@ use std::process;", } #[test] - fn unmerge_use_item_with_attributes() { + fn unmerge_import_with_attributes() { check_assist( - unmerge_use, + unmerge_imports, r" #[allow(deprecated)] use foo::{bar, baz$0};", diff --git a/crates/ide-assists/src/lib.rs b/crates/ide-assists/src/lib.rs index 7e9d59661481..ac963ef3adbc 100644 --- a/crates/ide-assists/src/lib.rs +++ b/crates/ide-assists/src/lib.rs @@ -221,8 +221,8 @@ mod handlers { mod toggle_async_sugar; mod toggle_ignore; mod toggle_macro_delimiter; + mod unmerge_imports; mod unmerge_match_arm; - mod unmerge_use; mod unnecessary_async; mod unqualify_method_call; mod unwrap_block; @@ -361,7 +361,7 @@ mod handlers { toggle_ignore::toggle_ignore, toggle_macro_delimiter::toggle_macro_delimiter, unmerge_match_arm::unmerge_match_arm, - unmerge_use::unmerge_use, + unmerge_imports::unmerge_imports, unnecessary_async::unnecessary_async, unqualify_method_call::unqualify_method_call, unwrap_block::unwrap_block, diff --git a/crates/ide-assists/src/tests/generated.rs b/crates/ide-assists/src/tests/generated.rs index 359de438ed6d..8adaed9f2433 100644 --- a/crates/ide-assists/src/tests/generated.rs +++ b/crates/ide-assists/src/tests/generated.rs @@ -3320,6 +3320,20 @@ sth!{ } ) } +#[test] +fn doctest_unmerge_imports() { + check_doc_test( + "unmerge_imports", + r#####" +use std::fmt::{Debug, Display$0}; +"#####, + r#####" +use std::fmt::{Debug}; +use std::fmt::Display; +"#####, + ) +} + #[test] fn doctest_unmerge_match_arm() { check_doc_test( @@ -3346,20 +3360,6 @@ fn handle(action: Action) { ) } -#[test] -fn doctest_unmerge_use() { - check_doc_test( - "unmerge_use", - r#####" -use std::fmt::{Debug, Display$0}; -"#####, - r#####" -use std::fmt::{Debug}; -use std::fmt::Display; -"#####, - ) -} - #[test] fn doctest_unnecessary_async() { check_doc_test(