Add new rustdoc broken_footnote lint - #137803
Conversation
rustbot
added
S-waiting-on-review
I think you might have misunderstood the issue description. "The term of what is meant by anchors is borrowed from" a description of this style of anchor links. It doesn't say anything about footnotes. /// Consider [panics](std::alloc::GlobalAllocator#Panics) on allocation.
// ^^^^^^^ anchor |
|
I indeed misunderstood and went on a completely different path. Although I still think that this PR is useful. I plan to send a follow-up PR for unused footnote references as well. ^^' I removed the "fixes" mention though. |
|
This lint seems too noisy to belong in core. Review case F1 of #121659 (comment), where [^a-zA-Z0-9] is syntactically valid as a footnote reference, but not intended to be parsed as one. Following the suggestion (to add If the goal of this lint is to catch typo-ed footnotes, then there should be an unused footnote definition, somewhere, that the author tried and failed to reference. Those are guaranteed to be a problem: either they typo-ed the reference, or they never intended a footnote at all (but still got one). |
|
That's why we suggest to escape the |
Doing that makes the doc comment less plain-text (noisier). |
c9627d0 to
9976037
Compare
|
After discussing with @notriddle, it was decided to move the implementation of this lint into its own file. |
This comment has been minimized.
This comment has been minimized.
9976037 to
8f7f71c
Compare
|
☔ The latest upstream changes (presumably #140726) made this pull request unmergeable. Please resolve the merge conflicts. |
changelog: [`doc_suspicious_footnotes`]: lint for text that looks like a footnote reference but has no definition This is an alternative to rust-lang/rust#137803, meant to address the concerns about false positives. This lint only fires when the apparent footnote reference has a name that's made from pure ASCII digits. This choice is justified by running lintcheck on the top 200 crates, plus the clippy default set: 1. [I ran lintcheck](https://gist.github.com/notriddle/59072476c9c1fd569fee421270dad665) with a modded version of this lint that didn't check for digits only. It produced a false positive warning on a line in mdbook that had a regex, and no true positives at all. 2. [I also ran lintcheck](https://gist.github.com/notriddle/74eb8c9e1939b9f5c5549bf1d4fa238a) with a custom lint that fired on any valid footnote reference with a non-ascii-digit name. `cargo` uses one in its job_queue module, and that's all it found. cc @GuillaumeGomez
8f7f71c to
10dd66a
Compare
This comment has been minimized.
This comment has been minimized.
|
Fixed merge conflict. Ping @notriddle |
This comment has been minimized.
This comment has been minimized.
10dd66a to
5e69d80
Compare
This comment has been minimized.
This comment has been minimized.
5deff4b to
e7b4bc8
Compare
|
And updated to new APIs. |
This comment has been minimized.
This comment has been minimized.
d827e02 to
10323f8
Compare
|
I was sure |
There was a problem hiding this comment.
Still missing a few:
//! [^*] special characters can appear within footnote references
//~^ ERROR: no footnote definition matching this footnote
//!
//! [^**]
//!
//! [^**]: not an error
//!
//! [^\_] so can escaped characters
//~^ ERROR: no footnote definition matching this footnote
There was a problem hiding this comment.
Ah good point, adding them as well.
…, r=notriddle,GuillaumeGomez Add new `unused_footnote_definition` rustdoc lint Follow-up of rust-lang#137803 (where the two first commits come from). It adds a new lint which checks for unused footnote definitions. r? @notriddle
…, r=notriddle,GuillaumeGomez Add new `unused_footnote_definition` rustdoc lint Follow-up of rust-lang#137803 (where the two first commits come from). It adds a new lint which checks for unused footnote definitions. r? @notriddle
c470ca8 to
48e4a7b
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Cherry-picked your commit @notriddle (wow, it's been 5 months already...). Once CI pass, I'll r+ the PR. |
|
@bors r=notriddle |
rust-bors
Bot
added
S-waiting-on-bors
…notriddle Add new rustdoc `broken_footnote` lint I think in this case, instead of spawning `unpportable_markdown`, we should instead notify that this footnote reference doesn't have an associated definition. r? @notriddle
…uwer Rollup of 4 pull requests Successful merges: - #137803 (Add new rustdoc `broken_footnote` lint) - #160975 (Remove target argument from get_proc_macros) - #160861 (rustc_parse: suggest removing semicolon before `if` block) - #160990 (Remove old cfg parser which is now dead code) Failed merges: - #137858 (Add new `unused_footnote_definition` rustdoc lint)
|
💔 I suspect this PR failed tests as part of a rollup After fixing the problem, consider running a try job for the failed job before re-approving. Link to failure: #160992 (comment) |
rust-bors
Bot
added
S-waiting-on-author
|
This pull request was unapproved. This PR was contained in a rollup (#160992), which was unapproved. |
…, r=notriddle Add new `unused_footnote_definition` rustdoc lint Follow-up of rust-lang#137803 (where the two first commits come from). It adds a new lint which checks for unused footnote definitions. r? @notriddle
…, r=notriddle Add new `unused_footnote_definition` rustdoc lint Follow-up of rust-lang#137803 (where the two first commits come from). It adds a new lint which checks for unused footnote definitions. r? @notriddle
…, r=notriddle Add new `unused_footnote_definition` rustdoc lint Follow-up of rust-lang#137803 (where the two first commits come from). It adds a new lint which checks for unused footnote definitions. r? @notriddle
|
☔ The latest upstream changes (presumably #161014) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
Rollup merge of #137858 - GuillaumeGomez:unused_footnote_def, r=notriddle Add new `unused_footnote_definition` rustdoc lint Follow-up of #137803 (where the two first commits come from). It adds a new lint which checks for unused footnote definitions. r? @notriddle
6 participants
View all comments
I think in this case, instead of spawning
unpportable_markdown, we should instead notify that this footnote reference doesn't have an associated definition.r? @notriddle