Skip to content

Commit 436af1c

Browse files
harrysarsonHarry Sarson
authored and
Harry Sarson
committed
do not normalize use foo::{self} to use foo
It changes behaviour and can cause collisions. E.g. for the following snippet ```rs mod foo { pub mod bar {} pub const bar: i32 = 8; } // tranforming the below to `use foo::bar;` causes the error: // // the name `bar` is defined multiple times use foo::bar::{self}; const bar: u32 = 99; fn main() { let local_bar = bar; } ``` we still normalize ```rs use foo::bar; use foo::bar::{self}; ``` to `use foo::bar;` because this cannot cause collisions. See: #17140 (comment)
1 parent 67f7eb5 commit 436af1c

File tree

3 files changed

+84
-4
lines changed

3 files changed

+84
-4
lines changed

crates/ide-assists/src/handlers/merge_imports.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,28 @@ use foo::bar;
489489
);
490490
}
491491

492+
493+
#[test]
494+
fn test_merge_nested_empty_and_self_with_other() {
495+
check_assist(
496+
497+
merge_imports,
498+
r"
499+
use foo::$0{bar};
500+
use foo::{bar::{self, other}};
501+
",
502+
r"
503+
use foo::bar::{self, other};
504+
",
505+
);
506+
check_assist_import_one_variations!(
507+
"foo::$0{bar}",
508+
"foo::{bar::{self, other}}",
509+
"use {foo::bar::{self, other}};"
510+
);
511+
}
512+
513+
492514
#[test]
493515
fn test_merge_nested_list_self_and_glob() {
494516
check_assist(

crates/ide-assists/src/handlers/normalize_import.rs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,41 @@ mod tests {
119119
);
120120
}
121121

122+
#[test]
123+
fn test_braces_kept() {
124+
check_assist_not_applicable_variations!(
125+
"foo::bar::{$0self}"
126+
);
127+
128+
// This code compiles but transforming "bar::{self}" into "bar" causes a
129+
// compilation error (the name `bar` is defined multiple times).
130+
// Therefore, the normalize_input assist must not apply here.
131+
check_assist_not_applicable(
132+
normalize_import,
133+
r"
134+
mod foo {
135+
136+
pub mod bar {}
137+
138+
pub const bar: i32 = 8;
139+
}
140+
141+
use foo::bar::{$0self};
142+
143+
const bar: u32 = 99;
144+
145+
fn main() {
146+
let local_bar = bar;
147+
}
148+
149+
"
150+
);
151+
}
152+
122153
#[test]
123154
fn test_redundant_braces() {
124155
check_assist_variations!("foo::{bar::{baz, Qux}}", "foo::bar::{baz, Qux}");
125-
check_assist_variations!("foo::{bar::{self}}", "foo::bar");
156+
check_assist_variations!("foo::{bar::{self}}", "foo::bar::{self}");
126157
check_assist_variations!("foo::{bar::{*}}", "foo::bar::*");
127158
check_assist_variations!("foo::{bar::{Qux as Quux}}", "foo::bar::Qux as Quux");
128159
check_assist_variations!(

crates/ide-db/src/imports/merge_imports.rs

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,14 @@ fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior)
157157
}
158158

159159
match (tree_contains_self(lhs_t), tree_contains_self(&rhs_t)) {
160-
(Some(true), None) => continue,
160+
(Some(true), None) => {
161+
remove_subtree_if_only_self(lhs_t);
162+
continue;
163+
}
161164
(None, Some(true)) => {
162165
ted::replace(lhs_t.syntax(), rhs_t.syntax());
163166
*lhs_t = rhs_t;
167+
remove_subtree_if_only_self(lhs_t);
164168
continue;
165169
}
166170
_ => (),
@@ -278,14 +282,20 @@ pub fn try_normalize_use_tree_mut(
278282
fn recursive_normalize(use_tree: &ast::UseTree, style: NormalizationStyle) -> Option<()> {
279283
let use_tree_list = use_tree.use_tree_list()?;
280284
let merge_subtree_into_parent_tree = |single_subtree: &ast::UseTree| {
285+
let subtree_is_only_self = single_subtree.path().as_ref().map_or(false, path_is_self);
286+
281287
let merged_path = match (use_tree.path(), single_subtree.path()) {
288+
// If the subtree is `{self}` then we cannot merge: `use
289+
// foo::bar::{self}` is not equivalent to `use foo::bar`. See
290+
// https://github.com/rust-lang/rust-analyzer/pull/17140#issuecomment-2079189725.
291+
_ if subtree_is_only_self => None,
292+
282293
(None, None) => None,
283294
(Some(outer), None) => Some(outer),
284-
(None, Some(inner)) if path_is_self(&inner) => None,
285295
(None, Some(inner)) => Some(inner),
286-
(Some(outer), Some(inner)) if path_is_self(&inner) => Some(outer),
287296
(Some(outer), Some(inner)) => Some(make::path_concat(outer, inner).clone_for_update()),
288297
};
298+
289299
if merged_path.is_some()
290300
|| single_subtree.use_tree_list().is_some()
291301
|| single_subtree.star_token().is_some()
@@ -706,3 +716,20 @@ fn path_is_self(path: &ast::Path) -> bool {
706716
fn path_len(path: ast::Path) -> usize {
707717
path.segments().count()
708718
}
719+
720+
fn get_single_subtree(use_tree: &ast::UseTree) -> Option<ast::UseTree> {
721+
use_tree
722+
.use_tree_list()
723+
.and_then(|tree_list| tree_list.use_trees().collect_tuple())
724+
.map(|(single_subtree,)| single_subtree)
725+
}
726+
727+
fn remove_subtree_if_only_self(use_tree: &ast::UseTree) {
728+
let Some(single_subtree) = get_single_subtree(use_tree) else { return };
729+
match (use_tree.path(), single_subtree.path()) {
730+
(Some(_), Some(inner)) if path_is_self(&inner) => {
731+
ted::remove_all_iter(single_subtree.syntax().children_with_tokens());
732+
}
733+
_ => (),
734+
}
735+
}

0 commit comments

Comments
 (0)