Repository: rust-lang/rustfmt (rust)

Issue:
bug: imports_granularity = "Module"/"Crate"/"One" crash code when de-duplicate dependencies
Recently, I enabled a rustfmt rule:
```toml
imports_granularity = "Module"
```

After I re-fmt, the code failed to be compiled. And thus I found an unexpected rustfmt behaviour:

before fmt:
```rust
use risingwave_pb::data::DataType;
use risingwave_pb::data::DataType as DataTypeProst;
```

after fmt:
```rust
use risingwave_pb::data::DataType as DataTypeProst;
```

But the codes that use "DataType" are not changed.

I don't think that rustfmt should turn the correct code into incorrect in any condition.














Pull request:
Fix import_granularity option when the use tree has an alias
Close #5131

Patch:
diff --git a/src/imports.rs b/src/imports.rs
index 40e0d06f..c60bec6d 100644
--- a/src/imports.rs
+++ b/src/imports.rs
@@ -238,7 +238,8 @@ impl fmt::Display for UseSegment {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
             UseSegment::Glob => write!(f, "*"),
-            UseSegment::Ident(ref s, _) => write!(f, "{}", s),
+            UseSegment::Ident(ref s, Some(ref alias)) => write!(f, "{} as {}", s, alias),
+            UseSegment::Ident(ref s, None) => write!(f, "{}", s),
             UseSegment::Slf(..) => write!(f, "self"),
             UseSegment::Super(..) => write!(f, "super"),
             UseSegment::Crate(..) => write!(f, "crate"),
@@ -622,7 +623,8 @@ impl UseTree {
     fn merge(&mut self, other: &UseTree, merge_by: SharedPrefix) {
         let mut prefix = 0;
         for (a, b) in self.path.iter().zip(other.path.iter()) {
-            if a.equal_except_alias(b) {
+            // only discard the alias at the root of the tree
+            if (prefix == 0 && a.equal_except_alias(b)) || a == b {
                 prefix += 1;
             } else {
                 break;
diff --git a/src/lists.rs b/src/lists.rs
index 7aa0315f..97eea19f 100644
--- a/src/lists.rs
+++ b/src/lists.rs
@@ -611,15 +611,30 @@ pub(crate) fn extract_post_comment(
     post_snippet: &str,
     comment_end: usize,
     separator: &str,
+    is_last: bool,
 ) -> Option<String> {
     let white_space: &[_] = &[' ', '\t'];
 
     // Cleanup post-comment: strip separators and whitespace.
     let post_snippet = post_snippet[..comment_end].trim();
+
+    let last_inline_comment_ends_with_separator = if is_last {
+        if let Some(line) = post_snippet.lines().last() {
+            line.ends_with(separator) && line.trim().starts_with("//")
+        } else {
+            false
+        }
+    } else {
+        false
+    };
+
     let post_snippet_trimmed = if post_snippet.starts_with(|c| c == ',' || c == ':') {
         post_snippet[1..].trim_matches(white_space)
     } else if let Some(stripped) = post_snippet.strip_prefix(separator) {
         stripped.trim_matches(white_space)
+    } else if last_inline_comment_ends_with_separator {
+        // since we're on the last item it's fine to keep any trailing separators in comments
+        post_snippet.trim_matches(white_space)
     }
     // not comment or over two lines
     else if post_snippet.ends_with(',')
@@ -748,14 +763,12 @@ where
                 .snippet_provider
                 .span_to_snippet(mk_sp((self.get_hi)(&item), next_start))
                 .unwrap_or("");
-            let comment_end = get_comment_end(
-                post_snippet,
-                self.separator,
-                self.terminator,
-                self.inner.peek().is_none(),
-            );
+            let is_last = self.inner.peek().is_none();
+            let comment_end =
+                get_comment_end(post_snippet, self.separator, self.terminator, is_last);
             let new_lines = has_extra_newline(post_snippet, comment_end);
-            let post_comment = extract_post_comment(post_snippet, comment_end, self.separator);
+            let post_comment =
+                extract_post_comment(post_snippet, comment_end, self.separator, is_last);
 
             self.prev_span_end = (self.get_hi)(&item) + BytePos(comment_end as u32);
 


Test patch:
diff --git a/tests/source/5131_crate.rs b/tests/source/5131_crate.rs
new file mode 100644
index 00000000..96a31659
--- /dev/null
+++ b/tests/source/5131_crate.rs
@@ -0,0 +1,14 @@
+// rustfmt-imports_granularity: Crate
+
+use foo::a;
+use foo::a;
+use foo::b;
+use foo::b as b2;
+use foo::b::f;
+use foo::b::g;
+use foo::b::g as g2;
+use foo::c;
+use foo::d::e;
+use qux::h;
+use qux::h as h2;
+use qux::i;
diff --git a/tests/source/5131_module.rs b/tests/source/5131_module.rs
new file mode 100644
index 00000000..3e913917
--- /dev/null
+++ b/tests/source/5131_module.rs
@@ -0,0 +1,33 @@
+// rustfmt-imports_granularity: Module
+
+#![allow(dead_code)]
+
+mod a {
+    pub mod b {
+        pub struct Data {
+            pub a: i32,
+        }
+    }
+
+    use crate::a::b::Data;
+    use crate::a::b::Data as Data2;
+
+    pub fn data(a: i32) -> Data {
+        Data { a }
+    }
+
+    pub fn data2(a: i32) -> Data2 {
+        Data2 { a }
+    }
+
+    #[cfg(test)]
+    mod tests {
+        use super::*;
+
+        #[test]
+        pub fn test() {
+            data(1);
+            data2(1);
+        }
+    }
+}
diff --git a/tests/source/5131_one.rs b/tests/source/5131_one.rs
new file mode 100644
index 00000000..61ddf134
--- /dev/null
+++ b/tests/source/5131_one.rs
@@ -0,0 +1,15 @@
+// rustfmt-imports_granularity: One
+
+pub use foo::x;
+pub use foo::x as x2;
+pub use foo::y;
+use bar::a;
+use bar::b;
+use bar::b::f;
+use bar::b::f as f2;
+use bar::b::g;
+use bar::c;
+use bar::d::e;
+use bar::d::e as e2;
+use qux::h;
+use qux::i;
diff --git a/tests/source/issue-5042/multi-line_comment_with_trailing_comma.rs b/tests/source/issue-5042/multi-line_comment_with_trailing_comma.rs
new file mode 100644
index 00000000..5d171f32
--- /dev/null
+++ b/tests/source/issue-5042/multi-line_comment_with_trailing_comma.rs
@@ -0,0 +1,24 @@
+fn main() {
+    // 5042 deals with trailing commas, not the indentation issue of these comments
+    // When a future PR fixes the inentation issues these test can be updated
+    let _ = std::ops::Add::add(10, 20
+        // ...
+        // ...,
+        );
+
+    let _ = std::ops::Add::add(10, 20
+        /* ... */
+        // ...,
+        );
+
+    let _ = std::ops::Add::add(10, 20
+        // ...,
+        // ...,
+        );
+
+    let _ = std::ops::Add::add(10, 20
+        // ...,
+        /* ...
+        */,
+        );
+}
diff --git a/tests/source/issue-5042/multi-line_comment_without_trailing_comma.rs b/tests/source/issue-5042/multi-line_comment_without_trailing_comma.rs
new file mode 100644
index 00000000..b8a824b3
--- /dev/null
+++ b/tests/source/issue-5042/multi-line_comment_without_trailing_comma.rs
@@ -0,0 +1,24 @@
+fn main() {
+    // 5042 deals with trailing commas, not the indentation issue of these comments
+    // When a future PR fixes the inentation issues these test can be updated
+    let _ = std::ops::Add::add(10, 20
+        // ...
+        // ...
+        );
+
+    let _ = std::ops::Add::add(10, 20
+        /* ... */
+        // ...
+        );
+
+    let _ = std::ops::Add::add(10, 20
+        // ...
+        // ...
+        );
+
+    let _ = std::ops::Add::add(10, 20
+        // ...
+        /* ...
+        */
+        );
+}
diff --git a/tests/source/issue-5042/single-line_comment_with_trailing_comma.rs b/tests/source/issue-5042/single-line_comment_with_trailing_comma.rs
new file mode 100644
index 00000000..bd765b7b
--- /dev/null
+++ b/tests/source/issue-5042/single-line_comment_with_trailing_comma.rs
@@ -0,0 +1,9 @@
+fn main() {
+    let _ = std::ops::Add::add(10, 20
+        // ...,
+        );
+
+    let _ = std::ops::Add::add(10, 20
+        /* ... */,
+        );
+}
diff --git a/tests/source/issue-5042/single-line_comment_without_trailing_comma.rs b/tests/source/issue-5042/single-line_comment_without_trailing_comma.rs
new file mode 100644
index 00000000..2ed8de87
--- /dev/null
+++ b/tests/source/issue-5042/single-line_comment_without_trailing_comma.rs
@@ -0,0 +1,10 @@
+fn main() {
+    let _ = std::ops::Add::add(10, 20
+        // ...
+        );
+
+    let _ = std::ops::Add::add(10, 20
+        /* ... */
+        );
+}
+
diff --git a/tests/target/5131_crate.rs b/tests/target/5131_crate.rs
new file mode 100644
index 00000000..557d6670
--- /dev/null
+++ b/tests/target/5131_crate.rs
@@ -0,0 +1,9 @@
+// rustfmt-imports_granularity: Crate
+
+use foo::{
+    a, b, b as b2,
+    b::{f, g, g as g2},
+    c,
+    d::e,
+};
+use qux::{h, h as h2, i};
diff --git a/tests/target/5131_module.rs b/tests/target/5131_module.rs
new file mode 100644
index 00000000..763024d6
--- /dev/null
+++ b/tests/target/5131_module.rs
@@ -0,0 +1,32 @@
+// rustfmt-imports_granularity: Module
+
+#![allow(dead_code)]
+
+mod a {
+    pub mod b {
+        pub struct Data {
+            pub a: i32,
+        }
+    }
+
+    use crate::a::b::{Data, Data as Data2};
+
+    pub fn data(a: i32) -> Data {
+        Data { a }
+    }
+
+    pub fn data2(a: i32) -> Data2 {
+        Data2 { a }
+    }
+
+    #[cfg(test)]
+    mod tests {
+        use super::*;
+
+        #[test]
+        pub fn test() {
+            data(1);
+            data2(1);
+        }
+    }
+}
diff --git a/tests/target/5131_one.rs b/tests/target/5131_one.rs
new file mode 100644
index 00000000..a086dae5
--- /dev/null
+++ b/tests/target/5131_one.rs
@@ -0,0 +1,12 @@
+// rustfmt-imports_granularity: One
+
+pub use foo::{x, x as x2, y};
+use {
+    bar::{
+        a,
+        b::{self, f, g},
+        c,
+        d::{e, e as e2},
+    },
+    qux::{h, i},
+};
diff --git a/tests/target/issue-5042/multi-line_comment_with_trailing_comma.rs b/tests/target/issue-5042/multi-line_comment_with_trailing_comma.rs
new file mode 100644
index 00000000..1ae1212b
--- /dev/null
+++ b/tests/target/issue-5042/multi-line_comment_with_trailing_comma.rs
@@ -0,0 +1,24 @@
+fn main() {
+    // 5042 deals with trailing commas, not the indentation issue of these comments
+    // When a future PR fixes the inentation issues these test can be updated
+    let _ = std::ops::Add::add(
+        10, 20, // ...
+           // ...,
+    );
+
+    let _ = std::ops::Add::add(
+        10, 20, /* ... */
+           // ...,
+    );
+
+    let _ = std::ops::Add::add(
+        10, 20, // ...,
+           // ...,
+    );
+
+    let _ = std::ops::Add::add(
+        10, 20, // ...,
+           /* ...
+            */
+    );
+}
diff --git a/tests/target/issue-5042/multi-line_comment_without_trailing_comma.rs b/tests/target/issue-5042/multi-line_comment_without_trailing_comma.rs
new file mode 100644
index 00000000..30d17466
--- /dev/null
+++ b/tests/target/issue-5042/multi-line_comment_without_trailing_comma.rs
@@ -0,0 +1,24 @@
+fn main() {
+    // 5042 deals with trailing commas, not the indentation issue of these comments
+    // When a future PR fixes the inentation issues these test can be updated
+    let _ = std::ops::Add::add(
+        10, 20, // ...
+           // ...
+    );
+
+    let _ = std::ops::Add::add(
+        10, 20, /* ... */
+           // ...
+    );
+
+    let _ = std::ops::Add::add(
+        10, 20, // ...
+           // ...
+    );
+
+    let _ = std::ops::Add::add(
+        10, 20, // ...
+           /* ...
+            */
+    );
+}
diff --git a/tests/target/issue-5042/single-line_comment_with_trailing_comma.rs b/tests/target/issue-5042/single-line_comment_with_trailing_comma.rs
new file mode 100644
index 00000000..87b651dd
--- /dev/null
+++ b/tests/target/issue-5042/single-line_comment_with_trailing_comma.rs
@@ -0,0 +1,7 @@
+fn main() {
+    let _ = std::ops::Add::add(
+        10, 20, // ...,
+    );
+
+    let _ = std::ops::Add::add(10, 20 /* ... */);
+}
diff --git a/tests/target/issue-5042/single-line_comment_without_trailing_comma.rs b/tests/target/issue-5042/single-line_comment_without_trailing_comma.rs
new file mode 100644
index 00000000..116df86a
--- /dev/null
+++ b/tests/target/issue-5042/single-line_comment_without_trailing_comma.rs
@@ -0,0 +1,7 @@
+fn main() {
+    let _ = std::ops::Add::add(
+        10, 20, // ...
+    );
+
+    let _ = std::ops::Add::add(10, 20 /* ... */);
+}