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

False warnings with cargo doc. #58745

Closed
prataprc opened this issue Feb 26, 2019 · 6 comments
Closed

False warnings with cargo doc. #58745

prataprc opened this issue Feb 26, 2019 · 6 comments
Assignees
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@prataprc
Copy link

First raised this issue with rust-random/rand rust-random/rand#737. But the documentation links seem to work fine. So may be it is false-warning raised by cargo doc ?

How to reproduce ?

$ git clone git@github.com:bnclabs/llrb-index.git
$ cd llrb-index

$ rustup which cargo
<path>/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo

$ cargo doc
@jonas-schievink jonas-schievink added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. labels Feb 26, 2019
@GuillaumeGomez
Copy link
Member

I'll take a look.

@QuietMisdreavus
Copy link
Member

QuietMisdreavus commented Mar 6, 2019

I took a closer look at this yesterday and today, and found some of the warnings that were in fact spurious - the ones that corresponded to a link that was actually resolved on the item's page. I've opened #58972 to fix those.

However, there were some left that were not spurious in the same way. These all tripped a warning because the links can resolve in the original location in the original crate, but not the re-exported location in the facade crate. (The links are resolved all over again each time the item re-exports into a full page - the information isn't saved across crates, so as a hack we use the location of the re-export to resolve links.) Once #58972 is merged, the remaining warnings will need more effort to fix.

EDIT: I'd meant to give an example of an actual resolution failure - in the RngCore trait docs, it links to an impls module, which is present in the rand_core crate where RngCore is declared, but is not present in the rand crate where it's re-exported. This causes the link to fail to render, causing an erroneous [impls] to appear in the rendered documentation.

Another example is the TimerError enum, re-exported from the rand_jitter crate. It links back to a method its original JitterRng struct via the intra-doc link crate::JitterRng::test_timer, which is the correct path in rand_jitter, but not in rand - where the full path is crate::rngs::JitterRng::test_timer.

@dhardy
Copy link
Contributor

dhardy commented Mar 7, 2019

Thanks for looking into this.

The latter issues sound tricky to solve — correct documentation for an item presented differently in two different places. I wonder if doc attributes could be made optional depending on which crate the item is being exported from? Ideally here we'd just want a different path for test_timer but significantly different docs for RngCore.

@GuillaumeGomez
Copy link
Member

@QuietMisdreavus and I handled the issue differently: I don't show errors on items that are outside of the current crate and they prevent to read non-local items (so no errors since it's not parsed).

Centril added a commit to Centril/rust that referenced this issue Apr 8, 2019
…rts, r=GuillaumeGomez

rustdoc: don't process `Crate::external_traits` when collecting intra-doc links

Part of rust-lang#58745, closes rust-lang#58917

The `collect-intra-doc-links` pass keeps track of the modules it recurses through as it processes items. This is used to know what module to give the resolver when looking up links. When looking through the regular items of the crate, this works fine, but the `DocFolder` trait as written doesn't just process the main crate hierarchy - it also processes the trait items in the `external_traits` map. This is useful for other passes (so they can strip out `#[doc(hidden)]` items, for example), but here it creates a situation where we're processing items "outside" the regular module hierarchy. Since everything in `external_traits` is defined outside the current crate, we can't fall back to finding its module scope like we do with local items.

Skipping this collection saves us from emitting some spurious warnings. We don't even lose anything by skipping it, either - the docs loaded from here are only ever rendered through `html::render::document_short` which strips any links out, so the fact that the links haven't been loaded doesn't matter. Hopefully this removes most of the remaining spurious resolution warnings from intra-doc links.

r? @GuillaumeGomez
bors added a commit that referenced this issue Apr 9, 2019
…laumeGomez

rustdoc: don't process `Crate::external_traits` when collecting intra-doc links

Part of #58745, closes #58917

The `collect-intra-doc-links` pass keeps track of the modules it recurses through as it processes items. This is used to know what module to give the resolver when looking up links. When looking through the regular items of the crate, this works fine, but the `DocFolder` trait as written doesn't just process the main crate hierarchy - it also processes the trait items in the `external_traits` map. This is useful for other passes (so they can strip out `#[doc(hidden)]` items, for example), but here it creates a situation where we're processing items "outside" the regular module hierarchy. Since everything in `external_traits` is defined outside the current crate, we can't fall back to finding its module scope like we do with local items.

Skipping this collection saves us from emitting some spurious warnings. We don't even lose anything by skipping it, either - the docs loaded from here are only ever rendered through `html::render::document_short` which strips any links out, so the fact that the links haven't been loaded doesn't matter. Hopefully this removes most of the remaining spurious resolution warnings from intra-doc links.

r? @GuillaumeGomez
bors added a commit that referenced this issue Apr 9, 2019
…laumeGomez

rustdoc: don't process `Crate::external_traits` when collecting intra-doc links

Part of #58745, closes #58917

The `collect-intra-doc-links` pass keeps track of the modules it recurses through as it processes items. This is used to know what module to give the resolver when looking up links. When looking through the regular items of the crate, this works fine, but the `DocFolder` trait as written doesn't just process the main crate hierarchy - it also processes the trait items in the `external_traits` map. This is useful for other passes (so they can strip out `#[doc(hidden)]` items, for example), but here it creates a situation where we're processing items "outside" the regular module hierarchy. Since everything in `external_traits` is defined outside the current crate, we can't fall back to finding its module scope like we do with local items.

Skipping this collection saves us from emitting some spurious warnings. We don't even lose anything by skipping it, either - the docs loaded from here are only ever rendered through `html::render::document_short` which strips any links out, so the fact that the links haven't been loaded doesn't matter. Hopefully this removes most of the remaining spurious resolution warnings from intra-doc links.

r? @GuillaumeGomez
bors added a commit that referenced this issue Apr 11, 2019
…laumeGomez

rustdoc: don't process `Crate::external_traits` when collecting intra-doc links

Part of #58745, closes #58917

The `collect-intra-doc-links` pass keeps track of the modules it recurses through as it processes items. This is used to know what module to give the resolver when looking up links. When looking through the regular items of the crate, this works fine, but the `DocFolder` trait as written doesn't just process the main crate hierarchy - it also processes the trait items in the `external_traits` map. This is useful for other passes (so they can strip out `#[doc(hidden)]` items, for example), but here it creates a situation where we're processing items "outside" the regular module hierarchy. Since everything in `external_traits` is defined outside the current crate, we can't fall back to finding its module scope like we do with local items.

Skipping this collection saves us from emitting some spurious warnings. We don't even lose anything by skipping it, either - the docs loaded from here are only ever rendered through `html::render::document_short` which strips any links out, so the fact that the links haven't been loaded doesn't matter. Hopefully this removes most of the remaining spurious resolution warnings from intra-doc links.

r? @GuillaumeGomez
@jyn514
Copy link
Member

jyn514 commented Jul 6, 2020

These all tripped a warning because the links can resolve in the original location in the original crate, but not the re-exported location in the facade crate.

This sounds like it will be fixed by #73101, but I haven't tested on these exact crates.

@jyn514
Copy link
Member

jyn514 commented Aug 19, 2020

I documented rand without warnings on the latest nightly, and the upstream issue has been closed (rust-random/rand#737), so I'm going to close this as well. Feel free to re-open if you still have issues!

@jyn514 jyn514 closed this as completed Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name 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

6 participants