rustdoc: Document whether traits are sealed or not

[obi1kenobi]

A trait is considered sealed if it can only be implemented in the crate that defined it. This obviously has important semver implications. Many changes that are breaking and semver-major on unsealed traits are completely fine and non-breaking on sealed traits:

• adding trait functions, associated types, or constants, even if they don't have assigned defaults
• adding supertraits
• removing unsafe from a trait's function

Determining whether a trait is sealed is complex, since there are many ways to seal a trait -- this blog post digs into the subject. I believe the following to be an exhaustive list:

• if the trait is pub-in-priv and not publicly re-exported
• if the trait has a sealed supertrait (sealed in any way, not just pub-in-priv) that is defined in the same crate
• if the trait has a "sealed method" without a default impl, in any of the ways that allow sealing a method:
  • a pub-in-priv type as a parameter or return type, including as a &dyn Trait (described here)
  • a sealed trait from the same crate used as a bound (described here — even if the bound is on a type that does not itself appear in the parameters or return type) this turned out to be incorrect due to possible refinement
• if the trait has an associated type with a sealed trait from the same crate as a bound incorrect due to possible refinement
• if the trait has an associated const of a pub-in-priv type that doesn't have a default incorrect due to possible refinement

Additionally, if the trait in question has a blanket impl, it is sealed if and only if the blanket impl has a bound on a trait that is itself sealed and defined in the same crate.

All the "from the same crate" qualifiers are important because without them, the trait becomes "sealed with respect to the crate where the other trait is defined" and only possible to implement in that crate.

As you can see, it's quite difficult to determine whether a trait is sealed or not when all the compiler's machinery is not present 😅

While RFC 3323 describes restrictions syntax that would add native support for sealed traits, I personally do not believe this is a replacement for implementing the rules above. It's unclear whether the RFC will be accepted, when it will become implemented and generally available, and when users of existing ways to seal traits will migrate to them. Since sealed traits by definition are meant for crate-internal implementation only, I don't think there will be strong pressure to change code that already works and I expect a very long migration period.

If tools such as cargo-semver-checks are only able to detect restrictions-based trait sealing, that means we'd have to make a no-win choice:

• either we don't implement a dozen lints that require precise knowledge of whether the trait is sealed or not, missing out on semver breakage we could have caught
• or we implement the lints anyway and report tons of false-positives when traits sealed with these "traditional methods" have non-breaking changes, frustrating maintainers to the point where I think they'd justifiably stop using the tool.

[Nemo157]

It's unclear whether the RFC will be accepted

It's already accepted, just waiting on implementation.

I don't think there will be strong pressure to change code that already works and I expect a very long migration period.

If rustdoc supports showing it, and cargo-semver-checks uses it, I think that should apply a decent amount of pressure to get users to migrate. The sort of users that will be running cargo-semver-checks should have a good overlap with the sort of users that will put in a trivial amount of effort to improve their code so that it provides precise metadata necessary for those checks.

[obi1kenobi]

Ah sorry, I confused it with a different RFC. My bad!

I don't think there will be strong pressure to change code that already works and I expect a very long migration period.

If rustdoc supports showing it, and cargo-semver-checks uses it, I think that should apply a decent amount of pressure to get users to migrate. The sort of users that will be running cargo-semver-checks should have a good overlap with the sort of users that will put in a trivial amount of effort to improve their code so that it provides precise metadata necessary for those checks.

While the overlap is definitely there, I'm not a fan of using it to drive adoption of the new sealing feature in that RFC. Open-source maintainers are already overworked and underpaid, and I think it can reasonably be considered maintainer-hostile to make tools that misbehave unless maintainers do extra work they hadn't planned on doing. I certainly wouldn't be happy if a tool I use did that to me.

It also adds friction to adopting and using cargo-semver-checks. I think it would likely discourage users from adopting cargo-semver-checks if they risk false-positives until they use the new sealing syntax. cargo-semver-checks has a policy against adding semver lints that cause false-positives, and if forced to make that choice, I think I would prefer to not have those lints at all than to risk false-positives.

