Add missing_docs lint to the rustdoc lint group by GuillaumeGomez · Pull Request #77364 · rust-lang/rust

GuillaumeGomez · 2020-09-30T13:37:53Z

Just realized that the rustdoc lint group didn't have the missing_docs lint inside.

r? @jyn514

jyn514 · 2020-09-30T14:03:04Z

I don't think I'm the right person to review this.

r? @ollie27
cc @rust-lang/lang

pickfire

Looks good to me. (don't take/rollup based on this review)

ollie27 · 2020-09-30T20:54:35Z

When the rustdoc lint group was added in #56689missing_docs was deliberately left out. Adding it would be changing what the group is for.

GuillaumeGomez · 2020-09-30T20:57:19Z

The lint is emitted by rustdoc too. :)

ollie27 · 2020-10-05T23:29:45Z

The lint is emitted by rustdoc too. :)

Yes, but it's also emitted by rustc and the rustdoc lint group is for lints only emitted by rustdoc. From #56689:

I deliberately didn't include missing_docs because this is kind of a stepping stone for moving our lints into tool lints (i.e. #![warn(rustdoc::private_doc_tests)]), since all of these are specifically emitted by rustdoc.

My main question is what would the rustdoc lint group be for if missing_docs is added?

GuillaumeGomez · 2020-10-06T09:39:15Z

Literally what it's supposed to be: lints emitted by rustdoc (even if they are also emitted by/from rustc).

jyn514 · 2020-10-06T13:08:15Z

Rustdoc also emits lints about unknown and renamed lints (#75903). But I think it's clear those shouldn't be part of the rustdoc group.

GuillaumeGomez · 2020-10-06T13:09:58Z

Good point. Then let me add more precision: "lints emitted by rustdoc (even if they are also emitted by/from rustc) and linked to documentation."

bors · 2020-10-07T12:16:32Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

camelid · 2020-10-23T02:58:00Z

@GuillaumeGomez Could you resolve the merge conflicts?

jyn514 · 2020-10-23T03:10:41Z

@camelid there's no point resolving the merge conflicts until the rustdoc team decides this is a change they want to accept (and IMO this should have feedback from T-compiler too).

camelid · 2020-10-23T03:26:39Z

Sorry, drive-by triaging 😄

jyn514 · 2020-11-04T07:22:57Z

The discussion has kind of stalled out ... there are two viewpoints of what warn(rustdoc) means:

Lints specific to rustdoc: Add missing_docs lint to the rustdoc lint group #77364 (comment)
Lints specific to documentation: Add missing_docs lint to the rustdoc lint group #77364 (comment)

I don't think we can do anything here until we decide which of those warn(rustdoc) should be. Maybe someone from T-lang has opinions?

nikomatsakis · 2020-11-04T14:34:01Z

My intuition is that users will expect rustdoc to mean "documentation-related lints emitted by rustdoc", and that it doesn't matter so much whether the lint is emitted by rustc or not, but I think it'd be useful to dig just a bit more into the implications of each choice, because I'm not sure I fully understand them.

I think that the argument in favor of a more limited definition is something like this:

If we say that rustdoc includes missing_docs, then when you do #![warn(rustdoc)], rustc will start to emit warnings for missing docs -- and that might be confusing.

In other words, maybe it's surprising the warn(rustdoc) changes rustc behavior at all?

GuillaumeGomez · 2020-12-26T19:26:07Z

I'd prefer to have it running in rustdoc directly indeed. :)

I can make a PR for that if you want?

camelid · 2020-12-26T20:33:58Z

@camelidmissing_docs is the only documentation-related lint that runs in rustc directly instead of rustdoc.

I figured, but my point is that that's an implementation detail and not that relevant to the user.

So maybe the better fix here is to move missing_docs out of rustc and into rustdoc instead? And then everything is consistent.

The one thing is that then you won't be able to run missing_docs by running rustc; you'll have to run rustdoc instead. It's probably not an issue, though, because there's not much reason to have documentation if you don't build it :)

jyn514 · 2020-12-26T21:14:47Z

The one thing is that then you won't be able to run missing_docs by running rustc; you'll have to run rustdoc instead

That's exactly why I don't consider this an implementation detail.

I can make a PR for that if you want?

I think this should have input from T-lang, I don't think it's really rustdoc's decision to make.

GuillaumeGomez · 2020-12-26T21:18:03Z

My proposition still stands. :p

LeSeulArtichaut · 2020-12-27T01:29:54Z

It's probably not an issue, though, because there's not much reason to have documentation if you don't build it :)

But do people run rustdoc in CI, for example?

camelid · 2020-12-27T02:06:49Z

It's probably not an issue, though, because there's not much reason to have documentation if you don't build it :)
But do people run rustdoc in CI, for example?

Yes, but this would mean that you can't run rustc --deny missing_docs, you instead run rustdoc --deny missing_docs. I'm not fully sure what your question is :)

