rustdoc: Add support for `#[rustc_must_implement_one_of]` #99235

WaffleLapkin · 2022-07-14T11:03:25Z

This PR adds support for #[rustc_must_implement_one_of] attribute added in #92164. There is a desire to eventually use this attribute of Read, so making it show up in docs is a good thing.

I "stole" the styling from cfg notes, not sure what would be a proper styling. Currently it looks like this:

Code to reproduce

#![feature(rustc_attrs)]

#[rustc_must_implement_one_of(a, b)]
pub trait Trait {
    fn req();
    fn a(){ Self::b() }
    fn b(){ Self::a() }
}

rustbot · 2022-07-14T11:03:29Z

Some changes occurred in HTML/CSS themes.

cc @GuillaumeGomez

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

A change occurred in the Ayu theme.

cc @Cldfire

rust-highfive · 2022-07-14T11:03:30Z

r? @GuillaumeGomez

(rust-highfive has picked a reviewer for you, use r? to override)

GuillaumeGomez · 2022-07-14T11:10:34Z

src/librustdoc/clean/inline.rs

@@ -216,6 +216,7 @@ pub(crate) fn build_external_trait(cx: &mut DocContext<'_>, did: DefId) -> clean
    let (generics, supertrait_bounds) = separate_supertrait_bounds(generics);
    let is_auto = cx.tcx.trait_is_auto(did);
    clean::Trait {
+        def_id: did,


I wonder: if we have the trait DefId, I think most (if not all) other fields can be computed on demand, no?

Probably? What is the purpose of clean types anyway? 🤔

Types "cleaned up" for rustdoc use. :)

It just feels (from an outside perspective) that a lot of this can be replaced by DefId newtypes + getters 😅

unsafety or is_auto for example. :)

Well, can be done later on.

GuillaumeGomez · 2022-07-14T11:11:27Z

Also: please add a test in src/test/rustdoc to ensure the attribute is generated in the HTML as expected.

fmease · 2022-07-14T15:37:52Z

