Rollup merge of #96008 - fmease:warn-on-useless-doc-hidden-on-assoc-impl-items, r=lcnr

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

[Zulip conversation](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/.E2.9C.94.20Validy.20checks.20for.20.60.23.5Bdoc.28hidden.29.5D.60).

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:

```rust
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

18 files changed, 225 insertions, 26 deletions

diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
index b297012f19f..2b0fa57cba8 100644
--- a/compiler/rustc_passes/src/check_attr.rs
+++ b/compiler/rustc_passes/src/check_attr.rs
@@ -4,7 +4,8 @@
 //! conflicts between multiple such attributes attached to the same
 //! item.
 
-use rustc_ast::{ast, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
+use rustc_ast::tokenstream::DelimSpan;
+use rustc_ast::{ast, AttrStyle, Attribute, Lit, LitKind, MacArgs, MetaItemKind, NestedMetaItem};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::{pluralize, struct_span_err, Applicability, MultiSpan};
 use rustc_feature::{AttributeDuplicates, AttributeType, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP};
@@ -810,6 +811,68 @@ impl CheckAttrVisitor<'_> {
         }
     }
 
+    /// Checks `#[doc(hidden)]` attributes. Returns `true` if valid.
+    fn check_doc_hidden(
+        &self,
+        attr: &Attribute,
+        meta_index: usize,
+        meta: &NestedMetaItem,
+        hir_id: HirId,
+        target: Target,
+    ) -> bool {
+        if let Target::AssocConst
+        | Target::AssocTy
+        | Target::Method(MethodKind::Trait { body: true }) = target
+        {
+            let parent_hir_id = self.tcx.hir().get_parent_item(hir_id);
+            let containing_item = self.tcx.hir().expect_item(parent_hir_id);
+
+            if Target::from_item(containing_item) == Target::Impl {
+                let meta_items = attr.meta_item_list().unwrap();
+
+                let (span, replacement_span) = if meta_items.len() == 1 {
+                    (attr.span, attr.span)
+                } else {
+                    let meta_span = meta.span();
+                    (
+                        meta_span,
+                        meta_span.until(match meta_items.get(meta_index + 1) {
+                            Some(next_item) => next_item.span(),
+                            None => match attr.get_normal_item().args {
+                                MacArgs::Delimited(DelimSpan { close, .. }, ..) => close,
+                                _ => unreachable!(),
+                            },
+                        }),
+                    )
+                };
+
+                // FIXME: #[doc(hidden)] was previously erroneously allowed on trait impl items,
+                // so for backward compatibility only emit a warning and do not mark it as invalid.
+                self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, span, |lint| {
+                    lint.build("`#[doc(hidden)]` is ignored on trait impl items")
+                        .warn(
+                            "this was previously accepted by the compiler but is \
+                             being phased out; it will become a hard error in \
+                             a future release!",
+                        )
+                        .note(
+                            "whether the impl item is `doc(hidden)` or not \
+                             entirely depends on the corresponding trait item",
+                        )
+                        .span_suggestion(
+                            replacement_span,
+                            "remove this attribute",
+                            String::new(),
+                            Applicability::MachineApplicable,
+                        )
+                        .emit();
+                });
+            }
+        }
+
+        true
+    }
+
     /// Checks that an attribute is *not* used at the crate level. Returns `true` if valid.
     fn check_attr_not_crate_level(
         &self,
@@ -928,7 +991,7 @@ impl CheckAttrVisitor<'_> {
         let mut is_valid = true;
 
         if let Some(mi) = attr.meta() && let Some(list) = mi.meta_item_list() {
-            for meta in list {
+            for (meta_index, meta) in list.into_iter().enumerate() {
                 if let Some(i_meta) = meta.meta_item() {
                     match i_meta.name_or_empty() {
                         sym::alias
@@ -969,6 +1032,15 @@ impl CheckAttrVisitor<'_> {
                             is_valid = false;
                         }
 
+                        sym::hidden if !self.check_doc_hidden(attr,
+                            meta_index,
+                            meta,
+                            hir_id,
+                            target,
+                            ) => {
+                            is_valid = false;
+                        }
+
                         // no_default_passes: deprecated
                         // passes: deprecated
                         // plugins: removed, but rustdoc warns about it itself
diff --git a/library/alloc/src/collections/vec_deque/iter.rs b/library/alloc/src/collections/vec_deque/iter.rs
index e8290809276..19198ab3aa1 100644
--- a/library/alloc/src/collections/vec_deque/iter.rs
+++ b/library/alloc/src/collections/vec_deque/iter.rs
@@ -122,7 +122,6 @@ impl<'a, T> Iterator for Iter<'a, T> {
     }
 
     #[inline]
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
         // Safety: The TrustedRandomAccess contract requires that callers only pass an index
         // that is in bounds.
diff --git a/library/alloc/src/collections/vec_deque/iter_mut.rs b/library/alloc/src/collections/vec_deque/iter_mut.rs
index ee2df0d5160..b78c0d5e1b3 100644
--- a/library/alloc/src/collections/vec_deque/iter_mut.rs
+++ b/library/alloc/src/collections/vec_deque/iter_mut.rs
@@ -100,7 +100,6 @@ impl<'a, T> Iterator for IterMut<'a, T> {
     }
 
     #[inline]
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
         // Safety: The TrustedRandomAccess contract requires that callers only pass an index
         // that is in bounds.
diff --git a/library/alloc/src/vec/into_iter.rs b/library/alloc/src/vec/into_iter.rs
index 8134eea570a..a7df6f59b59 100644
--- a/library/alloc/src/vec/into_iter.rs
+++ b/library/alloc/src/vec/into_iter.rs
@@ -202,7 +202,6 @@ impl<T, A: Allocator> Iterator for IntoIter<T, A> {
         self.len()
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, i: usize) -> Self::Item
     where
         Self: TrustedRandomAccessNoCoerce,
diff --git a/library/core/src/convert/num.rs b/library/core/src/convert/num.rs
index 2b6ea90bf04..4fa5d129bc6 100644
--- a/library/core/src/convert/num.rs
+++ b/library/core/src/convert/num.rs
@@ -25,7 +25,6 @@ macro_rules! impl_float_to_int {
         $(
             #[unstable(feature = "convert_float_to_int", issue = "67057")]
             impl FloatToInt<$Int> for $Float {
-                #[doc(hidden)]
                 #[inline]
                 unsafe fn to_int_unchecked(self) -> $Int {
                     // SAFETY: the safety contract must be upheld by the caller.
diff --git a/library/core/src/iter/adapters/cloned.rs b/library/core/src/iter/adapters/cloned.rs
index 71a5a4ea831..aba24a79dcf 100644
--- a/library/core/src/iter/adapters/cloned.rs
+++ b/library/core/src/iter/adapters/cloned.rs
@@ -60,7 +60,6 @@ where
         self.it.map(T::clone).fold(init, f)
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> T
     where
         Self: TrustedRandomAccessNoCoerce,
diff --git a/library/core/src/iter/adapters/copied.rs b/library/core/src/iter/adapters/copied.rs
index e5f2886dcaf..f9bfd77d7fb 100644
--- a/library/core/src/iter/adapters/copied.rs
+++ b/library/core/src/iter/adapters/copied.rs
@@ -81,7 +81,6 @@ where
         self.it.advance_by(n)
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> T
     where
         Self: TrustedRandomAccessNoCoerce,
diff --git a/library/core/src/iter/adapters/enumerate.rs b/library/core/src/iter/adapters/enumerate.rs
index 10b4db84b39..14a12695111 100644
--- a/library/core/src/iter/adapters/enumerate.rs
+++ b/library/core/src/iter/adapters/enumerate.rs
@@ -128,7 +128,6 @@ where
     }
 
     #[rustc_inherit_overflow_checks]
-    #[doc(hidden)]
     #[inline]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> <Self as Iterator>::Item
     where
diff --git a/library/core/src/iter/adapters/fuse.rs b/library/core/src/iter/adapters/fuse.rs
index fbf752c6f20..8adb53c6714 100644
--- a/library/core/src/iter/adapters/fuse.rs
+++ b/library/core/src/iter/adapters/fuse.rs
@@ -129,7 +129,6 @@ where
     }
 
     #[inline]
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item
     where
         Self: TrustedRandomAccessNoCoerce,
diff --git a/library/core/src/iter/adapters/map.rs b/library/core/src/iter/adapters/map.rs
index 6cbb35dc7c6..9e25dbe462c 100644
--- a/library/core/src/iter/adapters/map.rs
+++ b/library/core/src/iter/adapters/map.rs
@@ -124,7 +124,6 @@ where
         self.iter.fold(init, map_fold(self.f, g))
     }
 
-    #[doc(hidden)]
     #[inline]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> B
     where
diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs
index de44bd66501..8153c8cfef1 100644
--- a/library/core/src/iter/adapters/zip.rs
+++ b/library/core/src/iter/adapters/zip.rs
@@ -95,7 +95,6 @@ where
     }
 
     #[inline]
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item
     where
         Self: TrustedRandomAccessNoCoerce,
diff --git a/library/core/src/iter/range.rs b/library/core/src/iter/range.rs
index 0ae94c05da6..f7aeee8c9ad 100644
--- a/library/core/src/iter/range.rs
+++ b/library/core/src/iter/range.rs
@@ -752,7 +752,6 @@ impl<A: Step> Iterator for ops::Range<A> {
     }
 
     #[inline]
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item
     where
         Self: TrustedRandomAccessNoCoerce,
diff --git a/library/core/src/slice/iter.rs b/library/core/src/slice/iter.rs
index 98dd1521d0e..772a9698d84 100644
--- a/library/core/src/slice/iter.rs
+++ b/library/core/src/slice/iter.rs
@@ -1322,7 +1322,6 @@ impl<'a, T> Iterator for Windows<'a, T> {
         }
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
         // SAFETY: since the caller guarantees that `i` is in bounds,
         // which means that `i` cannot overflow an `isize`, and the
@@ -1478,7 +1477,6 @@ impl<'a, T> Iterator for Chunks<'a, T> {
         }
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
         let start = idx * self.chunk_size;
         // SAFETY: the caller guarantees that `i` is in bounds,
@@ -1657,7 +1655,6 @@ impl<'a, T> Iterator for ChunksMut<'a, T> {
         }
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
         let start = idx * self.chunk_size;
         // SAFETY: see comments for `Chunks::__iterator_get_unchecked`.
@@ -1830,7 +1827,6 @@ impl<'a, T> Iterator for ChunksExact<'a, T> {
         self.next_back()
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
         let start = idx * self.chunk_size;
         // SAFETY: mostly identical to `Chunks::__iterator_get_unchecked`.
@@ -1984,7 +1980,6 @@ impl<'a, T> Iterator for ChunksExactMut<'a, T> {
         self.next_back()
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
         let start = idx * self.chunk_size;
         // SAFETY: see comments for `ChunksMut::__iterator_get_unchecked`.
@@ -2248,7 +2243,6 @@ impl<'a, T, const N: usize> Iterator for ArrayChunks<'a, T, N> {
         self.iter.last()
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, i: usize) -> &'a [T; N] {
         // SAFETY: The safety guarantees of `__iterator_get_unchecked` are
         // transferred to the caller.
@@ -2367,7 +2361,6 @@ impl<'a, T, const N: usize> Iterator for ArrayChunksMut<'a, T, N> {
         self.iter.last()
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, i: usize) -> &'a mut [T; N] {
         // SAFETY: The safety guarantees of `__iterator_get_unchecked` are transferred to
         // the caller.
@@ -2520,7 +2513,6 @@ impl<'a, T> Iterator for RChunks<'a, T> {
         }
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
         let end = self.v.len() - idx * self.chunk_size;
         let start = match end.checked_sub(self.chunk_size) {
@@ -2689,7 +2681,6 @@ impl<'a, T> Iterator for RChunksMut<'a, T> {
         }
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
         let end = self.v.len() - idx * self.chunk_size;
         let start = match end.checked_sub(self.chunk_size) {
@@ -2856,7 +2847,6 @@ impl<'a, T> Iterator for RChunksExact<'a, T> {
         self.next_back()
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
         let end = self.v.len() - idx * self.chunk_size;
         let start = end - self.chunk_size;
@@ -3016,7 +3006,6 @@ impl<'a, T> Iterator for RChunksExactMut<'a, T> {
         self.next_back()
     }
 
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
         let end = self.v.len() - idx * self.chunk_size;
         let start = end - self.chunk_size;
diff --git a/library/core/src/slice/iter/macros.rs b/library/core/src/slice/iter/macros.rs
index 78bf3381b4d..c05242222dd 100644
--- a/library/core/src/slice/iter/macros.rs
+++ b/library/core/src/slice/iter/macros.rs
@@ -325,7 +325,6 @@ macro_rules! iterator {
                 None
             }
 
-            #[doc(hidden)]
             #[inline]
             unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item {
                 // SAFETY: the caller must guarantee that `i` is in bounds of
diff --git a/library/core/src/str/iter.rs b/library/core/src/str/iter.rs
index e00c6d377c7..24083ee6af4 100644
--- a/library/core/src/str/iter.rs
+++ b/library/core/src/str/iter.rs
@@ -298,7 +298,6 @@ impl Iterator for Bytes<'_> {
     }
 
     #[inline]
-    #[doc(hidden)]
     unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> u8 {
         // SAFETY: the caller must uphold the safety contract
         // for `Iterator::__iterator_get_unchecked`.
diff --git a/src/test/ui/lint/unused/unused-attr-doc-hidden.fixed b/src/test/ui/lint/unused/unused-attr-doc-hidden.fixed
new file mode 100644
index 00000000000..36a14097ac3
--- /dev/null
+++ b/src/test/ui/lint/unused/unused-attr-doc-hidden.fixed
@@ -0,0 +1,42 @@
+#![deny(unused_attributes)]
+#![crate_type = "lib"]
+// run-rustfix
+
+pub trait Trait {
+    type It;
+    const IT: ();
+    fn it0();
+    fn it1();
+    fn it2();
+}
+
+pub struct Implementor;
+
+impl Trait for Implementor {
+    
+    type It = ();
+    //~^^ ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+
+    
+    const IT: () = ();
+    //~^^ ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+
+    #[doc(alias = "aka")]
+    fn it0() {}
+    //~^^ ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+
+    #[doc(alias = "this", )]
+    fn it1() {}
+    //~^^ ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+
+    #[doc()]
+    fn it2() {}
+    //~^^ ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+    //~|  ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+}
diff --git a/src/test/ui/lint/unused/unused-attr-doc-hidden.rs b/src/test/ui/lint/unused/unused-attr-doc-hidden.rs
new file mode 100644
index 00000000000..e58c4f22f31
--- /dev/null
+++ b/src/test/ui/lint/unused/unused-attr-doc-hidden.rs
@@ -0,0 +1,42 @@
+#![deny(unused_attributes)]
+#![crate_type = "lib"]
+// run-rustfix
+
+pub trait Trait {
+    type It;
+    const IT: ();
+    fn it0();
+    fn it1();
+    fn it2();
+}
+
+pub struct Implementor;
+
+impl Trait for Implementor {
+    #[doc(hidden)]
+    type It = ();
+    //~^^ ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+
+    #[doc(hidden)]
+    const IT: () = ();
+    //~^^ ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+
+    #[doc(hidden, alias = "aka")]
+    fn it0() {}
+    //~^^ ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+
+    #[doc(alias = "this", hidden,)]
+    fn it1() {}
+    //~^^ ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+
+    #[doc(hidden, hidden)]
+    fn it2() {}
+    //~^^ ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+    //~|  ERROR `#[doc(hidden)]` is ignored
+    //~|  WARNING this was previously accepted
+}
diff --git a/src/test/ui/lint/unused/unused-attr-doc-hidden.stderr b/src/test/ui/lint/unused/unused-attr-doc-hidden.stderr
new file mode 100644
index 00000000000..fd1202a29de
--- /dev/null
+++ b/src/test/ui/lint/unused/unused-attr-doc-hidden.stderr
@@ -0,0 +1,67 @@
+error: `#[doc(hidden)]` is ignored on trait impl items
+  --> $DIR/unused-attr-doc-hidden.rs:16:5
+   |
+LL |     #[doc(hidden)]
+   |     ^^^^^^^^^^^^^^ help: remove this attribute
+   |
+note: the lint level is defined here
+  --> $DIR/unused-attr-doc-hidden.rs:1:9
+   |
+LL | #![deny(unused_attributes)]
+   |         ^^^^^^^^^^^^^^^^^
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: whether the impl item is `doc(hidden)` or not entirely depends on the corresponding trait item
+
+error: `#[doc(hidden)]` is ignored on trait impl items
+  --> $DIR/unused-attr-doc-hidden.rs:21:5
+   |
+LL |     #[doc(hidden)]
+   |     ^^^^^^^^^^^^^^ help: remove this attribute
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: whether the impl item is `doc(hidden)` or not entirely depends on the corresponding trait item
+
+error: `#[doc(hidden)]` is ignored on trait impl items
+  --> $DIR/unused-attr-doc-hidden.rs:26:11
+   |
+LL |     #[doc(hidden, alias = "aka")]
+   |           ^^^^^^--
+   |           |
+   |           help: remove this attribute
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: whether the impl item is `doc(hidden)` or not entirely depends on the corresponding trait item
+
+error: `#[doc(hidden)]` is ignored on trait impl items
+  --> $DIR/unused-attr-doc-hidden.rs:31:27
+   |
+LL |     #[doc(alias = "this", hidden,)]
+   |                           ^^^^^^-
+   |                           |
+   |                           help: remove this attribute
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: whether the impl item is `doc(hidden)` or not entirely depends on the corresponding trait item
+
+error: `#[doc(hidden)]` is ignored on trait impl items
+  --> $DIR/unused-attr-doc-hidden.rs:36:11
+   |
+LL |     #[doc(hidden, hidden)]
+   |           ^^^^^^--
+   |           |
+   |           help: remove this attribute
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: whether the impl item is `doc(hidden)` or not entirely depends on the corresponding trait item
+
+error: `#[doc(hidden)]` is ignored on trait impl items
+  --> $DIR/unused-attr-doc-hidden.rs:36:19
+   |
+LL |     #[doc(hidden, hidden)]
+   |                   ^^^^^^ help: remove this attribute
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: whether the impl item is `doc(hidden)` or not entirely depends on the corresponding trait item
+
+error: aborting due to 6 previous errors
+