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

rustdoc: private_doc_tests lint no longer fires on stable #75951

Closed
ehuss opened this issue Aug 26, 2020 · 9 comments · Fixed by #75953
Closed

rustdoc: private_doc_tests lint no longer fires on stable #75951

ehuss opened this issue Aug 26, 2020 · 9 comments · Fixed by #75953
Labels
A-doctests Area: Documentation tests, run by rustdoc A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Milestone
1.47.0

Comments

@ehuss
Copy link
Contributor

ehuss commented Aug 26, 2020

As of rustc 1.47.0-beta.1, the private_doc_tests lint is no longer working.

#![warn(private_doc_tests)]
/// hi!
/// ```
/// let x = 1;
/// ```
fn f() {}

I expected to see this happen: cargo doc should issue warning: documentation test in private item

Instead, this happened: No warnings emitted.

This appears to have changed in #74855. I don't see any discussion there about this, so I'm guessing it was unintentional.

cc @jyn514.
@ehuss ehuss added the C-bug Category: This is a bug. label Aug 26, 2020
@ehuss ehuss mentioned this issue Aug 26, 2020
Merged
@jonas-schievink jonas-schievink added A-doctests Area: Documentation tests, run by rustdoc A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Aug 26, 2020
@jyn514 jyn514 added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label Aug 26, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Aug 26, 2020
@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

This was definitely unintentional. I thought I added a test for this ...

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020 •
edited
Loading

Hmm, it tests with deny(rustdoc) but not with deny(missing_doc_code_examples). Maybe the name of the lint wasn't registered?

https://github.com/rust-lang/rust/pull/74855/files#diff-2009d75a18198cb86d168844baa3c607

@Mark-Simulacrum Mark-Simulacrum added this to the 1.47 milestone Aug 26, 2020
@ehuss
Copy link
Contributor Author

ehuss commented Aug 26, 2020

The issue is this line. Compiletest runs with RUSTC_BOOTSTRAP set. I don't recall off the top of my head how to force a test to be stable.

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

Wait now I'm confused. I thought the point of this lint was that it only runs on nightly, because we're not sure if we want to stabilize it.

cc @rust-lang/rustdoc

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

@ehuss are you sure this used to work?

@ehuss
Copy link
Contributor Author

ehuss commented Aug 26, 2020

The private_doc_tests lint has been on stable since 1.32. It is documented here.

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

Got it, I mixed it up with missing_doc_code_examples. Yeah this is a bug then - the two lints should be treated differently.

bors added a commit to rust-lang/cargo that referenced this issue Aug 26, 2020
@bors
Auto merge of #8653 - ehuss:fix-cache-messages-rustdoc-test, r=alexcr…
1d66f90 
…ichton

Fix cache_messages::rustdoc test broken on beta.

The most recent beta `rustc 1.47.0-beta.1` broke this test (rust-lang/rust#75951).  Just switch to a different lint to get the test working again.
@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

Actually it looks like I introduced the opposite bug for missing_doc_code_examples - it now runs on beta when it shouldn't.

@jyn514 jyn514 mentioned this issue Aug 26, 2020
Merged
@spastorino
Copy link
Member

Assigning P-high as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@spastorino spastorino added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 26, 2020
@bors bors closed this as completed in c2a0168 Aug 27, 2020
ehuss pushed a commit to ehuss/cargo that referenced this issue Aug 28, 2020
@bors @ehuss
Auto merge of rust-lang#8653 - ehuss:fix-cache-messages-rustdoc-test,…
2ceb172 
… r=alexcrichton

Fix cache_messages::rustdoc test broken on beta.

The most recent beta `rustc 1.47.0-beta.1` broke this test (rust-lang/rust#75951).  Just switch to a different lint to get the test working again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-doctests Area: Documentation tests, run by rustdoc A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.47.0
Development

Successfully merging a pull request may close this issue.

Fix swapped stability attributes for rustdoc lints jyn514/rust
6 participants
@ehuss @spastorino @jonas-schievink @Mark-Simulacrum @jyn514 @rustbot