Skip to content

Commit

Permalink
Traits cannot be generic-sealed, that only prevents method calls. (#348
Browse files Browse the repository at this point in the history
…) (#351)
  • Loading branch information
obi1kenobi authored Aug 18, 2024
1 parent eaebb87 commit fe08efb
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 74 deletions.
4 changes: 2 additions & 2 deletions src/adapter/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,8 @@ fn rustdoc_sealed_traits() {
sealed: false,
},
Output {
name: "GenericSealed".into(),
sealed: true,
name: "TraitUnsealedButMethodGenericSealed".into(),
sealed: false,
},
Output {
name: "NotGenericSealedBecauseOfDefaultImpl".into(),
Expand Down
71 changes: 4 additions & 67 deletions src/sealed_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@ pub(crate) fn is_trait_sealed<'a>(indexed_crate: &IndexedCrate<'a>, trait_item:
}

// Does the trait have a method that:
// - does not have a default impl, and either:
// a. takes at least one non-`self` argument that is pub-in-priv, or
// b. has a generic parameter that causes it to be generic-sealed
// If so, the trait is method-sealed or generic-sealed, per:
// - does not have a default impl, and
// - takes at least one non-`self` argument that is pub-in-priv
//
// If so, the trait is method-sealed, per:
// https://predr.ag/blog/definitive-guide-to-sealed-traits-in-rust/#sealing-traits-via-method-signatures
// https://jack.wrenn.fyi/blog/private-trait-methods/
// https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3393c3ae143cb75c9da7bfe6e8ff8084
if is_method_sealed(indexed_crate, trait_inner) {
return true;
}
Expand Down Expand Up @@ -108,73 +106,12 @@ fn is_method_sealed<'a>(indexed_crate: &IndexedCrate<'a>, trait_inner: &'a Trait
}
}
}

// Check for generics-sealed methods, as described in:
// https://jack.wrenn.fyi/blog/private-trait-methods/
// https://www.reddit.com/r/rust/comments/12cj6as/comment/jf21zsm/
for generic_param in &func.generics.params {
match &generic_param.kind {
rustdoc_types::GenericParamDefKind::Type {
bounds, default, ..
} => {
// If the generic parameter has a default, it can't be used to seal.
if default.is_none() {
for bound in bounds {
if is_generic_type_bound_sealed(indexed_crate, bound) {
return true;
}
}
}
}
_ => continue,
}
}
}
}

false
}

fn is_generic_type_bound_sealed<'a>(
indexed_crate: &IndexedCrate<'a>,
bound: &'a rustdoc_types::GenericBound,
) -> bool {
match bound {
rustdoc_types::GenericBound::TraitBound {
trait_: trait_path, ..
} => {
// For the bound to be sealing, it needs to:
// - point to a pub-in-priv trait in this crate.
// - all supertraits of the pub-in-priv trait must also be pub-in-priv
let Some(item) = indexed_crate.inner.index.get(&trait_path.id) else {
// Not an item in this crate.
return false;
};
if !is_pub_in_priv_item(indexed_crate, &item.id) {
return false;
}

let trait_item = unwrap_trait(item);

// Check all supertraits to ensure they are pub-in-priv as well.
for trait_bounds in &trait_item.bounds {
if let rustdoc_types::GenericBound::TraitBound {
trait_: inner_trait_path,
..
} = trait_bounds
{
if !is_local_pub_in_priv_path(indexed_crate, inner_trait_path) {
return false;
}
}
}

true
}
_ => false,
}
}

fn is_local_pub_in_priv_path<'a>(
indexed_crate: &IndexedCrate<'a>,
path: &'a rustdoc_types::Path,
Expand Down
31 changes: 26 additions & 5 deletions test_crates/sealed_traits/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,23 @@ pub trait NotMethodSealedBecauseOfDefaultImpl {
/// This trait is *not* sealed. Its supertrait is also not sealed.
pub trait NotTransitivelySealed: NotMethodSealedBecauseOfDefaultImpl {}

/// This trait's method is generic-sealed: downstream implementors are not able
/// to write the trait bound since the internal marker trait is pub-in-priv.
pub trait GenericSealed {
/// This trait's method is generic-sealed, but the trait itself is *not* sealed!
/// Downstream users aren't able to call this method because
/// type-privacy will prevent inference of `IM` since the marker trait is pub-in-priv.
/// But downstream implementations are possible:
///
/// ```rust
/// struct Foo;
///
/// impl sealed_traits::TraitUnsealedButMethodGenericSealed for Foo {
/// fn method<IM>(&self) {}
/// }
/// ```
pub trait TraitUnsealedButMethodGenericSealed {
fn method<IM: private::InternalMarker>(&self);
}

/// This trait is *not* sealed. Its method cannot be overridden,
/// This trait is *not* sealed. Its method cannot be called, but can be overridden and
/// but implementing it is not required since the trait offers a default impl.
pub trait NotGenericSealedBecauseOfDefaultImpl {
fn private_method<IM: private::InternalMarker>(&self) {}
Expand All @@ -65,7 +75,7 @@ pub mod generic_seal {
pub trait Marker: super::Super {}
}

/// This trait is *not* generic-sealed.
/// This trait is *not* generic-sealed, and neither is its method.
///
/// While the `Marker` trait bound is pub-in-priv, it has a public supertrait `Super`.
/// The `Super` bound can be used downstream to implement this trait:
Expand All @@ -79,6 +89,17 @@ pub mod generic_seal {
/// fn method<IM: Super>(&self) {}
/// }
/// ```
///
/// An unbounded `IM` implementation is also allowed:
/// ```rust
/// use sealed_traits::generic_seal::NotGenericSealedBecauseOfPubSupertrait;
///
/// struct Example;
///
/// impl NotGenericSealedBecauseOfPubSupertrait for Example {
/// fn method<IM>(&self) {}
/// }
/// ```
pub trait NotGenericSealedBecauseOfPubSupertrait {
fn method<IM: private::Marker>(&self);
}
Expand Down

0 comments on commit fe08efb

Please sign in to comment.