From 1356e7673c8b86bac8a1e7ab1eb2111c52d0bcfd Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 29 Apr 2025 15:06:20 -0700 Subject: [PATCH 1/3] doc_suspicious_footnotes: lint text that looks like a footnote --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + .../src/doc/doc_suspicious_footnotes.rs | 48 +++++++++++++++++++ clippy_lints/src/doc/mod.rs | 35 +++++++++++++- tests/ui/doc_suspicious_footnotes.fixed | 19 ++++++++ tests/ui/doc_suspicious_footnotes.rs | 19 ++++++++ tests/ui/doc_suspicious_footnotes.stderr | 16 +++++++ 7 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 clippy_lints/src/doc/doc_suspicious_footnotes.rs create mode 100644 tests/ui/doc_suspicious_footnotes.fixed create mode 100644 tests/ui/doc_suspicious_footnotes.rs create mode 100644 tests/ui/doc_suspicious_footnotes.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b62c9a59aa5..2b4f4ede9216 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5636,6 +5636,7 @@ Released 2018-09-13 [`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown [`doc_nested_refdefs`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs [`doc_overindented_list_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_overindented_list_items +[`doc_suspicious_footnotes`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_suspicious_footnotes [`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons [`double_ended_iterator_last`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_ended_iterator_last [`double_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_must_use diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2cccd6ba2702..c1d8bcccac2b 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -117,6 +117,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::doc::DOC_MARKDOWN_INFO, crate::doc::DOC_NESTED_REFDEFS_INFO, crate::doc::DOC_OVERINDENTED_LIST_ITEMS_INFO, + crate::doc::DOC_SUSPICIOUS_FOOTNOTES_INFO, crate::doc::EMPTY_DOCS_INFO, crate::doc::MISSING_ERRORS_DOC_INFO, crate::doc::MISSING_PANICS_DOC_INFO, diff --git a/clippy_lints/src/doc/doc_suspicious_footnotes.rs b/clippy_lints/src/doc/doc_suspicious_footnotes.rs new file mode 100644 index 000000000000..8fcdf43e6ee1 --- /dev/null +++ b/clippy_lints/src/doc/doc_suspicious_footnotes.rs @@ -0,0 +1,48 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet_with_applicability; +use rustc_errors::Applicability; +use rustc_lint::LateContext; + +use std::ops::Range; + +use super::{DOC_SUSPICIOUS_FOOTNOTES, Fragments}; + +pub fn check(cx: &LateContext<'_>, doc: &str, range: Range, fragments: &Fragments<'_>) { + for i in doc[range.clone()] + .bytes() + .enumerate() + .filter_map(|(i, c)| if c == b'[' { Some(i) } else { None }) + { + let start = i + range.start; + if doc.as_bytes().get(start + 1) == Some(&b'^') + && let Some(end) = all_numbers_upto_brace(doc, start + 2) + && doc.as_bytes().get(end) != Some(&b':') + && doc.as_bytes().get(start - 1) != Some(&b'\\') + && let Some(span) = fragments.span(cx, start..end) + { + span_lint_and_then( + cx, + DOC_SUSPICIOUS_FOOTNOTES, + span, + "looks like a footnote ref, but no matching footnote", + |diag| { + let mut applicability = Applicability::MachineApplicable; + let snippet = snippet_with_applicability(cx, span, "..", &mut applicability); + diag.span_suggestion_verbose(span, "try", format!("`{snippet}`"), applicability); + }, + ); + } + } +} + +fn all_numbers_upto_brace(text: &str, i: usize) -> Option { + for (j, c) in text.as_bytes()[i..].iter().copied().enumerate().take(64) { + if c == b']' && j != 0 { + return Some(i + j + 1); + } + if !c.is_ascii_digit() || j >= 64 { + break; + } + } + None +} diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index ab77edf1147c..4b4c762aaba9 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -26,6 +26,7 @@ use std::ops::Range; use url::Url; mod doc_comment_double_space_linebreaks; +mod doc_suspicious_footnotes; mod include_in_doc_without_cfg; mod lazy_continuation; mod link_with_quotes; @@ -608,6 +609,34 @@ declare_clippy_lint! { "double-space used for doc comment linebreak instead of `\\`" } +declare_clippy_lint! { + /// ### What it does + /// Detects syntax that looks like a footnote reference, + /// because it matches the regexp `\[\^[0-9]+\]`, + /// but has no referent. + /// + /// ### Why is this bad? + /// This probably means that a definition was meant to exist, + /// but was not written. + /// + /// ### Example + /// ```no_run + /// /// This is not a footnote[^1], because no definition exists. + /// fn my_fn() {} + /// ``` + /// Use instead: + /// ```no_run + /// /// This is a footnote[^1]. + /// /// + /// /// [^1]: defined here + /// fn my_fn() {} + /// ``` + #[clippy::version = "1.88.0"] + pub DOC_SUSPICIOUS_FOOTNOTES, + suspicious, + "looks like a link or footnote ref, but with no definition" +} + pub struct Documentation { valid_idents: FxHashSet, check_private_items: bool, @@ -639,7 +668,8 @@ impl_lint_pass!(Documentation => [ DOC_OVERINDENTED_LIST_ITEMS, TOO_LONG_FIRST_DOC_PARAGRAPH, DOC_INCLUDE_WITHOUT_CFG, - DOC_COMMENT_DOUBLE_SPACE_LINEBREAKS + DOC_COMMENT_DOUBLE_SPACE_LINEBREAKS, + DOC_SUSPICIOUS_FOOTNOTES, ]); impl EarlyLintPass for Documentation { @@ -1147,7 +1177,8 @@ fn check_doc<'a, Events: Iterator, Range {} diff --git a/tests/ui/doc_suspicious_footnotes.fixed b/tests/ui/doc_suspicious_footnotes.fixed new file mode 100644 index 000000000000..59ff5778e7fa --- /dev/null +++ b/tests/ui/doc_suspicious_footnotes.fixed @@ -0,0 +1,19 @@ +#![warn(clippy::doc_suspicious_footnotes)] + +/// This is not a footnote`[^1]`. +//~^ doc_suspicious_footnotes +/// +/// This is not a footnote[^either], but it doesn't warn. +/// +/// This is not a footnote\[^1], but it also doesn't warn. +/// +/// This is not a footnote[^1\], but it also doesn't warn. +/// +/// This is not a `footnote[^1]`, but it also doesn't warn. +/// +/// This is a footnote[^2]. +/// +/// [^2]: hello world +pub fn footnotes() { + // test code goes here +} diff --git a/tests/ui/doc_suspicious_footnotes.rs b/tests/ui/doc_suspicious_footnotes.rs new file mode 100644 index 000000000000..12077f318089 --- /dev/null +++ b/tests/ui/doc_suspicious_footnotes.rs @@ -0,0 +1,19 @@ +#![warn(clippy::doc_suspicious_footnotes)] + +/// This is not a footnote[^1]. +//~^ doc_suspicious_footnotes +/// +/// This is not a footnote[^either], but it doesn't warn. +/// +/// This is not a footnote\[^1], but it also doesn't warn. +/// +/// This is not a footnote[^1\], but it also doesn't warn. +/// +/// This is not a `footnote[^1]`, but it also doesn't warn. +/// +/// This is a footnote[^2]. +/// +/// [^2]: hello world +pub fn footnotes() { + // test code goes here +} diff --git a/tests/ui/doc_suspicious_footnotes.stderr b/tests/ui/doc_suspicious_footnotes.stderr new file mode 100644 index 000000000000..4cd4c379962a --- /dev/null +++ b/tests/ui/doc_suspicious_footnotes.stderr @@ -0,0 +1,16 @@ +error: looks like a footnote ref, but no matching footnote + --> tests/ui/doc_suspicious_footnotes.rs:3:27 + | +LL | /// This is not a footnote[^1]. + | ^^^^ + | + = note: `-D clippy::doc-suspicious-footnotes` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::doc_suspicious_footnotes)]` +help: try + | +LL - /// This is not a footnote[^1]. +LL + /// This is not a footnote`[^1]`. + | + +error: aborting due to 1 previous error + From 99b16ae1cfe8ff105948eeec37b78511fa3d166b Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 5 May 2025 14:02:47 -0700 Subject: [PATCH 2/3] Improve suggestions and add more tests --- .../src/doc/doc_suspicious_footnotes.rs | 73 ++++++++++-- clippy_lints/src/doc/mod.rs | 11 +- tests/ui/doc_suspicious_footnotes.fixed | 102 ++++++++++++++++- tests/ui/doc_suspicious_footnotes.rs | 80 +++++++++++++ tests/ui/doc_suspicious_footnotes.stderr | 105 +++++++++++++++++- tests/ui/doc_suspicious_footnotes_include.rs | 4 + .../doc_suspicious_footnotes_include.stderr | 17 +++ tests/ui/doc_suspicious_footnotes_include.txt | 13 +++ 8 files changed, 390 insertions(+), 15 deletions(-) create mode 100644 tests/ui/doc_suspicious_footnotes_include.rs create mode 100644 tests/ui/doc_suspicious_footnotes_include.stderr create mode 100644 tests/ui/doc_suspicious_footnotes_include.txt diff --git a/clippy_lints/src/doc/doc_suspicious_footnotes.rs b/clippy_lints/src/doc/doc_suspicious_footnotes.rs index 8fcdf43e6ee1..614939e28106 100644 --- a/clippy_lints/src/doc/doc_suspicious_footnotes.rs +++ b/clippy_lints/src/doc/doc_suspicious_footnotes.rs @@ -1,7 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::source::snippet_with_applicability; use rustc_errors::Applicability; -use rustc_lint::LateContext; +use rustc_lint::{LateContext, LintContext}; use std::ops::Range; @@ -14,21 +13,81 @@ pub fn check(cx: &LateContext<'_>, doc: &str, range: Range, fragments: &F .filter_map(|(i, c)| if c == b'[' { Some(i) } else { None }) { let start = i + range.start; + let mut this_fragment_start = start; if doc.as_bytes().get(start + 1) == Some(&b'^') && let Some(end) = all_numbers_upto_brace(doc, start + 2) && doc.as_bytes().get(end) != Some(&b':') && doc.as_bytes().get(start - 1) != Some(&b'\\') - && let Some(span) = fragments.span(cx, start..end) + && 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 + }) + .or(fragments.fragments.last()) { + let span = fragments.span(cx, start..end).unwrap_or(this_fragment.span); span_lint_and_then( cx, DOC_SUSPICIOUS_FOOTNOTES, span, - "looks like a footnote ref, but no matching footnote", + "looks like a footnote ref, but has no matching footnote", |diag| { - let mut applicability = Applicability::MachineApplicable; - let snippet = snippet_with_applicability(cx, span, "..", &mut applicability); - diag.span_suggestion_verbose(span, "try", format!("`{snippet}`"), applicability); + let applicability = Applicability::HasPlaceholders; + let start_of_md_line = doc.as_bytes()[..start] + .iter() + .rposition(|&c| c == b'\n' || c == b'\r') + .unwrap_or(0); + let end_of_md_line = doc.as_bytes()[start..] + .iter() + .position(|&c| c == b'\n' || c == b'\r') + .unwrap_or(doc.len() - start) + + start; + let span_md_line = fragments + .span(cx, start_of_md_line..end_of_md_line) + .unwrap_or(this_fragment.span); + let span_whole_line = cx.sess().source_map().span_extend_to_line(span_md_line); + if let Ok(mut pfx) = cx + .sess() + .source_map() + .span_to_snippet(span_whole_line.until(span_md_line)) + && let Ok(mut sfx) = cx + .sess() + .source_map() + .span_to_snippet(span_md_line.shrink_to_hi().until(span_whole_line.shrink_to_hi())) + { + let mut insert_before = String::new(); + let mut insert_after = String::new(); + let span = if this_fragment.kind == rustc_resolve::rustdoc::DocFragmentKind::RawDoc + && (!pfx.is_empty() || !sfx.is_empty()) + { + if (pfx.trim() == "#[doc=" || pfx.trim() == "#![doc=") && sfx.trim() == "]" { + // try to use per-line doc fragments if that's what the author did + pfx.push('"'); + sfx.insert(0, '"'); + span_whole_line.shrink_to_hi() + } else { + // otherwise, replace the whole line with the result + pfx = String::new(); + sfx = String::new(); + insert_before = format!(r#"r###"{}"#, this_fragment.doc); + r####""###"####.clone_into(&mut insert_after); + span_md_line + } + } else { + span_whole_line.shrink_to_hi() + }; + diag.span_suggestion_verbose( + span, + "add footnote definition", + format!("{insert_before}\n{pfx}{sfx}\n{pfx}{label}: {sfx}\n{pfx}{sfx}{insert_after}", label = &doc[start..end]), + applicability, + ); + } }, ); } diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 4b4c762aaba9..70dc1aad53a9 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -615,8 +615,17 @@ declare_clippy_lint! { /// because it matches the regexp `\[\^[0-9]+\]`, /// but has no referent. /// + /// Rustdoc footnotes are compatible with GitHub-Flavored Markdown (GFM). + /// They are not parsed as footnotes unless a definition also exists, + /// so they usually "do what you mean" if you want to write the text + /// literally—usually in a regular expression. + /// + /// However, footnote references are usually numbers, and regex + /// negative character classes usually contain other characters, so this + /// lint can make a practical guess for which is meant. + /// /// ### Why is this bad? - /// This probably means that a definition was meant to exist, + /// This probably means that a footnote was meant to exist, /// but was not written. /// /// ### Example diff --git a/tests/ui/doc_suspicious_footnotes.fixed b/tests/ui/doc_suspicious_footnotes.fixed index 59ff5778e7fa..3b433a7419f9 100644 --- a/tests/ui/doc_suspicious_footnotes.fixed +++ b/tests/ui/doc_suspicious_footnotes.fixed @@ -1,6 +1,27 @@ #![warn(clippy::doc_suspicious_footnotes)] +#![allow(clippy::needless_raw_string_hashes)] +//! This is not a footnote[^1]. +//! +//! [^1]: +//! +//~^ doc_suspicious_footnotes +//! +//! This is not a footnote[^either], but it doesn't warn. +//! +//! This is not a footnote\[^1], but it also doesn't warn. +//! +//! This is not a footnote[^1\], but it also doesn't warn. +//! +//! This is not a `footnote[^1]`, but it also doesn't warn. +//! +//! This is a footnote[^2]. +//! +//! [^2]: hello world -/// This is not a footnote`[^1]`. +/// This is not a footnote[^1]. +/// +/// [^1]: +/// //~^ doc_suspicious_footnotes /// /// This is not a footnote[^either], but it doesn't warn. @@ -17,3 +38,82 @@ pub fn footnotes() { // test code goes here } + +pub struct Foo; +impl Foo { + #[doc = r###"This is not a footnote[^1]. + +[^1]: +"###] + //~^ doc_suspicious_footnotes + #[doc = r#""#] + #[doc = r#"This is not a footnote[^either], but it doesn't warn."#] + #[doc = r#""#] + #[doc = r#"This is not a footnote\[^1], but it also doesn't warn."#] + #[doc = r#""#] + #[doc = r#"This is not a footnote[^1\], but it also doesn't warn."#] + #[doc = r#""#] + #[doc = r#"This is not a `footnote[^1]`, but it also doesn't warn."#] + #[doc = r#""#] + #[doc = r#"This is a footnote[^2]."#] + #[doc = r#""#] + #[doc = r#"[^2]: hello world"#] + pub fn footnotes() { + // test code goes here + } + #[doc = r###"This is not a footnote[^1]. + + This is not a footnote[^either], but it doesn't warn. + + This is not a footnote\[^1], but it also doesn't warn. + + This is not a footnote[^1\], but it also doesn't warn. + + This is not a `footnote[^1]`, but it also doesn't warn. + + This is a footnote[^2]. + + [^2]: hello world + + +[^1]: +"###] + //~^^^^^^^^^^^^^^ doc_suspicious_footnotes + pub fn footnotes2() { + // test code goes here + } + #[cfg_attr( + not(FALSE), + doc = r###"This is not a footnote[^1]. + +This is not a footnote[^either], but it doesn't warn. + +[^1]: +"### + //~^ doc_suspicious_footnotes + )] + pub fn footnotes3() { + // test code goes here + } +} + +#[doc = r###"This is not a footnote[^1]. + +[^1]: +"###] +//~^ doc_suspicious_footnotes +#[doc = r""] +#[doc = r"This is not a footnote[^either], but it doesn't warn."] +#[doc = r""] +#[doc = r"This is not a footnote\[^1], but it also doesn't warn."] +#[doc = r""] +#[doc = r"This is not a footnote[^1\], but it also doesn't warn."] +#[doc = r""] +#[doc = r"This is not a `footnote[^1]`, but it also doesn't warn."] +#[doc = r""] +#[doc = r"This is a footnote[^2]."] +#[doc = r""] +#[doc = r"[^2]: hello world"] +pub fn footnotes_attrs() { + // test code goes here +} diff --git a/tests/ui/doc_suspicious_footnotes.rs b/tests/ui/doc_suspicious_footnotes.rs index 12077f318089..75f5029a630b 100644 --- a/tests/ui/doc_suspicious_footnotes.rs +++ b/tests/ui/doc_suspicious_footnotes.rs @@ -1,4 +1,19 @@ #![warn(clippy::doc_suspicious_footnotes)] +#![allow(clippy::needless_raw_string_hashes)] +//! This is not a footnote[^1]. +//~^ doc_suspicious_footnotes +//! +//! This is not a footnote[^either], but it doesn't warn. +//! +//! This is not a footnote\[^1], but it also doesn't warn. +//! +//! This is not a footnote[^1\], but it also doesn't warn. +//! +//! This is not a `footnote[^1]`, but it also doesn't warn. +//! +//! This is a footnote[^2]. +//! +//! [^2]: hello world /// This is not a footnote[^1]. //~^ doc_suspicious_footnotes @@ -17,3 +32,68 @@ pub fn footnotes() { // test code goes here } + +pub struct Foo; +impl Foo { + #[doc = r#"This is not a footnote[^1]."#] + //~^ doc_suspicious_footnotes + #[doc = r#""#] + #[doc = r#"This is not a footnote[^either], but it doesn't warn."#] + #[doc = r#""#] + #[doc = r#"This is not a footnote\[^1], but it also doesn't warn."#] + #[doc = r#""#] + #[doc = r#"This is not a footnote[^1\], but it also doesn't warn."#] + #[doc = r#""#] + #[doc = r#"This is not a `footnote[^1]`, but it also doesn't warn."#] + #[doc = r#""#] + #[doc = r#"This is a footnote[^2]."#] + #[doc = r#""#] + #[doc = r#"[^2]: hello world"#] + pub fn footnotes() { + // test code goes here + } + #[doc = "This is not a footnote[^1]. + + This is not a footnote[^either], but it doesn't warn. + + This is not a footnote\\[^1], but it also doesn't warn. + + This is not a footnote[^1\\], but it also doesn't warn. + + This is not a `footnote[^1]`, but it also doesn't warn. + + This is a footnote[^2]. + + [^2]: hello world + "] + //~^^^^^^^^^^^^^^ doc_suspicious_footnotes + pub fn footnotes2() { + // test code goes here + } + #[cfg_attr( + not(FALSE), + doc = "This is not a footnote[^1].\n\nThis is not a footnote[^either], but it doesn't warn." + //~^ doc_suspicious_footnotes + )] + pub fn footnotes3() { + // test code goes here + } +} + +#[doc = r"This is not a footnote[^1]."] +//~^ doc_suspicious_footnotes +#[doc = r""] +#[doc = r"This is not a footnote[^either], but it doesn't warn."] +#[doc = r""] +#[doc = r"This is not a footnote\[^1], but it also doesn't warn."] +#[doc = r""] +#[doc = r"This is not a footnote[^1\], but it also doesn't warn."] +#[doc = r""] +#[doc = r"This is not a `footnote[^1]`, but it also doesn't warn."] +#[doc = r""] +#[doc = r"This is a footnote[^2]."] +#[doc = r""] +#[doc = r"[^2]: hello world"] +pub fn footnotes_attrs() { + // test code goes here +} diff --git a/tests/ui/doc_suspicious_footnotes.stderr b/tests/ui/doc_suspicious_footnotes.stderr index 4cd4c379962a..9b5a33ccbff1 100644 --- a/tests/ui/doc_suspicious_footnotes.stderr +++ b/tests/ui/doc_suspicious_footnotes.stderr @@ -1,16 +1,109 @@ -error: looks like a footnote ref, but no matching footnote +error: looks like a footnote ref, but has no matching footnote --> tests/ui/doc_suspicious_footnotes.rs:3:27 | -LL | /// This is not a footnote[^1]. +LL | //! This is not a footnote[^1]. | ^^^^ | = note: `-D clippy::doc-suspicious-footnotes` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::doc_suspicious_footnotes)]` -help: try +help: add footnote definition + | +LL ~ //! This is not a footnote[^1]. +LL + //! +LL + //! [^1]: +LL + //! + | + +error: looks like a footnote ref, but has no matching footnote + --> tests/ui/doc_suspicious_footnotes.rs:18:27 + | +LL | /// This is not a footnote[^1]. + | ^^^^ + | +help: add footnote definition + | +LL ~ /// This is not a footnote[^1]. +LL + /// +LL + /// [^1]: +LL + /// + | + +error: looks like a footnote ref, but has no matching footnote + --> tests/ui/doc_suspicious_footnotes.rs:38:13 + | +LL | #[doc = r#"This is not a footnote[^1]."#] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: add footnote definition + | +LL ~ #[doc = r###"This is not a footnote[^1]. +LL + +LL + [^1]: +LL ~ "###] + | + +error: looks like a footnote ref, but has no matching footnote + --> tests/ui/doc_suspicious_footnotes.rs:55:13 + | +LL | #[doc = "This is not a footnote[^1]. + | _____________^ +LL | | +LL | | This is not a footnote[^either], but it doesn't warn. +... | +LL | | [^2]: hello world +LL | | "] + | |_____^ + | +help: add footnote definition + | +LL ~ #[doc = r###"This is not a footnote[^1]. +LL + +LL + This is not a footnote[^either], but it doesn't warn. +LL + +LL + This is not a footnote\[^1], but it also doesn't warn. +LL + +LL + This is not a footnote[^1\], but it also doesn't warn. +LL + +LL + This is not a `footnote[^1]`, but it also doesn't warn. +LL + +LL + This is a footnote[^2]. +LL + +LL + [^2]: hello world +LL + +LL + +LL + [^1]: +LL ~ "###] + | + +error: looks like a footnote ref, but has no matching footnote + --> tests/ui/doc_suspicious_footnotes.rs:75:15 + | +LL | doc = "This is not a footnote[^1].\n\nThis is not a footnote[^either], but it doesn't warn." + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: add footnote definition + | +LL ~ doc = r###"This is not a footnote[^1]. +LL + +LL + This is not a footnote[^either], but it doesn't warn. +LL + +LL + [^1]: +LL + "### + | + +error: looks like a footnote ref, but has no matching footnote + --> tests/ui/doc_suspicious_footnotes.rs:83:9 + | +LL | #[doc = r"This is not a footnote[^1]."] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: add footnote definition | -LL - /// This is not a footnote[^1]. -LL + /// This is not a footnote`[^1]`. +LL ~ #[doc = r###"This is not a footnote[^1]. +LL + +LL + [^1]: +LL ~ "###] | -error: aborting due to 1 previous error +error: aborting due to 6 previous errors diff --git a/tests/ui/doc_suspicious_footnotes_include.rs b/tests/ui/doc_suspicious_footnotes_include.rs new file mode 100644 index 000000000000..4f75ad94eafd --- /dev/null +++ b/tests/ui/doc_suspicious_footnotes_include.rs @@ -0,0 +1,4 @@ +//@ error-in-other-file: footnote +//@ no-rustfix +#![warn(clippy::doc_suspicious_footnotes)] +#![doc=include_str!("doc_suspicious_footnotes_include.txt")] diff --git a/tests/ui/doc_suspicious_footnotes_include.stderr b/tests/ui/doc_suspicious_footnotes_include.stderr new file mode 100644 index 000000000000..cf5ba9b71261 --- /dev/null +++ b/tests/ui/doc_suspicious_footnotes_include.stderr @@ -0,0 +1,17 @@ +error: looks like a footnote ref, but has no matching footnote + --> tests/ui/doc_suspicious_footnotes_include.txt:1:23 + | +LL | This is not a footnote[^1]. + | ^^^^ + | + = note: `-D clippy::doc-suspicious-footnotes` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::doc_suspicious_footnotes)]` +help: add footnote definition + | +LL ~ This is not a footnote[^1]. +LL + +LL + [^1]: + | + +error: aborting due to 1 previous error + diff --git a/tests/ui/doc_suspicious_footnotes_include.txt b/tests/ui/doc_suspicious_footnotes_include.txt new file mode 100644 index 000000000000..2a533e32c4a6 --- /dev/null +++ b/tests/ui/doc_suspicious_footnotes_include.txt @@ -0,0 +1,13 @@ +This is not a footnote[^1]. //~ doc_suspicious_footnotes + +This is not a footnote[^either], but it doesn't warn. + +This is not a footnote\[^1], but it also doesn't warn. + +This is not a footnote[^1\], but it also doesn't warn. + +This is not a `footnote[^1]`, but it also doesn't warn. + +This is a footnote[^2]. + +[^2]: hello world From 98426f5fdf1a514a735c70b25a6a089e7e1210a1 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 19 May 2025 16:33:07 -0700 Subject: [PATCH 3/3] Use a simpler way to generate these doc comments --- .../src/doc/doc_suspicious_footnotes.rs | 92 +++++++++---------- clippy_lints/src/doc/mod.rs | 4 +- tests/ui/doc_suspicious_footnotes.fixed | 58 ++++++++---- tests/ui/doc_suspicious_footnotes.rs | 26 ++++++ tests/ui/doc_suspicious_footnotes.stderr | 56 +++++++---- .../doc_suspicious_footnotes_include.stderr | 2 +- 6 files changed, 152 insertions(+), 86 deletions(-) diff --git a/clippy_lints/src/doc/doc_suspicious_footnotes.rs b/clippy_lints/src/doc/doc_suspicious_footnotes.rs index 614939e28106..c4d896946783 100644 --- a/clippy_lints/src/doc/doc_suspicious_footnotes.rs +++ b/clippy_lints/src/doc/doc_suspicious_footnotes.rs @@ -1,12 +1,14 @@ use clippy_utils::diagnostics::span_lint_and_then; +use rustc_ast::token::CommentKind; use rustc_errors::Applicability; +use rustc_hir::{AttrStyle, Attribute}; use rustc_lint::{LateContext, LintContext}; use std::ops::Range; use super::{DOC_SUSPICIOUS_FOOTNOTES, Fragments}; -pub fn check(cx: &LateContext<'_>, doc: &str, range: Range, fragments: &Fragments<'_>) { +pub fn check(cx: &LateContext<'_>, doc: &str, range: Range, fragments: &Fragments<'_>, attrs: &[Attribute]) { for i in doc[range.clone()] .bytes() .enumerate() @@ -29,6 +31,10 @@ pub fn check(cx: &LateContext<'_>, doc: &str, range: Range, fragments: &F found }) .or(fragments.fragments.last()) + && let Some((last_doc_attr, (last_doc_attr_str, last_doc_attr_comment_kind))) = attrs + .iter() + .rev() + .find_map(|attr| Some((attr, attr.doc_str_and_comment_kind()?))) { let span = fragments.span(cx, start..end).unwrap_or(this_fragment.span); span_lint_and_then( @@ -37,56 +43,48 @@ pub fn check(cx: &LateContext<'_>, doc: &str, range: Range, fragments: &F span, "looks like a footnote ref, but has no matching footnote", |diag| { - let applicability = Applicability::HasPlaceholders; - let start_of_md_line = doc.as_bytes()[..start] - .iter() - .rposition(|&c| c == b'\n' || c == b'\r') - .unwrap_or(0); - let end_of_md_line = doc.as_bytes()[start..] - .iter() - .position(|&c| c == b'\n' || c == b'\r') - .unwrap_or(doc.len() - start) - + start; - let span_md_line = fragments - .span(cx, start_of_md_line..end_of_md_line) - .unwrap_or(this_fragment.span); - let span_whole_line = cx.sess().source_map().span_extend_to_line(span_md_line); - if let Ok(mut pfx) = cx - .sess() - .source_map() - .span_to_snippet(span_whole_line.until(span_md_line)) - && let Ok(mut sfx) = cx - .sess() - .source_map() - .span_to_snippet(span_md_line.shrink_to_hi().until(span_whole_line.shrink_to_hi())) - { - let mut insert_before = String::new(); - let mut insert_after = String::new(); - let span = if this_fragment.kind == rustc_resolve::rustdoc::DocFragmentKind::RawDoc - && (!pfx.is_empty() || !sfx.is_empty()) - { - if (pfx.trim() == "#[doc=" || pfx.trim() == "#![doc=") && sfx.trim() == "]" { - // try to use per-line doc fragments if that's what the author did - pfx.push('"'); - sfx.insert(0, '"'); - span_whole_line.shrink_to_hi() - } else { - // otherwise, replace the whole line with the result - pfx = String::new(); - sfx = String::new(); - insert_before = format!(r#"r###"{}"#, this_fragment.doc); - r####""###"####.clone_into(&mut insert_after); - span_md_line - } - } else { - span_whole_line.shrink_to_hi() + if last_doc_attr.is_doc_comment() { + let (pfx, sfx) = match (last_doc_attr_comment_kind, last_doc_attr.style()) { + (CommentKind::Line, AttrStyle::Outer) => ("\n///\n/// ", ""), + (CommentKind::Line, AttrStyle::Inner) => ("\n//!\n//! ", ""), + (CommentKind::Block, AttrStyle::Outer) => ("\n/** ", " */"), + (CommentKind::Block, AttrStyle::Inner) => ("\n/*! ", " */"), }; diag.span_suggestion_verbose( - span, + last_doc_attr.span().shrink_to_hi(), "add footnote definition", - format!("{insert_before}\n{pfx}{sfx}\n{pfx}{label}: {sfx}\n{pfx}{sfx}{insert_after}", label = &doc[start..end]), - applicability, + format!("{pfx}{label}: {sfx}", label = &doc[start..end]), + Applicability::HasPlaceholders, ); + } else { + let is_file_include = cx + .sess() + .source_map() + .span_to_snippet(this_fragment.span) + .as_ref() + .map(|vdoc| vdoc.trim()) + == Ok(doc); + if is_file_include { + // if this is a file include, then there's no quote marks + diag.span_suggestion_verbose( + this_fragment.span.shrink_to_hi(), + "add footnote definition", + format!("\n\n{label}: ", label = &doc[start..end],), + Applicability::HasPlaceholders, + ); + } else { + // otherwise, we wrap in a string + diag.span_suggestion_verbose( + this_fragment.span, + "add footnote definition", + format!( + "r#\"{doc}\n\n{label}: \"#", + doc = last_doc_attr_str, + label = &doc[start..end], + ), + Applicability::HasPlaceholders, + ); + } } }, ); diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 70dc1aad53a9..8347d7e4827a 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -862,6 +862,7 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[ doc: &doc, fragments: &fragments, }, + attrs, )) } @@ -942,6 +943,7 @@ fn check_doc<'a, Events: Iterator, Range, + attrs: &[Attribute], ) -> DocHeaders { // true if a safety header was found let mut headers = DocHeaders::default(); @@ -1187,7 +1189,7 @@ fn check_doc<'a, Events: Iterator, Range {} diff --git a/tests/ui/doc_suspicious_footnotes.fixed b/tests/ui/doc_suspicious_footnotes.fixed index 3b433a7419f9..4d167f7e4fea 100644 --- a/tests/ui/doc_suspicious_footnotes.fixed +++ b/tests/ui/doc_suspicious_footnotes.fixed @@ -1,9 +1,6 @@ #![warn(clippy::doc_suspicious_footnotes)] #![allow(clippy::needless_raw_string_hashes)] //! This is not a footnote[^1]. -//! -//! [^1]: -//! //~^ doc_suspicious_footnotes //! //! This is not a footnote[^either], but it doesn't warn. @@ -17,11 +14,10 @@ //! This is a footnote[^2]. //! //! [^2]: hello world +//! +//! [^1]: /// This is not a footnote[^1]. -/// -/// [^1]: -/// //~^ doc_suspicious_footnotes /// /// This is not a footnote[^either], but it doesn't warn. @@ -35,16 +31,17 @@ /// This is a footnote[^2]. /// /// [^2]: hello world +/// +/// [^1]: pub fn footnotes() { // test code goes here } pub struct Foo; impl Foo { - #[doc = r###"This is not a footnote[^1]. + #[doc = r#"[^2]: hello world -[^1]: -"###] +[^1]: "#] //~^ doc_suspicious_footnotes #[doc = r#""#] #[doc = r#"This is not a footnote[^either], but it doesn't warn."#] @@ -61,7 +58,7 @@ impl Foo { pub fn footnotes() { // test code goes here } - #[doc = r###"This is not a footnote[^1]. + #[doc = r#"This is not a footnote[^1]. This is not a footnote[^either], but it doesn't warn. @@ -76,20 +73,18 @@ impl Foo { [^2]: hello world -[^1]: -"###] +[^1]: "#] //~^^^^^^^^^^^^^^ doc_suspicious_footnotes pub fn footnotes2() { // test code goes here } #[cfg_attr( not(FALSE), - doc = r###"This is not a footnote[^1]. + doc = r#"This is not a footnote[^1]. This is not a footnote[^either], but it doesn't warn. -[^1]: -"### +[^1]: "# //~^ doc_suspicious_footnotes )] pub fn footnotes3() { @@ -97,10 +92,9 @@ This is not a footnote[^either], but it doesn't warn. } } -#[doc = r###"This is not a footnote[^1]. +#[doc = r#"[^2]: hello world -[^1]: -"###] +[^1]: "#] //~^ doc_suspicious_footnotes #[doc = r""] #[doc = r"This is not a footnote[^either], but it doesn't warn."] @@ -117,3 +111,31 @@ This is not a footnote[^either], but it doesn't warn. pub fn footnotes_attrs() { // test code goes here } + +pub mod multiline { + /*! + * This is not a footnote[^1]. //~ doc_suspicious_footnotes + * + * This is not a footnote\[^1], but it doesn't warn. + * + * This is a footnote[^2]. + * + * These give weird results, but correct ones, so it works. + * + * [^2]: hello world + */ +/*! [^1]: */ + /** + * This is not a footnote[^1]. //~ doc_suspicious_footnotes + * + * This is not a footnote\[^1], but it doesn't warn. + * + * This is a footnote[^2]. + * + * These give weird results, but correct ones, so it works. + * + * [^2]: hello world + */ +/** [^1]: */ + pub fn foo() {} +} diff --git a/tests/ui/doc_suspicious_footnotes.rs b/tests/ui/doc_suspicious_footnotes.rs index 75f5029a630b..7cd0e0d451e7 100644 --- a/tests/ui/doc_suspicious_footnotes.rs +++ b/tests/ui/doc_suspicious_footnotes.rs @@ -97,3 +97,29 @@ impl Foo { pub fn footnotes_attrs() { // test code goes here } + +pub mod multiline { + /*! + * This is not a footnote[^1]. //~ doc_suspicious_footnotes + * + * This is not a footnote\[^1], but it doesn't warn. + * + * This is a footnote[^2]. + * + * These give weird results, but correct ones, so it works. + * + * [^2]: hello world + */ + /** + * This is not a footnote[^1]. //~ doc_suspicious_footnotes + * + * This is not a footnote\[^1], but it doesn't warn. + * + * This is a footnote[^2]. + * + * These give weird results, but correct ones, so it works. + * + * [^2]: hello world + */ + pub fn foo() {} +} diff --git a/tests/ui/doc_suspicious_footnotes.stderr b/tests/ui/doc_suspicious_footnotes.stderr index 9b5a33ccbff1..049862dedfc6 100644 --- a/tests/ui/doc_suspicious_footnotes.stderr +++ b/tests/ui/doc_suspicious_footnotes.stderr @@ -8,10 +8,9 @@ LL | //! This is not a footnote[^1]. = help: to override `-D warnings` add `#[allow(clippy::doc_suspicious_footnotes)]` help: add footnote definition | -LL ~ //! This is not a footnote[^1]. -LL + //! +LL ~ //! [^2]: hello world +LL + //! LL + //! [^1]: -LL + //! | error: looks like a footnote ref, but has no matching footnote @@ -22,10 +21,9 @@ LL | /// This is not a footnote[^1]. | help: add footnote definition | -LL ~ /// This is not a footnote[^1]. -LL + /// +LL ~ /// [^2]: hello world +LL + /// LL + /// [^1]: -LL + /// | error: looks like a footnote ref, but has no matching footnote @@ -36,10 +34,9 @@ LL | #[doc = r#"This is not a footnote[^1]."#] | help: add footnote definition | -LL ~ #[doc = r###"This is not a footnote[^1]. +LL ~ #[doc = r#"[^2]: hello world LL + -LL + [^1]: -LL ~ "###] +LL ~ [^1]: "#] | error: looks like a footnote ref, but has no matching footnote @@ -56,7 +53,7 @@ LL | | "] | help: add footnote definition | -LL ~ #[doc = r###"This is not a footnote[^1]. +LL ~ #[doc = r#"This is not a footnote[^1]. LL + LL + This is not a footnote[^either], but it doesn't warn. LL + @@ -71,8 +68,7 @@ LL + LL + [^2]: hello world LL + LL + -LL + [^1]: -LL ~ "###] +LL ~ [^1]: "#] | error: looks like a footnote ref, but has no matching footnote @@ -83,12 +79,11 @@ LL | doc = "This is not a footnote[^1].\n\nThis is not a footnote[^eithe | help: add footnote definition | -LL ~ doc = r###"This is not a footnote[^1]. +LL ~ doc = r#"This is not a footnote[^1]. LL + LL + This is not a footnote[^either], but it doesn't warn. LL + -LL + [^1]: -LL + "### +LL + [^1]: "# | error: looks like a footnote ref, but has no matching footnote @@ -99,11 +94,34 @@ LL | #[doc = r"This is not a footnote[^1]."] | help: add footnote definition | -LL ~ #[doc = r###"This is not a footnote[^1]. +LL ~ #[doc = r#"[^2]: hello world LL + -LL + [^1]: -LL ~ "###] +LL ~ [^1]: "#] | -error: aborting due to 6 previous errors +error: looks like a footnote ref, but has no matching footnote + --> tests/ui/doc_suspicious_footnotes.rs:103:30 + | +LL | * This is not a footnote[^1]. + | ^^^^ + | +help: add footnote definition + | +LL ~ */ +LL + /*! [^1]: */ + | + +error: looks like a footnote ref, but has no matching footnote + --> tests/ui/doc_suspicious_footnotes.rs:114:30 + | +LL | * This is not a footnote[^1]. + | ^^^^ + | +help: add footnote definition + | +LL ~ */ +LL + /** [^1]: */ + | + +error: aborting due to 8 previous errors diff --git a/tests/ui/doc_suspicious_footnotes_include.stderr b/tests/ui/doc_suspicious_footnotes_include.stderr index cf5ba9b71261..74154e3f4ef3 100644 --- a/tests/ui/doc_suspicious_footnotes_include.stderr +++ b/tests/ui/doc_suspicious_footnotes_include.stderr @@ -8,7 +8,7 @@ LL | This is not a footnote[^1]. = help: to override `-D warnings` add `#[allow(clippy::doc_suspicious_footnotes)]` help: add footnote definition | -LL ~ This is not a footnote[^1]. +LL ~ [^2]: hello world LL + LL + [^1]: |