Slightly off-topic: For T-rustdoc reviewers and for future reference, this is how the equivalent feature in Haskell ({-# MINIMAL … #-}) looks like in the documentation generated by Haddock:

WaffleLapkin · 2022-07-17T17:03:27Z

Took me a while to figure out how to do rustdoc tests, but I added the test 😄

GuillaumeGomez · 2022-07-18T09:19:06Z

src/librustdoc/html/render/print_item.rs

+        if let Some(list) = must_implement_one_of_functions.as_deref() {
+            write!(
+                w,
+                "<div class=\"stab must_implement\">At least one of {} methods is required.</div>",


There can be one method right? If so, the s at the end of methods should be conditional.

Also, can you add backticks around each method name please? Makes it easier to read it in a console.

There can be one method right? If so, the s at the end of methods should be conditional.

No, there are always at least two methods.

Oh indeed. So my first comment can be ignored. Remains the second one. ;)

Like that a81b4304e0d5668e74fd62443faf44da8a2083c8?

src/test/rustdoc/must_implement_one_of.rs

GuillaumeGomez · 2022-07-19T17:07:43Z

src/librustdoc/html/static/css/themes/light.css

@@ -158,6 +158,7 @@ details.undocumented > summary::before {
 .stab.empty-impl { background: #FFF5D6; border-color: #FFC600; }
 .stab.unstable { background: #FFF5D6; border-color: #FFC600; }
 .stab.deprecated { background: #ffc4c4; border-color: #db7b7b; }
+.stab.must_implement { background: #F3DFFF; border-color: #b07bdb; }


You need to add this into ayu.css as well.

It was already there though. Adding .stab.must_implement{} fixed the issue, seems like a bug in the checker...

No, the rule isn't the same so to speak. They needed to be all split (hence why it was unhappy). The joy of CSS. :)

GuillaumeGomez · 2022-07-19T17:08:06Z

Looks all good to me. Please add the missing CSS rule in the ayu theme and it should be good to go!

bors · 2022-07-22T21:26:55Z

☔ The latest upstream changes (presumably #99598) made this pull request unmergeable. Please resolve the merge conflicts.

WaffleLapkin · 2022-07-24T15:53:52Z

Can someone explain why the tests fail/how to fix them? I don't really get gui tests 😅

GuillaumeGomez · 2022-07-24T19:00:18Z

All the GUI tests are failing because there is a change in the body width. Basically, you broke the layout. 😆

src/librustdoc/html/static/css/rustdoc.css

src/librustdoc/html/render/print_item.rs

GuillaumeGomez · 2022-07-26T09:33:38Z

Thanks!

@bors r+ rollup

bors · 2022-07-26T09:33:39Z

📌 Commit ed8c2c2 has been approved by GuillaumeGomez

It is now in the queue for this repository.

…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#99235 (rustdoc: Add support for `#[rustc_must_implement_one_of]`) - rust-lang#99716 (remove useless mut from examples) - rust-lang#99724 (Fix some broken link fragments.) - rust-lang#99729 (Remove unused tuple fields) - rust-lang#99757 (Make `transmute_copy` docs read better) - rust-lang#99758 (remove useless `#[allow]` in a test) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned GuillaumeGomez Jul 14, 2022

rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jul 14, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 14, 2022

GuillaumeGomez reviewed Jul 14, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

WaffleLapkin force-pushed the rustdoc_implement_support_for_must_implement branch from df48600 to fd980c0 Compare July 17, 2022 16:58

This comment has been minimized.

Sign in to view

GuillaumeGomez reviewed Jul 18, 2022

View reviewed changes

src/test/rustdoc/must_implement_one_of.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

GuillaumeGomez approved these changes Jul 19, 2022

View reviewed changes

GuillaumeGomez reviewed Jul 19, 2022

View reviewed changes

WaffleLapkin added 4 commits July 24, 2022 15:10

Add support for #[rustc_must_implement_one_of] to rustdoc

3da2553

Add rustdoc test for #[rustc_must_implement_one_of]

9b9693c

add backticks

d566334

Satisfy theme check

7f1af42

WaffleLapkin force-pushed the rustdoc_implement_support_for_must_implement branch from a81b430 to 7f1af42 Compare July 24, 2022 12:49

This comment has been minimized.

Sign in to view

GuillaumeGomez reviewed Jul 24, 2022

View reviewed changes

src/librustdoc/html/static/css/rustdoc.css Outdated Show resolved Hide resolved

camelid reviewed Jul 24, 2022

View reviewed changes

src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved

fix rustdoc-gui tests

1c4c398

apply review suggestions

ed8c2c2

GuillaumeGomez approved these changes Jul 26, 2022

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 26, 2022

matthiaskrgr mentioned this pull request Jul 26, 2022

Rollup of 6 pull requests #99764

Merged

bors merged commit 811b4b8 into rust-lang:master Jul 26, 2022

rustbot added this to the 1.64.0 milestone Jul 26, 2022

WaffleLapkin mentioned this pull request Jan 30, 2023

Tracking Issue for attributes changing "Minimal Complete Definition" of a trait #107460

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: Add support for `#[rustc_must_implement_one_of]` #99235

rustdoc: Add support for `#[rustc_must_implement_one_of]` #99235

WaffleLapkin commented Jul 14, 2022

rustbot commented Jul 14, 2022

rust-highfive commented Jul 14, 2022

GuillaumeGomez Jul 14, 2022

WaffleLapkin Jul 17, 2022

GuillaumeGomez Jul 17, 2022

WaffleLapkin Jul 17, 2022

GuillaumeGomez Jul 18, 2022

GuillaumeGomez commented Jul 14, 2022

This comment has been minimized.

fmease commented Jul 14, 2022

WaffleLapkin commented Jul 17, 2022

This comment has been minimized.

GuillaumeGomez Jul 18, 2022

GuillaumeGomez Jul 18, 2022

WaffleLapkin Jul 19, 2022

GuillaumeGomez Jul 19, 2022

WaffleLapkin Jul 19, 2022

GuillaumeGomez Jul 19, 2022

This comment has been minimized.

GuillaumeGomez Jul 19, 2022

WaffleLapkin Jul 24, 2022

GuillaumeGomez Jul 24, 2022

GuillaumeGomez commented Jul 19, 2022

bors commented Jul 22, 2022

This comment has been minimized.

WaffleLapkin commented Jul 24, 2022

GuillaumeGomez commented Jul 24, 2022

GuillaumeGomez commented Jul 26, 2022

bors commented Jul 26, 2022

rustdoc: Add support for #[rustc_must_implement_one_of] #99235

rustdoc: Add support for #[rustc_must_implement_one_of] #99235

Conversation

WaffleLapkin commented Jul 14, 2022

rustbot commented Jul 14, 2022

rust-highfive commented Jul 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Jul 14, 2022

This comment has been minimized.

fmease commented Jul 14, 2022

WaffleLapkin commented Jul 17, 2022

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Jul 19, 2022

bors commented Jul 22, 2022

This comment has been minimized.

WaffleLapkin commented Jul 24, 2022

GuillaumeGomez commented Jul 24, 2022

GuillaumeGomez commented Jul 26, 2022

bors commented Jul 26, 2022

rustdoc: Add support for `#[rustc_must_implement_one_of]` #99235

rustdoc: Add support for `#[rustc_must_implement_one_of]` #99235