From 9a8ae768febc7c55cffca06c42dc81172d9702ba Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 13 Feb 2023 16:05:54 +0100 Subject: [PATCH 1/4] Add lint to check if non-inlined local reexports have documentation --- src/librustdoc/clean/mod.rs | 2 +- src/librustdoc/lint.rs | 12 +++++ src/librustdoc/passes/lint.rs | 2 + .../lint/unused_doc_comment_on_reexport.rs | 46 +++++++++++++++++++ 4 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 src/librustdoc/passes/lint/unused_doc_comment_on_reexport.rs diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 80493b100bb45..bb6445ddcb781 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2050,7 +2050,7 @@ fn clean_bare_fn_ty<'tcx>( /// This visitor is used to go through only the "top level" of a item and not enter any sub /// item while looking for a given `Ident` which is stored into `item` if found. -struct OneLevelVisitor<'hir> { +pub(crate) struct OneLevelVisitor<'hir> { map: rustc_middle::hir::map::Map<'hir>, item: Option<&'hir hir::Item<'hir>>, looking_for: Ident, diff --git a/src/librustdoc/lint.rs b/src/librustdoc/lint.rs index 3aad97bc296fb..1bcd7b2a28c90 100644 --- a/src/librustdoc/lint.rs +++ b/src/librustdoc/lint.rs @@ -174,6 +174,18 @@ declare_rustdoc_lint! { "codeblock could not be parsed as valid Rust or is empty" } +declare_rustdoc_lint! { + /// The `documented_local_non_inlined_reexport` lint detects when a reexport + /// (`pub use`) of an item from the same crate which doesn't have + /// `#[doc(inline)]` is documented. + /// This is a `rustdoc` only lint, see the documentation in the [rustdoc book]. + /// + /// [rustdoc book]: ../../../rustdoc/lints.html#documented_non_inlined_local_reexport + DOCUMENTED_LOCAL_NON_INLINED_REEXPORT, + Warn, + "doc comments on local non-inlined reexports are ignored", +} + pub(crate) static RUSTDOC_LINTS: Lazy> = Lazy::new(|| { vec![ BROKEN_INTRA_DOC_LINKS, diff --git a/src/librustdoc/passes/lint.rs b/src/librustdoc/passes/lint.rs index 97031c4f028f4..bf9cb36a58b7b 100644 --- a/src/librustdoc/passes/lint.rs +++ b/src/librustdoc/passes/lint.rs @@ -4,6 +4,7 @@ mod bare_urls; mod check_code_block_syntax; mod html_tags; +mod unused_doc_comment_on_reexport; use super::Pass; use crate::clean::*; @@ -27,6 +28,7 @@ impl<'a, 'tcx> DocVisitor for Linter<'a, 'tcx> { bare_urls::visit_item(self.cx, item); check_code_block_syntax::visit_item(self.cx, item); html_tags::visit_item(self.cx, item); + unused_doc_comment_on_reexport::visit_item(self.cx, item); self.visit_item_recur(item) } diff --git a/src/librustdoc/passes/lint/unused_doc_comment_on_reexport.rs b/src/librustdoc/passes/lint/unused_doc_comment_on_reexport.rs new file mode 100644 index 0000000000000..c4f1db17de48a --- /dev/null +++ b/src/librustdoc/passes/lint/unused_doc_comment_on_reexport.rs @@ -0,0 +1,46 @@ +//! Detects non-inlined local reexports with doc comments. +use crate::clean::*; +use crate::core::DocContext; + +use rustc_errors::Applicability; +use rustc_span::DUMMY_SP; + +pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) { + if item.attrs.doc_strings.is_empty() || !matches!(*item.kind, ItemKind::ImportItem(_)) { + return; + } + let Some(hir_id) = DocContext::as_local_hir_id(cx.tcx, item.item_id) + else { + // If non-local, no need to check anything. + return; + }; + let sp = item.attrs.doc_strings[0].span; + let item_sp = item.span(cx.tcx).map(|sp| sp.inner()).unwrap_or(DUMMY_SP).shrink_to_lo(); + + // If we see an import item with doc, then it's wrong in any case because otherwise, if it + // had `#[doc(inline)]`, we wouldn't see it here in the first place. + cx.tcx.struct_span_lint_hir( + crate::lint::DOCUMENTED_LOCAL_NON_INLINED_REEXPORT, + hir_id, + sp, + "doc comments on local non-inlined reexports are ignored", + |lint| { + lint.note( + "the documentation won't be visible because reexports don't have their own page", + ); + lint.span_suggestion( + item_sp, + "try adding `#[doc(inline)]`", + "#[doc(inline)] ", + Applicability::MaybeIncorrect, + ); + lint.span_suggestion( + sp, + "or move the documentation directly on the reexported item", + "", + Applicability::MaybeIncorrect, + ); + lint + }, + ); +} From c17ec3fc082c84b639b1d110a692b26582da8c2c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 13 Feb 2023 17:29:10 +0100 Subject: [PATCH 2/4] Add lint for unused documentation on reexports --- src/librustdoc/lint.rs | 10 +++---- .../lint/unused_doc_comment_on_reexport.rs | 28 ++++++++++++++++--- src/librustdoc/passes/mod.rs | 11 ++++++++ src/librustdoc/visit_ast.rs | 5 ++++ 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/librustdoc/lint.rs b/src/librustdoc/lint.rs index 1bcd7b2a28c90..86aa4cb5c4031 100644 --- a/src/librustdoc/lint.rs +++ b/src/librustdoc/lint.rs @@ -176,14 +176,13 @@ declare_rustdoc_lint! { declare_rustdoc_lint! { /// The `documented_local_non_inlined_reexport` lint detects when a reexport - /// (`pub use`) of an item from the same crate which doesn't have - /// `#[doc(inline)]` is documented. + /// (`pub use`) documentation is unused. /// This is a `rustdoc` only lint, see the documentation in the [rustdoc book]. /// - /// [rustdoc book]: ../../../rustdoc/lints.html#documented_non_inlined_local_reexport - DOCUMENTED_LOCAL_NON_INLINED_REEXPORT, + /// [rustdoc book]: ../../../rustdoc/lints.html#unused_reexport_documentation + UNUSED_REEXPORT_DOCUMENTATION, Warn, - "doc comments on local non-inlined reexports are ignored", + "doc comments are ignored on some cases for reexports", } pub(crate) static RUSTDOC_LINTS: Lazy> = Lazy::new(|| { @@ -197,6 +196,7 @@ pub(crate) static RUSTDOC_LINTS: Lazy> = Lazy::new(|| { INVALID_HTML_TAGS, BARE_URLS, MISSING_CRATE_LEVEL_DOCS, + UNUSED_REEXPORT_DOCUMENTATION, ] }); diff --git a/src/librustdoc/passes/lint/unused_doc_comment_on_reexport.rs b/src/librustdoc/passes/lint/unused_doc_comment_on_reexport.rs index c4f1db17de48a..9d0df874ad4a9 100644 --- a/src/librustdoc/passes/lint/unused_doc_comment_on_reexport.rs +++ b/src/librustdoc/passes/lint/unused_doc_comment_on_reexport.rs @@ -1,12 +1,13 @@ //! Detects non-inlined local reexports with doc comments. -use crate::clean::*; +use crate::clean::{ImportKind, Item, ItemKind}; use crate::core::DocContext; use rustc_errors::Applicability; +use rustc_span::symbol::sym; use rustc_span::DUMMY_SP; pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) { - if item.attrs.doc_strings.is_empty() || !matches!(*item.kind, ItemKind::ImportItem(_)) { + if item.attrs.doc_strings.is_empty() { return; } let Some(hir_id) = DocContext::as_local_hir_id(cx.tcx, item.item_id) @@ -15,15 +16,34 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) { return; }; let sp = item.attrs.doc_strings[0].span; + if let ItemKind::ImportItem(ref i) = *item.kind { + match i.kind { + ImportKind::Simple(s) => { + // We don't emit this lint if it's an anonymous reexport or a non-local item since + // it'll be directly inlined. + if s == sym::underscore_imports + || !i.source.did.map(|did| did.is_local()).unwrap_or(false) + { + return; + } + } + ImportKind::Glob => { + // The warning was already emitted in `visit_ast` for glob imports. + return; + } + } + } else { + return; + } let item_sp = item.span(cx.tcx).map(|sp| sp.inner()).unwrap_or(DUMMY_SP).shrink_to_lo(); // If we see an import item with doc, then it's wrong in any case because otherwise, if it // had `#[doc(inline)]`, we wouldn't see it here in the first place. cx.tcx.struct_span_lint_hir( - crate::lint::DOCUMENTED_LOCAL_NON_INLINED_REEXPORT, + crate::lint::UNUSED_REEXPORT_DOCUMENTATION, hir_id, sp, - "doc comments on local non-inlined reexports are ignored", + "doc comments on non-inlined reexports of local items are ignored", |lint| { lint.note( "the documentation won't be visible because reexports don't have their own page", diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 634e70ec97a0d..4b86bcd040f92 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -200,3 +200,14 @@ pub(crate) fn source_span_for_markdown_range( md_range.end + start_bytes + end_bytes, ))) } + +/// Emit the warning for a document `pub use *`. +pub(crate) fn emit_documented_glob_warning(hir_id: rustc_hir::HirId, span: Span, tcx: TyCtxt<'_>) { + tcx.struct_span_lint_hir( + crate::lint::UNUSED_REEXPORT_DOCUMENTATION, + hir_id, + span, + "doc comment on glob reexports are always ignored", + |lint| lint, + ); +} diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 088cb3f339492..688fa1426fd7a 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -193,6 +193,11 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // Later passes in rustdoc will de-duplicate by name and kind, so if glob- // imported items appear last, then they'll be the ones that get discarded. if matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) { + let hir_id = item.hir_id(); + let attrs = self.cx.tcx.hir().attrs(hir_id); + if let Some(attr) = attrs.iter().find(|attr| attr.doc_str().is_some()) { + crate::passes::emit_documented_glob_warning(hir_id, attr.span, self.cx.tcx); + } self.visit_item(item); } } From 6d944393e3bb768b4bc9141838dfa6e56ecee9ee Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 13 Feb 2023 17:29:39 +0100 Subject: [PATCH 3/4] Add rustdoc UI tests for unused doc comments on reexports --- tests/rustdoc-ui/issue-103997.rs | 2 + tests/rustdoc-ui/issue-103997.stderr | 2 +- .../unused-doc-comment-on-reexport.rs | 29 +++++++++++++++ .../unused-doc-comment-on-reexport.stderr | 37 +++++++++++++++++++ 4 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 tests/rustdoc-ui/unused-doc-comment-on-reexport.rs create mode 100644 tests/rustdoc-ui/unused-doc-comment-on-reexport.stderr diff --git a/tests/rustdoc-ui/issue-103997.rs b/tests/rustdoc-ui/issue-103997.rs index 36f42fb15f7e4..dec13443d2fa9 100644 --- a/tests/rustdoc-ui/issue-103997.rs +++ b/tests/rustdoc-ui/issue-103997.rs @@ -1,5 +1,7 @@ // check-pass +#![allow(rustdoc::unused_reexport_documentation)] + pub fn foo() {} /// [`foo`](Self::foo) //~ WARNING unresolved link to `Self::foo` diff --git a/tests/rustdoc-ui/issue-103997.stderr b/tests/rustdoc-ui/issue-103997.stderr index c06db91496f86..aa87c13a69df7 100644 --- a/tests/rustdoc-ui/issue-103997.stderr +++ b/tests/rustdoc-ui/issue-103997.stderr @@ -1,5 +1,5 @@ warning: unresolved link to `Self::foo` - --> $DIR/issue-103997.rs:5:13 + --> $DIR/issue-103997.rs:7:13 | LL | /// [`foo`](Self::foo) | ^^^^^^^^^ no item named `Self` in scope diff --git a/tests/rustdoc-ui/unused-doc-comment-on-reexport.rs b/tests/rustdoc-ui/unused-doc-comment-on-reexport.rs new file mode 100644 index 0000000000000..fa636290f128a --- /dev/null +++ b/tests/rustdoc-ui/unused-doc-comment-on-reexport.rs @@ -0,0 +1,29 @@ +#![deny(rustdoc::unused_reexport_documentation)] + +pub mod a { + pub struct Foo; +} + +mod b { + pub struct Priv; +} + +pub mod c { + /// Error! + //~^ ERROR + pub use a::Foo as Bar; + /// No error for this one. + #[doc(inline)] + pub use a::Foo as Bar2; + /// No error for this one. + pub use b::Priv as Priv; +} + +/// No error for this one. +pub use std::option::Option as AnotherOption; +/// Error! +//~^ ERROR +pub use std::option::*; +/// Error! +//~^ ERROR +pub use b::*; diff --git a/tests/rustdoc-ui/unused-doc-comment-on-reexport.stderr b/tests/rustdoc-ui/unused-doc-comment-on-reexport.stderr new file mode 100644 index 0000000000000..22202e2b38929 --- /dev/null +++ b/tests/rustdoc-ui/unused-doc-comment-on-reexport.stderr @@ -0,0 +1,37 @@ +error: doc comment on glob reexports are always ignored + --> $DIR/unused-doc-comment-on-reexport.rs:24:1 + | +LL | /// Error! + | ^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/unused-doc-comment-on-reexport.rs:1:9 + | +LL | #![deny(rustdoc::unused_reexport_documentation)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: doc comment on glob reexports are always ignored + --> $DIR/unused-doc-comment-on-reexport.rs:27:1 + | +LL | /// Error! + | ^^^^^^^^^^ + +error: doc comments on non-inlined reexports of local items are ignored + --> $DIR/unused-doc-comment-on-reexport.rs:12:5 + | +LL | /// Error! + | ^^^^^^^^^^ + | + = note: the documentation won't be visible because reexports don't have their own page +help: try adding `#[doc(inline)]` + | +LL | #[doc(inline)] pub use a::Foo as Bar; + | ++++++++++++++ +help: or move the documentation directly on the reexported item + | +LL - /// Error! +LL + + | + +error: aborting due to 3 previous errors + From 8059af54116f875eaae94759793207388d60c0bb Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 13 Feb 2023 17:34:52 +0100 Subject: [PATCH 4/4] Add documentation for new `unused_reexport_documentation` lint --- src/doc/rustdoc/src/lints.md | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/doc/rustdoc/src/lints.md b/src/doc/rustdoc/src/lints.md index 45db3bb9b007b..490e258bfd02d 100644 --- a/src/doc/rustdoc/src/lints.md +++ b/src/doc/rustdoc/src/lints.md @@ -374,3 +374,51 @@ warning: this URL is not a hyperlink warning: 2 warnings emitted ``` + +## `unused_reexport_documentation` + +This lint in **warn-by-default**. It detects documentation which isn't used on +reexports. There are two possible cases: + * Glob reexports (`pub use *`). + * Non-inlined reexport of local item. + +```rust +pub struct Foo; + +/// Unused because `Bar` doesn't have its own documentation +/// page unless we add `#[doc(inline)]` on it. +pub use Foo as Bar; + +/// Documentation from glob imports is never used. +pub use std::option::*; +``` + +Which will give: + +```text +warning: doc comment on glob reexports are always ignored + --> unused.rs:7:1 + | +7 | /// Documentation from glob imports is never used. + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(rustdoc::unused_reexport_documentation)]` on by default + +warning: doc comments on non-inlined reexports of local items are ignored + --> unused.rs:3:1 + | +3 | /// Unused because `Bar` doesn't have its own documentation + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the documentation won't be visible because reexports don't have their own page +help: try adding `#[doc(inline)]` + | +5 | #[doc(inline)] pub use Foo as Bar; + | ++++++++++++++ +help: or move the documentation directly on the reexported item + | +3 - /// Unused because `Bar` doesn't have its own documentation + | + +warning: 2 warnings emitted +```