GuillaumeGomez · 2020-12-27T15:03:35Z

I wanted cargo check to run rustdoc --check too. The PR is open but I don't think it's gonna get merged any time soon...

Make rustdoc lints a tool lint instead of built-in - Rename `broken_intra_doc_links` to `rustdoc::broken_intra_doc_links` (and similar for other rustdoc lints; I don't expect any others to be used frequently, though). - Ensure that the old lint names still work and give deprecation errors - Register lints even when running doctests - Move lint machinery into a separate file - Add `declare_rustdoc_lint!` macro Unblocks rust-lang#80300, rust-lang#79816, rust-lang#80965. Makes the strangeness in rust-lang#77364 more apparent to the end user (note that `missing_docs` is *not* moved to rustdoc in this PR). ## Current status This is waiting on FCP: rust-lang#80527 (comment)

Make rustdoc lints a tool lint instead of built-in - Rename `broken_intra_doc_links` to `rustdoc::broken_intra_doc_links` (and similar for other rustdoc lints; I don't expect any others to be used frequently, though). - Ensure that the old lint names still work and give deprecation errors - Register lints even when running doctests - Move lint machinery into a separate file - Add `declare_rustdoc_lint!` macro Unblocks rust-lang#80300, rust-lang#79816, rust-lang#80965. Makes the strangeness in rust-lang#77364 more apparent to the end user (note that `missing_docs` is *not* moved to rustdoc in this PR). Closesrust-lang#78786. ## Current status This is blocked on rust-lang#82620 (see rust-lang#80527 (comment))

GuillaumeGomez · 2021-04-28T13:56:08Z

Rustdoc lints are now declared in rustdoc so no need for this PR anymore.

Add missing_docs lint to the rustdoc lint group
2ede5a0

rust-highfive assigned jyn514 Sep 30, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 30, 2020

rust-highfive assigned ollie27 and unassigned jyn514 Sep 30, 2020

jyn514 added A-lints 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 Sep 30, 2020

pickfire approved these changes Sep 30, 2020
View reviewed changes

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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2020

jyn514 added I-needs-decision Issue: In need of a decision. S-waiting-on-team and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 4, 2020

crlf0710 added S-waiting-on-team and removed S-waiting-on-team labels Nov 20, 2020

crlf0710 added S-waiting-on-team and removed S-waiting-on-team labels Dec 11, 2020

This was referenced Dec 28, 2020
New rustdoc lint to respect -Dwarnings correctly #80456
Closed
Cleanup rustdoc lint listing #80455
Closed
Make rustdoc lints a tool lint instead of built-in #80527
Merged

crlf0710 added S-waiting-on-team and removed S-waiting-on-team labels Jan 15, 2021

crlf0710 added S-waiting-on-team and removed S-waiting-on-team labels Mar 5, 2021

crlf0710 added S-waiting-on-team and removed S-waiting-on-team labels Apr 2, 2021

crlf0710 added S-waiting-on-team and removed S-waiting-on-team labels Apr 23, 2021

apiraino added the T-rustdoc label Apr 28, 2021

GuillaumeGomez closed this Apr 28, 2021

GuillaumeGomez deleted the missing-docs-lint-group branch April 28, 2021 13:56

Uh oh!

Uh oh!

Conversation

GuillaumeGomez commented Sep 30, 2020

Uh oh!

jyn514 commented Sep 30, 2020

Uh oh!

pickfire left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ollie27 commented Sep 30, 2020

Uh oh!

GuillaumeGomez commented Sep 30, 2020

Uh oh!

ollie27 commented Oct 5, 2020

Uh oh!

GuillaumeGomez commented Oct 6, 2020

Uh oh!

jyn514 commented Oct 6, 2020

Uh oh!

GuillaumeGomez commented Oct 6, 2020

Uh oh!

bors commented Oct 7, 2020

Uh oh!

camelid commented Oct 23, 2020

Uh oh!

jyn514 commented Oct 23, 2020

Uh oh!

camelid commented Oct 23, 2020

Uh oh!

jyn514 commented Nov 4, 2020

Uh oh!

nikomatsakis commented Nov 4, 2020

Uh oh!

GuillaumeGomez commented Dec 26, 2020

Uh oh!

camelid commented Dec 26, 2020

Uh oh!

jyn514 commented Dec 26, 2020

Uh oh!

GuillaumeGomez commented Dec 26, 2020

Uh oh!

LeSeulArtichaut commented Dec 27, 2020

Uh oh!

camelid commented Dec 27, 2020

Uh oh!

GuillaumeGomez commented Dec 27, 2020

Uh oh!

GuillaumeGomez commented Apr 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

pickfire left a comment •
edited
Loading