The RFC sealing syntax also doesn't address all the use cases. I'm hoping we'd eventually be able to reuse this trait-sealing logic to help address the remaining use cases by exposing more information about trait associated items.

For example, consider an unsealed trait that has a sealed method — a method that is made impossible to override e.g. by including a pub-in-priv parameter, as described here. Function signature changes to that method are not breaking, since even though 3rd party types could implement the trait, they couldn't have overridden that method. The check necessary to determine this is a subset of the checks for trait sealing.

[yottalogical]

It seems like as of Rust 1.72, rustc is able to detect whether a trait is sealed:

mod example_mod {
    pub trait ExampleTrait: private::Sealed {}

    mod private {
        pub trait Sealed {}
    }
}

struct ExampleStruct;

impl example_mod::ExampleTrait for ExampleStruct {}

error[E0277]: the trait bound `ExampleStruct: Sealed` is not satisfied
  --> src/lib.rs:11:36
   |
11 | impl example_mod::ExampleTrait for ExampleStruct {}
   |                                    ^^^^^^^^^^^^^ the trait `Sealed` is not implemented for `ExampleStruct`
   |
help: this trait has no implementations, consider adding one
  --> src/lib.rs:5:9
   |
5  |         pub trait Sealed {}
   |         ^^^^^^^^^^^^^^^^
note: required by a bound in `ExampleTrait`
  --> src/lib.rs:2:29
   |
2  |     pub trait ExampleTrait: private::Sealed {}
   |                             ^^^^^^^^^^^^^^^ required by this bound in `ExampleTrait`
   = note: `ExampleTrait` is a "sealed trait", because to implement it you also need to implement `example_mod::private::Sealed`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it

Playground Link

Perhaps this could be reused in rustdoc.

[yottalogical]

It also appears that as of Rust 1.74, you can have private supertraits. This is a new way to seal a trait that wasn't possible before. See RFC 2145.

#[allow(private_bounds)]
pub trait ExampleTrait: Sealed {}

trait Sealed {}

Playground Link

[aDotInTheVoid]

I don't think this is a good idea, at least for rustdoc JSON.

Rust doesn't have a language level concept of sealed traits. Instead their constructed from other features, such as pub-in-priv types, or a pub-in-priv super-trait. Their more of an ecosystem idiom, than something that would be directly found in the spec.

The fact that their are so many different non-obvious ways of sealing a trait.

The "object-safety" of a trait is a language level property, that the compiler (and reference/spec) work with directly. The "sealed-ness" of a trait is an emergent property, that the compiler (and refernce/spec) construct out of the language level features. I think Rustdoc JSON should only expose the base language level features.

Instead, I'd rather this was something that JSON consumers computed from information rustdoc exposed to them, (as oposed to computing it in rustdoc, and dumping a bool in json). If their are limitations on the rustdoc JSON format that make this information impossible to calculate from JSON alone, I'd rather fix those fundamental limitations.

[jhpratt]

For clarity, impl restrictions were accepted in principle and semantics. Given that the language does not currently have the concept of sealed traits (aside from error messages), I'm not sure this is the best way forward. Over time, hopefully proper sealing will take over and this issue will be diminished.

[obi1kenobi]

@aDotInTheVoid and I are planning to pair-program an attempt to figure out if a trait is sealed based on rustdoc JSON data, to get a better sense of how feasible it is and whether there's something rustdoc JSON can do to make it easier.

I agree that in the long run we want to support proper sealing, but it'll take years until that's commonplace enough to be the only thing we can afford to support. This also doesn't address the partial sealing use cases I described above, for which to my knowledge there's no RFC nor official lang support planned yet.

[jhpratt]

Correct. It wasn't mentioned in the RFC, but partial sealing will still be used for "private" trait methods.

[obi1kenobi]

I recently became aware of the refined trait impls RFC and had a chance to revisit this question with that in mind.

It turns out that refinements + variance simplify the sealed trait reasoning quite a bit. I was able to implement sealed trait analysis for cargo-semver-checks, so this issue is no longer required.