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

Conversation

GuillaumeGomez
Copy link
Member

Fixes #43087.

Everything can't be checked directly from the newly added pass unfortunately since glob reexports have already been inlined at this point.

r? @notriddle

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 13, 2023
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-13 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v3' (SHA:ac593985615ec2ede58e132d2e21d2b1cbd6127c)
Download action repository 'rust-lang/simpleinfra@master' (SHA:b9dd95b10bcfe24d57bf54db874f81a7c8315a80)
Complete job name: PR (x86_64-gnu-llvm-13, false, ubuntu-20.04-xl)
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
  CI_JOB_NAME: x86_64-gnu-llvm-13
---
---- [rustdoc] tests/rustdoc/multiple-import-levels.rs stdout ----

error: rustdoc failed!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/multiple-import-levels/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/multiple-import-levels" "--deny" "warnings" "/checkout/tests/rustdoc/multiple-import-levels.rs"
stdout: none
--- stderr -------------------------------
error: doc comments on non-inlined reexports of local items are ignored
   |
12 |     /// 2
   |     ^^^^^
   |
   |
   = note: the documentation won't be visible because reexports don't have their own page
   = note: `-D rustdoc::unused-reexport-documentation` implied by `-D warnings`
help: try adding `#[doc(inline)]`
   |
13 |     #[doc(inline)] pub use crate::a::Type;
help: or move the documentation directly on the reexported item
   |
12 -     /// 2
12 +     
12 +     
   |

error: doc comments on non-inlined reexports of local items are ignored
   |
17 |     /// 3
   |     ^^^^^
   |
   |
   = note: the documentation won't be visible because reexports don't have their own page
help: try adding `#[doc(inline)]`
   |
18 |     #[doc(inline)] pub use crate::b::Type;
help: or move the documentation directly on the reexported item
   |
17 -     /// 3
17 +     
17 +     
   |

error: doc comments on non-inlined reexports of local items are ignored
   |
19 |     /// 4
   |     ^^^^^
   |
   |
   = note: the documentation won't be visible because reexports don't have their own page
help: try adding `#[doc(inline)]`
   |
20 |     #[doc(inline)] pub use crate::b::Type as Woof;
help: or move the documentation directly on the reexported item
   |
19 -     /// 4
19 +     

@GuillaumeGomez
Copy link
Member Author

So, the failure is actually quite interesting because if a reexport is itself reexported, then it shouldn't emit a warning. This lint implementation will require a bit more work to take this into account. But before that, does this lint make sense or not?

@notriddle
Copy link
Contributor

I think it does make sense to produce a warning in any case where actually using the doc comment is impractical.

@GuillaumeGomez
Copy link
Member Author

We have some unused_attr lint emitted for doc comments in some places, but impractical, not that I know of.

@bors
Copy link
Contributor

bors commented Mar 10, 2023

☔ The latest upstream changes (presumably #108934) made this pull request unmergeable. Please resolve the merge conflicts.

@anden3 anden3 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 14, 2023
@anden3
Copy link
Contributor

anden3 commented May 14, 2023

Hello @GuillaumeGomez! This PR has some merge conflicts, what's the status of it?

@GuillaumeGomez
Copy link
Member Author

I need to reimplement it as described above. I keep this PR open as a reminder for future me when I have time. ^^'

@GuillaumeGomez GuillaumeGomez marked this pull request as draft May 15, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not all doc-comments on re-exports are displayed
7 participants