Skip to content

Commit c90c7c4

Browse files
blyxyasblyxyas
authored
doc_suspicious_footnotes: lint text that looks like a footnote (#14708)
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
2 parents a76c2b3 + fcfbcc8 commit c90c7c4

10 files changed

+714
-2
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5736,6 +5736,7 @@ Released 2018-09-13
57365736
[`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
57375737
[`doc_nested_refdefs`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_nested_refdefs
57385738
[`doc_overindented_list_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_overindented_list_items
5739+
[`doc_suspicious_footnotes`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_suspicious_footnotes
57395740
[`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons
57405741
[`double_ended_iterator_last`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_ended_iterator_last
57415742
[`double_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_must_use

‎clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
119119
crate::doc::DOC_MARKDOWN_INFO,
120120
crate::doc::DOC_NESTED_REFDEFS_INFO,
121121
crate::doc::DOC_OVERINDENTED_LIST_ITEMS_INFO,
122+
crate::doc::DOC_SUSPICIOUS_FOOTNOTES_INFO,
122123
crate::doc::EMPTY_DOCS_INFO,
123124
crate::doc::MISSING_ERRORS_DOC_INFO,
124125
crate::doc::MISSING_PANICS_DOC_INFO,

‎clippy_lints/src/doc/doc_suspicious_footnotes.rs

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use rustc_ast::token::CommentKind;
3+
use rustc_errors::Applicability;
4+
use rustc_hir::{AttrStyle, Attribute};
5+
use rustc_lint::{LateContext, LintContext};
6+
use rustc_resolve::rustdoc::DocFragmentKind;
7+
8+
use std::ops::Range;
9+
10+
use super::{DOC_SUSPICIOUS_FOOTNOTES, Fragments};
11+
12+
pub fn check(cx: &LateContext<'_>, doc: &str, range: Range<usize>, fragments: &Fragments<'_>, attrs: &[Attribute]) {
13+
for i in doc[range.clone()]
14+
.bytes()
15+
.enumerate()
16+
.filter_map(|(i, c)| if c == b'[' { Some(i) } else { None })
17+
{
18+
let start = i + range.start;
19+
if doc.as_bytes().get(start + 1) == Some(&b'^')
20+
&& let Some(end) = all_numbers_upto_brace(doc, start + 2)
21+
&& doc.as_bytes().get(end) != Some(&b':')
22+
&& doc.as_bytes().get(start - 1) != Some(&b'\\')
23+
&& let Some(this_fragment) = {
24+
// the `doc` string contains all fragments concatenated together
25+
// figure out which one this suspicious footnote comes from
26+
let mut starting_position = 0;
27+
let mut found_fragment = fragments.fragments.last();
28+
for fragment in fragments.fragments {
29+
if start >= starting_position && start < starting_position + fragment.doc.as_str().len() {
30+
found_fragment = Some(fragment);
31+
break;
32+
}
33+
starting_position += fragment.doc.as_str().len();
34+
}
35+
found_fragment
36+
}
37+
{
38+
let span = fragments.span(cx, start..end).unwrap_or(this_fragment.span);
39+
span_lint_and_then(
40+
cx,
41+
DOC_SUSPICIOUS_FOOTNOTES,
42+
span,
43+
"looks like a footnote ref, but has no matching footnote",
44+
|diag| {
45+
if this_fragment.kind == DocFragmentKind::SugaredDoc {
46+
let (doc_attr, (_, doc_attr_comment_kind)) = attrs
47+
.iter()
48+
.filter(|attr| attr.span().overlaps(this_fragment.span))
49+
.rev()
50+
.find_map(|attr| Some((attr, attr.doc_str_and_comment_kind()?)))
51+
.unwrap();
52+
let (to_add, terminator) = match (doc_attr_comment_kind, doc_attr.style()) {
53+
(CommentKind::Line, AttrStyle::Outer) => ("\n///\n/// ", ""),
54+
(CommentKind::Line, AttrStyle::Inner) => ("\n//!\n//! ", ""),
55+
(CommentKind::Block, AttrStyle::Outer) => ("\n/** ", " */"),
56+
(CommentKind::Block, AttrStyle::Inner) => ("\n/*! ", " */"),
57+
};
58+
diag.span_suggestion_verbose(
59+
doc_attr.span().shrink_to_hi(),
60+
"add footnote definition",
61+
format!(
62+
"{to_add}{label}: <!-- description -->{terminator}",
63+
label = &doc[start..end]
64+
),
65+
Applicability::HasPlaceholders,
66+
);
67+
} else {
68+
let is_file_include = cx
69+
.sess()
70+
.source_map()
71+
.span_to_snippet(this_fragment.span)
72+
.as_ref()
73+
.map(|vdoc| vdoc.trim())
74+
== Ok(doc);
75+
if is_file_include {
76+
// if this is a file include, then there's no quote marks
77+
diag.span_suggestion_verbose(
78+
this_fragment.span.shrink_to_hi(),
79+
"add footnote definition",
80+
format!("\n\n{label}: <!-- description -->", label = &doc[start..end],),
81+
Applicability::HasPlaceholders,
82+
);
83+
} else {
84+
// otherwise, we wrap in a string
85+
diag.span_suggestion_verbose(
86+
this_fragment.span,
87+
"add footnote definition",
88+
format!(
89+
"r#\"{doc}\n\n{label}: <!-- description -->\"#",
90+
doc = this_fragment.doc,
91+
label = &doc[start..end],
92+
),
93+
Applicability::HasPlaceholders,
94+
);
95+
}
96+
}
97+
},
98+
);
99+
}
100+
}
101+
}
102+
103+
fn all_numbers_upto_brace(text: &str, i: usize) -> Option<usize> {
104+
for (j, c) in text.as_bytes()[i..].iter().copied().enumerate().take(64) {
105+
if c == b']' && j != 0 {
106+
return Some(i + j + 1);
107+
}
108+
if !c.is_ascii_digit() || j >= 64 {
109+
break;
110+
}
111+
}
112+
None
113+
}

‎clippy_lints/src/doc/mod.rs

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use std::ops::Range;
2525
use url::Url;
2626

2727
mod doc_comment_double_space_linebreaks;
28+
mod doc_suspicious_footnotes;
2829
mod include_in_doc_without_cfg;
2930
mod lazy_continuation;
3031
mod link_with_quotes;
@@ -607,6 +608,37 @@ declare_clippy_lint! {
607608
"double-space used for doc comment linebreak instead of `\\`"
608609
}
609610

611+
declare_clippy_lint! {
612+
/// ### What it does
613+
/// Detects syntax that looks like a footnote reference.
614+
///
615+
/// Rustdoc footnotes are compatible with GitHub-Flavored Markdown (GFM).
616+
/// GFM does not parse a footnote reference unless its definition also
617+
/// exists. This lint checks for footnote references with missing
618+
/// definitions, unless it thinks you're writing a regex.
619+
///
620+
/// ### Why is this bad?
621+
/// This probably means that a footnote was meant to exist,
622+
/// but was not written.
623+
///
624+
/// ### Example
625+
/// ```no_run
626+
/// /// This is not a footnote[^1], because no definition exists.
627+
/// fn my_fn() {}
628+
/// ```
629+
/// Use instead:
630+
/// ```no_run
631+
/// /// This is a footnote[^1].
632+
/// ///
633+
/// /// [^1]: defined here
634+
/// fn my_fn() {}
635+
/// ```
636+
#[clippy::version = "1.88.0"]
637+
pub DOC_SUSPICIOUS_FOOTNOTES,
638+
suspicious,
639+
"looks like a link or footnote ref, but with no definition"
640+
}
641+
610642
pub struct Documentation {
611643
valid_idents: FxHashSet<String>,
612644
check_private_items: bool,
@@ -638,7 +670,8 @@ impl_lint_pass!(Documentation => [
638670
DOC_OVERINDENTED_LIST_ITEMS,
639671
TOO_LONG_FIRST_DOC_PARAGRAPH,
640672
DOC_INCLUDE_WITHOUT_CFG,
641-
DOC_COMMENT_DOUBLE_SPACE_LINEBREAKS
673+
DOC_COMMENT_DOUBLE_SPACE_LINEBREAKS,
674+
DOC_SUSPICIOUS_FOOTNOTES,
642675
]);
643676

644677
impl EarlyLintPass for Documentation {
@@ -825,6 +858,7 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
825858
doc: &doc,
826859
fragments: &fragments,
827860
},
861+
attrs,
828862
))
829863
}
830864

@@ -905,6 +939,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
905939
events: Events,
906940
doc: &str,
907941
fragments: Fragments<'_>,
942+
attrs: &[Attribute],
908943
) -> DocHeaders {
909944
// true if a safety header was found
910945
let mut headers = DocHeaders::default();
@@ -1148,7 +1183,8 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
11481183
// Don't check the text associated with external URLs
11491184
continue;
11501185
}
1151-
text_to_check.push((text, range, code_level));
1186+
text_to_check.push((text, range.clone(), code_level));
1187+
doc_suspicious_footnotes::check(cx, doc, range, &fragments, attrs);
11521188
}
11531189
}
11541190
FootnoteReference(_) => {}

