Allow looking up ImplementedTrait even if the trait item is missing. (

#530) (#541)

* Allow looking up `ImplementedTrait` even if the trait item is missing. (#530)

The trait item is only present if the trait is local to our crate, or if
it's one of a handful of common Rust built-in traits whose definitions
we manually inline. The previous behavior of the adapter was to
_not produce_ an `ImplementedTrait` vertex if the item wasn't present.
This made it look like generics were missing some bounds, etc.

This PR implements a better choice: it produces an `ImplementedTrait`
always, and instead makes it so that the edge to the underlying `Trait`
might be missing. We also provide the trait ID and a renaming-proof
trait name, to further improve the robustness of this code and make it
possible for downstream use cases to look up the foreign trait by ID.

* Fix compilation error.

* Update edges.rs

src/adapter/edges.rs

@@ -1,4 +1,4 @@
-use rustdoc_types::{GenericBound::TraitBound, Id, ItemEnum, VariantKind};
+use rustdoc_types::{GenericBound::TraitBound, Id, ItemEnum, VariantKind, WherePredicate};
 use std::rc::Rc;
 use trustfall::provider::{
     resolve_neighbors_with, AsVertex, ContextIterator, ContextOutcomeIterator, ResolveEdgeInfo,
@@ -198,10 +198,11 @@ pub(super) fn resolve_generic_parameter_edge<'a, V: AsVertex<Vertex<'a>> + 'a>(
             Box::new(
                 vertex
                     .as_generics()
-                    .map(move |g| {
-                        g.params
+                    .map(move |generics| {
+                        generics
+                            .params
                             .iter()
-                            .map(move |param| origin.make_generic_parameter_vertex(param))
+                            .map(move |param| origin.make_generic_parameter_vertex(generics, param))
                     })
                     .into_iter()
                     .flatten(),
@@ -522,13 +523,10 @@ pub(super) fn resolve_impl_edge<'a, V: AsVertex<Vertex<'a>> + 'a>(
                     };
                     manually_inlined_builtin_traits.get(&path.id)
                 });
-                if let Some(item) = found_item {
-                    Box::new(std::iter::once(
-                        origin.make_implemented_trait_vertex(path, item),
-                    ))
-                } else {
-                    Box::new(std::iter::empty())
-                }
+
+                Box::new(std::iter::once(
+                    origin.make_implemented_trait_vertex(path, found_item),
+                ))
             } else {
                 Box::new(std::iter::empty())
             }
@@ -601,8 +599,7 @@ pub(super) fn resolve_trait_edge<'a, V: AsVertex<Vertex<'a>> + 'a>(
                         manually_inlined_builtin_traits.get(&trait_.id)
                     });
 
-                    found_item
-                        .map(|next_item| origin.make_implemented_trait_vertex(trait_, next_item))
+                    Some(origin.make_implemented_trait_vertex(trait_, found_item))
                 } else {
                     None
                 }
@@ -699,7 +696,12 @@ pub(super) fn resolve_implemented_trait_edge<'a, V: AsVertex<Vertex<'a>> + 'a>(
             let (_, trait_item) = vertex
                 .as_implemented_trait()
                 .expect("vertex was not an ImplementedTrait");
-            Box::new(std::iter::once(origin.make_item_vertex(trait_item)))
+
+            Box::new(
+                trait_item
+                    .into_iter()
+                    .map(move |item| origin.make_item_vertex(item)),
+            )
         }),
         _ => unreachable!("resolve_implemented_trait_edge {edge_name}"),
     }
@@ -788,13 +790,54 @@ pub(super) fn resolve_generic_type_parameter_edge<'a, V: AsVertex<Vertex<'a>> +
                 }
             };
 
-            let generic = vertex
+            let (generics, param): (
+                &'a rustdoc_types::Generics,
+                &'a rustdoc_types::GenericParamDef,
+            ) = vertex
                 .as_generic_parameter()
                 .expect("vertex was not a GenericTypeParameter");
 
