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

Rename intra_doc_link_resolution_failure #74926

Merged
merged 10 commits into from
Jul 31, 2020
Merged

Conversation

Manishearth
Copy link
Member

@rust-highfive
Copy link
Collaborator

Some changes occurred in intra-doc-links.

cc @jyn514

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(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 Jul 29, 2020
src/librustc_lint/lib.rs Outdated Show resolved Hide resolved
@jyn514 jyn514 assigned jyn514 and unassigned davidtwco Jul 29, 2020
@Manishearth
Copy link
Member Author

Done!

@jyn514
Copy link
Member

jyn514 commented Jul 29, 2020

BTW, the failure before the second rename was

error: lint `intra_doc_link_resolution_failure` has been renamed to `intra_doc_link_resolution_failures`
  --> library/core/src/lib.rs:64:9
   |
64 | #![deny(intra_doc_link_resolution_failure)] // rustdoc is run without -D warnings
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `intra_doc_link_resolution_failures`
   |
   = note: `-D renamed-and-removed-lints` implied by `-D warnings`

error: lint `intra_doc_link_resolution_failure` has been renamed to `intra_doc_link_resolution_failures`
   --> library/core/src/lib.rs:152:9
    |
152 | #![deny(intra_doc_link_resolution_failure)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `intra_doc_link_resolution_failures`

So you probably have to update those still.

@Manishearth
Copy link
Member Author

Ah, I ran my replaces in src/

@jyn514
Copy link
Member

jyn514 commented Jul 30, 2020

Ugh, looks like you need cfg_attr(not(bootstrap)) in core.

error: unknown lint: `intra_doc_resolution_failures`
  --> library/core/src/lib.rs:64:9
   |
64 | #![deny(intra_doc_resolution_failures)] // rustdoc is run without -D warnings
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `intra_doc_link_resolution_failure`
   |
   = note: `-D unknown-lints` implied by `-D warnings`

error: unknown lint: `intra_doc_resolution_failures`
   --> library/core/src/lib.rs:152:9
    |
152 | #![deny(intra_doc_resolution_failures)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `intra_doc_link_resolution_failure`

r=me once CI is fixed.

@Mark-Simulacrum
Copy link
Member

Noticed in passing - do we have an issue about rustdoc not denying warnings? If that's still true today, please file one and cc me on it. We should almost certainly not have any deny attributes here.

@jyn514
Copy link
Member

jyn514 commented Jul 30, 2020

I think this was added shortly before we denied rustdoc warnings (#71670, #74300). So it can probably be removed now.

@jyn514
Copy link
Member

jyn514 commented Jul 30, 2020

@Manishearth you need to run x.py fmt.

@Manishearth
Copy link
Member Author

Aaah I thought I'd done this, but not each time

@ollie27 ollie27 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jul 30, 2020
@jyn514
Copy link
Member

jyn514 commented Jul 30, 2020

You also need to run x.py test src/test/rustdoc --bless 😆

@jyn514 jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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 Jul 30, 2020
@bors
Copy link
Contributor

bors commented Jul 30, 2020

📌 Commit c17eb56 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 30, 2020
@Manishearth
Copy link
Member Author

@bors rollup=iffy

still haven't been able to locally test it

@bors
Copy link
Contributor

bors commented Jul 31, 2020

⌛ Testing commit c17eb56 with merge ffa80f0...

@bors
Copy link
Contributor

bors commented Jul 31, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: jyn514
Pushing ffa80f0 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 31, 2020
@bors bors merged commit ffa80f0 into rust-lang:master Jul 31, 2020
@Manishearth Manishearth deleted the rename-lint branch July 31, 2020 07:12
bors added a commit to rust-lang/cargo that referenced this pull request Aug 3, 2020
Fix intra-doc tests for renamed lint.

The lint was renamed in rust-lang/rust#74926.
@lopopolo
Copy link
Contributor

lopopolo commented Aug 4, 2020

Thanks for naming the lint to something that's both consistent with the API guidelines and also more memorable. One side effect is that it is now difficult to build existing projects on the current stable and latest nightly.

Running on nightly with -D warnings in CI for generating rustdoc results in renamed-and-removed-lints warnings. Updating the lint to the new name results in unknown_lints warnings on stable.

The only options are to either blanket allow one of the failing lints (since allow attributes can't be applied to allow attributes) or do a dance with -A lint flags on CI and unwind them once the new name hits stable.

@jyn514
Copy link
Member

jyn514 commented Aug 4, 2020

@lopopolo can't you allow(unknown_lints) on stable? But in general warnings are not guaranteed to be stable, see #43466 (comment)

@jyn514
Copy link
Member

jyn514 commented Aug 4, 2020

Or in other words, I don't know a way to prevent the situation you mentioned other than never renaming lints ever.

@Manishearth
Copy link
Member Author

Renaming lints is not uncommon, library authors are expected to apply allow(unknown_lints) for that duration.

lopopolo added a commit to artichoke/focaccia that referenced this pull request Aug 6, 2020
This lint was renamed to broken_intra_doc_links as part of feature
stabilization.

The lint was renamed in rust-lang/rust#74926
lopopolo added a commit to artichoke/intaglio that referenced this pull request Aug 6, 2020
This lint was renamed to broken_intra_doc_links as part of feature
stabilization.

The lint was renamed in rust-lang/rust#74926
lopopolo added a commit to artichoke/rand_mt that referenced this pull request Aug 6, 2020
This lint was renamed to broken_intra_doc_links as part of feature
stabilization.

The lint was renamed in rust-lang/rust#74926
lopopolo added a commit to artichoke/strudel that referenced this pull request Aug 6, 2020
This lint was renamed to broken_intra_doc_links as part of feature
stabilization.

The lint was renamed in rust-lang/rust#74926
daviddrysdale added a commit to daviddrysdale/tink that referenced this pull request Aug 7, 2020
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. merged-by-bors This PR was explicitly merged by bors. 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
Development

Successfully merging this pull request may close these issues.

10 participants