From fe08efb215622c555b66be23128cfca605852ebd Mon Sep 17 00:00:00 2001 From: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> Date: Sun, 18 Aug 2024 16:15:54 -0400 Subject: [PATCH] Traits cannot be generic-sealed, that only prevents method calls. (#348) (#351) --- src/adapter/tests.rs | 4 +- src/sealed_trait.rs | 71 ++-------------------------- test_crates/sealed_traits/src/lib.rs | 31 ++++++++++-- 3 files changed, 32 insertions(+), 74 deletions(-) diff --git a/src/adapter/tests.rs b/src/adapter/tests.rs index 1d00db0..26cd5d4 100644 --- a/src/adapter/tests.rs +++ b/src/adapter/tests.rs @@ -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(), diff --git a/src/sealed_trait.rs b/src/sealed_trait.rs index 6ed9978..47d23d9 100644 --- a/src/sealed_trait.rs +++ b/src/sealed_trait.rs @@ -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; } @@ -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, diff --git a/test_crates/sealed_traits/src/lib.rs b/test_crates/sealed_traits/src/lib.rs index 5eba563..6844f61 100644 --- a/test_crates/sealed_traits/src/lib.rs +++ b/test_crates/sealed_traits/src/lib.rs @@ -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(&self) {} +/// } +/// ``` +pub trait TraitUnsealedButMethodGenericSealed { fn method(&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(&self) {} @@ -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: @@ -79,6 +89,17 @@ pub mod generic_seal { /// fn method(&self) {} /// } /// ``` + /// + /// An unbounded `IM` implementation is also allowed: + /// ```rust + /// use sealed_traits::generic_seal::NotGenericSealedBecauseOfPubSupertrait; + /// + /// struct Example; + /// + /// impl NotGenericSealedBecauseOfPubSupertrait for Example { + /// fn method(&self) {} + /// } + /// ``` pub trait NotGenericSealedBecauseOfPubSupertrait { fn method(&self); }