-            match &generic.kind {
-                rustdoc_types::GenericParamDefKind::Type { bounds, .. } => {
-                    Box::new(bounds.iter().filter_map(move |bound| {
+            // Bounds directly applied to the generic, like `<T: Clone>`.
+            let explicit_bounds = match &param.kind {
+                rustdoc_types::GenericParamDefKind::Type { bounds, .. } => bounds.as_slice(),
+                _ => unreachable!("vertex was not a GenericTypeParameter: {vertex:?}"),
+            };
+
+            // Lift `where` bounds that could have been written as bounds on the generic.
+            // For example: `where T: Clone` is the same as `<T: Clone>` so we want to extract it.
+            // For cases like `where T: Iterator, T::Item: Clone`, we only extract `<T: Iterator>`.
+            // We leave more complex cases alone, like `where Arc<T>: Clone`
+            // or `where for<'a> &'a: Iterator`.
+            let where_bounds = generics.where_predicates.iter().filter_map(move |predicate| {
+                match predicate {
+                    WherePredicate::BoundPredicate { type_, bounds, generic_params } => {
+                        if !generic_params.is_empty() {
+                            // `generic_params` is only used for HRTBs,
+                            // which can't be represented as bounds on the generic itself.
+                            return None;
+                        }
+
+                        if !matches!(type_, rustdoc_types::Type::Generic(name) if name == &param.name) {
+                            // This bound is not directly on the generic we're looking at.
+                            // For example, it might be `where T::Item: Clone`,
+                            // or it might be on a different generic parameter, like `U: Clone`.
+                            return None;
+                        }
+
+                        Some(bounds.as_slice())
+                    }
+                    WherePredicate::RegionPredicate { .. } | WherePredicate::EqPredicate { .. } => {
+                        // Neither of these cases can be written as a bound on a generic parameter.
+                        None
+                    }
+                }
+            }).flatten();
+
+            Box::new(
+                explicit_bounds
+                    .iter()
+                    .chain(where_bounds)
+                    .filter_map(move |bound| {
                         if let TraitBound { trait_, .. } = &bound {
                             // When the implemented trait is from the same crate
                             // as its definition, the trait is expected to be present
@@ -818,17 +861,13 @@ pub(super) fn resolve_generic_type_parameter_edge<'a, V: AsVertex<Vertex<'a>> +
                                 manually_inlined_builtin_traits.get(&trait_.id)
                             });
 
-                            found_item.map(|next_item| {
-                                origin.make_implemented_trait_vertex(trait_, next_item)
-                            })
+                            Some(origin.make_implemented_trait_vertex(trait_, found_item))
                         } else {
                             None
                         }
-                    }))
-                }
-                _ => unreachable!("vertex was not a GenericTypeParameter: {vertex:?}"),
-            }
+                    }),
+            )
         }),
-        _ => unreachable!("resolve_derive_proc_macro_edge {edge_name}"),
+        _ => unreachable!("resolve_generic_type_parameter_edge {edge_name}"),
     }
 }

src/adapter/mod.rs

@@ -166,9 +166,12 @@ impl<'a> Adapter<'a> for RustdocAdapter<'a> {
                     self.current_crate,
                     self.previous_crate,
                 ),
-                "ImplementedTrait" => {
-                    properties::resolve_implemented_trait_property(contexts, property_name)
-                }
+                "ImplementedTrait" => properties::resolve_implemented_trait_property(
+                    contexts,
+                    property_name,
+                    self.current_crate,
+                    self.previous_crate,
+                ),
                 "Static" => properties::resolve_static_property(contexts, property_name),
                 "RawType" | "ResolvedPathType" if matches!(property_name.as_ref(), "name") => {
                     // fields from "RawType"

src/adapter/origin.rs

@@ -78,7 +78,7 @@ impl Origin {
     pub(super) fn make_implemented_trait_vertex<'a>(
         &self,
         path: &'a rustdoc_types::Path,
-        trait_def: &'a Item,
+        trait_def: Option<&'a Item>,
     ) -> Vertex<'a> {
         Vertex {
             origin: *self,
@@ -128,11 +128,12 @@ impl Origin {
 
     pub(super) fn make_generic_parameter_vertex<'a>(
         &self,
+        generics: &'a rustdoc_types::Generics,
         param: &'a rustdoc_types::GenericParamDef,
     ) -> Vertex<'a> {
         Vertex {
             origin: *self,
-            kind: VertexKind::GenericParameter(param),
+            kind: VertexKind::GenericParameter(generics, param),
         }
     }
 }

