about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-07-01 10:20:01 +0000
committerbors <bors@rust-lang.org>2024-07-01 10:20:01 +0000
commit29fbc2414d63e985a0fb1547489bf2fb3b093a78 (patch)
treee9b8b6c6c868c3873c07ad841b62a57089094c3c
parentdffbad50c1d8bbd92dacac89222f4832f7d518a7 (diff)
parent742c2ea6217067b69ea82b12cdbdd0c1ee6d4c09 (diff)
downloadrust-29fbc2414d63e985a0fb1547489bf2fb3b093a78.tar.gz
rust-29fbc2414d63e985a0fb1547489bf2fb3b093a78.zip
Auto merge of #17494 - harrysarson:harry/keep-braces, r=Veykril
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;
}

// transforming 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: https://github.com/rust-lang/rust-analyzer/pull/17140#issuecomment-2079189725
-rw-r--r--src/tools/rust-analyzer/crates/ide-assists/src/handlers/merge_imports.rs19
-rw-r--r--src/tools/rust-analyzer/crates/ide-assists/src/handlers/normalize_import.rs31
-rw-r--r--src/tools/rust-analyzer/crates/ide-db/src/imports/merge_imports.rs33
3 files changed, 79 insertions, 4 deletions
diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/merge_imports.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/merge_imports.rs
index 797c5c06533..7f751c93e48 100644
--- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/merge_imports.rs
+++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/merge_imports.rs
@@ -490,6 +490,25 @@ use foo::bar;
     }
 
     #[test]
+    fn test_merge_nested_empty_and_self_with_other() {
+        check_assist(
+            merge_imports,
+            r"
+use foo::$0{bar};
+use foo::{bar::{self, other}};
+",
+            r"
+use foo::bar::{self, other};
+",
+        );
+        check_assist_import_one_variations!(
+            "foo::$0{bar}",
+            "foo::{bar::{self, other}}",
+            "use {foo::bar::{self, other}};"
+        );
+    }
+
+    #[test]
     fn test_merge_nested_list_self_and_glob() {
         check_assist(
             merge_imports,
diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/normalize_import.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/normalize_import.rs
index 7d003efe721..0b91eb676df 100644
--- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/normalize_import.rs
+++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/normalize_import.rs
@@ -120,9 +120,38 @@ mod tests {
     }
 
     #[test]
+    fn test_braces_kept() {
+        check_assist_not_applicable_variations!("foo::bar::{$0self}");
+
+        // This code compiles but transforming "bar::{self}" into "bar" causes a
+        // compilation error (the name `bar` is defined multiple times).
+        // Therefore, the normalize_input assist must not apply here.
+        check_assist_not_applicable(
+            normalize_import,
+            r"
+mod foo {
+
+    pub mod bar {}
+
+    pub const bar: i32 = 8;
+}
+
+use foo::bar::{$0self};
+
+const bar: u32 = 99;
+
+fn main() {
+    let local_bar = bar;
+}
+
+",
+        );
+    }
+
+    #[test]
     fn test_redundant_braces() {
         check_assist_variations!("foo::{bar::{baz, Qux}}", "foo::bar::{baz, Qux}");
-        check_assist_variations!("foo::{bar::{self}}", "foo::bar");
+        check_assist_variations!("foo::{bar::{self}}", "foo::bar::{self}");
         check_assist_variations!("foo::{bar::{*}}", "foo::bar::*");
         check_assist_variations!("foo::{bar::{Qux as Quux}}", "foo::bar::Qux as Quux");
         check_assist_variations!(
diff --git a/src/tools/rust-analyzer/crates/ide-db/src/imports/merge_imports.rs b/src/tools/rust-analyzer/crates/ide-db/src/imports/merge_imports.rs
index b153aafa0e1..9cacb6b1a60 100644
--- a/src/tools/rust-analyzer/crates/ide-db/src/imports/merge_imports.rs
+++ b/src/tools/rust-analyzer/crates/ide-db/src/imports/merge_imports.rs
@@ -157,10 +157,14 @@ fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior)
                     }
 
                     match (tree_contains_self(lhs_t), tree_contains_self(&rhs_t)) {
-                        (Some(true), None) => continue,
+                        (Some(true), None) => {
+                            remove_subtree_if_only_self(lhs_t);
+                            continue;
+                        }
                         (None, Some(true)) => {
                             ted::replace(lhs_t.syntax(), rhs_t.syntax());
                             *lhs_t = rhs_t;
+                            remove_subtree_if_only_self(lhs_t);
                             continue;
                         }
                         _ => (),
@@ -278,14 +282,20 @@ pub fn try_normalize_use_tree_mut(
 fn recursive_normalize(use_tree: &ast::UseTree, style: NormalizationStyle) -> Option<()> {
     let use_tree_list = use_tree.use_tree_list()?;
     let merge_subtree_into_parent_tree = |single_subtree: &ast::UseTree| {
+        let subtree_is_only_self = single_subtree.path().as_ref().map_or(false, path_is_self);
+
         let merged_path = match (use_tree.path(), single_subtree.path()) {
+            // If the subtree is `{self}` then we cannot merge: `use
+            // foo::bar::{self}` is not equivalent to `use foo::bar`. See
+            // https://github.com/rust-lang/rust-analyzer/pull/17140#issuecomment-2079189725.
+            _ if subtree_is_only_self => None,
+
             (None, None) => None,
             (Some(outer), None) => Some(outer),
-            (None, Some(inner)) if path_is_self(&inner) => None,
             (None, Some(inner)) => Some(inner),
-            (Some(outer), Some(inner)) if path_is_self(&inner) => Some(outer),
             (Some(outer), Some(inner)) => Some(make::path_concat(outer, inner).clone_for_update()),
         };
+
         if merged_path.is_some()
             || single_subtree.use_tree_list().is_some()
             || single_subtree.star_token().is_some()
@@ -706,3 +716,20 @@ fn path_is_self(path: &ast::Path) -> bool {
 fn path_len(path: ast::Path) -> usize {
     path.segments().count()
 }
+
+fn get_single_subtree(use_tree: &ast::UseTree) -> Option<ast::UseTree> {
+    use_tree
+        .use_tree_list()
+        .and_then(|tree_list| tree_list.use_trees().collect_tuple())
+        .map(|(single_subtree,)| single_subtree)
+}
+
+fn remove_subtree_if_only_self(use_tree: &ast::UseTree) {
+    let Some(single_subtree) = get_single_subtree(use_tree) else { return };
+    match (use_tree.path(), single_subtree.path()) {
+        (Some(_), Some(inner)) if path_is_self(&inner) => {
+            ted::remove_all_iter(single_subtree.syntax().children_with_tokens());
+        }
+        _ => (),
+    }
+}