Document required components with a marker trait

[bushrat011899]

Objective

• Builds on Fix Component require() IDE integration #18165
• Alternative to Rustdoc: Restore visibility of required components #18171

Solution

• Added a new trait, Require<C: Component>: Component, which is automatically implemented in the Component derive macro.

Testing

• CI

---

Notes

• This trait is purely for documentation purposes and has no impact on the functionality of required components.
• I recommend using cargo doc --open with this PR and inspecting the Button and Node components as examples for the effects of this PR.
• I have based this PR off of Fix Component require() IDE integration #18165 and will rebase once it is merged.

Examples

Button required components are listed logically in Trait Implementations

The method used to insert a required component can also be documented

Node can document components it requires and components that require it

Comparison to #18171 (Rustdoc Extension)

Cons

• Can only place information within the trait implementation section, whereas the extension can place it anywhere, including at the top of the page as-if a user had done so themselves.
• Information is more verbose, as it includes where bounds, etc.

Pros

• This PR works for all ecosystem and user crates automatically, whereas the extension requires opt-in configuration during doc generation.
• List format is better suited for items with many required components.
• Can easily show additional information, such as the method used to insert a required component if it is not present.
• Automatically shows documentation on the required and requiring components (e.g., Button and Node both get documentation for Button requiring Node)

[JMS55]

Can you add #[cfg(doc)] to the generated impl/trait?

[bushrat011899]

Sadly not, I tried but cargo doc doesn't propagate doc to dependencies or macros in a way that worked reliably.

[SpecificProtagonist]

Yep, that's because of rust-lang/cargo#8811. For docs.rs and dev-doc.bevyengine.org you could set the flags in Cargo.toml/docs.yaml.

[jnhyatt]

I'm not exactly a new user, but my knee-jerk reaction to seeing impl Require<FocusPolicy> for Button was "oh, I guess I can impl Require<X> for my component to require X". I'd be wary of newer users (especially new to Rust) having this misconception.

[bushrat011899]

I'm not exactly a new user, but my knee-jerk reaction to seeing impl Require<FocusPolicy> for Button was "oh, I guess I can impl Require<X> for my component to require X". I'd be wary of newer users (especially new to Rust) having this misconception.

Agreed. I'm going to mark the trait as unsafe and document that you must sync implementations of this trait with the implementation of Component itself.

[SpecificProtagonist]

Saw this just after making my own solution: #18171 :P

[alice-i-cecile]

I'm on board with this plan: the backlinks are incredibly useful. That said, this trait needs to be way more explicit about a) it just being a marker for docs and b) the fact that users should effectively never ever implement this because it's automatically generated.

I think that we should do that and remove the unsafe: there's no actual memory safety hazards here.

[bushrat011899]

I've moved the trait into a doc(hidden) module named document_required_components, which ensures Require is never suggested by an IDE, and clearly communicates to users importing the trait that it's a documentation marker. I've also amended the documentation for Require to make it clear that it's up to the implementer to ensure Component actually requires the components as marked by Require.

[cart]

For me, this removes all Require listings from the docs, which defeats the purpose of this feature :)

[bushrat011899]

Ah whoops, should be tested first! I assumed keeping the trait public would keep it in the documentation.

[cart]

I'm not convinced that this is meaningfully "better" than the current "required list in Component docs":

1. Because traits are listed alphabetically, users will hit the Component trait before they hit the Require<X> traits.
2. The require list in Component is significantly less "noisy". You can consume the entire list at a glance, rather than needing to parse out multiple lines of docs for each required component (and needing to look "around" generics / formatting / etc)
3. The Require trait impl is confusing / introduces a weirdness factor (even if we manage to hide it). This is non-standard trait usage (traits are generally about "rust behaviors"). People reading the docs need to deal with the fact that
4. This introduces additional codegen, which is something we should be wary of, given that we are already "heavy" on the codegen side.

From my perspective the only real win here is that the require list shows up in the sidebar, but that is also a downside, as a long list takes up significant space and obscures other "real" trait impls. And it doesn't necessarily meaningfully improve the prominence of the require list. The sidebar is often littered with other things (ex: I have to scroll to see the list for Node).

Edit: The backlinks are kind of nice, but the fact that A->B and C->A requires are interleaved together in the list is super confusing to me.

[cart]

Additionally, our docs are just generally already very "noisy" and this adds even more to the pile.

[bushrat011899]

I think these are valid concerns, and I don't think this technique can be amended to satisfy them. Thanks for providing the feedback! I'll close this out

[bushrat011899]

Closed based on feedback as not being the desired path forward. Thanks for the reviews and feedback! Good to explore and experiment even if it doesn't get merged.