‎tests/ui/doc_suspicious_footnotes.fixed

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
#![warn(clippy::doc_suspicious_footnotes)]
2+
#![allow(clippy::needless_raw_string_hashes)]
3+
//! This is not a footnote[^1].
4+
//!
5+
//! [^1]: <!-- description -->
6+
//~^ doc_suspicious_footnotes
7+
//!
8+
//! This is not a footnote[^either], but it doesn't warn.
9+
//!
10+
//! This is not a footnote\[^1], but it also doesn't warn.
11+
//!
12+
//! This is not a footnote[^1\], but it also doesn't warn.
13+
//!
14+
//! This is not a `footnote[^1]`, but it also doesn't warn.
15+
//!
16+
//! This is a footnote[^2].
17+
//!
18+
//! [^2]: hello world
19+
20+
/// This is not a footnote[^1].
21+
///
22+
/// [^1]: <!-- description -->
23+
//~^ doc_suspicious_footnotes
24+
///
25+
/// This is not a footnote[^either], but it doesn't warn.
26+
///
27+
/// This is not a footnote\[^1], but it also doesn't warn.
28+
///
29+
/// This is not a footnote[^1\], but it also doesn't warn.
30+
///
31+
/// This is not a `footnote[^1]`, but it also doesn't warn.
32+
///
33+
/// This is a footnote[^2].
34+
///
35+
/// [^2]: hello world
36+
pub fn footnotes() {
37+
// test code goes here
38+
}
39+
40+
pub struct Foo;
41+
#[rustfmt::skip]
42+
impl Foo {
43+
#[doc = r#"This is not a footnote[^1].
44+
45+
[^1]: <!-- description -->"#]
46+
//~^ doc_suspicious_footnotes
47+
#[doc = r#""#]
48+
#[doc = r#"This is not a footnote[^either], but it doesn't warn."#]
49+
#[doc = r#""#]
50+
#[doc = r#"This is not a footnote\[^1], but it also doesn't warn."#]
51+
#[doc = r#""#]
52+
#[doc = r#"This is not a footnote[^1\], but it also doesn't warn."#]
53+
#[doc = r#""#]
54+
#[doc = r#"This is not a `footnote[^1]`, but it also doesn't warn."#]
55+
#[doc = r#""#]
56+
#[doc = r#"This is a footnote[^2]."#]
57+
#[doc = r#""#]
58+
#[doc = r#"[^2]: hello world"#]
59+
pub fn footnotes() {
60+
// test code goes here
61+
}
62+
#[doc = r#"This is not a footnote[^1].
63+
64+
This is not a footnote[^either], but it doesn't warn.
65+
66+
This is not a footnote\[^1], but it also doesn't warn.
67+
68+
This is not a footnote[^1\], but it also doesn't warn.
69+
70+
This is not a `footnote[^1]`, but it also doesn't warn.
71+
72+
This is a footnote[^2].
73+
74+
[^2]: hello world
75+
76+
77+
[^1]: <!-- description -->"#]
78+
//~^^^^^^^^^^^^^^ doc_suspicious_footnotes
79+
pub fn footnotes2() {
80+
// test code goes here
81+
}
82+
#[cfg_attr(
83+
not(FALSE),
84+
doc = r#"This is not a footnote[^1].
85+
86+
This is not a footnote[^either], but it doesn't warn.
87+
88+
[^1]: <!-- description -->"#
89+
//~^ doc_suspicious_footnotes
90+
)]
91+
pub fn footnotes3() {
92+
// test code goes here
93+
}
94+
#[doc = "My footnote [^foot\note]"]
95+
pub fn footnote4() {
96+
// test code goes here
97+
}
98+
#[doc = "Hihi"]pub fn footnote5() {
99+
// test code goes here
100+
}
101+
}
102+
103+
#[doc = r#"This is not a footnote[^1].
104+
105+
[^1]: <!-- description -->"#]
106+
//~^ doc_suspicious_footnotes
107+
#[doc = r""]
108+
#[doc = r"This is not a footnote[^either], but it doesn't warn."]
109+
#[doc = r""]
110+
#[doc = r"This is not a footnote\[^1], but it also doesn't warn."]
111+
#[doc = r""]
112+
#[doc = r"This is not a footnote[^1\], but it also doesn't warn."]
113+
#[doc = r""]
114+
#[doc = r"This is not a `footnote[^1]`, but it also doesn't warn."]
115+
#[doc = r""]
116+
#[doc = r"This is a footnote[^2]."]
117+
#[doc = r""]
118+
#[doc = r"[^2]: hello world"]
119+
pub fn footnotes_attrs() {
120+
// test code goes here
121+
}
122+
123+
pub mod multiline {
124+
/*!
125+
* This is not a footnote[^1]. //~ doc_suspicious_footnotes
126+
*
127+
* This is not a footnote\[^1], but it doesn't warn.
128+
*
129+
* This is a footnote[^2].
130+
*
131+
* These give weird results, but correct ones, so it works.
132+
*
133+
* [^2]: hello world
134+
*/
135+
/*! [^1]: <!-- description --> */
136+
/**
137+
* This is not a footnote[^1]. //~ doc_suspicious_footnotes
138+
*
139+
* This is not a footnote\[^1], but it doesn't warn.
140+
*
141+
* This is a footnote[^2].
142+
*
143+
* These give weird results, but correct ones, so it works.
144+
*
145+
* [^2]: hello world
146+
*/
147+
/** [^1]: <!-- description --> */
148+
pub fn foo() {}
149+
}
150+
151+
/// This is not a footnote [^1]
152+
///
153+
/// [^1]: <!-- description -->
154+
//~^ doc_suspicious_footnotes
155+
///
156+
/// This one is [^2]
157+
///
158+
/// [^2]: contents
159+
#[doc = r#"This is not a footnote [^3]
160+
161+
[^3]: <!-- description -->"#]
162+
//~^ doc_suspicious_footnotes
163+
#[doc = ""]
164+
#[doc = "This one is [^4]"]
165+
#[doc = ""]
166+
#[doc = "[^4]: contents"]
167+
pub struct MultiFragmentFootnote;
168+
169+
#[doc(inline)]
170+
/// This is not a footnote [^5]
171+
///
172+
/// [^5]: <!-- description -->
173+
//~^ doc_suspicious_footnotes
174+
///
175+
/// This one is [^6]
176+
///
177+
/// [^6]: contents
178+
#[doc = r#"This is not a footnote [^7]
179+
180+
[^7]: <!-- description -->"#]
181+
//~^ doc_suspicious_footnotes
182+
#[doc = ""]
183+
#[doc = "This one is [^8]"]
184+
#[doc = ""]
185+
#[doc = "[^8]: contents"]
186+
pub use MultiFragmentFootnote as OtherInlinedFootnote;

0 commit comments

Comments
 (0)