Warn on unused #[doc(hidden)] attributes on trait impl items

[fmease]

Zulip conversation.

Whether an associated item in a trait impl is shown or hidden in the documentation entirely depends on the corresponding item in the trait declaration. Rustdoc completely ignores #[doc(hidden)] attributes on impl items. No error or warning is emitted:

pub trait Tr { fn f(); }
pub struct Ty;
impl Tr for Ty { #[doc(hidden)] fn f() {} }
//               ^^^^^^^^^^^^^^ ignored by rustdoc and currently
//                              no error or warning issued

This may lead users to the wrong belief that the attribute has an effect. In fact, several such cases are found in the standard library (I've removed all of them in this PR).
There does not seem to exist any incentive to allow this in the future either: Impl'ing a trait for a type means the type fully conforms to its API. Users can add #[doc(hidden)] to the whole impl if they want to hide the implementation or add the attribute to the corresponding associated item in the trait declaration to hide the specific item. Hiding an implementation of an associated item does not make much sense: The associated item can still be found on the trait page.

This PR emits the warn-by-default lint unused_attribute for this case with a future-incompat warning.

@rustbot label T-compiler T-rustdoc A-lint

[rust-highfive]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• a stabilization of a library feature
• introducing new or changes existing unstable library APIs
• changes to public documentation in ways that create new stability guarantees

[rust-highfive]

r? @lcnr

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

[GuillaumeGomez]

For the bug you detected, please open an issue (and assign it to yourself) and fix it in another PR.

Otherwise, this PR looks good to me, thanks!

[lcnr]

I personally think we should treat this similar to other, previously silently ignored attributes. E.g.

struct Foo(#[inline] ());

warning: `#[inline]` is ignored on struct fields, match arms and macro defs
 --> src/lib.rs:1:12
  |
1 | struct Foo(#[inline] ());
  |            ^^^^^^^^^
  |
  = note: `#[warn(unused_attributes)]` on by default
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: see issue #80564 <https://github.com/rust-lang/rust/issues/80564> for more information

Imo this should be a unused_attributes future compat lint.

Though that's for @rust-lang/lang @rust-lang/docs to decide.

[bors]

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

[fmease]

Since nobody else voiced their opinion, I will integrate the lint unused_doc_hidden into unused_attributes.

[scottmcm]

👍 to treating it like the other attributes -- there have been a ton of these, like #[repr] on fields, etc.

[fmease]

@rustbot author

[fmease]

@rustbot ready
@rustbot label -T-libs

[lcnr]

cnsidering that this is

1. a pretty straightforward bug fix
2. only adds a new lint, so it isn't a breaking change
3. i've already notified t-lang and got an unofficial signoff by @scottmcm

@bors r+ rollup

[bors]

📌 Commit 9d157ad has been approved by lcnr

[scottmcm]

only adds a new lint, so it isn't a breaking change

👍. So long as (like here) reverting it doesn't break people, it doesn't need a full FCP.

[dtolnay]

This may lead users to the wrong belief that the attribute has an effect. In fact, several such cases are found in the standard library (I've removed all of them in this PR).

There does not seem to exist any incentive to allow this in the future either

FWIW the rustdoc that shipped with 1.56.0 and older versions of Rust does look at doc(hidden) on impl items and it does affect the rendered output, so you might find that these attributes exist (for good reason) in the ecosystem.

A typical case looks like:

pub trait Trait {
    fn public_fn();
    #[doc(hidden)]
    fn __private_fn() {…}
}

impl Trait for Struct {
    fn public_fn() {…}
    #[doc(hidden)]
    fn __private_fn() {…}
}

Without doc(hidden) in the impl, old rustdoc will render __private_fn on Struct's page. Versions 1.54.0 and older will additionally render it in the impl section of Trait's page.

Rust 1.56.0 is only 6 months old so it's not unreasonable that libraries would want to support it.

[dtolnay]

I noticed in a different one of my crates that the doc(hidden) attribute on an associated type is still used by rustdoc, so this lint is a false positive in some cases.

I filed #96890 to follow up.

[fmease]

Oh, I didn't know older versions of rustdoc behave differently in this scenario.
@dtolnay Do you suggest reverting this PR and re-introducing this change “later” like in a year? How can we make sure this won't be forgotten?
As a middle ground, what do you think about only linting this case in the 2021 edition (stabilized in 1.56) as a temporary measure? With the rationale that people who successfully migrated to the latest edition potentially do not care so much about supporting older versions of rustc (edit: only at the time of this writing, obviously, since the edition is relatively recent)? Or is this utter nonsense?
And adding a FIXME to backport it to all editions eventually?

Is 1.56 the last version where rustdoc behaved differently? (Well, for 1.62 I even extended the new behavior, see #95769)