Add new `unused_footnote_definition` rustdoc lint by GuillaumeGomez · Pull Request #137858 · rust-lang/rust

GuillaumeGomez · 2025-03-01T16:22:08Z

Follow-up of #137803 (where the two first commits come from).

It adds a new lint which checks for unused footnote definitions.

r? @notriddle

GuillaumeGomez · 2025-03-01T19:58:23Z

Fixed tidy.

GuillaumeGomez · 2025-03-11T14:15:00Z

Updated to use new footnote lint code instead.

bors · 2025-05-07T05:04:56Z

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

lolbinarycat

One main issue (possibly parsing footnotes as code blocks) and a bunch of small nits.

lolbinarycat · 2025-05-22T17:56:45Z

src/librustdoc/passes/lint/footnotes.rs

+    let mut footnote_definitions = FxHashMap::default();
+
+    let options = Options::ENABLE_FOOTNOTES;
+    let mut parser = Parser::new_ext(dox, options).into_offset_iter().peekable();


Should we be making sure the lint is enabled before invoking the parser? I know the other lints don't do this, but maybe they should?

lolbinarycat · 2025-05-22T18:00:57Z

src/librustdoc/passes/lint/footnotes.rs

+        let (ref_span, precise) =
+            source_span_for_markdown_range(tcx, dox, &span, &item.attrs.doc_strings)
+                .map(|span| (span, true))
+                .unwrap_or_else(|| (item.attr_span(tcx), false));


Couldn't this just be a let Some(ref_span) = ... else { continue }; ?

It doesn't seem like we use ref_span if precise is false.

lolbinarycat · 2025-05-22T18:31:58Z

src/librustdoc/passes/lint/footnotes.rs

+            Event::Text(text)
+                if &*text == "["
+                    && let Some((Event::Text(text), _)) = parser.peek()
+                    && text.trim_start().starts_with('^')
+                    && parser.next().is_some()
+                    && let Some((Event::Text(text), end_span)) = parser.peek()
+                    && &**text == "]" =>
+            {
+                missing_footnote_references.insert(Range { start: span.start, end: end_span.end });
+            }


It is quite odd that pulldown_cmark isn't emmitting some form of FootnoteReference here despite the docs saying they might not map to an actual definition.

In any case, I don't think this implementation is correct, since Text events are emitted for the bodies of all blocks. Crucially, this includes code blocks, which certainly should not be parsed for footnotes. An integration test should be added to show that we are not wrongfully parsing footnotes within code blocks.

One way to handle this is to track the type of the last Start event and make sure it isn't CodeBlock. luckily other blocks can't appear within code blocks so we don't have to track the full stack of tags. We might want to clear that variable whenever we reach an End event, but I'm not sure if the text after a code block will always get its own separate Paragraph event or not. An integration test with an unused footnote directly after a code block should clear this up.

The docs are outdated. FootnoteReference is only emitted if the footnote definition exists.

pulldown-cmark/pulldown-cmark#1038

glad to see the docs getting fixed, but i still believe this code handles code blocks incorrectly.

lolbinarycat · 2025-05-22T18:34:43Z

tests/rustdoc-ui/lints/unused-footnote.rs

@@ -0,0 +1,9 @@
+// This test ensures that the rustdoc `unused_footnote` is working as expected.


Suggested change

// This test ensures that the rustdoc `unused_footnote` is working as expected.

// This test ensures that the `rustdoc::unused_footnote` lint is working as expected.

lolbinarycat · 2025-05-22T18:35:02Z

tests/rustdoc-ui/lints/unused-footnote.rs

+//!
+//! [^1]: footnote defined
+//! [^2]: footnote defined
+//~^^ unused_footnote_definition


Suggested change

//~^^ unused_footnote_definition

//~^^ ERROR: unused footnote definition

Nit: for consistency with other tests, look for the actual error and not the note telling you why it is enabled

lolbinarycat · 2025-05-22T18:36:39Z

src/librustdoc/lint.rs

+    "footnote reference with no associated definition"
+}
+
+declare_rustdoc_lint! {
+    /// This lint checks if all footnote definitions are used.
+    UNUSED_FOOTNOTE_DEFINITION,
+    Warn,
+    "unused footnote definition"


Suggested change

"footnote reference with no associated definition"

}

