From 1cef731a8b35a140bedc648037b0870e7ebafca0 Mon Sep 17 00:00:00 2001 From: SomeFlyingThing <306498559+SomeFlyingThing@users.noreply.github.com> Date: Sun, 23 Aug 2026 21:01:12 +1200 Subject: [PATCH 1/5] Fix auto-import namespace collision merges --- .../ide-assists/src/handlers/auto_import.rs | 86 ++++++++++++++++++- crates/ide-completion/src/lib.rs | 8 +- crates/ide-completion/src/tests/flyimport.rs | 61 +++++++++++++ crates/ide-db/src/imports/insert_use.rs | 60 +++++++++++-- crates/ide-db/src/imports/merge_imports.rs | 60 +++++++++++++ 5 files changed, 264 insertions(+), 11 deletions(-) diff --git a/crates/ide-assists/src/handlers/auto_import.rs b/crates/ide-assists/src/handlers/auto_import.rs index dd082476d2d6..ea52abe69cc1 100644 --- a/crates/ide-assists/src/handlers/auto_import.rs +++ b/crates/ide-assists/src/handlers/auto_import.rs @@ -7,7 +7,10 @@ use ide_db::{ helpers::mod_path_to_ast, imports::{ import_assets::{ImportAssets, ImportCandidate, LocatedImport, TraitImportCandidate}, - insert_use::{ImportScope, insert_use_as_alias_with_editor, insert_use_with_editor}, + insert_use::{ + ImportScope, insert_use_as_alias_with_editor, + insert_use_with_editor_preserving_namespaces, + }, }, }; use syntax::{AstNode, Edition, SyntaxNode, ast, match_ast}; @@ -126,7 +129,8 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext<'_, '_>) -> Opt let add_normal_import = |acc: &mut Assists, label| { acc.add_group(&group_label, assist_id, label, range, |builder| { let editor = builder.make_editor(scope.as_syntax_node()); - insert_use_with_editor( + insert_use_with_editor_preserving_namespaces( + &ctx.sema, &scope, mod_path_to_ast(&import_path, edition), &ctx.config.insert_use, @@ -352,8 +356,8 @@ mod tests { use test_fixture::WithFixture; use crate::tests::{ - TEST_CONFIG, check_assist, check_assist_by_label, check_assist_not_applicable, - check_assist_target, + TEST_CONFIG, check_assist, check_assist_by_label, check_assist_import_one, + check_assist_not_applicable, check_assist_target, }; fn check_auto_import_order(before: &str, order: &[&str]) { @@ -553,6 +557,80 @@ mod baz { ); } + #[test] + fn preserves_value_namespace_when_merging() { + check_assist( + auto_import, + r#" +use foo::bar; + +mod foo { + pub mod bar { + pub struct Baz; + } + pub fn bar() {} +} + +fn main() { + let _: Baz$0; + bar(); +} +"#, + r#" +use foo::{bar, bar::Baz}; + +mod foo { + pub mod bar { + pub struct Baz; + } + pub fn bar() {} +} + +fn main() { + let _: Baz; + bar(); +} +"#, + ); + } + + #[test] + fn preserves_value_namespace_when_merging_all_imports() { + check_assist_import_one( + auto_import, + r#" +use foo::bar; + +mod foo { + pub mod bar { + pub struct Baz; + } + pub fn bar() {} +} + +fn main() { + let _: Baz$0; + bar(); +} +"#, + r#" +use {foo::bar, foo::bar::Baz}; + +mod foo { + pub mod bar { + pub struct Baz; + } + pub fn bar() {} +} + +fn main() { + let _: Baz; + bar(); +} +"#, + ); + } + #[test] fn applicable_when_found_an_import() { check_assist( diff --git a/crates/ide-completion/src/lib.rs b/crates/ide-completion/src/lib.rs index 4ca3257c5cba..58cb7cdd13c2 100644 --- a/crates/ide-completion/src/lib.rs +++ b/crates/ide-completion/src/lib.rs @@ -311,7 +311,13 @@ pub fn resolve_completion_edits( &editor, ); } else { - insert_use::insert_use_with_editor(&scope, full_path, &config.insert_use, &editor); + insert_use::insert_use_with_editor_preserving_namespaces( + &sema, + &scope, + full_path, + &config.insert_use, + &editor, + ); } }); diff --git a/crates/ide-completion/src/tests/flyimport.rs b/crates/ide-completion/src/tests/flyimport.rs index 8a5025caf901..918b2428ddf7 100644 --- a/crates/ide-completion/src/tests/flyimport.rs +++ b/crates/ide-completion/src/tests/flyimport.rs @@ -100,6 +100,67 @@ fn main() { ); } +#[test] +fn flyimport_preserves_value_namespace_when_merging() { + check_edit( + "Baz", + r#" +//- /lib.rs crate:dep +pub mod bar { + pub struct Baz; +} +pub fn bar() {} + +//- /main.rs crate:main deps:dep +use dep::bar; + +fn main() { + Ba$0; + bar(); +} +"#, + r#" +use dep::{bar, bar::Baz}; + +fn main() { + Baz; + bar(); +} +"#, + ); +} + +#[test] +fn flyimport_preserves_macro_namespace_when_merging() { + check_edit( + "vec!", + r#" +//- /lib.rs crate:dep +pub mod vec { + pub struct Vec; +} +#[macro_export] +macro_rules! vec { + () => {}; +} + +//- /main.rs crate:main deps:dep +use dep::vec::Vec; + +fn main() { + ve$0 +} +"#, + r#" +use dep::{vec, vec::Vec}; + +fn main() { + vec!($0) +} +"#, + ); +} + #[test] fn struct_fuzzy_completion() { check_edit( diff --git a/crates/ide-db/src/imports/insert_use.rs b/crates/ide-db/src/imports/insert_use.rs index 023538976308..a0d6e8174b86 100644 --- a/crates/ide-db/src/imports/insert_use.rs +++ b/crates/ide-db/src/imports/insert_use.rs @@ -4,7 +4,7 @@ mod tests; use std::cmp::Ordering; -use hir::Semantics; +use hir::{PathResolution, Semantics}; use syntax::{ NodeOrToken, SyntaxKind, SyntaxNode, ast::{ @@ -17,7 +17,7 @@ use crate::{ RootDatabase, imports::merge_imports::{ MergeBehavior, NormalizationStyle, common_prefix, eq_attrs, eq_visibility, - try_merge_imports, use_tree_cmp, wrap_in_tree_list, + try_merge_imports, try_merge_imports_preserving_path, use_tree_cmp, wrap_in_tree_list, }, }; @@ -157,7 +157,37 @@ pub fn insert_use_with_editor( cfg: &InsertUseConfig, syntax_editor: &SyntaxEditor, ) { - insert_use_with_alias_option_with_editor(scope, path, cfg, None, syntax_editor); + insert_use_with_alias_option_with_editor( + scope, + path, + cfg, + None, + syntax_editor, + try_merge_imports, + ); +} + +/// Inserts a use without turning a path that resolves in multiple namespaces into a nested `self`. +pub fn insert_use_with_editor_preserving_namespaces( + sema: &Semantics<'_, RootDatabase>, + scope: &ImportScope, + path: ast::Path, + cfg: &InsertUseConfig, + syntax_editor: &SyntaxEditor, +) { + insert_use_with_alias_option_with_editor( + scope, + path, + cfg, + None, + syntax_editor, + |make, lhs, rhs, merge_behavior| match namespace_collision_prefix_len(sema, lhs) { + Some(prefix_len) => { + try_merge_imports_preserving_path(make, lhs, rhs, merge_behavior, prefix_len) + } + None => try_merge_imports(make, lhs, rhs, merge_behavior), + }, + ); } pub fn insert_uses_with_editor( @@ -208,7 +238,7 @@ pub fn insert_use_as_alias_with_editor( .expect("Failed to make ast node `Rename`"); let alias = node.rename(); - insert_use_with_alias_option_with_editor(scope, path, cfg, alias, editor); + insert_use_with_alias_option_with_editor(scope, path, cfg, alias, editor, try_merge_imports); } fn insert_use_with_alias_option_with_editor( @@ -217,6 +247,12 @@ fn insert_use_with_alias_option_with_editor( cfg: &InsertUseConfig, alias: Option, syntax_editor: &SyntaxEditor, + merge_imports: impl Fn( + &syntax::ast::syntax_factory::SyntaxFactory, + &ast::Use, + &ast::Use, + MergeBehavior, + ) -> Option, ) { let make = syntax_editor.make(); let _p = tracing::info_span!("insert_use_with_alias_option").entered(); @@ -263,8 +299,7 @@ fn insert_use_with_alias_option_with_editor( for existing_use in scope.as_syntax_node().children().filter_map(ast::Use::cast).filter(filter) { - if let Some(merged) = - try_merge_imports(syntax_editor.make(), &existing_use, &use_item, mb) + if let Some(merged) = merge_imports(syntax_editor.make(), &existing_use, &use_item, mb) { syntax_editor.replace(existing_use.syntax(), merged.syntax()); return; @@ -276,6 +311,19 @@ fn insert_use_with_alias_option_with_editor( insert_use_with_editor_(scope, use_item, cfg.group, syntax_editor); } +fn namespace_collision_prefix_len( + sema: &Semantics<'_, RootDatabase>, + existing_use: &ast::Use, +) -> Option { + existing_use.syntax().descendants().filter_map(ast::Path::cast).find_map(|path| { + let resolution = sema.resolve_path_per_ns(&path)?; + let resolves_to_module = + matches!(resolution.type_ns, Some(PathResolution::Def(hir::ModuleDef::Module(_)))); + (resolves_to_module && (resolution.value_ns.is_some() || resolution.macro_ns.is_some())) + .then(|| path.segments().count()) + }) +} + pub fn remove_use_tree_if_simple(use_tree: &ast::UseTree, editor: &SyntaxEditor) { if use_tree.use_tree_list().is_some() || use_tree.star_token().is_some() { return; diff --git a/crates/ide-db/src/imports/merge_imports.rs b/crates/ide-db/src/imports/merge_imports.rs index f9251458fd02..ce548cbe014b 100644 --- a/crates/ide-db/src/imports/merge_imports.rs +++ b/crates/ide-db/src/imports/merge_imports.rs @@ -64,6 +64,66 @@ pub fn try_merge_imports( make_use_with_tree(lhs, use_tree) } +/// Merges imports without replacing the protected path with a nested `self` import. +pub(super) fn try_merge_imports_preserving_path( + make: &SyntaxFactory, + lhs: &ast::Use, + rhs: &ast::Use, + merge_behavior: MergeBehavior, + preserve_path_len: usize, +) -> Option { + if !eq_visibility(lhs.visibility(), rhs.visibility()) || !eq_attrs(lhs.attrs(), rhs.attrs()) { + return None; + } + + let lhs_tree = lhs.use_tree()?; + let rhs_tree = rhs.use_tree()?; + if merge_behavior == MergeBehavior::One { + let merge_lhs = wrap_in_tree_list(&lhs_tree, make).unwrap_or(lhs_tree); + let merge_rhs = wrap_in_tree_list(&rhs_tree, make).unwrap_or(rhs_tree); + let mut use_trees = merge_lhs + .use_tree_list()? + .use_trees() + .chain(merge_rhs.use_tree_list()?.use_trees()) + .collect::>(); + use_trees.sort_unstable_by(use_tree_cmp); + return make_use_with_tree(lhs, with_use_tree_list(&merge_lhs, use_trees, make)?); + } + if merge_behavior == MergeBehavior::Module { + return None; + } + + let lhs_path = lhs_tree.path()?; + let rhs_path = rhs_tree.path()?; + let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; + if path_len(lhs_prefix.clone()) != preserve_path_len { + return None; + } + let lhs_is_prefix = + lhs_tree.is_simple_path() && lhs_tree.rename().is_none() && lhs_path == lhs_prefix; + let rhs_is_prefix = + rhs_tree.is_simple_path() && rhs_tree.rename().is_none() && rhs_path == rhs_prefix; + if lhs_is_prefix == rhs_is_prefix { + return None; + } + + let (merge_lhs, merge_rhs) = match (lhs_prefix.qualifier(), rhs_prefix.qualifier()) { + (Some(lhs_parent), Some(rhs_parent)) => ( + split_prefix(&lhs_tree, &lhs_parent, make)?, + split_prefix(&rhs_tree, &rhs_parent, make)?, + ), + (None, None) => (wrap_in_tree_list(&lhs_tree, make)?, wrap_in_tree_list(&rhs_tree, make)?), + _ => return None, + }; + let mut use_trees = merge_lhs + .use_tree_list()? + .use_trees() + .chain(merge_rhs.use_tree_list()?.use_trees()) + .collect::>(); + use_trees.sort_unstable_by(use_tree_cmp); + make_use_with_tree(lhs, with_use_tree_list(&merge_lhs, use_trees, make)?) +} + /// Merge `rhs` into `lhs` keeping both intact. pub fn try_merge_trees( make: &SyntaxFactory, From a9ca72103f50778de44e6a613367aadc63eea123 Mon Sep 17 00:00:00 2001 From: SomeFlyingThing <306498559+SomeFlyingThing@users.noreply.github.com> Date: Mon, 24 Aug 2026 09:48:50 +1200 Subject: [PATCH 2/5] Refactor namespace-preserving import merges --- .../ide-assists/src/handlers/auto_import.rs | 6 +- .../src/handlers/convert_bool_to_enum.rs | 8 +- .../convert_tuple_return_type_to_struct.rs | 8 +- .../src/handlers/extract_function.rs | 1 + .../extract_struct_from_enum_variant.rs | 15 +- .../ide-assists/src/handlers/merge_imports.rs | 2 +- .../replace_qualified_name_with_use.rs | 8 +- .../src/handlers/unqualify_method_call.rs | 1 + crates/ide-completion/src/lib.rs | 3 +- crates/ide-db/src/imports/insert_use.rs | 56 ++----- crates/ide-db/src/imports/insert_use/tests.rs | 8 +- crates/ide-db/src/imports/merge_imports.rs | 137 +++++++++--------- .../src/handlers/json_is_not_rust.rs | 1 + 13 files changed, 131 insertions(+), 123 deletions(-) diff --git a/crates/ide-assists/src/handlers/auto_import.rs b/crates/ide-assists/src/handlers/auto_import.rs index ea52abe69cc1..15707b26f074 100644 --- a/crates/ide-assists/src/handlers/auto_import.rs +++ b/crates/ide-assists/src/handlers/auto_import.rs @@ -8,8 +8,7 @@ use ide_db::{ imports::{ import_assets::{ImportAssets, ImportCandidate, LocatedImport, TraitImportCandidate}, insert_use::{ - ImportScope, insert_use_as_alias_with_editor, - insert_use_with_editor_preserving_namespaces, + ImportScope, insert_use_as_alias_with_editor, insert_use_with_editor, }, }, }; @@ -129,7 +128,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext<'_, '_>) -> Opt let add_normal_import = |acc: &mut Assists, label| { acc.add_group(&group_label, assist_id, label, range, |builder| { let editor = builder.make_editor(scope.as_syntax_node()); - insert_use_with_editor_preserving_namespaces( + insert_use_with_editor( &ctx.sema, &scope, mod_path_to_ast(&import_path, edition), @@ -151,6 +150,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext<'_, '_>) -> Opt acc.add_group(&group_label, assist_id, label, range, |builder| { let editor = builder.make_editor(scope.as_syntax_node()); insert_use_as_alias_with_editor( + &ctx.sema, &scope, mod_path_to_ast(&import_path, edition), &ctx.config.insert_use, diff --git a/crates/ide-assists/src/handlers/convert_bool_to_enum.rs b/crates/ide-assists/src/handlers/convert_bool_to_enum.rs index 865995def013..44121f686584 100644 --- a/crates/ide-assists/src/handlers/convert_bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/convert_bool_to_enum.rs @@ -88,7 +88,13 @@ pub(crate) fn convert_bool_to_enum(acc: &mut Assists, ctx: &AssistContext<'_, '_ ); for (file_id, scope, path) in delayed_mutations { let editor = edit.make_editor(scope.as_syntax_node()); - insert_use_with_editor(&scope, path, &ctx.config.insert_use, &editor); + insert_use_with_editor( + &ctx.sema, + &scope, + path, + &ctx.config.insert_use, + &editor, + ); edit.add_file_edits(file_id, editor); } }, diff --git a/crates/ide-assists/src/handlers/convert_tuple_return_type_to_struct.rs b/crates/ide-assists/src/handlers/convert_tuple_return_type_to_struct.rs index 46e2917f72f1..c28bfa2faf88 100644 --- a/crates/ide-assists/src/handlers/convert_tuple_return_type_to_struct.rs +++ b/crates/ide-assists/src/handlers/convert_tuple_return_type_to_struct.rs @@ -155,7 +155,13 @@ fn replace_usages( } } if let Some((import_scope, path)) = import_data { - insert_use_with_editor(&import_scope, path, &ctx.config.insert_use, &editor); + insert_use_with_editor( + &ctx.sema, + &import_scope, + path, + &ctx.config.insert_use, + &editor, + ); } }); edit.add_file_edits(file_id.file_id(ctx.db()), editor); diff --git a/crates/ide-assists/src/handlers/extract_function.rs b/crates/ide-assists/src/handlers/extract_function.rs index 46333ed72638..cdd849b07255 100644 --- a/crates/ide-assists/src/handlers/extract_function.rs +++ b/crates/ide-assists/src/handlers/extract_function.rs @@ -204,6 +204,7 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext<'_, '_>) - if let Some(mod_path) = mod_path { insert_use_with_editor( + &ctx.sema, &scope, mod_path_to_ast_with_factory(make, &mod_path, edition), &ctx.config.insert_use, diff --git a/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs index c1ac4f172489..0f5c1e1f2b5e 100644 --- a/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/ide-assists/src/handlers/extract_struct_from_enum_variant.rs @@ -1,7 +1,7 @@ use std::iter; use either::Either; -use hir::{EnumVariant, HasCrate, Module, ModuleDef, Name}; +use hir::{EnumVariant, HasCrate, Module, ModuleDef, Name, Semantics}; use ide_db::{ FxHashSet, RootDatabase, defs::Definition, @@ -90,6 +90,7 @@ pub(crate) fn extract_struct_from_enum_variant( let file_editor = builder.make_editor(processed[0].0.syntax()); processed.into_iter().for_each(|(path, node, import)| { apply_references( + &ctx.sema, ctx.config.insert_use, path, node, @@ -110,7 +111,15 @@ pub(crate) fn extract_struct_from_enum_variant( references, ); processed.into_iter().for_each(|(path, node, import)| { - apply_references(ctx.config.insert_use, path, node, import, edition, &editor) + apply_references( + &ctx.sema, + ctx.config.insert_use, + path, + node, + import, + edition, + &editor, + ) }); } @@ -385,6 +394,7 @@ fn collect_variant_comments( } fn apply_references( + sema: &Semantics<'_, RootDatabase>, insert_use_cfg: InsertUseConfig, segment: ast::PathSegment, node: SyntaxNode, @@ -395,6 +405,7 @@ fn apply_references( let make = editor.make(); if let Some((scope, path)) = import { insert_use_with_editor( + sema, &scope, mod_path_to_ast_with_factory(make, &path, edition), &insert_use_cfg, diff --git a/crates/ide-assists/src/handlers/merge_imports.rs b/crates/ide-assists/src/handlers/merge_imports.rs index a4edb556d457..c77c10b1e7f9 100644 --- a/crates/ide-assists/src/handlers/merge_imports.rs +++ b/crates/ide-assists/src/handlers/merge_imports.rs @@ -98,7 +98,7 @@ fn merge_uses( }; let mut merged = first.clone(); for item in &rest { - merged = try_merge_imports(editor.make(), &merged, item, mb)?; + merged = try_merge_imports(editor.make(), &merged, item, mb, None)?; } for item in rest { item.remove(editor); diff --git a/crates/ide-assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ide-assists/src/handlers/replace_qualified_name_with_use.rs index d7f00bf669cc..cfa2cabfcbc6 100644 --- a/crates/ide-assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/ide-assists/src/handlers/replace_qualified_name_with_use.rs @@ -95,7 +95,13 @@ pub(crate) fn replace_qualified_name_with_use( Some(qualifier) => make.path_concat(qualifier, path), None => path, }; - insert_use_with_editor(&scope, path, &ctx.config.insert_use, &editor); + insert_use_with_editor( + &ctx.sema, + &scope, + path, + &ctx.config.insert_use, + &editor, + ); builder.add_file_edits(ctx.vfs_file_id(), editor); }, ) diff --git a/crates/ide-assists/src/handlers/unqualify_method_call.rs b/crates/ide-assists/src/handlers/unqualify_method_call.rs index 242ebc4063d0..3613e4ab3f97 100644 --- a/crates/ide-assists/src/handlers/unqualify_method_call.rs +++ b/crates/ide-assists/src/handlers/unqualify_method_call.rs @@ -106,6 +106,7 @@ fn add_import( if let Some(scope) = scope { ide_db::imports::insert_use::insert_use_with_editor( + &ctx.sema, &scope, import, &ctx.config.insert_use, diff --git a/crates/ide-completion/src/lib.rs b/crates/ide-completion/src/lib.rs index 58cb7cdd13c2..95c17edca08b 100644 --- a/crates/ide-completion/src/lib.rs +++ b/crates/ide-completion/src/lib.rs @@ -304,6 +304,7 @@ pub fn resolve_completion_edits( let full_path = make.path_from_text_with_edition(&import.path, current_edition); if import.as_underscore { insert_use::insert_use_as_alias_with_editor( + &sema, &scope, full_path, &config.insert_use, @@ -311,7 +312,7 @@ pub fn resolve_completion_edits( &editor, ); } else { - insert_use::insert_use_with_editor_preserving_namespaces( + insert_use::insert_use_with_editor( &sema, &scope, full_path, diff --git a/crates/ide-db/src/imports/insert_use.rs b/crates/ide-db/src/imports/insert_use.rs index a0d6e8174b86..0bb017c6c705 100644 --- a/crates/ide-db/src/imports/insert_use.rs +++ b/crates/ide-db/src/imports/insert_use.rs @@ -17,7 +17,7 @@ use crate::{ RootDatabase, imports::merge_imports::{ MergeBehavior, NormalizationStyle, common_prefix, eq_attrs, eq_visibility, - try_merge_imports, try_merge_imports_preserving_path, use_tree_cmp, wrap_in_tree_list, + try_merge_imports, use_tree_cmp, wrap_in_tree_list, }, }; @@ -152,45 +152,17 @@ impl ImportScope { /// Insert an import path into the given file/node. A `merge` value of none indicates that no import merging is allowed to occur. pub fn insert_use_with_editor( - scope: &ImportScope, - path: ast::Path, - cfg: &InsertUseConfig, - syntax_editor: &SyntaxEditor, -) { - insert_use_with_alias_option_with_editor( - scope, - path, - cfg, - None, - syntax_editor, - try_merge_imports, - ); -} - -/// Inserts a use without turning a path that resolves in multiple namespaces into a nested `self`. -pub fn insert_use_with_editor_preserving_namespaces( sema: &Semantics<'_, RootDatabase>, scope: &ImportScope, path: ast::Path, cfg: &InsertUseConfig, syntax_editor: &SyntaxEditor, ) { - insert_use_with_alias_option_with_editor( - scope, - path, - cfg, - None, - syntax_editor, - |make, lhs, rhs, merge_behavior| match namespace_collision_prefix_len(sema, lhs) { - Some(prefix_len) => { - try_merge_imports_preserving_path(make, lhs, rhs, merge_behavior, prefix_len) - } - None => try_merge_imports(make, lhs, rhs, merge_behavior), - }, - ); + insert_use_with_alias_option_with_editor(sema, scope, path, cfg, None, syntax_editor); } pub fn insert_uses_with_editor( + sema: &Semantics<'_, RootDatabase>, scope: &ImportScope, paths: impl IntoIterator, cfg: &InsertUseConfig, @@ -217,11 +189,12 @@ pub fn insert_uses_with_editor( } for path in paths { - insert_use_with_editor(scope, path, cfg, syntax_editor); + insert_use_with_editor(sema, scope, path, cfg, syntax_editor); } } pub fn insert_use_as_alias_with_editor( + sema: &Semantics<'_, RootDatabase>, scope: &ImportScope, path: ast::Path, cfg: &InsertUseConfig, @@ -238,21 +211,16 @@ pub fn insert_use_as_alias_with_editor( .expect("Failed to make ast node `Rename`"); let alias = node.rename(); - insert_use_with_alias_option_with_editor(scope, path, cfg, alias, editor, try_merge_imports); + insert_use_with_alias_option_with_editor(sema, scope, path, cfg, alias, editor); } fn insert_use_with_alias_option_with_editor( + sema: &Semantics<'_, RootDatabase>, scope: &ImportScope, path: ast::Path, cfg: &InsertUseConfig, alias: Option, syntax_editor: &SyntaxEditor, - merge_imports: impl Fn( - &syntax::ast::syntax_factory::SyntaxFactory, - &ast::Use, - &ast::Use, - MergeBehavior, - ) -> Option, ) { let make = syntax_editor.make(); let _p = tracing::info_span!("insert_use_with_alias_option").entered(); @@ -299,8 +267,14 @@ fn insert_use_with_alias_option_with_editor( for existing_use in scope.as_syntax_node().children().filter_map(ast::Use::cast).filter(filter) { - if let Some(merged) = merge_imports(syntax_editor.make(), &existing_use, &use_item, mb) - { + let preserve_path_len = namespace_collision_prefix_len(sema, &existing_use); + if let Some(merged) = try_merge_imports( + syntax_editor.make(), + &existing_use, + &use_item, + mb, + preserve_path_len, + ) { syntax_editor.replace(existing_use.syntax(), merged.syntax()); return; } diff --git a/crates/ide-db/src/imports/insert_use/tests.rs b/crates/ide-db/src/imports/insert_use/tests.rs index 34ff7c8d5b42..c7ffa3fd6abf 100644 --- a/crates/ide-db/src/imports/insert_use/tests.rs +++ b/crates/ide-db/src/imports/insert_use/tests.rs @@ -1358,7 +1358,7 @@ fn check_with_config( .find_map(ast::Path::cast) .unwrap(); - insert_use_with_editor(&file, path, config, &editor); + insert_use_with_editor(sema, &file, path, config, &editor); let edit = editor.finish(); let result = edit.new_root().to_string(); assert_eq_text!(&trim_indent(ra_fixture_after), &result); @@ -1432,7 +1432,7 @@ fn check_merge_only_fail(ra_fixture0: &str, ra_fixture1: &str, mb: MergeBehavior .unwrap(); let make = SyntaxFactory::without_mappings(); - let result = try_merge_imports(&make, &use0, &use1, mb); + let result = try_merge_imports(&make, &use0, &use1, mb, None); assert_eq!(result.map(|u| u.to_string()), None); } @@ -1498,7 +1498,7 @@ fn check_merge(ra_fixture0: &str, ra_fixture1: &str, last: &str, mb: MergeBehavi .unwrap(); let make = SyntaxFactory::without_mappings(); - let result = try_merge_imports(&make, &use0, &use1, mb); + let result = try_merge_imports(&make, &use0, &use1, mb, None); assert_eq!(result.map(|u| u.to_string().trim().to_owned()), Some(last.trim().to_owned())); } @@ -1529,7 +1529,7 @@ fn merge_gated_imports_with_different_values() { .unwrap(); let make = SyntaxFactory::without_mappings(); - let result = try_merge_imports(&make, &use0, &use1, MergeBehavior::Crate); + let result = try_merge_imports(&make, &use0, &use1, MergeBehavior::Crate, None); assert_eq!(result, None); } diff --git a/crates/ide-db/src/imports/merge_imports.rs b/crates/ide-db/src/imports/merge_imports.rs index ce548cbe014b..ba5bfafc2fcf 100644 --- a/crates/ide-db/src/imports/merge_imports.rs +++ b/crates/ide-db/src/imports/merge_imports.rs @@ -39,11 +39,15 @@ impl MergeBehavior { } /// Merge `rhs` into `lhs` keeping both intact. +/// +/// `preserve_path_len` prevents an existing multi-namespace path of that length from being +/// replaced with a nested `self` import. pub fn try_merge_imports( make: &SyntaxFactory, lhs: &ast::Use, rhs: &ast::Use, merge_behavior: MergeBehavior, + preserve_path_len: Option, ) -> Option { // don't merge imports with different visibilities if !eq_visibility(lhs.visibility(), rhs.visibility()) { @@ -55,75 +59,25 @@ pub fn try_merge_imports( let lhs_tree = lhs.use_tree()?; let rhs_tree = rhs.use_tree()?; - let merged_tree = try_merge_trees_with_factory(lhs_tree, rhs_tree, merge_behavior, make)?; + let (merged_tree, normalize) = try_merge_trees_with_factory( + lhs_tree, + rhs_tree, + merge_behavior, + preserve_path_len, + make, + )?; - // Ignore `None` result because normalization should not affect the merge result. - let use_tree = try_normalize_use_tree(merged_tree.clone(), merge_behavior.into(), make) - .unwrap_or(merged_tree); + let use_tree = if normalize { + // Ignore `None` result because normalization should not affect the merge result. + try_normalize_use_tree(merged_tree.clone(), merge_behavior.into(), make) + .unwrap_or(merged_tree) + } else { + merged_tree + }; make_use_with_tree(lhs, use_tree) } -/// Merges imports without replacing the protected path with a nested `self` import. -pub(super) fn try_merge_imports_preserving_path( - make: &SyntaxFactory, - lhs: &ast::Use, - rhs: &ast::Use, - merge_behavior: MergeBehavior, - preserve_path_len: usize, -) -> Option { - if !eq_visibility(lhs.visibility(), rhs.visibility()) || !eq_attrs(lhs.attrs(), rhs.attrs()) { - return None; - } - - let lhs_tree = lhs.use_tree()?; - let rhs_tree = rhs.use_tree()?; - if merge_behavior == MergeBehavior::One { - let merge_lhs = wrap_in_tree_list(&lhs_tree, make).unwrap_or(lhs_tree); - let merge_rhs = wrap_in_tree_list(&rhs_tree, make).unwrap_or(rhs_tree); - let mut use_trees = merge_lhs - .use_tree_list()? - .use_trees() - .chain(merge_rhs.use_tree_list()?.use_trees()) - .collect::>(); - use_trees.sort_unstable_by(use_tree_cmp); - return make_use_with_tree(lhs, with_use_tree_list(&merge_lhs, use_trees, make)?); - } - if merge_behavior == MergeBehavior::Module { - return None; - } - - let lhs_path = lhs_tree.path()?; - let rhs_path = rhs_tree.path()?; - let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; - if path_len(lhs_prefix.clone()) != preserve_path_len { - return None; - } - let lhs_is_prefix = - lhs_tree.is_simple_path() && lhs_tree.rename().is_none() && lhs_path == lhs_prefix; - let rhs_is_prefix = - rhs_tree.is_simple_path() && rhs_tree.rename().is_none() && rhs_path == rhs_prefix; - if lhs_is_prefix == rhs_is_prefix { - return None; - } - - let (merge_lhs, merge_rhs) = match (lhs_prefix.qualifier(), rhs_prefix.qualifier()) { - (Some(lhs_parent), Some(rhs_parent)) => ( - split_prefix(&lhs_tree, &lhs_parent, make)?, - split_prefix(&rhs_tree, &rhs_parent, make)?, - ), - (None, None) => (wrap_in_tree_list(&lhs_tree, make)?, wrap_in_tree_list(&rhs_tree, make)?), - _ => return None, - }; - let mut use_trees = merge_lhs - .use_tree_list()? - .use_trees() - .chain(merge_rhs.use_tree_list()?.use_trees()) - .collect::>(); - use_trees.sort_unstable_by(use_tree_cmp); - make_use_with_tree(lhs, with_use_tree_list(&merge_lhs, use_trees, make)?) -} - /// Merge `rhs` into `lhs` keeping both intact. pub fn try_merge_trees( make: &SyntaxFactory, @@ -131,7 +85,8 @@ pub fn try_merge_trees( rhs: &ast::UseTree, merge: MergeBehavior, ) -> Option { - let merged = try_merge_trees_with_factory(lhs.clone(), rhs.clone(), merge, make)?; + let (merged, _) = + try_merge_trees_with_factory(lhs.clone(), rhs.clone(), merge, None, make)?; // Ignore `None` result because normalization should not affect the merge result. Some(try_normalize_use_tree(merged.clone(), merge.into(), make).unwrap_or(merged)) @@ -141,16 +96,61 @@ fn try_merge_trees_with_factory( mut lhs: ast::UseTree, mut rhs: ast::UseTree, merge: MergeBehavior, + preserve_path_len: Option, make: &SyntaxFactory, -) -> Option { +) -> Option<(ast::UseTree, bool)> { if merge == MergeBehavior::One { lhs = wrap_in_tree_list(&lhs, make).unwrap_or(lhs); rhs = wrap_in_tree_list(&rhs, make).unwrap_or(rhs); + + if preserve_path_len.is_some() { + let mut use_trees = lhs + .use_tree_list()? + .use_trees() + .chain(rhs.use_tree_list()?.use_trees()) + .collect::>(); + use_trees.sort_unstable_by(use_tree_cmp); + return Some((with_use_tree_list(&lhs, use_trees, make)?, false)); + } } else { let lhs_path = lhs.path()?; let rhs_path = rhs.path()?; let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; + if let Some(preserve_path_len) = preserve_path_len { + if merge == MergeBehavior::Module + || path_len(lhs_prefix.clone()) != preserve_path_len + { + return None; + } + + let lhs_is_prefix = + lhs.is_simple_path() && lhs.rename().is_none() && lhs_path == lhs_prefix; + let rhs_is_prefix = + rhs.is_simple_path() && rhs.rename().is_none() && rhs_path == rhs_prefix; + if lhs_is_prefix == rhs_is_prefix { + return None; + } + + let (merge_lhs, merge_rhs) = match (lhs_prefix.qualifier(), rhs_prefix.qualifier()) { + (Some(lhs_parent), Some(rhs_parent)) => ( + split_prefix(&lhs, &lhs_parent, make)?, + split_prefix(&rhs, &rhs_parent, make)?, + ), + (None, None) => { + (wrap_in_tree_list(&lhs, make)?, wrap_in_tree_list(&rhs, make)?) + } + _ => return None, + }; + let mut use_trees = merge_lhs + .use_tree_list()? + .use_trees() + .chain(merge_rhs.use_tree_list()?.use_trees()) + .collect::>(); + use_trees.sort_unstable_by(use_tree_cmp); + return Some((with_use_tree_list(&merge_lhs, use_trees, make)?, false)); + } + if lhs.is_simple_path() && rhs.is_simple_path() && lhs_path == lhs_prefix @@ -172,7 +172,7 @@ fn try_merge_trees_with_factory( rhs = split_prefix(&rhs, &rhs_prefix, make)?; } } - recursive_merge(lhs, rhs, merge, make) + Some((recursive_merge(lhs, rhs, merge, make)?, true)) } /// Recursively merges rhs to lhs @@ -394,10 +394,11 @@ fn recursive_normalize( while anchor_idx < use_tree_list.len() { let mut candidate_idx = anchor_idx + 1; while candidate_idx < use_tree_list.len() { - if let Some(mut merged) = try_merge_trees_with_factory( + if let Some((mut merged, _)) = try_merge_trees_with_factory( use_tree_list[anchor_idx].clone(), use_tree_list[candidate_idx].clone(), MergeBehavior::Crate, + None, make, ) { if let Some(normalized) = diff --git a/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs b/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs index 24f1e3ad836a..3f6de39149eb 100644 --- a/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs +++ b/crates/ide-diagnostics/src/handlers/json_is_not_rust.rs @@ -172,6 +172,7 @@ pub(crate) fn json_in_items( } insert_uses_with_editor( + sema, &import_scope, imports_to_insert, &config.insert_use, From 8c00a569589c78e6435b08ac34772e8f4768e7f0 Mon Sep 17 00:00:00 2001 From: SomeFlyingThing <306498559+SomeFlyingThing@users.noreply.github.com> Date: Mon, 24 Aug 2026 09:50:19 +1200 Subject: [PATCH 3/5] Apply rustfmt --- .../ide-assists/src/handlers/auto_import.rs | 4 +-- .../src/handlers/convert_bool_to_enum.rs | 8 +----- .../replace_qualified_name_with_use.rs | 8 +----- crates/ide-db/src/imports/merge_imports.rs | 25 ++++++------------- 4 files changed, 10 insertions(+), 35 deletions(-) diff --git a/crates/ide-assists/src/handlers/auto_import.rs b/crates/ide-assists/src/handlers/auto_import.rs index 15707b26f074..c312fafc5fda 100644 --- a/crates/ide-assists/src/handlers/auto_import.rs +++ b/crates/ide-assists/src/handlers/auto_import.rs @@ -7,9 +7,7 @@ use ide_db::{ helpers::mod_path_to_ast, imports::{ import_assets::{ImportAssets, ImportCandidate, LocatedImport, TraitImportCandidate}, - insert_use::{ - ImportScope, insert_use_as_alias_with_editor, insert_use_with_editor, - }, + insert_use::{ImportScope, insert_use_as_alias_with_editor, insert_use_with_editor}, }, }; use syntax::{AstNode, Edition, SyntaxNode, ast, match_ast}; diff --git a/crates/ide-assists/src/handlers/convert_bool_to_enum.rs b/crates/ide-assists/src/handlers/convert_bool_to_enum.rs index 44121f686584..0e42c91a86ea 100644 --- a/crates/ide-assists/src/handlers/convert_bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/convert_bool_to_enum.rs @@ -88,13 +88,7 @@ pub(crate) fn convert_bool_to_enum(acc: &mut Assists, ctx: &AssistContext<'_, '_ ); for (file_id, scope, path) in delayed_mutations { let editor = edit.make_editor(scope.as_syntax_node()); - insert_use_with_editor( - &ctx.sema, - &scope, - path, - &ctx.config.insert_use, - &editor, - ); + insert_use_with_editor(&ctx.sema, &scope, path, &ctx.config.insert_use, &editor); edit.add_file_edits(file_id, editor); } }, diff --git a/crates/ide-assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ide-assists/src/handlers/replace_qualified_name_with_use.rs index cfa2cabfcbc6..eaa9c2d78039 100644 --- a/crates/ide-assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/ide-assists/src/handlers/replace_qualified_name_with_use.rs @@ -95,13 +95,7 @@ pub(crate) fn replace_qualified_name_with_use( Some(qualifier) => make.path_concat(qualifier, path), None => path, }; - insert_use_with_editor( - &ctx.sema, - &scope, - path, - &ctx.config.insert_use, - &editor, - ); + insert_use_with_editor(&ctx.sema, &scope, path, &ctx.config.insert_use, &editor); builder.add_file_edits(ctx.vfs_file_id(), editor); }, ) diff --git a/crates/ide-db/src/imports/merge_imports.rs b/crates/ide-db/src/imports/merge_imports.rs index ba5bfafc2fcf..02a8b95fa19a 100644 --- a/crates/ide-db/src/imports/merge_imports.rs +++ b/crates/ide-db/src/imports/merge_imports.rs @@ -59,13 +59,8 @@ pub fn try_merge_imports( let lhs_tree = lhs.use_tree()?; let rhs_tree = rhs.use_tree()?; - let (merged_tree, normalize) = try_merge_trees_with_factory( - lhs_tree, - rhs_tree, - merge_behavior, - preserve_path_len, - make, - )?; + let (merged_tree, normalize) = + try_merge_trees_with_factory(lhs_tree, rhs_tree, merge_behavior, preserve_path_len, make)?; let use_tree = if normalize { // Ignore `None` result because normalization should not affect the merge result. @@ -85,8 +80,7 @@ pub fn try_merge_trees( rhs: &ast::UseTree, merge: MergeBehavior, ) -> Option { - let (merged, _) = - try_merge_trees_with_factory(lhs.clone(), rhs.clone(), merge, None, make)?; + let (merged, _) = try_merge_trees_with_factory(lhs.clone(), rhs.clone(), merge, None, make)?; // Ignore `None` result because normalization should not affect the merge result. Some(try_normalize_use_tree(merged.clone(), merge.into(), make).unwrap_or(merged)) @@ -118,9 +112,7 @@ fn try_merge_trees_with_factory( let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; if let Some(preserve_path_len) = preserve_path_len { - if merge == MergeBehavior::Module - || path_len(lhs_prefix.clone()) != preserve_path_len - { + if merge == MergeBehavior::Module || path_len(lhs_prefix.clone()) != preserve_path_len { return None; } @@ -133,13 +125,10 @@ fn try_merge_trees_with_factory( } let (merge_lhs, merge_rhs) = match (lhs_prefix.qualifier(), rhs_prefix.qualifier()) { - (Some(lhs_parent), Some(rhs_parent)) => ( - split_prefix(&lhs, &lhs_parent, make)?, - split_prefix(&rhs, &rhs_parent, make)?, - ), - (None, None) => { - (wrap_in_tree_list(&lhs, make)?, wrap_in_tree_list(&rhs, make)?) + (Some(lhs_parent), Some(rhs_parent)) => { + (split_prefix(&lhs, &lhs_parent, make)?, split_prefix(&rhs, &rhs_parent, make)?) } + (None, None) => (wrap_in_tree_list(&lhs, make)?, wrap_in_tree_list(&rhs, make)?), _ => return None, }; let mut use_trees = merge_lhs From 82466cf258a4f465c4b68db3981f722ec6339b31 Mon Sep 17 00:00:00 2001 From: SomeFlyingThing <306498559+SomeFlyingThing@users.noreply.github.com> Date: Mon, 24 Aug 2026 09:51:16 +1200 Subject: [PATCH 4/5] Fix merge result type --- crates/ide-db/src/imports/merge_imports.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ide-db/src/imports/merge_imports.rs b/crates/ide-db/src/imports/merge_imports.rs index 02a8b95fa19a..a40a4aa39837 100644 --- a/crates/ide-db/src/imports/merge_imports.rs +++ b/crates/ide-db/src/imports/merge_imports.rs @@ -155,7 +155,7 @@ fn try_merge_trees_with_factory( return None; } - return Some(rhs); + return Some((rhs, true)); } else { lhs = split_prefix(&lhs, &lhs_prefix, make)?; rhs = split_prefix(&rhs, &rhs_prefix, make)?; From 7c96e39e929c436dcaccdc7e1136d7f8c4f4a91a Mon Sep 17 00:00:00 2001 From: SomeFlyingThing <306498559+SomeFlyingThing@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:42:31 +1200 Subject: [PATCH 5/5] Fix redundant self bindings when inserting imports Remove self imports covered by the new explicit import, preserving distinct aliases and unrelated namespace bindings. Handle all import granularities and self imports appearing after another merge candidate. Add regression tests for macro completion, nested imports, aliases, and imports kept separate. --- crates/ide-completion/src/tests/flyimport.rs | 31 ++++++ crates/ide-db/src/imports/insert_use.rs | 55 +++++++--- crates/ide-db/src/imports/insert_use/tests.rs | 100 ++++++++++++++++++ crates/ide-db/src/imports/merge_imports.rs | 67 ++++++++++++ 4 files changed, 236 insertions(+), 17 deletions(-) diff --git a/crates/ide-completion/src/tests/flyimport.rs b/crates/ide-completion/src/tests/flyimport.rs index 918b2428ddf7..0a7cf6ba8a0f 100644 --- a/crates/ide-completion/src/tests/flyimport.rs +++ b/crates/ide-completion/src/tests/flyimport.rs @@ -161,6 +161,37 @@ fn main() { ); } +#[test] +fn flyimport_preserves_macro_namespace_with_existing_self() { + check_edit( + "vec!", + r#" +//- /lib.rs crate:dep +pub mod vec { + pub struct Vec; +} +#[macro_export] +macro_rules! vec { + () => {}; +} + +//- /main.rs crate:main deps:dep +use dep::vec::{self, Vec}; + +fn main() { + ve$0 +} +"#, + r#" +use dep::{vec, vec::Vec}; + +fn main() { + vec!($0) +} +"#, + ); +} + #[test] fn struct_fuzzy_completion() { check_edit( diff --git a/crates/ide-db/src/imports/insert_use.rs b/crates/ide-db/src/imports/insert_use.rs index 0bb017c6c705..513ecfcade08 100644 --- a/crates/ide-db/src/imports/insert_use.rs +++ b/crates/ide-db/src/imports/insert_use.rs @@ -10,14 +10,14 @@ use syntax::{ ast::{ self, AstNode, HasAttrs, HasModuleItem, HasVisibility, PathSegmentKind, edit::IndentLevel, }, - syntax_editor::{Position, SyntaxEditor}, + syntax_editor::{Position, Removable, SyntaxEditor}, }; use crate::{ RootDatabase, imports::merge_imports::{ MergeBehavior, NormalizationStyle, common_prefix, eq_attrs, eq_visibility, - try_merge_imports, use_tree_cmp, wrap_in_tree_list, + remove_redundant_self_import, try_merge_imports, use_tree_cmp, wrap_in_tree_list, }, }; @@ -252,7 +252,8 @@ fn insert_use_with_alias_option_with_editor( }; } - let mut use_tree = make.use_tree(path, None, alias, false); + let remove_self = alias.is_none(); + let mut use_tree = make.use_tree(path.clone(), None, alias, false); if mb == Some(MergeBehavior::One) && use_tree.path().is_some() && let Some(wrapped) = wrap_in_tree_list(&use_tree, make) @@ -261,24 +262,44 @@ fn insert_use_with_alias_option_with_editor( } let use_item = make.use_(scope.required_cfgs.iter().cloned().rev(), None, use_tree); - // merge into existing imports if possible - if let Some(mb) = mb { - let filter = |it: &_| !(cfg.skip_glob_imports && ast::Use::is_simple_glob(it)); - for existing_use in - scope.as_syntax_node().children().filter_map(ast::Use::cast).filter(filter) + let mut inserted = false; + for existing_use in scope.as_syntax_node().children().filter_map(ast::Use::cast) { + let mut updated = existing_use.clone(); + if remove_self + && eq_visibility(existing_use.visibility(), use_item.visibility()) + && eq_attrs(existing_use.attrs(), use_item.attrs()) + { + let Some(cleaned) = remove_redundant_self_import(&existing_use, &path, make) else { + if inserted { + existing_use.remove(syntax_editor); + } else { + syntax_editor.replace(existing_use.syntax(), use_item.syntax()); + inserted = true; + } + continue; + }; + updated = cleaned; + } + // Keep scanning after insertion: a later use may still contain a + // redundant `self` binding for the newly imported path. + if !inserted + && let Some(mb) = mb + && !(cfg.skip_glob_imports && existing_use.is_simple_glob()) { let preserve_path_len = namespace_collision_prefix_len(sema, &existing_use); - if let Some(merged) = try_merge_imports( - syntax_editor.make(), - &existing_use, - &use_item, - mb, - preserve_path_len, - ) { - syntax_editor.replace(existing_use.syntax(), merged.syntax()); - return; + if let Some(merged) = + try_merge_imports(make, &updated, &use_item, mb, preserve_path_len) + { + updated = merged; + inserted = true; } } + if updated != existing_use { + syntax_editor.replace(existing_use.syntax(), updated.syntax()); + } + } + if inserted { + return; } // either we weren't allowed to merge or there is no import that fits the merge conditions // so look for the place we have to insert to diff --git a/crates/ide-db/src/imports/insert_use/tests.rs b/crates/ide-db/src/imports/insert_use/tests.rs index c7ffa3fd6abf..b41e1af085c8 100644 --- a/crates/ide-db/src/imports/insert_use/tests.rs +++ b/crates/ide-db/src/imports/insert_use/tests.rs @@ -1326,6 +1326,106 @@ use ::ext::foo::Foo; ); } +#[test] +fn insert_replaces_self_import_in_all_granularities() { + let definitions = "mod foo { pub mod bar { pub struct Baz; } pub fn bar() {} }"; + for (granularity, imports) in [ + (ImportGranularity::Crate, "use foo::{bar, bar::Baz};"), + (ImportGranularity::One, "use {foo::bar, foo::bar::Baz};"), + (ImportGranularity::Module, "use foo::bar;\nuse foo::bar::Baz;"), + (ImportGranularity::Item, "use foo::bar;\nuse foo::bar::Baz;"), + ] { + check( + "foo::bar", + &format!("use foo::bar::{{self, Baz}};\n\n{definitions}"), + &format!("{imports}\n\n{definitions}"), + granularity, + ); + } +} + +#[test] +fn insert_replaces_only_self_import() { + for granularity in + [ImportGranularity::Crate, ImportGranularity::Module, ImportGranularity::Item] + { + check("foo::bar", "use foo::bar::{self};", "use foo::bar;", granularity); + } + check_one("foo::bar", "use {foo::bar::{self}};", "use {foo::bar};"); +} + +#[test] +fn insert_replaces_self_import_preserving_aliases() { + let definitions = "mod foo { pub mod bar { pub struct Baz; } pub fn bar() {} }"; + for (existing, expected) in [ + ("self as bar, Baz", "use foo::{bar, bar::Baz};"), + ("self as renamed, Baz", "use foo::{bar, bar::{self as renamed, Baz}};"), + ("self, self as renamed", "use foo::{bar, bar::{self as renamed}};"), + ("self, self as _", "use foo::{bar, bar::{self as _}};"), + ] { + check_crate( + "foo::bar", + &format!("use foo::bar::{{{existing}}};\n\n{definitions}"), + &format!("{expected}\n\n{definitions}"), + ); + } +} + +#[test] +fn insert_replaces_nested_self_without_normalizing_other_imports() { + check_crate( + "foo::bar", + r#" +use foo::{bar::{self, Baz}, quux, quux::Item}; + +mod foo { + pub mod bar { pub struct Baz; } + pub fn bar() {} + pub mod quux { pub struct Item; } + pub fn quux() {} +} +"#, + r#" +use foo::bar; +use foo::{bar::Baz, quux, quux::Item}; + +mod foo { + pub mod bar { pub struct Baz; } + pub fn bar() {} + pub mod quux { pub struct Item; } + pub fn quux() {} +} +"#, + ); +} + +#[test] +fn insert_replaces_self_after_merging_another_import() { + check_crate( + "foo::bar", + r#" +use foo::Other; +use foo::bar::{self, Baz}; + +mod foo { + pub struct Other; + pub mod bar { pub struct Baz; } + pub fn bar() {} +} +"#, + r#" +use foo::{Other, bar}; +use foo::bar::Baz; + +mod foo { + pub struct Other; + pub mod bar { pub struct Baz; } + pub fn bar() {} +} +"#, + ); +} + fn check_with_config( path: &str, #[rust_analyzer::rust_fixture] ra_fixture_before: &str, diff --git a/crates/ide-db/src/imports/merge_imports.rs b/crates/ide-db/src/imports/merge_imports.rs index a40a4aa39837..b8d8daa8cc01 100644 --- a/crates/ide-db/src/imports/merge_imports.rs +++ b/crates/ide-db/src/imports/merge_imports.rs @@ -73,6 +73,73 @@ pub fn try_merge_imports( make_use_with_tree(lhs, use_tree) } +/// Remove `self` bindings supplied by an explicit import of `path`. +/// Returns `None` if no imports remain. +pub(super) fn remove_redundant_self_import( + use_item: &ast::Use, + path: &ast::Path, + make: &SyntaxFactory, +) -> Option { + let Some(tree) = use_item.use_tree() else { return Some(use_item.clone()) }; + let segments = path.segments().collect::>(); + let name = path.segment().and_then(|segment| segment.name_ref()); + let updated = remove_redundant_self(tree.clone(), &segments, name.as_ref(), make)?; + if updated == tree { + Some(use_item.clone()) + } else { + Some(make_use_with_tree(use_item, updated).unwrap_or_else(|| use_item.clone())) + } +} + +fn remove_redundant_self( + tree: ast::UseTree, + path: &[ast::PathSegment], + name: Option<&ast::NameRef>, + make: &SyntaxFactory, +) -> Option { + let Some(list) = tree.use_tree_list() else { return Some(tree) }; + let prefix = tree.path().map(|path| path.segments().collect::>()).unwrap_or_default(); + if prefix.len() > path.len() + || prefix.iter().zip(path).any(|(a, b)| a.syntax().text() != b.syntax().text()) + { + return Some(tree); + } + let path = &path[prefix.len()..]; + let mut changed = false; + let subtrees = list + .use_trees() + .filter_map(|subtree| { + let redundant_self = path.is_empty() + && subtree.path().as_ref().is_some_and(path_is_self) + && subtree.rename().is_none_or(|rename| { + rename.name().zip(name).is_some_and(|(alias, name)| alias.text() == name.text()) + }); + if redundant_self { + changed = true; + return None; + } + let updated = remove_redundant_self(subtree.clone(), path, name, make); + changed |= updated.as_ref() != Some(&subtree); + updated + }) + .collect::>(); + if !changed { + return Some(tree); + } + if subtrees.is_empty() { + return None; + } + // Only collapse this level. General normalization could turn another + // explicit multi-namespace import back into a type-only `self` import. + let collapse = subtrees.len() == 1 && !subtrees[0].path().as_ref().is_some_and(path_is_self); + let Some(updated) = with_use_tree_list(&tree, subtrees, make) else { return Some(tree) }; + if collapse { + Some(merge_single_subtree_into_parent_tree(updated.clone(), make).unwrap_or(updated)) + } else { + Some(updated) + } +} + /// Merge `rhs` into `lhs` keeping both intact. pub fn try_merge_trees( make: &SyntaxFactory,