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

Rustdoc lints don't fire for pub re-exports #77230

Open
jyn514 opened this issue Sep 26, 2020 · 3 comments
Open

Rustdoc lints don't fire for pub re-exports #77230

jyn514 opened this issue Sep 26, 2020 · 3 comments
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Sep 26, 2020

This is the general form of #77200, since it appears this was unintentional. It came up again in #77119 (comment).

I tried this code:

#![deny(missing_doc_code_examples)]
//! ```
//! unimplemented!()
//! ```

pub use std::task::RawWakerVTable;

I expected to see this happen: Rustdoc gives an error that RawWakerVTable is missing examples.

Instead, this happened: Rustdoc ignores the re-export.

Meta

rustdoc --version: rustdoc 1.48.0-nightly (8b40853 2020-09-23)

Possible Cause

@GuillaumeGomez and I found yesterday that rustdoc does not generate an ImportItem for pub re-exports. Instead it generates an StructItem, which has the DefId of the original item, not the HirId of an item in the current crate. This means that anywhere that needs a HirId, like lints, is out of luck.

Relevant pieces of code:

_ if !i.def_id.is_local() => {
// non-local items are skipped because they can be out of the users control,
// especially in the case of trait impls, which rustdoc eagerly inlines
return Some(i);
}

let hir_id = match cx.as_local_hir_id(item.def_id) {
Some(hir_id) => hir_id,
None => {
// If non-local, no need to check anything.
return;
}
};

https://github.com/rust-lang/rust/blob/f9546afbe1300fa3780470db1d10eb60c2cd3bbc/src/librustdoc/passes/html_tags.rs#L106-L112

check_code_block_syntax doesn't have this issue, but only by sidestepping the lint machinery altogether and being impossible to silence.

let mut diag = self.cx.sess().struct_span_warn(sp, warning_message);

I'm not yet sure what's causing the issues in clean - it looks like it should be making an ImportItem but it's not happening in practice.

vec![Item {
name: None,
attrs: self.attrs.clean(cx),
source: self.whence.clean(cx),
def_id: DefId::local(CRATE_DEF_INDEX),
visibility: self.vis.clean(cx),
stability: None,
deprecation: None,
inner: ImportItem(inner),
}]

Below is some debug output from the original code sample. There are various other RawWakerVTable items being skipped but they're either fields or impls, so I didn't include them here. In particular there are no ImportItems.

