Warn on unused doc(hidden) on trait impl items

2022-04-09 17:19:33 +02:00 · 2022-04-09 17:19:33 +02:00 · 9d157ada35
parent 83322c557f
commit 9d157ada35
18 changed files with 225 additions and 26 deletions
--- 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
--- 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.
--- 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.
--- 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,
--- 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.
--- 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,
--- 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,
--- 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
--- 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,
--- 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
--- 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,
--- 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,
--- 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;
--- 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
--- 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`.
--- a/src/test/ui/lint/unused/unused-attr-doc-hidden.fixed
+++ 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
+}
--- a/src/test/ui/lint/unused/unused-attr-doc-hidden.rs
+++ 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
+}
--- a/src/test/ui/lint/unused/unused-attr-doc-hidden.stderr
+++ 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
+