declare_rustdoc_lint! {

/// This lint checks if all footnote definitions are used.

UNUSED_FOOTNOTE_DEFINITION,

Warn,

"unused footnote definition"

"detects footnote references with no associated definition"

}

declare_rustdoc_lint! {

/// This lint checks if all footnote definitions are used.

UNUSED_FOOTNOTE_DEFINITION,

Warn,

"detects unused footnote definitions"

for consistency with other lint descriptions

lolbinarycat · 2025-08-21T17:19:49Z

@rustbot author

notriddle · 2025-08-21T17:54:05Z

This PR is still based on #137803, which I thought was a bad idea because of the false positives.

The unused_footnote_definition lint shouldn't have any false positives, so it's fine, but this PR needs rebased to remove the other, rejected lint.

rustbot · 2026-01-26T13:40:41Z

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.

GuillaumeGomez · 2026-01-26T13:40:57Z

Sorry for the delay, finally applied suggestions.

notriddle · 2026-01-31T17:37:09Z

src/librustdoc/passes/lint/footnotes.rs

+//! Detects specific markdown syntax that's different between pulldown-cmark
+//! 0.9 and 0.11.
+//!
+//! This is a mitigation for old parser bugs that affected some
+//! real crates' docs. The old parser claimed to comply with CommonMark,
+//! but it did not. These warnings will eventually be removed,
+//! though some of them may become Clippy lints.
+//!
+//! <https://github.com/rust-lang/rust/pull/121659#issuecomment-1992752820>
+//!
+//! <https://rustc-dev-guide.rust-lang.org/bug-fix-procedure.html#add-the-lint-to-the-list-of-removed-lists>


This doc comment is wrong.

notriddle · 2026-01-31T17:40:56Z

src/librustdoc/passes/lint/footnotes.rs

+                .unwrap_or_else(|| (item.attr_span(tcx), false));
+
+        tcx.node_span_lint(crate::lint::BROKEN_FOOTNOTE, hir_id, ref_span, |lint| {
+            lint.primary_message("no footnote definition matching this footnote");


This lint is redundant with the doc_suspicious_footnotes clippy lint, but it doesn't implement the number checking the same way. Shouldn't that be added?

rustbot assigned notriddle Mar 1, 2025

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 Mar 1, 2025

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the unused_footnote_def branch from c17e182 to 8550b0a Compare March 1, 2025 19:58

GuillaumeGomez force-pushed the unused_footnote_def branch 2 times, most recently from 40b5fa2 to 27c7ec4 Compare March 11, 2025 14:14

lolbinarycat requested changes May 22, 2025

View reviewed changes

rustbot 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 Aug 21, 2025

GuillaumeGomez added 5 commits January 26, 2026 11:22

Add new rustdoc broken_footnote lint

1740c93

Add ui test for rustdoc broken_footnote lint

66dc579

Add new unused_footnote_definition rustdoc lint

e0d8136

Add ui test for new unused_footnote_definition rustdoc lint

3406533

Improve description of new rustdoc lints

2cdc54b

GuillaumeGomez force-pushed the unused_footnote_def branch from 27c7ec4 to 2cdc54b Compare January 26, 2026 13:40

notriddle reviewed Jan 31, 2026

View reviewed changes

		@@ -0,0 +1,9 @@
		// This test ensures that the rustdoc `unused_footnote` is working as expected.

	// This test ensures that the rustdoc `unused_footnote` is working as expected.
	// This test ensures that the `rustdoc::unused_footnote` lint is working as expected.

	//~^^ unused_footnote_definition
	//~^^ ERROR: unused footnote definition

Uh oh!

Conversation

GuillaumeGomez commented Mar 1, 2025

Uh oh!

This comment has been minimized.

GuillaumeGomez commented Mar 1, 2025

Uh oh!

GuillaumeGomez commented Mar 11, 2025

Uh oh!

bors commented May 7, 2025

Uh oh!

lolbinarycat left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lolbinarycat commented Aug 21, 2025

Uh oh!

notriddle commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jan 26, 2026

Uh oh!

GuillaumeGomez commented Jan 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

notriddle commented Aug 21, 2025 •

edited

Loading