diff --git a/src/adapter/edges.rs b/src/adapter/edges.rs index 25bca2bd..7e0bf4e4 100644 --- a/src/adapter/edges.rs +++ b/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> + '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> + '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> + '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> + '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> + } }; - 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 ``. + let explicit_bounds = match ¶m.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 `` so we want to extract it. + // For cases like `where T: Iterator, T::Item: Clone`, we only extract ``. + // We leave more complex cases alone, like `where Arc: 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 == ¶m.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> + 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}"), } } diff --git a/src/adapter/mod.rs b/src/adapter/mod.rs index 64445693..4b98bd32 100644 --- a/src/adapter/mod.rs +++ b/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" diff --git a/src/adapter/origin.rs b/src/adapter/origin.rs index 69f6cd0b..fc981278 100644 --- a/src/adapter/origin.rs +++ b/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), } } } diff --git a/src/adapter/properties.rs b/src/adapter/properties.rs index 8cbcf81a..cc8ed028 100644 --- a/src/adapter/properties.rs +++ b/src/adapter/properties.rs @@ -484,14 +484,56 @@ pub(super) fn resolve_trait_property<'a, V: AsVertex> + 'a>( pub(super) fn resolve_implemented_trait_property<'a, V: AsVertex> + '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> + ' ) -> 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 ) -> 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 } }), "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 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"); diff --git a/src/adapter/tests.rs b/src/adapter/tests.rs index 2fc944d7..46b89f26 100644 --- a/src/adapter/tests.rs +++ b/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, } - let mut results: Vec<_> = trustfall::execute_query( + let mut results: Vec = 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(), diff --git a/src/adapter/vertex.rs b/src/adapter/vertex.rs index 6ec94cf8..e5ac7270 100644 --- a/src/adapter/vertex.rs +++ b/src/adapter/vertex.rs @@ -33,13 +33,13 @@ pub enum VertexKind<'a> { RawType(&'a Type), Attribute(Attribute<'a>), AttributeMetaItem(Rc>), - ImplementedTrait(&'a Path, &'a Item), + ImplementedTrait(&'a Path, Option<&'a Item>), // the second item is `None` if not in our crate FunctionParameter(&'a str), FunctionAbi(&'a Abi), Discriminant(Cow<'a, str>), Variant(EnumVariant<'a>), DeriveHelperAttr(&'a str), - GenericParameter(&'a GenericParamDef), + GenericParameter(&'a rustdoc_types::Generics, &'a GenericParamDef), } impl Typename for Vertex<'_> { @@ -94,7 +94,7 @@ impl Typename for Vertex<'_> { VariantKind::Struct { .. } => "StructVariant", }, VertexKind::DeriveHelperAttr(..) => "DeriveMacroHelperAttribute", - VertexKind::GenericParameter(param) => match ¶m.kind { + VertexKind::GenericParameter(_, param) => match ¶m.kind { rustdoc_types::GenericParamDefKind::Lifetime { .. } => "GenericLifetimeParameter", rustdoc_types::GenericParamDefKind::Type { .. } => "GenericTypeParameter", rustdoc_types::GenericParamDefKind::Const { .. } => "GenericConstParameter", @@ -278,7 +278,9 @@ impl<'a> Vertex<'a> { } } - pub(super) fn as_implemented_trait(&self) -> Option<(&'a rustdoc_types::Path, &'a Item)> { + pub(super) fn as_implemented_trait( + &self, + ) -> Option<(&'a rustdoc_types::Path, Option<&'a Item>)> { match &self.kind { VertexKind::ImplementedTrait(path, trait_item) => Some((*path, *trait_item)), _ => None, @@ -299,9 +301,11 @@ impl<'a> Vertex<'a> { } } - pub(super) fn as_generic_parameter(&self) -> Option<&'a GenericParamDef> { + pub(super) fn as_generic_parameter( + &self, + ) -> Option<(&'a rustdoc_types::Generics, &'a GenericParamDef)> { match &self.kind { - VertexKind::GenericParameter(value) => Some(value), + VertexKind::GenericParameter(generics, param) => Some((generics, param)), _ => None, } } diff --git a/src/rustdoc_schema.graphql b/src/rustdoc_schema.graphql index 5343b563..9ab2bf88 100644 --- a/src/rustdoc_schema.graphql +++ b/src/rustdoc_schema.graphql @@ -1708,11 +1708,36 @@ The trait that is being implemented in an impl block. In `impl Foo for Bar`, this is the `Foo` part. """ type ImplementedTrait { + """ + The name of the trait being implemented. + + In `impl Foo for Bar`, this is `"Foo"`. + """ name: String! + """ + The item identifier of the trait being implemented here. + + Useful in case the trait's definition isn't available for querying, for example + if the trait is located in a different crate than the currently-queried one. + In such a case, there won't be any instances of the `trait` edge, but this ID + will make it possible to look up more information on the trait regardless. + """ + trait_id: String! + # own edges """ In `impl Foo for Bar`, this refers to `trait Foo`. + + Currently, item information for items from a foreign crate is not generally available, + so if the trait is defined in a different crate then this edge won't have any instances. + + Even the `std` and `core` built-in crates are considered foreign. + However, a limited number of common built-in Rust traits have their definitions hard-coded + into our adapter and their information will be available for inspection. + The set of such traits will include, at minimum: + `Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Hash, Sized, Send, Sync, Unpin, UnwindSafe, + RefUnwindSafe`. """ trait: Trait } diff --git a/test_crates/generic_parameters/src/lib.rs b/test_crates/generic_parameters/src/lib.rs index 115bd693..76bc3ea1 100644 --- a/test_crates/generic_parameters/src/lib.rs +++ b/test_crates/generic_parameters/src/lib.rs @@ -1,6 +1,7 @@ use std::hash::Hash; use std::marker::PhantomData; use std::sync::Arc; +use std::io::Write as IoWrite; pub struct GenericStruct<'a, T: Clone + PartialOrd, const N: usize> { _marker: &'a PhantomData<[T; N]>, @@ -29,10 +30,32 @@ pub fn impl_trait<'a, T: Clone + PartialOrd, const N: usize>( // The `Arc: Clone` bound involves `T` but isn't `T`'s own bound. pub fn non_included_bound(value: T) where Arc: Clone {} +// The generics here are equivalent to ``, +// so `T: Iterator` should be `T`'s own type bound even though it's written in the `where` portion. +pub fn explicit_where_bound(value: T) where T: Iterator {} + +// The generics here are equivalent to ``, +// both bounds should be `T`'s own type bounds even though they are written in the `where` portion. +pub fn combined_explicit_where_bound(value: T) where T: Iterator, T: Clone {} + // The generics here are equivalent to ` where T::Item: Clone`, // so `T: Iterator` should be `T`'s own type bound even though it's written in the `where` portion. -pub fn explicit_where_bound(value: T) where T: Iterator, T::Item: Clone {} +pub fn complex_explicit_where_bound(value: T) where T: Iterator, T::Item: Clone {} + +// The generics here are equivalent to ``, or equivalently, +// ` where T: Clone, T: Iterator` so bounds should be `T`'s own type bounds +// regardless of where they appear. +pub fn combined_bounds(value: T) where T: Iterator {} + +// We expect the `ImplementedTrait` in the bound here to still have name `Debug`, +// even though locally we're using a full path instead of an import. +pub fn full_path_trait_bound(value: T) {} + +// We expect the `ImplementedTrait` in the bound here to still have name `Write`, +// even though we've locally renamed the import to `IoWrite`. +pub fn renamed_trait_bound(value: T) {} +// Default values for generic parameters should be observed and reported. pub struct DefaultGenerics { value: [T; N], }