Allow supertrait edge to resolve to common built-in traits too. (#464…

…) (#467)
obi1kenobi · Sep 8, 2024 · 53efc48 · 53efc48
1 parent b276b13
commit 53efc48
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 33 deletions.
diff --git a/src/adapter/edges.rs b/src/adapter/edges.rs
@@ -558,9 +558,27 @@ pub(super) fn resolve_trait_edge<'a, V: AsVertex<Vertex<'a>> + 'a>(
             let trait_vertex = vertex.as_trait().expect("not a Trait vertex");
             Box::new(trait_vertex.bounds.iter().filter_map(move |bound| {
                 if let TraitBound { trait_, .. } = &bound {
-                    item_index
-                        .get(&trait_.id)
-                        .as_ref()
+                    // When the implemented trait is from the same crate
+                    // as its definition, the trait is expected to be present
+                    // in `item_index`. Otherwise, the
+                    // `rustdoc_types::Trait` is not in this rustdoc,
+                    // even if the trait is part of Rust `core` or `std`.
+                    // As a temporary workaround, some common
+                    // Rust built-in traits are manually "inlined"
+                    // with items stored in `manually_inlined_builtin_traits`.
+                    let found_item = item_index.get(&trait_.id).or_else(|| {
+                        let manually_inlined_builtin_traits = match origin {
+                            Origin::CurrentCrate => &current_crate.manually_inlined_builtin_traits,
+                            Origin::PreviousCrate => {
+                                &previous_crate
+                                    .expect("no previous crate provided")
+                                    .manually_inlined_builtin_traits
+                            }
+                        };
+                        manually_inlined_builtin_traits.get(&trait_.id)
+                    });
+
+                    found_item
                         .map(|next_item| origin.make_implemented_trait_vertex(trait_, next_item))
                 } else {
                     None

diff --git a/src/adapter/properties.rs b/src/adapter/properties.rs
@@ -487,10 +487,11 @@ pub(super) fn resolve_implemented_trait_property<'a, V: AsVertex<Vertex<'a>> + '
 ) -> ContextOutcomeIterator<'a, V, FieldValue> {
     match property_name {
         "name" => resolve_property_with(contexts, |vertex| {
-            let (path, _) = vertex
+            let (_, item) = vertex
                 .as_implemented_trait()
                 .expect("not an ImplementedTrait");
-            path.name.clone().into()
+
+            item.name.clone().into()
         }),
         _ => unreachable!("ImplementedTrait property {property_name}"),
     }

diff --git a/src/adapter/tests.rs b/src/adapter/tests.rs
@@ -135,8 +135,10 @@ fn rustdoc_finds_supertrait() {
     Crate {
         item {
             ... on Trait {
+                name @output
+
                 supertrait {
-                    name @output
+                    supertrait: name @output
                 }
             }
         }
@@ -152,6 +154,7 @@ fn rustdoc_finds_supertrait() {
     #[derive(Debug, PartialOrd, Ord, PartialEq, Eq, serde::Deserialize)]
     struct Output {
         name: String,
+        supertrait: String,
     }
 
     let mut results: Vec<_> =
@@ -164,10 +167,23 @@ fn rustdoc_finds_supertrait() {
     similar_asserts::assert_eq!(
         vec![
             Output {
-                name: "Supertrait".into(),
+                name: "DebugPartialOrd".into(),
+                // We *specifically* require the supertrait name to be "Debug",
+                // not "std::fmt::Debug" or any other option. Failing to do this
+                // could cause false-positives in cargo-semver-checks.
+                supertrait: "Debug".into(),
+            },
+            Output {
+                name: "DebugPartialOrd".into(),
+                supertrait: "PartialOrd".into(),
+            },
+            Output {
+                name: "MyTrait".into(),
+                supertrait: "Supertrait".into(),
             },
             Output {
-                name: "Supertrait2".into(),
+                name: "MyTrait".into(),
+                supertrait: "Supertrait2".into(),
             },
         ],
         results

diff --git a/src/indexed_crate.rs b/src/indexed_crate.rs
@@ -240,6 +240,11 @@ impl<'a> IndexedCrate<'a> {
             impl_index: None,
         };
 
+        debug_assert!(
+            !value.manually_inlined_builtin_traits.is_empty(),
+            "failed to find any traits to manually inline",
+        );
+
         // Build the imports index
         //
         // This is inlined because we need access to `value`, but `value` is not a valid
@@ -413,6 +418,7 @@ impl<'a: 'b, 'b> Borrow<(&'b Id, &'b str)> for ImplEntry<'a> {
 #[derive(Debug)]
 struct ManualTraitItem {
     name: &'static str,
+    path: &'static [&'static str],
     is_auto: bool,
     is_unsafe: bool,
 }
@@ -423,71 +429,85 @@ struct ManualTraitItem {
 const MANUAL_TRAIT_ITEMS: [ManualTraitItem; 14] = [
     ManualTraitItem {
         name: "Debug",
+        path: &["core", "fmt", "Debug"],
         is_auto: false,
         is_unsafe: false,
     },
     ManualTraitItem {
         name: "Clone",
+        path: &["core", "clone", "Clone"],
         is_auto: false,
         is_unsafe: false,
     },
     ManualTraitItem {
         name: "Copy",
+        path: &["core", "marker", "Copy"],
         is_auto: false,
         is_unsafe: false,
     },
     ManualTraitItem {
         name: "PartialOrd",
+        path: &["core", "cmp", "PartialOrd"],
         is_auto: false,
         is_unsafe: false,
     },
     ManualTraitItem {
         name: "Ord",
+        path: &["core", "cmp", "Ord"],
         is_auto: false,
         is_unsafe: false,
     },
     ManualTraitItem {
         name: "PartialEq",
+        path: &["core", "cmp", "PartialEq"],
         is_auto: false,
         is_unsafe: false,
     },
     ManualTraitItem {
         name: "Eq",
+        path: &["core", "cmp", "Eq"],
         is_auto: false,
         is_unsafe: false,
     },
     ManualTraitItem {
         name: "Hash",
+        path: &["core", "hash", "Hash"],
         is_auto: false,
         is_unsafe: false,
     },
     ManualTraitItem {
         name: "Send",
+        path: &["core", "marker", "Send"],
         is_auto: true,
         is_unsafe: true,
     },
     ManualTraitItem {
         name: "Sync",
+        path: &["core", "marker", "Sync"],
         is_auto: true,
         is_unsafe: true,
     },
     ManualTraitItem {
         name: "Unpin",
+        path: &["core", "marker", "Unpin"],
         is_auto: true,
         is_unsafe: false,
     },
     ManualTraitItem {
         name: "RefUnwindSafe",
+        path: &["core", "panic", "unwind_safe", "RefUnwindSafe"],
         is_auto: true,
         is_unsafe: false,
     },
     ManualTraitItem {
         name: "UnwindSafe",
+        path: &["core", "panic", "unwind_safe", "UnwindSafe"],
         is_auto: true,
         is_unsafe: false,
     },
     ManualTraitItem {
         name: "Sized",
+        path: &["core", "marker", "Sized"],
         is_auto: false,
         is_unsafe: false,
     },
@@ -534,31 +554,27 @@ fn new_trait(manual_trait_item: &ManualTraitItem, id: Id, crate_id: u32) -> Item
 }
 
 fn create_manually_inlined_builtin_traits(crate_: &Crate) -> HashMap<Id, Item> {
-    let paths = crate_
-        .index
-        .values()
-        .map(|item| &item.inner)
-        .filter_map(|item_enum| match item_enum {
-            rustdoc_types::ItemEnum::Impl(impl_) => Some(impl_),
-            _ => None,
-        })
-        .filter_map(|impl_| impl_.trait_.as_ref());
+    let paths = &crate_.paths;
 
-    paths
-        .filter_map(|path| {
-            MANUAL_TRAIT_ITEMS
-                .iter()
-                .find(|manual| manual.name == path.name)
-                .and_then(|manual| {
-                    crate_.paths.get(&path.id).map(|item_summary| {
-                        (
-                            path.id.clone(),
-                            new_trait(manual, path.id.clone(), item_summary.crate_id),
-                        )
-                    })
-                })
-        })
-        .collect()
+    // `paths` may have thousands of items.
+    #[cfg(feature = "rayon")]
+    let iter = paths.par_iter();
+    #[cfg(not(feature = "rayon"))]
+    let iter = paths.iter();
+
+    iter.filter_map(|(id, entry)| {
+        if entry.kind != rustdoc_types::ItemKind::Trait {
+            return None;
+        }
+
+        // This is a linear scan, but across a tiny array.
+        // It isn't worth doing anything fancier here.
+        MANUAL_TRAIT_ITEMS
+            .iter()
+            .find(|t| t.path == entry.path)
+            .map(|manual| (id.clone(), new_trait(manual, id.clone(), entry.crate_id)))
+    })
+    .collect()
 }
 
 #[cfg(test)]

diff --git a/test_crates/supertrait/src/lib.rs b/test_crates/supertrait/src/lib.rs
@@ -2,4 +2,9 @@ pub trait Supertrait {}
 
 pub trait Supertrait2 {}
 
-pub trait BaseTrait : Supertrait2 + Supertrait {}
+pub trait MyTrait : Supertrait2 + Supertrait {}
+
+// Ensure `std::fmt::Debug` is a supertrait here, verbatim.
+// It's a load-bearing part of the test, since we want our query output names to be invariant
+// to how they are written down in the source.
+pub trait DebugPartialOrd : std::fmt::Debug + PartialOrd {}