From: León Orell Valerian Liehr Date: Sat, 9 Apr 2022 15:19:33 +0000 (+0200) Subject: Warn on unused doc(hidden) on trait impl items X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=9d157ada35c0e363e30344526755649c3399f7de;hp=83322c557fcaa9b6750955ceb6b9591df6c53a65;p=rust.git Warn on unused doc(hidden) on trait impl items --- 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 @@ fn check_doc_inline( } } + /// 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 @@ fn check_doc_attrs( 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 @@ fn check_doc_attrs( 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 @@ fn last(mut self) -> Option<&'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 @@ fn last(mut self) -> Option<&'a mut 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 @@ fn count(self) -> usize { 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 @@ impl private::Sealed for $Float {} $( #[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 @@ fn fold(self, init: Acc, f: F) -> Acc 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 @@ fn advance_by(&mut self, n: usize) -> Result<(), usize> { 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 @@ fn advance_by(&mut self, n: usize) -> Result<(), usize> { } #[rustc_inherit_overflow_checks] - #[doc(hidden)] #[inline] unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> ::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 @@ fn find

(&mut self, predicate: P) -> Option } #[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 @@ fn fold(self, init: Acc, g: G) -> Acc 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 @@ fn nth(&mut self, n: usize) -> Option { } #[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 @@ fn advance_by(&mut self, n: usize) -> Result<(), usize> { } #[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 @@ fn last(self) -> Option { } } - #[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 @@ fn last(self) -> Option { } } - #[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 @@ fn last(self) -> Option { } } - #[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 @@ fn last(mut self) -> Option { 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 @@ fn last(mut self) -> Option { 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 @@ fn last(self) -> Option { 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 @@ fn last(self) -> Option { 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 @@ fn last(self) -> Option { } } - #[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 @@ fn last(self) -> Option { } } - #[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 @@ fn last(mut self) -> Option { 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 @@ fn last(mut self) -> Option { 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 @@ fn rposition

(&mut self, mut predicate: P) -> Option where 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 e529bccbc79..f46acc11f2d 100644 --- a/library/core/src/str/iter.rs +++ b/library/core/src/str/iter.rs @@ -298,7 +298,6 @@ fn rposition

(&mut self, predicate: P) -> Option } #[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 +