src/adapter/properties.rs

@@ -484,14 +484,56 @@ pub(super) fn resolve_trait_property<'a, V: AsVertex<Vertex<'a>> + 'a>(
 pub(super) fn resolve_implemented_trait_property<'a, V: AsVertex<Vertex<'a>> + 'a>(
     contexts: ContextIterator<'a, V>,
     property_name: &str,
+    current_crate: &'a IndexedCrate<'a>,
+    previous_crate: Option<&'a IndexedCrate<'a>>,
 ) -> ContextOutcomeIterator<'a, V, FieldValue> {
     match property_name {
-        "name" => resolve_property_with(contexts, |vertex| {
-            let (_, item) = vertex
+        "name" => resolve_property_with(contexts, move |vertex| {
+            let origin_crate = match vertex.origin {
+                Origin::CurrentCrate => current_crate,
+                Origin::PreviousCrate => {
+                    previous_crate.as_ref().expect("no previous crate provided")
+                }
+            };
+            let (info, item) = vertex
+                .as_implemented_trait()
+                .expect("not an ImplementedTrait");
+
+            if let Some(item) = item {
+                // We have the full item already. Use the original declaration name.
+                item.name.clone().into()
+            } else if let Some(summary) = origin_crate.inner.paths.get(&info.id) {
+                // The item is from a foreign crate.
+                // The last component of the canonical path should match its declaration name,
+                // so use that.
+                summary
+                    .path
+                    .last()
+                    .unwrap_or_else(|| {
+                        panic!("empty path for id {} in vertex {vertex:?}", info.id.0)
+                    })
+                    .clone()
+                    .into()
+            } else if let Some((_, last)) = info.name.rsplit_once("::") {
+                // For some reason, we didn't find the item either locally or
+                // in the `paths` section of the rustdoc JSON.
+                //
+                // Use the name in the `implemented_trait` portion itself.
+                // That name seems to be sensitive to how the source represents the item:
+                // it might be a full path, or just a name, or anything in between.
+                // If it's a path, grab its last component (this block).
+                // Otherwise, fall through to the `else` block to return it as-is.
+                last.to_string().into()
+            } else {
+                info.name.clone().into()
+            }
+        }),
+        "trait_id" => resolve_property_with(contexts, |vertex| {
+            let (info, _) = vertex
                 .as_implemented_trait()
                 .expect("not an ImplementedTrait");
 
-            item.name.clone().into()
+            info.id.0.to_string().into()
         }),
         _ => unreachable!("ImplementedTrait property {property_name}"),
     }
@@ -617,12 +659,11 @@ pub(crate) fn resolve_generic_parameter_property<'a, V: AsVertex<Vertex<'a>> + '
 ) -> ContextOutcomeIterator<'a, V, FieldValue> {
     match property_name {
         "name" => resolve_property_with(contexts, |vertex| {
-            vertex
+            let (_, generic) = vertex
                 .as_generic_parameter()
-                .expect("vertex was not a GenericParameter")
-                .name
-                .clone()
-                .into()
+                .expect("vertex was not a GenericParameter");
+
+            generic.name.clone().into()
         }),
         _ => unreachable!("GenericParameter property {property_name}"),
     }
@@ -634,7 +675,7 @@ pub(crate) fn resolve_generic_type_parameter_property<'a, V: AsVertex<Vertex<'a>
 ) -> ContextOutcomeIterator<'a, V, FieldValue> {
     match property_name {
         "has_default" => resolve_property_with(contexts, |vertex| {
-            let generic = vertex
+            let (_, generic) = vertex
                 .as_generic_parameter()
                 .expect("vertex was not a GenericTypeParameter");
 
@@ -646,7 +687,7 @@ pub(crate) fn resolve_generic_type_parameter_property<'a, V: AsVertex<Vertex<'a>
             }
         }),
         "synthetic" => resolve_property_with(contexts, |vertex| {
-            let generic = vertex
+            let (_, generic) = vertex
                 .as_generic_parameter()
                 .expect("vertex was not a GenericTypeParameter");
 
@@ -665,7 +706,7 @@ pub(crate) fn resolve_generic_const_parameter_property<'a, V: AsVertex<Vertex<'a
 ) -> ContextOutcomeIterator<'a, V, FieldValue> {
     match property_name {
         "has_default" => resolve_property_with(contexts, |vertex| {
-            let generic = vertex
+            let (_, generic) = vertex
                 .as_generic_parameter()
                 .expect("vertex was not a GenericConstParameter");
 

src/adapter/tests.rs

@@ -2657,6 +2657,31 @@ fn generic_parameters() {
             generic_kind: "GenericTypeParameter".into(),
             generic_name: "T".into(),
         },
+        Output {
+            name: "combined_explicit_where_bound".into(),
+            generic_kind: "GenericTypeParameter".into(),
+            generic_name: "T".into(),
+        },
+        Output {
+            name: "complex_explicit_where_bound".into(),
+            generic_kind: "GenericTypeParameter".into(),
+            generic_name: "T".into(),
+        },
+        Output {
+            name: "combined_bounds".into(),
+            generic_kind: "GenericTypeParameter".into(),
+            generic_name: "T".into(),
+        },
+        Output {
+            name: "full_path_trait_bound".into(),
+            generic_kind: "GenericTypeParameter".into(),
+            generic_name: "T".into(),
+        },
+        Output {
+            name: "renamed_trait_bound".into(),
+            generic_kind: "GenericTypeParameter".into(),
+            generic_name: "T".into(),
+        },
         Output {
             name: "DefaultGenerics".into(),
             generic_kind: "GenericTypeParameter".into(),
@@ -2782,7 +2807,7 @@ fn generic_type_parameters() {
         bound: Vec<String>,
     }
 
-    let mut results: Vec<_> = trustfall::execute_query(
+    let mut results: Vec<Output> = trustfall::execute_query(
         &schema,
         adapter.clone(),
         top_level_query,
@@ -2809,7 +2834,10 @@ fn generic_type_parameters() {
     )
     .map(|row| row.try_into_struct().expect("shape mismatch"))
     .collect();
+
+    // Ensure that the results are in sorted order, and also that the aggregated bounds are sorted.
     results.sort_unstable();
+    results.iter_mut().for_each(|row| row.bound.sort_unstable());
 
     // We write the results in the order the items appear in the test file,
     // and sort them afterward in order to compare with the (sorted) query results.
@@ -2905,16 +2933,53 @@ fn generic_type_parameters() {
             generic_name: "T".into(),
             synthetic: false,
             has_default: false,
+            bound: { ["Iterator"].into_iter().map(ToString::to_string).collect() },
+        },
+        Output {
+            name: "combined_explicit_where_bound".into(),
+            generic_name: "T".into(),
+            synthetic: false,
+            has_default: false,
+            bound: {
+                ["Clone", "Iterator"]
+                    .into_iter()
+                    .map(ToString::to_string)
+                    .collect()
+            },
+        },
+        Output {
+            name: "complex_explicit_where_bound".into(),
+            generic_name: "T".into(),
+            synthetic: false,
+            has_default: false,
+            bound: { ["Iterator"].into_iter().map(ToString::to_string).collect() },
+        },
+        Output {
+            name: "combined_bounds".into(),
+            generic_name: "T".into(),
+            synthetic: false,
+            has_default: false,
             bound: {
-                // TODO: FIXME, the correct bound here should be ["Iterator"]
-                //       but because we only partially inline built-in trait definitions,
-                //       `Iterator` is not resolved and is invisible.
-                //       When that's fixed, switch implementations to:
-                //
-                // ["Iterator"].into_iter().map(ToString::to_string).collect(),
-                vec![]
+                ["Clone", "Iterator"]
+                    .into_iter()
+                    .map(ToString::to_string)
+                    .collect()
             },
         },
+        Output {
+            name: "full_path_trait_bound".into(),
+            generic_name: "T".into(),
+            synthetic: false,
+            has_default: false,
+            bound: { ["Debug"].into_iter().map(ToString::to_string).collect() },
+        },
+        Output {
+            name: "renamed_trait_bound".into(),
+            generic_name: "T".into(),
+            synthetic: false,
+            has_default: false,
+            bound: { ["Write"].into_iter().map(ToString::to_string).collect() },
+        },
         Output {
             name: "DefaultGenerics".into(),
             generic_name: "T".into(),