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.

Sign up for GitHub

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

Jump to bottom

Don't warn for missing_doc_examples when item is #[doc(hidden)] #76716

Merged
merged 6 commits into from
Mar 5, 2021
Merged

Don't warn for missing_doc_examples when item is #[doc(hidden)] #76716

merged 6 commits into from
Mar 5, 2021

Conversation

GuillaumeGomez
Copy link
Member

r? @jyn514

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 14, 2020
@GuillaumeGomez GuillaumeGomez changed the title Stop complains on doc hidden Stop complains when item is #[doc(hidden)] Sep 14, 2020
@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. labels Sep 14, 2020
jyn514
jyn514 reviewed Sep 14, 2020
View reviewed changes
src/librustdoc/passes/doc_test_lints.rs Outdated Show resolved Hide resolved
src/librustdoc/visit_ast.rs Outdated Show resolved Hide resolved
src/librustdoc/passes/doc_test_lints.rs Outdated Show resolved Hide resolved
@jyn514 jyn514 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 Sep 14, 2020
@jyn514 jyn514 changed the title Stop complains when item is #[doc(hidden)] Don't warn for missing_doc_examples when item is #[doc(hidden)] Sep 14, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 14, 2020

I have a secondary concern too: it seems like a lot of code between this and missing_docs is being duplicated. Is there anyway to make them more consistent with one another? It seems a shame to have to update the same logic in two places.

@bors

This comment has been minimized.

Sign in to view
@crlf0710 crlf0710 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 8, 2020
@camelid camelid 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2020
@Dylan-DPC-zz Dylan-DPC-zz added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 30, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 8, 2020

@Dylan-DPC why did you mark this as waiting-on-team?

@GuillaumeGomez
Copy link
Member Author

I am the team maybe? XD

@Dylan-DPC-zz
Copy link

This is pending on discussion between both of you and anyone else from the team :P

@jyn514
Copy link
Member

jyn514 commented Nov 8, 2020

This is pending on discussion between both of you and anyone else from the team :P

Not that I'm aware of.

@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Nov 28, 2020
@jyn514 jyn514 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-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Nov 29, 2020
@crlf0710 crlf0710 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 18, 2020
@crlf0710 crlf0710 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 15, 2021
@camelid camelid removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 12, 2021
@GuillaumeGomez GuillaumeGomez added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 2, 2021
@bors
Copy link
Contributor

bors commented Mar 4, 2021

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

@jyn514
Copy link
Member

jyn514 commented Mar 4, 2021

This makes missing_doc_examples inconsistent with missing_docs:

warning: missing documentation for the crate
 --> hidden.rs:1:1
  |
1 | / #![warn(missing_docs)]
2 | | #[doc(hidden)]
3 | | pub fn f() {}
  | |_____________^
  |
note: the lint level is defined here
 --> hidden.rs:1:9
  |
1 | #![warn(missing_docs)]
  |         ^^^^^^^^^^^^

I do think the new behavior is better, but it also seems strange to have the two behave differently ... maybe we should also update missing_docs to have this behavior? But that requires keeping track of doc(hidden) in rustc itself, which would be a pretty big change from how it works now.

@Manishearth @ollie27 what do you think?

@jyn514
Copy link
Member

jyn514 commented Mar 4, 2021

Oh wait I'm just dumb, that was warning that the crate root is missing docs, not that the item is missing docs. Adding crate-level docs fixes the issue:

$ cat hidden.rs 
//! some docs
#![warn(missing_docs)]
#[doc(hidden)]
pub fn f() {}

@GuillaumeGomez r=me with the merge conflict fixed

@GuillaumeGomez
Copy link
Member Author

@bors: r=jyn514

@bors
Copy link
Contributor

bors commented Mar 4, 2021

📌 Commit 1683cb1 has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2021
@GuillaumeGomez GuillaumeGomez mentioned this pull request Mar 4, 2021
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 5, 2021
@Dylan-DPC
Rollup merge of rust-lang#76716 - GuillaumeGomez:stop-complains-on-do…
adb52a3 
…c-hidden, r=jyn514

Don't warn for `missing_doc_examples` when item is #[doc(hidden)]

r? ``@jyn514``
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 5, 2021
Closed
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 5, 2021
@JohnTitor
Rollup merge of rust-lang#76716 - GuillaumeGomez:stop-complains-on-do…
3f41802 
…c-hidden, r=jyn514

Don't warn for `missing_doc_examples` when item is #[doc(hidden)]

r? ```@jyn514```
@JohnTitor JohnTitor mentioned this pull request Mar 5, 2021
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2021
@bors
Auto merge of rust-lang#82777 - GuillaumeGomez:rollup-etcsupl, r=Guil…
8ccc89b 
…laumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#76716 (Don't warn for `missing_doc_examples` when item is #[doc(hidden)])
 - rust-lang#82088 (Shorten html::render)
 - rust-lang#82690 (Update rustdoc documentation)
 - rust-lang#82752 (Add a regression test for issue-81712)
 - rust-lang#82765 (Fix polymorphization ICE on associated types in trait decls using const generics in bounds)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Mar 5, 2021
@m-ou-se
Rollup merge of rust-lang#76716 - GuillaumeGomez:stop-complains-on-do…
7f2e413 
…c-hidden, r=jyn514

Don't warn for `missing_doc_examples` when item is #[doc(hidden)]

r? ````@jyn514````
@m-ou-se m-ou-se mentioned this pull request Mar 5, 2021
Closed
@bors bors merged commit 95bbc7e into rust-lang:master Mar 5, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 5, 2021
@GuillaumeGomez GuillaumeGomez deleted the stop-complains-on-doc-hidden branch March 5, 2021 13:36
@Nemo157 Nemo157 mentioned this pull request Sep 12, 2022
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 left review comments

Assignees

@jyn514 jyn514

Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.52.0
Development

Successfully merging this pull request may close these issues.
8 participants
@GuillaumeGomez @jyn514 @bors @Dylan-DPC-zz @crlf0710 @rust-highfive @camelid @rustbot