doc_suspicious_footnotes: lint text that looks like a footnote #14708

notriddle · 2025-04-29T23:37:57Z

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:

I ran lintcheck 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.
I also ran lintcheck 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

rustbot · 2025-04-29T23:38:03Z

r? @blyxyas

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

GuillaumeGomez · 2025-05-02T12:01:06Z

Looks good to me, thanks!

r? samueltardieu

rustbot · 2025-05-02T12:01:08Z

samueltardieu is on vacation.

Please choose another assignee.

blyxyas

Great initial patch (and a really good lint idea 👍)! I left some suggestions and some questions.

clippy_lints/src/doc/doc_suspicious_footnotes.rs

clippy_lints/src/doc/mod.rs

tests/ui/doc_suspicious_footnotes.rs

clippy_lints/src/doc/doc_suspicious_footnotes.rs

notriddle · 2025-05-05T21:44:14Z

@blyxyas Okay, I've implemented variants on all your suggestions.

clippy_lints/src/doc/doc_suspicious_footnotes.rs

blyxyas · 2025-05-14T13:02:04Z

clippy_lints/src/doc/mod.rs

                    }
-                    text_to_check.push((text, range, code_level));
+                    text_to_check.push((text, range.clone(), code_level));
+                    doc_suspicious_footnotes::check(cx, doc, range, &fragments);


I'm sure that you've already tried this, but is there a reason that you didn't use FootnoteReference from ķEvent? You would not have to do span magic and manually parse footnotes and it seems that this function doesn't actually use text

The FootnoteReference event is only emitted if the code is actually a footnote. This lint fires for text that looks like a footnote reference but isn't.

notriddle · 2025-05-19T23:54:50Z

@blyxyas

I've redone the suggestions using attr metadata instead of string manipulation.

This also has test cases for cfg_attr and block doc comments.

clippy_lints/src/doc/doc_suspicious_footnotes.rs

tests/ui/doc_suspicious_footnotes.fixed

blyxyas

Seems pretty good overall, I'll go open the FCP. Although some refactoring is needed.

blyxyas · 2025-05-26T16:39:46Z

clippy_lints/src/doc/doc_suspicious_footnotes.rs

+            && let Some(this_fragment) = fragments
+                .fragments
+                .iter()
+                .find(|frag| {
+                    let found = this_fragment_start < frag.doc.as_str().len();
+                    if !found {
+                        this_fragment_start -= frag.doc.as_str().len();
+                    }
+                    found
+                })


Could you refactor this piece of code so that it's clearer what its doing?

Okay, I've added some comments and simplified it down to just be a for loop.

blyxyas · 2025-05-26T17:41:20Z

clippy_lints/src/doc/doc_suspicious_footnotes.rs

+                .find(|frag| {
+                    let found = this_fragment_start < frag.doc.as_str().len();
+                    if !found {
+                        this_fragment_start -= frag.doc.as_str().len();


Also, could you add a test for this specific line of code? Seems that no tests are triggering this case.

Okay, I've added a test called MultiFragmentFootnote that checks the behavior of that code.

notriddle · 2025-05-27T23:10:50Z

@rustbot ready

blyxyas

The last nit (not a clear documentation) and I think this is to be merged! (+ We have team members backing this up)

clippy_lints/src/doc/mod.rs

blyxyas · 2025-06-02T19:31:54Z

After the change is done, could you squash these commits?

notriddle · 2025-06-02T21:10:25Z

@blyxyas Okay, it's done.

blyxyas

LGTM, thanks! ❤️

rustbot assigned blyxyas Apr 29, 2025

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 29, 2025

notriddle force-pushed the doc_suspicious_footnotes branch 2 times, most recently from c56ef28 to 1356e76 Compare April 29, 2025 23:50

GuillaumeGomez approved these changes May 2, 2025

View reviewed changes

blyxyas requested changes May 4, 2025

View reviewed changes

clippy_lints/src/doc/doc_suspicious_footnotes.rs Outdated Show resolved Hide resolved

clippy_lints/src/doc/mod.rs Show resolved Hide resolved

tests/ui/doc_suspicious_footnotes.rs Show resolved Hide resolved

clippy_lints/src/doc/doc_suspicious_footnotes.rs Outdated Show resolved Hide resolved

rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels May 4, 2025

notriddle force-pushed the doc_suspicious_footnotes branch from b0e3325 to 99b16ae Compare May 5, 2025 22:45

blyxyas reviewed May 14, 2025

View reviewed changes

notriddle force-pushed the doc_suspicious_footnotes branch from 00f6841 to 40040e1 Compare May 19, 2025 23:53

notriddle force-pushed the doc_suspicious_footnotes branch 2 times, most recently from 6c70997 to 98426f5 Compare May 20, 2025 00:13

blyxyas requested changes May 21, 2025

View reviewed changes

notriddle force-pushed the doc_suspicious_footnotes branch from 02eda3b to a5f4d5d Compare May 21, 2025 18:52

blyxyas requested changes May 26, 2025

View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels May 27, 2025

blyxyas reviewed Jun 1, 2025

View reviewed changes

clippy_lints/src/doc/mod.rs Outdated Show resolved Hide resolved

doc_suspicious_footnotes: lint text that looks like a footnote

fcfbcc8

notriddle force-pushed the doc_suspicious_footnotes branch from 8ba6d9f to fcfbcc8 Compare June 2, 2025 20:20

blyxyas approved these changes Jun 5, 2025

View reviewed changes

blyxyas added this pull request to the merge queue Jun 5, 2025

Merged via the queue into rust-lang:master with commit c90c7c4 Jun 5, 2025
11 checks passed

notriddle deleted the doc_suspicious_footnotes branch July 21, 2025 17:29

doc_suspicious_footnotes: lint text that looks like a footnote #14708

doc_suspicious_footnotes: lint text that looks like a footnote #14708

Uh oh!

Conversation

notriddle commented Apr 29, 2025

Uh oh!

rustbot commented Apr 29, 2025

Uh oh!

GuillaumeGomez commented May 2, 2025

Uh oh!

rustbot commented May 2, 2025

Uh oh!

blyxyas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

notriddle commented May 5, 2025

Uh oh!

Uh oh!

blyxyas May 14, 2025

Choose a reason for hiding this comment

Uh oh!

notriddle May 14, 2025

Choose a reason for hiding this comment

Uh oh!

notriddle commented May 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

blyxyas left a comment

Choose a reason for hiding this comment

Uh oh!

blyxyas May 26, 2025

Choose a reason for hiding this comment

Uh oh!

notriddle May 27, 2025

Choose a reason for hiding this comment

Uh oh!

blyxyas May 26, 2025

Choose a reason for hiding this comment

Uh oh!

notriddle May 27, 2025

Choose a reason for hiding this comment

Uh oh!

notriddle commented May 27, 2025

Uh oh!

blyxyas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

blyxyas commented Jun 2, 2025

Uh oh!

notriddle commented Jun 2, 2025

Uh oh!

blyxyas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants