Skip to content

Add new rustdoc broken_footnote lint #137803

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Feb 28, 2025

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 28, 2025
@notriddle
Copy link
Contributor

Fixes #137740.

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

@GuillaumeGomez
Copy link
Member Author

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.

@notriddle
Copy link
Contributor

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 \ to the front) makes the plain-text representation harder to read, and Rustdoc is already doing what the author wanted, so where's the problem?

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).

@GuillaumeGomez
Copy link
Member Author

That's why we suggest to escape the [ if it was not intended. Like I said, I'm also planning to send a PR for unused footnote definitions. But even without it, I think it makes sense to warn that a footnote reference doesn't match anything and suggest to escape it if it's not meant to be a footnote.

@notriddle
Copy link
Contributor

That's why we suggest to escape the [ if it was not intended.

Doing that makes the doc comment less plain-text (noisier).

@GuillaumeGomez
Copy link
Member Author

After discussing with @notriddle, it was decided to move the implementation of this lint into its own file.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented May 7, 2025

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

github-merge-queue bot pushed a commit to rust-lang/rust-clippy that referenced this pull request Jun 5, 2025
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.

5 participants