Skip to content

Make rustdoc lints a tool lint instead of built-in#80527

Merged
bors merged 7 commits intorust-lang:masterfrom
jyn514:rustdoc-lints
Mar 4, 2021
Merged

Make rustdoc lints a tool lint instead of built-in#80527
bors merged 7 commits intorust-lang:masterfrom
jyn514:rustdoc-lints

Conversation

@jyn514
Copy link
Member

@jyn514 jyn514 commented Dec 30, 2020

  • 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 #80300, #79816, #80965. Makes the strangeness in #77364 more apparent to the end user (note that missing_docs is not moved to rustdoc in this PR). Closes #78786.

Current status

This is blocked on #82620 (see #80527 (comment))

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-lang Relevant to the language team needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. labels Dec 30, 2020
@rust-highfive

This comment has been minimized.

@rust-highfive
Copy link
Contributor

r? @GuillaumeGomez

(rust-highfive has picked a reviewer for you, use r? to override)

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

This comment has been minimized.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 15, 2021
@jyn514 jyn514 changed the title Make rustdoc lints to a tool lint instead of built-in Make rustdoc lints a tool lint instead of built-in Jan 15, 2021
@jyn514 jyn514 added S-blocked Status: Blocked on something else such as an RFC or other implementation work. 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
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Mar 4, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 4, 2021
@camelid camelid mentioned this pull request Mar 4, 2021
3 tasks
@JohnTitor
Copy link
Member

@bors retry

@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
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 4, 2021
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). Closes rust-lang#78786.

## Current status

This is blocked on rust-lang#82620 (see rust-lang#80527 (comment))
@lopopolo
Copy link
Contributor

lopopolo commented Mar 6, 2021

I would have liked to see the rustdoc lint group/tool name exist on stable before this change was merged.

It is not possible to run both nightly and stable builds with -D warnings and warn(broken_intra_doc_links)/warn(rustdoc:: broken_intra_doc_links) now because, despite allow(unknown_lints), the unknown tool name results in a hard error:

error[E0710]: an unknown tool name found in scoped lint: `rustdoc::broken_intra_doc_links`
 --> artichoke-core/src/lib.rs:9:9
  |
9 | #![warn(rustdoc::broken_intra_doc_links)]
  |         ^^^^^^^

@jyn514
Copy link
Member Author

jyn514 commented Mar 6, 2021

It is not possible to run both nightly and stable builds with -D warnings and warn(broken_intra_doc_links)/warn(rustdoc:: broken_intra_doc_links) now because, despite allow(unknown_lints), the unknown tool name results in a hard error:

If you use the original name (broken_intra_doc_links) and allow(renamed_removed_lints), it should work.

@lopopolo
Copy link
Contributor

lopopolo commented Mar 6, 2021

@jyn514 does the introduction of a new tool name mean that declaring this lint as a crate level warn pragma will bump the MSRV of the crate?

@lopopolo
Copy link
Contributor

lopopolo commented Mar 6, 2021

If you use the original name (broken_intra_doc_links) and allow(renamed_removed_lints), it should work.

Since the lint has been removed, does this mean running nightly rustdoc will not detect broken intradoc links anymore?

@jyn514
Copy link
Member Author

jyn514 commented Mar 6, 2021

Since the lint has been removed, does this mean running nightly rustdoc will not detect broken intradoc links anymore?

No, the lint name will still apply (since #82620).

@jyn514 does the introduction of a new tool name mean that declaring this lint as a crate level warn pragma will bump the MSRV of the crate?

Yes, unfortunately. There's no way to fix this because past releases are immutable (ideally the unknown tool name would only be a warning and not a hard error).

@lopopolo
Copy link
Contributor

lopopolo commented Mar 6, 2021

Thanks for the context. I think the best path forward for me is to remove the pragmas and add -D rustdoc::broken_intra_doc_links to CI via RUSTDOCFLAGS.

ideally the unknown tool name would only be a warning and not a hard error

Has this change landed in nightly/is there a ticket for it?

@jyn514
Copy link
Member Author

jyn514 commented Mar 6, 2021

Has this change landed in nightly/is there a ticket for it?

I think #66079 (comment) is the closest issue.

@lopopolo
Copy link
Contributor

lopopolo commented Mar 6, 2021

Thank you!

@jyn514
Copy link
Member Author

jyn514 commented Mar 16, 2021

I would have liked to see the rustdoc lint group/tool name exist on stable before this change was merged.

@lopopolo I opened #83203 to turn off the warnings until this makes it to stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team 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.

Rustdoc should never silence lints from the rustdoc lint group