Repository: rust-lang/rustfmt (rust)

Issue:
`imports_granularity = "Module"` causes compile error when path contains `self`
**Describe the bug**

If a path contains `self`, this keyword must be enclosed in braces:

```rust
use crate::lexer::{self}; // valid
use crate::lexer::self;   // invalid
```

rustfmt with `imports_granularity = "Module"` sometimes splits `use` statements so that invalid code like above is emitted.

**To Reproduce**

```rust
use crate::lexer::{self, tokens::TokenData};
```

rustfmt formats it like this:

```rust
use crate::lexer::self;
use crate::lexer::tokens::TokenData;
```

When formatting the file again, the `::self` is removed.

**Expected behavior**

rustfmt should emit the following code without having to run it twice:

```rust
use crate::lexer;
use crate::lexer::tokens::TokenData;
```

Note that `use crate::lexer;` and `use crate::lexer::{self};` are not actually equivalent: The latter only imports the `lexer` module, while the former looks in all namespaces, so it can import up to 3 items at once, e.g. a function, a module and a macro of the same name.

To make sure that the behaviour doesn't change, it needs to be formatted like so:

```rust
use crate::lexer::{self};
use crate::lexer::tokens::TokenData;
```

However, I would find this rather annoying, because of the way rust-analyzer handles auto-imports.

**Meta**

- rustfmt version: rustfmt 1.4.34-nightly (ea268b9 2021-01-28)
- From where did you install rustfmt?: rustup
- How do you run rustfmt: `cargo fmt`


Pull request:
fix: imports_granularity module with path containing self
Closes #4681

(unintentionally) ports #4716 from `rustfmt-2.0.0-rc.2` to `master`, with some additional refactoring.

It looks like this issue was already fixed, but was then lost in the `rustfmt-2.0.0-rc.2` branch on this commit: https://github.com/rust-lang/rustfmt/commit/0e908555273391bc3aba087abae72791451fd84a

I implemented this separately but came up with a satisfyingly similar implementation - I've read the review comments on the original PR, so I don't think there should be much to add, but do let me know if anything needs changing!

Patch:
diff --git a/src/imports.rs b/src/imports.rs
index c60bec6d..02319809 100644
--- a/src/imports.rs
+++ b/src/imports.rs
@@ -190,13 +190,17 @@ pub(crate) fn merge_use_trees(use_trees: Vec<UseTree>, merge_by: SharedPrefix) -
             continue;
         }
 
-        for flattened in use_tree.flatten() {
+        for mut flattened in use_tree.flatten() {
             if let Some(tree) = result
                 .iter_mut()
                 .find(|tree| tree.share_prefix(&flattened, merge_by))
             {
                 tree.merge(&flattened, merge_by);
             } else {
+                // If this is the first tree with this prefix, handle potential trailing ::self
+                if merge_by == SharedPrefix::Module {
+                    flattened = flattened.nest_trailing_self();
+                }
                 result.push(flattened);
             }
         }
@@ -208,17 +212,7 @@ pub(crate) fn flatten_use_trees(use_trees: Vec<UseTree>) -> Vec<UseTree> {
     use_trees
         .into_iter()
         .flat_map(UseTree::flatten)
-        .map(|mut tree| {
-            // If a path ends in `::self`, rewrite it to `::{self}`.
-            if let Some(UseSegment::Slf(..)) = tree.path.last() {
-                let self_segment = tree.path.pop().unwrap();
-                tree.path.push(UseSegment::List(vec![UseTree::from_path(
-                    vec![self_segment],
-                    DUMMY_SP,
-                )]));
-            }
-            tree
-        })
+        .map(UseTree::nest_trailing_self)
         .collect()
 }
 
@@ -635,6 +629,18 @@ impl UseTree {
             self.span = self.span.to(other.span);
         }
     }
+
+    /// If this tree ends in `::self`, rewrite it to `::{self}`.
+    fn nest_trailing_self(mut self) -> UseTree {
+        if let Some(UseSegment::Slf(..)) = self.path.last() {
+            let self_segment = self.path.pop().unwrap();
+            self.path.push(UseSegment::List(vec![UseTree::from_path(
+                vec![self_segment],
+                DUMMY_SP,
+            )]));
+        }
+        self
+    }
 }
 
 fn merge_rest(
@@ -1311,4 +1317,24 @@ mod test {
                 < parse_use_tree("std::cmp::{b, e, g, f}").normalize()
         );
     }
+
+    #[test]
+    fn test_use_tree_nest_trailing_self() {
+        assert_eq!(
+            parse_use_tree("a::b::self").nest_trailing_self(),
+            parse_use_tree("a::b::{self}")
+        );
+        assert_eq!(
+            parse_use_tree("a::b::c").nest_trailing_self(),
+            parse_use_tree("a::b::c")
+        );
+        assert_eq!(
+            parse_use_tree("a::b::{c, d}").nest_trailing_self(),
+            parse_use_tree("a::b::{c, d}")
+        );
+        assert_eq!(
+            parse_use_tree("a::b::{self, c}").nest_trailing_self(),
+            parse_use_tree("a::b::{self, c}")
+        );
+    }
 }


Test patch:
diff --git a/tests/source/imports_granularity_module.rs b/tests/source/imports_granularity_module.rs
index 5a4fad58..2d7bb299 100644
--- a/tests/source/imports_granularity_module.rs
+++ b/tests/source/imports_granularity_module.rs
@@ -4,6 +4,7 @@ use a::{b::c, d::e};
 use a::{f, g::{h, i}};
 use a::{j::{self, k::{self, l}, m}, n::{o::p, q}};
 pub use a::{r::s, t};
+use b::{c::d, self};
 
 #[cfg(test)]
 use foo::{a::b, c::d};
diff --git a/tests/target/imports_granularity_module.rs b/tests/target/imports_granularity_module.rs
index 9c1387c4..e4e1a299 100644
--- a/tests/target/imports_granularity_module.rs
+++ b/tests/target/imports_granularity_module.rs
@@ -10,6 +10,8 @@ use a::n::o::p;
 use a::n::q;
 pub use a::r::s;
 pub use a::t;
+use b::c::d;
+use b::{self};
 
 use foo::e;
 #[cfg(test)]