diff --git a/src/adapter/tests.rs b/src/adapter/tests.rs index 14ea530..28ce035 100644 --- a/src/adapter/tests.rs +++ b/src/adapter/tests.rs @@ -297,6 +297,122 @@ fn rustdoc_sealed_traits() { name: "NotGenericSealedBecauseOfPubSupertrait".into(), sealed: false, }, + Output { + name: "FullBlanket".into(), + sealed: true, + }, + Output { + name: "PrivateBlanket".into(), + sealed: true, + }, + Output { + name: "RefBlanket".into(), + sealed: true, + }, + Output { + name: "ExternalSupertraitsBlanket".into(), + sealed: true, + }, + Output { + name: "BlanketWithWhereClause".into(), + sealed: true, + }, + Output { + name: "IteratorBlanket".into(), + sealed: true, + }, + Output { + name: "BlanketOverLocalUnsealedTrait".into(), + sealed: true, + }, + Output { + name: "BlanketOverSealedTrait".into(), + sealed: true, + }, + Output { + name: "BlanketOverSealedAndUnsealedTrait".into(), + sealed: true, + }, + Output { + name: "TransitiveBlanket".into(), + sealed: true, + }, + Output { + name: "BlanketOverArc".into(), + sealed: true, + }, + Output { + name: "BlanketOverTuple".into(), + sealed: true, + }, + Output { + name: "BlanketOverSlice".into(), + sealed: true, + }, + Output { + name: "BlanketOverArray".into(), + sealed: true, + }, + Output { + name: "BlanketOverPointer".into(), + sealed: true, + }, + Output { + name: "BlanketUnsealed".into(), + sealed: false, + }, + Output { + name: "RefBlanketUnsealed".into(), + sealed: false, + }, + Output { + name: "ExternalSupertraitsBlanketUnsealed".into(), + sealed: false, + }, + Output { + name: "BlanketWithWhereClauseUnsealed".into(), + sealed: false, + }, + Output { + name: "IteratorBlanketUnsealed".into(), + sealed: false, + }, + Output { + name: "BlanketOverLocalUnsealedTraitUnsealed".into(), + sealed: false, + }, + Output { + name: "BlanketOverSealedTraitSealed".into(), + sealed: true, + }, + Output { + name: "BlanketSealedOverMultiple".into(), + sealed: true, + }, + Output { + name: "TransitiveBlanketUnsealed".into(), + sealed: false, + }, + Output { + name: "BlanketOverArcSealed".into(), + sealed: true, + }, + Output { + name: "BlanketOverTupleSealed".into(), + sealed: true, + }, + Output { + name: "BlanketOverSliceSealed".into(), + sealed: true, + }, + Output { + name: "BlanketOverArraySealed".into(), + sealed: true, + }, + Output { + name: "BlanketOverPointerSealed".into(), + sealed: true, + }, ]; expected_results.sort_unstable(); diff --git a/src/sealed_trait.rs b/src/sealed_trait.rs index 0241293..8820e3e 100644 --- a/src/sealed_trait.rs +++ b/src/sealed_trait.rs @@ -2,18 +2,11 @@ use rustdoc_types::{Item, Trait}; use crate::IndexedCrate; -pub(crate) fn is_trait_sealed<'a>(indexed_crate: &IndexedCrate<'a>, trait_item: &'a Item) -> bool { - let trait_inner = unwrap_trait(trait_item); +pub(crate) fn is_trait_sealed<'a>(indexed_crate: &IndexedCrate<'a>, item: &'a Item) -> bool { + let trait_item = unwrap_trait(item); // If the trait is pub-in-priv, trivially sealed. - if is_pub_in_priv_item(indexed_crate, &trait_item.id) { - return true; - } - - // Does the trait have a supertrait that is: - // - defined in this crate - // - pub-in-priv, or otherwise sealed - if has_sealed_supertrait(indexed_crate, trait_inner) { + if is_pub_in_priv_item(indexed_crate, &item.id) { return true; } @@ -23,7 +16,15 @@ pub(crate) fn is_trait_sealed<'a>(indexed_crate: &IndexedCrate<'a>, trait_item: // // If so, the trait is method-sealed, per: // https://predr.ag/blog/definitive-guide-to-sealed-traits-in-rust/#sealing-traits-via-method-signatures - if is_method_sealed(indexed_crate, trait_inner) { + if is_method_sealed(indexed_crate, trait_item) { + return true; + } + + // Does the trait have a supertrait that is all of the below: + // - defined in this crate + // - pub-in-priv, or otherwise sealed + // - lacking a blanket impl whose bounds can be satisfied outside this crate + if has_sealed_supertrait(indexed_crate, trait_item) { return true; } @@ -42,6 +43,31 @@ fn unwrap_trait(item: &Item) -> &'_ Trait { } } +fn is_method_sealed<'a>(indexed_crate: &IndexedCrate<'a>, trait_inner: &'a Trait) -> bool { + for inner_item_id in &trait_inner.items { + let inner_item = &indexed_crate.inner.index[inner_item_id]; + if let rustdoc_types::ItemEnum::Function(func) = &inner_item.inner { + if func.has_body { + // This trait function has a default implementation. + // An implementation is not required in order to implement this trait on a type. + // Therefore, it cannot on its own cause the trait to be sealed. + continue; + } + + // Check for pub-in-priv function parameters. + for (_, param) in &func.decl.inputs { + if let rustdoc_types::Type::ResolvedPath(path) = param { + if is_local_pub_in_priv_path(indexed_crate, path) { + return true; + } + } + } + } + } + + false +} + fn has_sealed_supertrait<'a>(indexed_crate: &IndexedCrate<'a>, inner: &'a Trait) -> bool { for bound in &inner.bounds { let supertrait = match bound { @@ -78,38 +104,201 @@ fn has_sealed_supertrait<'a>(indexed_crate: &IndexedCrate<'a>, inner: &'a Trait) // ``` // // Here, both `First` and `Second` are sealed. + // + // N.B.: This cannot infinite-loop, since rustc denies cyclic trait bounds. if is_trait_sealed(indexed_crate, supertrait) { - // N.B.: This cannot infinite-loop, since rustc denies cyclic trait bounds. - return true; + // The supertrait is sealed, so a downstream crate cannot add a direct impl for it + // on any of its own types. + // + // However, this isn't the same thing as "downstream types cannot impl this trait"! + // We must check the supertrait for blanket impls that might result in + // a downstream type gaining an impl for that trait. + if has_no_externally_satisfiable_blanket_impls(indexed_crate, supertrait) { + return true; + } } } false } -fn is_method_sealed<'a>(indexed_crate: &IndexedCrate<'a>, trait_inner: &'a Trait) -> bool { - for inner_item_id in &trait_inner.items { - let inner_item = &indexed_crate.inner.index[inner_item_id]; - if let rustdoc_types::ItemEnum::Function(func) = &inner_item.inner { - if func.has_body { - // This trait function has a default implementation. - // An implementation is not required in order to implement this trait on a type. - // Therefore, it cannot on its own cause the trait to be sealed. +fn has_no_externally_satisfiable_blanket_impls( + indexed_crate: &IndexedCrate<'_>, + trait_: &Item, +) -> bool { + let trait_item = unwrap_trait(trait_); + for impl_id in &trait_item.implementations { + let impl_item = match indexed_crate.inner.index.get(impl_id).map(|item| { + let rustdoc_types::ItemEnum::Impl(impl_item) = &item.inner else { + panic!("impl Id {impl_id:?} did not refer to an impl item: {item:?}"); + }; + impl_item + }) { + Some(item) => item, + None => { + // Failed to find the impl item in the index. continue; } + }; - // Check for pub-in-priv function parameters. - for (_, param) in &func.decl.inputs { - if let rustdoc_types::Type::ResolvedPath(path) = param { - if is_local_pub_in_priv_path(indexed_crate, path) { - return true; + if is_externally_satisfiable_blanket_impl(indexed_crate, impl_item) { + return false; + } + } + + true +} + +fn is_externally_satisfiable_blanket_impl( + indexed_crate: &IndexedCrate<'_>, + impl_item: &rustdoc_types::Impl, +) -> bool { + let blanket_type = match get_impl_target_if_blanket_impl(impl_item) { + None => { + // Not a blanket impl, so not relevant here. + return false; + } + Some(blanket) => blanket, + }; + + // Can the blanket cover a type that a downstream crate might define? + // For example, `T` and `&T` count, whereas `Vec`, `[T]`, and `*const T` do not. + if !blanket_type_might_cover_types_in_downstream_crate(blanket_type) { + // This blanket impl doesn't cover types of a downstream crate. It isn't relevant here. + return false; + } + + // Can the bounds on this impl be satisfied by downstream crates' types? + for generic in &impl_item.generics.params { + match &generic.kind { + rustdoc_types::GenericParamDefKind::Type { + bounds, synthetic, .. + } => { + if *synthetic { + // Synthetic bounds don't count. We also don't really expect to find one here. + continue; + } + + // The blanket impl is only not externally satisfiable if at least one trait bound + // references a trait where all of the following apply: + // - The trait is local to the crate we're analyzing. + // - The trait is sealed. + // - The trait has no blanket impls that are externally satisfiable. + // (The same criterion we're in the middle of evaluating for another trait here.) + for bound in bounds { + if is_bounded_on_local_sealed_trait_without_blankets(indexed_crate, bound) { + return false; } } } + rustdoc_types::GenericParamDefKind::Lifetime { .. } + | rustdoc_types::GenericParamDefKind::Const { .. } => { + // Lifetime and const generics aren't relevant here. + continue; + } } } - false + true +} + +fn get_impl_target_if_blanket_impl( + impl_item: &rustdoc_types::Impl, +) -> Option<&rustdoc_types::Type> { + let mut current_type = &impl_item.for_; + + loop { + match current_type { + rustdoc_types::Type::ResolvedPath { .. } | // e.g. `Arc` + rustdoc_types::Type::DynTrait { .. } | // e.g. `dyn Iterator` + rustdoc_types::Type::Tuple { .. } | // e.g. `(T,)` + rustdoc_types::Type::Slice { .. } | // e.g. `[T]` + rustdoc_types::Type::Array { .. } | // e.g. `[T; 1]` + rustdoc_types::Type::Pat { .. } => { // unstable feature, syntax isn't finalized + // These are all specific types that simply have a generic parameter. + // They are not blanket implementations. + return None; + } + rustdoc_types::Type::Generic(..) => { + // Blanket impl! + break; + } + rustdoc_types::Type::Primitive { .. } | + rustdoc_types::Type::FunctionPointer { .. } | + rustdoc_types::Type::Infer | + rustdoc_types::Type::ImplTrait { .. } | + rustdoc_types::Type::QualifiedPath { .. } => { + // Not a blanket impl. + return None; + } + rustdoc_types::Type::RawPointer { type_, .. } | + rustdoc_types::Type::BorrowedRef { type_, .. } => { + current_type = type_; + } + } + } + + Some(&impl_item.for_) +} + +fn is_bounded_on_local_sealed_trait_without_blankets( + indexed_crate: &IndexedCrate<'_>, + bound: &rustdoc_types::GenericBound, +) -> bool { + match bound { + rustdoc_types::GenericBound::TraitBound { trait_, .. } => { + let bound_trait_id = &trait_.id; + let Some(item) = indexed_crate.inner.index.get(bound_trait_id) else { + // Not a trait from this crate. + return false; + }; + + // We cannot have an infinite loop here, since Rust won't allow cyclic bounds. + if !is_trait_sealed(indexed_crate, item) { + return false; + } + + has_no_externally_satisfiable_blanket_impls(indexed_crate, item) + } + rustdoc_types::GenericBound::Outlives(_) => { + // Other kinds of generic bounds aren't relevant here. + false + } + } +} + +fn blanket_type_might_cover_types_in_downstream_crate(blanket_type: &rustdoc_types::Type) -> bool { + match blanket_type { + rustdoc_types::Type::Generic(..) => { + // Blanket implementation over a bare generic type, like `T`. + // This matches! + true + } + rustdoc_types::Type::BorrowedRef { type_, .. } => { + // Blanket implementatio over a reference, like `&T`. + // It matches if the underlying type beheath the reference matches. + blanket_type_might_cover_types_in_downstream_crate(type_) + } + rustdoc_types::Type::ResolvedPath { .. } | // e.g. `Arc` + rustdoc_types::Type::Tuple { .. } | // e.g. `(T,)` + rustdoc_types::Type::Slice { .. } | // e.g. `[T]` + rustdoc_types::Type::Array { .. } | // e.g. `[T; 1]` + rustdoc_types::Type::RawPointer { .. } => { // e.g. `*const T` + // All these types are considered "foreign" by trait coherence, + // so Rust does not allow implementing another crate's trait on them. + false + } + rustdoc_types::Type::DynTrait { .. } | + rustdoc_types::Type::Primitive { .. } | + rustdoc_types::Type::FunctionPointer { .. } | + rustdoc_types::Type::Pat { .. } | + rustdoc_types::Type::ImplTrait { .. } | + rustdoc_types::Type::Infer { .. } | + rustdoc_types::Type::QualifiedPath { .. } => { + // None of these can cover a type in a downstream crate. + false + } + } } fn is_local_pub_in_priv_path<'a>( diff --git a/test_crates/sealed_traits/src/lib.rs b/test_crates/sealed_traits/src/lib.rs index 327536d..5cd4b82 100644 --- a/test_crates/sealed_traits/src/lib.rs +++ b/test_crates/sealed_traits/src/lib.rs @@ -113,3 +113,226 @@ pub mod generic_seal { fn method(&self); } } + +/// Traits whose supertraits have blanket impls, where the blanket bounds +/// can be satisfied in a downstream crate, are not sealed. +mod blanket_impls { + pub trait FullBlanket {} + impl FullBlanket for T {} + + // This trait is both crate-private and inside a private module. + pub(crate) trait PrivateBlanket {} + impl PrivateBlanket for T {} + + pub trait RefBlanket {} + impl RefBlanket for &T {} + + pub trait ExternalSupertraitsBlanket {} + impl ExternalSupertraitsBlanket for T {} + + // In Rust this is syntax sugar for `impl`, but let's make sure rustdoc thinks so too. + pub trait BlanketWithWhereClause {} + impl BlanketWithWhereClause for T where T: Clone {} + + // The iterator trait is special because we don't manually inline it into rustdoc info. + // See `MANUAL_TRAIT_ITEMS` inside `indexed_crate.rs` for more details. + pub trait IteratorBlanket {} + impl IteratorBlanket for T {} + + pub trait BlanketOverLocalUnsealedTrait {} + impl BlanketOverLocalUnsealedTrait for T {} + + pub trait BlanketOverSealedTrait {} + impl BlanketOverSealedTrait for T {} + + pub trait BlanketOverSealedAndUnsealedTrait {} + impl BlanketOverSealedAndUnsealedTrait for T {} + + // The blanket impl here is over everything, + // because `FullBlanket` has a blanket impl for everything. + pub trait TransitiveBlanket {} + impl TransitiveBlanket for T {} + + pub trait BlanketOverArc {} + impl BlanketOverArc for std::sync::Arc {} + + pub trait BlanketOverTuple {} + impl BlanketOverTuple for (T,) {} + + pub trait BlanketOverSlice {} + impl BlanketOverSlice for [T] {} + + pub trait BlanketOverArray {} + impl BlanketOverArray for [T; 1] {} + + pub trait BlanketOverPointer {} + impl BlanketOverPointer for *const T {} +} + +/// Not sealed due to blanket impl. +/// +/// Proof: +/// ```rust +/// struct Example; +/// +/// impl sealed_traits::BlanketUnsealed for Example {} +/// ``` +#[allow(private_bounds)] +pub trait BlanketUnsealed: blanket_impls::FullBlanket + blanket_impls::PrivateBlanket {} + +/// Not sealed due to blanket impl. +/// +/// Proof: +/// ```rust +/// struct Example; +/// +/// impl sealed_traits::RefBlanketUnsealed for &Example {} +/// ``` +pub trait RefBlanketUnsealed: blanket_impls::RefBlanket {} + +/// Not sealed due to blanket impl. +/// +/// Proof: +/// ```rust +/// #[derive(Debug, Clone)] +/// struct Example; +/// +/// impl sealed_traits::ExternalSupertraitsBlanketUnsealed for Example {} +/// ``` +pub trait ExternalSupertraitsBlanketUnsealed: blanket_impls::ExternalSupertraitsBlanket {} + +/// Not sealed due to blanket impl. +/// +/// Proof: +/// ```rust +/// #[derive(Clone)] +/// struct Example; +/// +/// impl sealed_traits::BlanketWithWhereClauseUnsealed for Example {} +/// ``` +pub trait BlanketWithWhereClauseUnsealed: blanket_impls::BlanketWithWhereClause {} + + +/// Not sealed due to blanket impl. +/// +/// Proof: +/// ```rust +/// struct ExampleIter; +/// +/// impl Iterator for ExampleIter { +/// type Item = (); +/// +/// fn next(&mut self) -> Option { +/// None +/// } +/// } +/// +/// impl sealed_traits::IteratorBlanketUnsealed for ExampleIter {} +/// ``` +pub trait IteratorBlanketUnsealed: blanket_impls::IteratorBlanket {} + +/// Not sealed due to blanket impl. +/// +/// Proof: +/// ```rust +/// struct Example; +/// +/// impl sealed_traits::Unsealed for Example {} +/// +/// impl sealed_traits::BlanketOverLocalUnsealedTraitUnsealed for Example {} +/// ``` +pub trait BlanketOverLocalUnsealedTraitUnsealed: blanket_impls::BlanketOverLocalUnsealedTrait {} + +/// This one is sealed, since the blanket is over a sealed trait which we cannot impl. +/// +/// Proof, in two parts: +/// +/// We cannot implement the supertrait ourselves: +/// ```compile_fail +/// struct Example; +/// +/// // The next line won't work, since the item is pub-in-priv +/// // so the trait is sealed and inaccessible. +/// impl sealed_traits::blanket_impls::BlanketOverSealedTrait for Example {} +/// +/// impl sealed_traits::BlanketOverSealedTraitSealed for Example {} +/// ``` +/// +/// And we cannot implement the trait in the blanket bound: +/// ```compile_fail +/// struct Example; +/// +/// // the next line won't work +/// impl sealed_traits::DirectlyTraitSealed for Example {} +/// +/// impl sealed_traits::BlanketOverSealedTraitSealed for Example {} +/// ``` +pub trait BlanketOverSealedTraitSealed: blanket_impls::BlanketOverSealedTrait {} + +/// This trait is sealed because the bound on the blanket impl +/// includes a trait we cannot impl. The proof is the same as above. +pub trait BlanketSealedOverMultiple: blanket_impls::BlanketOverSealedAndUnsealedTrait {} + +/// This trait is not sealed, since its supertrait has a blanket impl whose bound +/// is always satisfied due to its own blanket impl. +/// +/// Proof: +/// ```rust +/// struct Example; +/// +/// impl sealed_traits::TransitiveBlanketUnsealed for Example {} +/// ``` +pub trait TransitiveBlanketUnsealed: blanket_impls::TransitiveBlanket {} + +/// This trait is sealed. +/// - Its supertrait has a blanket impl over `Arc`. +/// - In order for a crate to implement a trait for a type, the crate needs to define +/// either the trait or the type. A downstream crate doesn't define either. +/// +/// Proof: +/// ```compile_fail +/// struct Example; +/// +/// impl sealed_traits::BlanketOverArcSealed for std::sync::Arc {} +/// ``` +pub trait BlanketOverArcSealed: blanket_impls::BlanketOverArc {} + +/// Sealed since tuples/slices/arrays/pointers are always considered foreign types. +/// +/// Proof: +/// ```compile_fail +/// struct Example; +/// +/// impl sealed_traits::BlanketOverTupleSealed for (Example,) {} +/// ``` +pub trait BlanketOverTupleSealed: blanket_impls::BlanketOverTuple {} + +/// Sealed since tuples/slices/arrays/pointers are always considered foreign types. +/// +/// Proof: +/// ```compile_fail +/// struct Example; +/// +/// impl sealed_traits::BlanketOverSliceSealed for [Example] {} +/// ``` +pub trait BlanketOverSliceSealed: blanket_impls::BlanketOverSlice {} + +/// Sealed since tuples/slices/arrays/pointers are always considered foreign types. +/// +/// Proof: +/// ```compile_fail +/// struct Example; +/// +/// impl sealed_traits::BlanketOverArraySealed for [Example; 1] {} +/// ``` +pub trait BlanketOverArraySealed: blanket_impls::BlanketOverArray {} + +/// Sealed since tuples/slices/arrays/pointers are always considered foreign types. +/// +/// Proof: +/// ```compile_fail +/// struct Example; +/// +/// impl sealed_traits::BlanketOverPointerSealed for *const Example {} +/// ``` +pub trait BlanketOverPointerSealed: blanket_impls::BlanketOverPointer {}