diff --git a/crates/ide-assists/src/handlers/auto_import.rs b/crates/ide-assists/src/handlers/auto_import.rs index dd082476d2d6..c312fafc5fda 100644 --- a/crates/ide-assists/src/handlers/auto_import.rs +++ b/crates/ide-assists/src/handlers/auto_import.rs @@ -127,6 +127,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_with_editor( + &ctx.sema, &scope, mod_path_to_ast(&import_path, edition), &ctx.config.insert_use, @@ -147,6 +148,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, @@ -352,8 +354,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 +555,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-assists/src/handlers/convert_bool_to_enum.rs b/crates/ide-assists/src/handlers/convert_bool_to_enum.rs index 865995def013..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,7 +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(&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..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,7 +95,7 @@ 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 4ca3257c5cba..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,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( + &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..0a7cf6ba8a0f 100644 --- a/crates/ide-completion/src/tests/flyimport.rs +++ b/crates/ide-completion/src/tests/flyimport.rs @@ -100,6 +100,98 @@ 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 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 023538976308..513ecfcade08 100644 --- a/crates/ide-db/src/imports/insert_use.rs +++ b/crates/ide-db/src/imports/insert_use.rs @@ -4,20 +4,20 @@ mod tests; use std::cmp::Ordering; -use hir::Semantics; +use hir::{PathResolution, Semantics}; use syntax::{ NodeOrToken, SyntaxKind, SyntaxNode, 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, }, }; @@ -152,15 +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( + 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); + 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, @@ -187,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, @@ -208,10 +211,11 @@ 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(sema, scope, path, cfg, alias, editor); } fn insert_use_with_alias_option_with_editor( + sema: &Semantics<'_, RootDatabase>, scope: &ImportScope, path: ast::Path, cfg: &InsertUseConfig, @@ -248,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) @@ -257,25 +262,63 @@ 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) + try_merge_imports(make, &updated, &use_item, mb, preserve_path_len) { - syntax_editor.replace(existing_use.syntax(), merged.syntax()); - return; + 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 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/insert_use/tests.rs b/crates/ide-db/src/imports/insert_use/tests.rs index 34ff7c8d5b42..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, @@ -1358,7 +1458,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 +1532,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 +1598,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 +1629,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 f9251458fd02..b8d8daa8cc01 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,15 +59,87 @@ 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) } +/// 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, @@ -71,7 +147,7 @@ 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)) @@ -81,16 +157,56 @@ 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 @@ -106,13 +222,13 @@ 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)?; } } - recursive_merge(lhs, rhs, merge, make) + Some((recursive_merge(lhs, rhs, merge, make)?, true)) } /// Recursively merges rhs to lhs @@ -334,10 +450,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,