rustdoc: handle generics better when matching notable traits

src/librustdoc/clean/types.rs

@@ -1471,27 +1471,68 @@ impl Type {
         result
     }
 
-    /// Check if two types are "potentially the same".
+    pub(crate) fn is_borrowed_ref(&self) -> bool {
+        matches!(self, Type::BorrowedRef { .. })
+    }
+
+    /// Check if two types are "the same" for documentation purposes.
+    ///
     /// This is different from `Eq`, because it knows that things like
     /// `Placeholder` are possible matches for everything.
-    pub(crate) fn is_same(&self, other: &Self, cache: &Cache) -> bool {
-        match (self, other) {
+    ///
+    /// This relation is not commutative when generics are involved:
+    ///
+    /// ```ignore(private)
+    /// # // see types/tests.rs:is_same_generic for the real test
+    /// use rustdoc::format::cache::Cache;
+    /// use rustdoc::clean::types::{Type, PrimitiveType};
+    /// let cache = Cache::new(false);
+    /// let generic = Type::Generic(rustc_span::symbol::sym::Any);
+    /// let unit = Type::Primitive(PrimitiveType::Unit);
+    /// assert!(!generic.is_same(&unit, &cache));
+    /// assert!(unit.is_same(&generic, &cache));
+    /// ```
+    ///
+    /// An owned type is also the same as its borrowed variants (this is commutative),
+    /// but `&T` is not the same as `&mut T`.
+    pub(crate) fn is_doc_subtype_of(&self, other: &Self, cache: &Cache) -> bool {
+        // Strip the references so that it can compare the actual types, unless both are references.
+        // If both are references, leave them alone and compare the mutabilities later.
+        let (self_cleared, other_cleared) = if !self.is_borrowed_ref() || !other.is_borrowed_ref() {
+            (self.without_borrowed_ref(), other.without_borrowed_ref())
+        } else {
+            (self, other)
+        };
+        match (self_cleared, other_cleared) {
             // Recursive cases.
             (Type::Tuple(a), Type::Tuple(b)) => {
-                a.len() == b.len() && a.iter().zip(b).all(|(a, b)| a.is_same(b, cache))
+                a.len() == b.len() && a.iter().zip(b).all(|(a, b)| a.is_doc_subtype_of(b, cache))
             }
-            (Type::Slice(a), Type::Slice(b)) => a.is_same(b, cache),
-            (Type::Array(a, al), Type::Array(b, bl)) => al == bl && a.is_same(b, cache),
+            (Type::Slice(a), Type::Slice(b)) => a.is_doc_subtype_of(b, cache),
+            (Type::Array(a, al), Type::Array(b, bl)) => al == bl && a.is_doc_subtype_of(b, cache),
             (Type::RawPointer(mutability, type_), Type::RawPointer(b_mutability, b_type_)) => {
-                mutability == b_mutability && type_.is_same(b_type_, cache)
+                mutability == b_mutability && type_.is_doc_subtype_of(b_type_, cache)
             }
             (
                 Type::BorrowedRef { mutability, type_, .. },
                 Type::BorrowedRef { mutability: b_mutability, type_: b_type_, .. },
-            ) => mutability == b_mutability && type_.is_same(b_type_, cache),
-            // Placeholders and generics are equal to all other types.
+            ) => mutability == b_mutability && type_.is_doc_subtype_of(b_type_, cache),
+            // Placeholders are equal to all other types.
             (Type::Infer, _) | (_, Type::Infer) => true,
-            (Type::Generic(_), _) | (_, Type::Generic(_)) => true,
+            // Generics match everything on the right, but not on the left.
+            // If both sides are generic, this returns true.
+            (_, Type::Generic(_)) => true,
+            (Type::Generic(_), _) => false,
+            // Paths account for both the path itself and its generics.
+            (Type::Path { path: a }, Type::Path { path: b }) => {
+                a.def_id() == b.def_id()
+                    && a.generics()
+                        .zip(b.generics())
+                        .map(|(ag, bg)| {
+                            ag.iter().zip(bg.iter()).all(|(at, bt)| at.is_doc_subtype_of(bt, cache))
+                        })
+                        .unwrap_or(true)
+            }
             // Other cases, such as primitives, just use recursion.
             (a, b) => a
                 .def_id(cache)

src/librustdoc/clean/types/tests.rs

@@ -69,3 +69,14 @@ fn should_not_trim() {
     run_test("\t    line1  \n\t    line2", "line1  \nline2");
     run_test("    \tline1  \n    \tline2", "line1  \nline2");
 }
+
+#[test]
+fn is_same_generic() {
+    use crate::clean::types::{PrimitiveType, Type};
+    use crate::formats::cache::Cache;
+    let cache = Cache::new(false);
+    let generic = Type::Generic(rustc_span::symbol::sym::Any);
+    let unit = Type::Primitive(PrimitiveType::Unit);
+    assert!(!generic.is_doc_subtype_of(&unit, &cache));
+    assert!(unit.is_doc_subtype_of(&generic, &cache));
+}

src/librustdoc/html/render/mod.rs

@@ -1291,7 +1291,7 @@ pub(crate) fn notable_traits_button(ty: &clean::Type, cx: &mut Context<'_>) -> O
     if let Some(impls) = cx.cache().impls.get(&did) {
         for i in impls {
             let impl_ = i.inner_impl();
-            if !impl_.for_.without_borrowed_ref().is_same(ty.without_borrowed_ref(), cx.cache()) {
+            if !ty.is_doc_subtype_of(&impl_.for_, cx.cache()) {
                 // Two different types might have the same did,
                 // without actually being the same.
                 continue;
@@ -1327,7 +1327,7 @@ fn notable_traits_decl(ty: &clean::Type, cx: &Context<'_>) -> (String, String) {
 
     for i in impls {
         let impl_ = i.inner_impl();
-        if !impl_.for_.without_borrowed_ref().is_same(ty.without_borrowed_ref(), cx.cache()) {
+        if !ty.is_doc_subtype_of(&impl_.for_, cx.cache()) {
             // Two different types might have the same did,
             // without actually being the same.
             continue;

tests/rustdoc/doc-notable_trait-slice.rs → tests/rustdoc/notable-trait/doc-notable_trait-slice.rs

@@ -18,3 +18,9 @@ pub fn bare_fn_matches() -> &'static [SomeStruct] {
 pub fn bare_fn_no_matches() -> &'static [OtherStruct] {
     &[]
 }
+
+// @has doc_notable_trait_slice/fn.bare_fn_mut_no_matches.html
+// @count - '//script[@id="notable-traits-data"]' 0
+pub fn bare_fn_mut_no_matches() -> &'static mut [SomeStruct] {
+    &mut []
+}

tests/rustdoc/notable-trait/notable-trait-generics.rs

@@ -0,0 +1,35 @@
+#![feature(doc_notable_trait)]
+
+// Notable traits SHOULD NOT be shown when the `impl` has a concrete type and
+// the return type has a generic type.
+pub mod generic_return {
+    pub struct Wrapper<T>(T);
+
+    #[doc(notable_trait)]
+    pub trait NotableTrait {}
+
+    impl NotableTrait for Wrapper<u8> {}
+
+    // @has notable_trait_generics/generic_return/fn.returning.html
+    // @!has - '//a[@class="tooltip"]/@data-notable-ty' 'Wrapper<T>'
+    pub fn returning<T>() -> Wrapper<T> {
+        loop {}
+    }
+}
+
+// Notable traits SHOULD be shown when the `impl` has a generic type and the
+// return type has a concrete type.
+pub mod generic_impl {
+    pub struct Wrapper<T>(T);
+
+    #[doc(notable_trait)]
+    pub trait NotableTrait {}
+
+    impl<T> NotableTrait for Wrapper<T> {}
+
+    // @has notable_trait_generics/generic_impl/fn.returning.html
+    // @has - '//a[@class="tooltip"]/@data-notable-ty' 'Wrapper<u8>'
+    pub fn returning() -> Wrapper<u8> {
+        loop {}
+    }
+}