Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lint to check if non-inlined local reexports have documentation #108001

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions src/doc/rustdoc/src/lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
2 changes: 1 addition & 1 deletion src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions src/librustdoc/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,17 @@ 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`) documentation is unused.
/// This is a `rustdoc` only lint, see the documentation in the [rustdoc book].
///
/// [rustdoc book]: ../../../rustdoc/lints.html#unused_reexport_documentation
UNUSED_REEXPORT_DOCUMENTATION,
Warn,
"doc comments are ignored on some cases for reexports",
}

pub(crate) static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
vec![
BROKEN_INTRA_DOC_LINKS,
Expand All @@ -185,6 +196,7 @@ pub(crate) static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
INVALID_HTML_TAGS,
BARE_URLS,
MISSING_CRATE_LEVEL_DOCS,
UNUSED_REEXPORT_DOCUMENTATION,
]
});

Expand Down
2 changes: 2 additions & 0 deletions src/librustdoc/passes/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -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)
}
Expand Down
66 changes: 66 additions & 0 deletions src/librustdoc/passes/lint/unused_doc_comment_on_reexport.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
//! Detects non-inlined local reexports with doc comments.
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() {
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;
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::UNUSED_REEXPORT_DOCUMENTATION,
hir_id,
sp,
"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",
);
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
},
);
}
11 changes: 11 additions & 0 deletions src/librustdoc/passes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
5 changes: 5 additions & 0 deletions src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
2 changes: 2 additions & 0 deletions tests/rustdoc-ui/issue-103997.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// check-pass

#![allow(rustdoc::unused_reexport_documentation)]

pub fn foo() {}

/// [`foo`](Self::foo) //~ WARNING unresolved link to `Self::foo`
Expand Down
2 changes: 1 addition & 1 deletion tests/rustdoc-ui/issue-103997.stderr
Original file line number Diff line number Diff line change
@@ -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
Expand Down
29 changes: 29 additions & 0 deletions tests/rustdoc-ui/unused-doc-comment-on-reexport.rs
Original file line number Diff line number Diff line change
@@ -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::*;
37 changes: 37 additions & 0 deletions tests/rustdoc-ui/unused-doc-comment-on-reexport.stderr
Original file line number Diff line number Diff line change
@@ -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