Debug output
Sep 26 10:09:35.580 DEBUG rustdoc::passes::doc_test_lints: skipping foreign item DefId(2:33796 ~ core[e6b8]::task[0]::wake[0]::RawWakerVTable[0]) with inner item StructItem(Struct { struct_type: Plain, generics: Generics { params: [], where_predicates: [] }, fields: [Item { source: Span { filename: Real(Named("/home/joshua/rustc/library/core/src/task/wake.rs")), cnum: crate2, loline: 67, locol: 4, hiline: 67, hicol: 43, original: /home/joshua/rustc/library/core/src/task/wake.rs:67:5: 67:44 (#0) }, name: Some("clone"), attrs: Attributes { doc_strings: [SugaredDoc(0, /home/joshua/rustc/library/core/src/task/wake.rs:60:5: 66:83 (#0), "This function will be called when the [`RawWaker`] gets cloned, e.g. when\nthe [`Waker`] in which the [`RawWaker`] is stored gets cloned.\n\nThe implementation of this function must retain all resources that are\nrequired for this additional instance of a [`RawWaker`] and associated\ntask. Calling `wake` on the resulting [`RawWaker`] should result in a wakeup\nof the same task that would have been awoken by the original [`RawWaker`].")], other_attrs: [], cfg: None, span: Some(/home/joshua/rustc/library/core/src/task/wake.rs:60:5: 60:82 (#0)), links: [], inner_docs: false }, inner: StructFieldItem(BareFunction(BareFunctionDecl { unsafety: Unsafe, generic_params: [], decl: FnDecl { inputs: Arguments { values: [Argument { type_: RawPointer(Not, Tuple([])), name: "" }] }, output: Return(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "RawWaker", args: AngleBracketed { args: [], bindings: [] } }] }, param_names: None, did: DefId(2:33787 ~ core[e6b8]::task[0]::wake[0]::RawWaker[0]), is_generic: false }), c_variadic: false, attrs: Attributes { doc_strings: [], other_attrs: [], cfg: None, span: None, links: [], inner_docs: false } }, abi: Rust })), visibility: Inherited, def_id: DefId(2:33797 ~ core[e6b8]::task[0]::wake[0]::RawWakerVTable[0]::clone[0]), stability: None, deprecation: None }, Item { source: Span { filename: Real(Named("/home/joshua/rustc/library/core/src/task/wake.rs")), cnum: crate2, loline: 75, locol: 4, hiline: 75, hicol: 30, original: /home/joshua/rustc/library/core/src/task/wake.rs:75:5: 75:31 (#0) }, name: Some("wake"), attrs: Attributes { doc_strings: [SugaredDoc(0, /home/joshua/rustc/library/core/src/task/wake.rs:69:5: 74:25 (#0), "This function will be called when `wake` is called on the [`Waker`].\nIt must wake up the task associated with this [`RawWaker`].\n\nThe implementation of this function must make sure to release any\nresources that are associated with this instance of a [`RawWaker`] and\nassociated task.")], other_attrs: [], cfg: None, span: Some(/home/joshua/rustc/library/core/src/task/wake.rs:69:5: 69:77 (#0)), links: [], inner_docs: false }, inner: StructFieldItem(BareFunction(BareFunctionDecl { unsafety: Unsafe, generic_params: [], decl: FnDecl { inputs: Arguments { values: [Argument { type_: RawPointer(Not, Tuple([])), name: "" }] }, output: Return(Tuple([])), c_variadic: false, attrs: Attributes { doc_strings: [], other_attrs: [], cfg: None, span: None, links: [], inner_docs: false } }, abi: Rust })), visibility: Inherited, def_id: DefId(2:33798 ~ core[e6b8]::task[0]::wake[0]::RawWakerVTable[0]::wake[0]), stability: None, deprecation: None }, Item { source: Span { filename: Real(Named("/home/joshua/rustc/library/core/src/task/wake.rs")), cnum: crate2, loline: 82, locol: 4, hiline: 82, hicol: 37, original: /home/joshua/rustc/library/core/src/task/wake.rs:82:5: 82:38 (#0) }, name: Some("wake_by_ref"), attrs: Attributes { doc_strings: [SugaredDoc(0, /home/joshua/rustc/library/core/src/task/wake.rs:77:5: 81:17 (#0), "This function will be called when `wake_by_ref` is called on the [`Waker`].\nIt must wake up the task associated with this [`RawWaker`].\n\nThis function is similar to `wake`, but must not consume the provided data\npointer.")], other_attrs: [], cfg: None, span: Some(/home/joshua/rustc/library/core/src/task/wake.rs:77:5: 77:84 (#0)), links: [], inner_docs: false }, inner: StructFieldItem(BareFunction(BareFunctionDecl { unsafety: Unsafe, generic_params: [], decl: FnDecl { inputs: Arguments { values: [Argument { type_: RawPointer(Not, Tuple([])), name: "" }] }, output: Return(Tuple([])), c_variadic: false, attrs: Attributes { doc_strings: [], other_attrs: [], cfg: None, span: None, links: [], inner_docs: false } }, abi: Rust })), visibility: Inherited, def_id: DefId(2:33799 ~ core[e6b8]::task[0]::wake[0]::RawWakerVTable[0]::wake_by_ref[0]), stability: None, deprecation: None }, Item { source: Span { filename: Real(Named("/home/joshua/rustc/library/core/src/task/wake.rs")), cnum: crate2, loline: 89, locol: 4, hiline: 89, hicol: 30, original: /home/joshua/rustc/library/core/src/task/wake.rs:89:5: 89:31 (#0) }, name: Some("drop"), attrs: Attributes { doc_strings: [SugaredDoc(0, /home/joshua/rustc/library/core/src/task/wake.rs:84:5: 88:25 (#0), "This function gets called when a [`RawWaker`] gets dropped.\n\nThe implementation of this function must make sure to release any\nresources that are associated with this instance of a [`RawWaker`] and\nassociated task.")], other_attrs: [], cfg: None, span: Some(/home/joshua/rustc/library/core/src/task/wake.rs:84:5: 84:68 (#0)), links: [], inner_docs: false }, inner: StructFieldItem(BareFunction(BareFunctionDecl { unsafety: Unsafe, generic_params: [], decl: FnDecl { inputs: Arguments { values: [Argument { type_: RawPointer(Not, Tuple([])), name: "" }] }, output: Return(Tuple([])), c_variadic: false, attrs: Attributes { doc_strings: [], other_attrs: [], cfg: None, span: None, links: [], inner_docs: false } }, abi: Rust })), visibility: Inherited, def_id: DefId(2:33800 ~ core[e6b8]::task[0]::wake[0]::RawWakerVTable[0]::drop[0]), stability: None, deprecation: None }], fields_stripped: false })
@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate labels Sep 26, 2020
@GuillaumeGomez

This comment has been minimized.

@jyn514

This comment has been minimized.

@GuillaumeGomez

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants