Merge #8897

8897: minor: Don't compare ast::Visibility by stringifying r=Veykril a=Veykril

bors r+

Co-authored-by: Lukas Tobias Wirth <lukastw97@gmail.com>

crates/ide_assists/src/handlers/merge_imports.rs

@@ -212,6 +212,20 @@ pub(crate) use std::fmt::{Debug, Display};
         )
     }
 
+    #[test]
+    fn merge_pub_in_path_crate() {
+        check_assist(
+            merge_imports,
+            r"
+pub(in this::path) use std::fmt$0::Debug;
+pub(in this::path) use std::fmt::Display;
+",
+            r"
+pub(in this::path) use std::fmt::{Debug, Display};
+",
+        )
+    }
+
     #[test]
     fn test_merge_nested() {
         check_assist(

crates/ide_db/src/helpers/merge_imports.rs

@@ -292,9 +292,7 @@ fn path_segment_cmp(a: &ast::PathSegment, b: &ast::PathSegment) -> Ordering {
 pub fn eq_visibility(vis0: Option<ast::Visibility>, vis1: Option<ast::Visibility>) -> bool {
     match (vis0, vis1) {
         (None, None) => true,
-        // FIXME: Don't use the string representation to check for equality
+        (Some(vis0), Some(vis1)) => vis0.is_eq_to(&vis1),
-        // spaces inside of the node would break this comparison
-        (Some(vis0), Some(vis1)) => vis0.to_string() == vis1.to_string(),
         _ => false,
     }
 }
@@ -303,9 +301,14 @@ pub fn eq_attrs(
     attrs0: impl Iterator<Item = ast::Attr>,
     attrs1: impl Iterator<Item = ast::Attr>,
 ) -> bool {
-    let attrs0 = attrs0.map(|attr| attr.to_string());
+    // FIXME order of attributes should not matter
-    let attrs1 = attrs1.map(|attr| attr.to_string());
+    let attrs0 = attrs0
-    attrs0.eq(attrs1)
+        .flat_map(|attr| attr.syntax().descendants_with_tokens())
+        .flat_map(|it| it.into_token());
+    let attrs1 = attrs1
+        .flat_map(|attr| attr.syntax().descendants_with_tokens())
+        .flat_map(|it| it.into_token());
+    stdx::iter_eq_by(attrs0, attrs1, |tok, tok2| tok.text() == tok2.text())
 }
 
 fn path_is_self(path: &ast::Path) -> bool {

crates/stdx/src/lib.rs

@@ -140,6 +140,34 @@ impl JodChild {
     }
 }
 
+// feature: iter_order_by
+// Iterator::eq_by
+pub fn iter_eq_by<I, I2, F>(this: I2, other: I, mut eq: F) -> bool
+where
+    I: IntoIterator,
+    I2: IntoIterator,
+    F: FnMut(I2::Item, I::Item) -> bool,
+{
+    let mut other = other.into_iter();
+    let mut this = this.into_iter();
+
+    loop {
+        let x = match this.next() {
+            None => return other.next().is_none(),
+            Some(val) => val,
+        };
+
+        let y = match other.next() {
+            None => return false,
+            Some(val) => val,
+        };
+
+        if !eq(x, y) {
+            return false;
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/syntax/src/ast/node_ext.rs

@@ -608,6 +608,29 @@ impl ast::Visibility {
             None => VisibilityKind::Pub,
         }
     }
+
+    pub fn is_eq_to(&self, other: &Self) -> bool {
+        match (self.kind(), other.kind()) {
+            (VisibilityKind::In(this), VisibilityKind::In(other)) => {
+                stdx::iter_eq_by(this.segments(), other.segments(), |lhs, rhs| {
+                    lhs.kind().zip(rhs.kind()).map_or(false, |it| match it {
+                        (PathSegmentKind::CrateKw, PathSegmentKind::CrateKw)
+                        | (PathSegmentKind::SelfKw, PathSegmentKind::SelfKw)
+                        | (PathSegmentKind::SuperKw, PathSegmentKind::SuperKw) => true,
+                        (PathSegmentKind::Name(lhs), PathSegmentKind::Name(rhs)) => {
+                            lhs.text() == rhs.text()
+                        }
+                        _ => false,
+                    })
+                })
+            }
+            (VisibilityKind::PubSelf, VisibilityKind::PubSelf)
+            | (VisibilityKind::PubSuper, VisibilityKind::PubSuper)
+            | (VisibilityKind::PubCrate, VisibilityKind::PubCrate)
+            | (VisibilityKind::Pub, VisibilityKind::Pub) => true,
+            _ => false,
+        }
+    }
 }
 
 impl ast::LifetimeParam {