Skip to content

Commit a6e40fa

Browse files
JarchoJarcho
authored
[Perf] Optimize documentation lints **a lot** (1/2) (18% -> 10%) (#14693)
Turns out that `doc_markdown` uses a non-cheap rustdoc function to convert from markdown ranges into source spans. And it was using it a lot (about once every 17 lines of documentation on `tokio`, which ends up being about 2000 times). This ended up being about 18% of the total Clippy runtime as discovered by lintcheck --perf in docs-heavy crates. This PR optimizes one of the cases in which Clippy calls the function, and a future PR once pulldown-cmark/pulldown-cmark#1034 is merged will be opened. This PR lands the use of the function into the single-digit zone. Note that not all crates were affected by this crate equally, those with more docs are affected far more than those light ones. changelog:[`clippy::doc_markdown`] has been optimized by 50%
2 parents 3da4c10 + acff5d3 commit a6e40fa

File tree

5 files changed

+179
-32
lines changed

5 files changed

+179
-32
lines changed

‎clippy_lints/src/doc/markdown.rs

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@ use rustc_lint::LateContext;
66
use rustc_span::{BytePos, Pos, Span};
77
use url::Url;
88

9-
use crate::doc::DOC_MARKDOWN;
9+
use crate::doc::{DOC_MARKDOWN, Fragments};
10+
use std::ops::Range;
1011

1112
pub fn check(
1213
cx: &LateContext<'_>,
1314
valid_idents: &FxHashSet<String>,
1415
text: &str,
15-
span: Span,
16+
fragments: &Fragments<'_>,
17+
fragment_range: Range<usize>,
1618
code_level: isize,
1719
blockquote_level: isize,
1820
) {
@@ -64,20 +66,31 @@ pub fn check(
6466
close_parens += 1;
6567
}
6668

69+
// We'll use this offset to calculate the span to lint.
70+
let fragment_offset = word.as_ptr() as usize - text.as_ptr() as usize;
71+
6772
// Adjust for the current word
68-
let offset = word.as_ptr() as usize - text.as_ptr() as usize;
69-
let span = Span::new(
70-
span.lo() + BytePos::from_usize(offset),
71-
span.lo() + BytePos::from_usize(offset + word.len()),
72-
span.ctxt(),
73-
span.parent(),
73+
check_word(
74+
cx,
75+
word,
76+
fragments,
77+
&fragment_range,
78+
fragment_offset,
79+
code_level,
80+
blockquote_level,
7481
);
75-
76-
check_word(cx, word, span, code_level, blockquote_level);
7782
}
7883
}
7984

80-
fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, blockquote_level: isize) {
85+
fn check_word(
86+
cx: &LateContext<'_>,
87+
word: &str,
88+
fragments: &Fragments<'_>,
89+
range: &Range<usize>,
90+
fragment_offset: usize,
91+
code_level: isize,
92+
blockquote_level: isize,
93+
) {
8194
/// Checks if a string is upper-camel-case, i.e., starts with an uppercase and
8295
/// contains at least two uppercase letters (`Clippy` is ok) and one lower-case
8396
/// letter (`NASA` is ok).
@@ -117,6 +130,16 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, b
117130
// try to get around the fact that `foo::bar` parses as a valid URL
118131
&& !url.cannot_be_a_base()
119132
{
133+
let Some(fragment_span) = fragments.span(cx, range.clone()) else {
134+
return;
135+
};
136+
let span = Span::new(
137+
fragment_span.lo() + BytePos::from_usize(fragment_offset),
138+
fragment_span.lo() + BytePos::from_usize(fragment_offset + word.len()),
139+
fragment_span.ctxt(),
140+
fragment_span.parent(),
141+
);
142+
120143
span_lint_and_sugg(
121144
cx,
122145
DOC_MARKDOWN,
@@ -137,6 +160,17 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, b
137160
}
138161

139162
if has_underscore(word) || word.contains("::") || is_camel_case(word) || word.ends_with("()") {
163+
let Some(fragment_span) = fragments.span(cx, range.clone()) else {
164+
return;
165+
};
166+
167+
let span = Span::new(
168+
fragment_span.lo() + BytePos::from_usize(fragment_offset),
169+
fragment_span.lo() + BytePos::from_usize(fragment_offset + word.len()),
170+
fragment_span.ctxt(),
171+
fragment_span.parent(),
172+
);
173+
140174
span_lint_and_then(
141175
cx,
142176
DOC_MARKDOWN,

‎clippy_lints/src/doc/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,10 @@ struct Fragments<'a> {
730730
}
731731

732732
impl Fragments<'_> {
733-
fn span(self, cx: &LateContext<'_>, range: Range<usize>) -> Option<Span> {
733+
/// get the span for the markdown range. Note that this function is not cheap, use it with
734+
/// caution.
735+
#[must_use]
736+
fn span(&self, cx: &LateContext<'_>, range: Range<usize>) -> Option<Span> {
734737
source_span_for_markdown_range(cx.tcx, self.doc, &range, self.fragments)
735738
}
736739
}
@@ -1068,9 +1071,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
10681071
);
10691072
} else {
10701073
for (text, range, assoc_code_level) in text_to_check {
1071-
if let Some(span) = fragments.span(cx, range) {
1072-
markdown::check(cx, valid_idents, &text, span, assoc_code_level, blockquote_level);
1073-
}
1074+
markdown::check(cx, valid_idents, &text, &fragments, range, assoc_code_level, blockquote_level);
10741075
}
10751076
}
10761077
text_to_check = Vec::new();

‎tests/ui/doc/doc_markdown-issue_13097.fixed

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,37 @@
1-
// This test checks that words starting with capital letters and ending with "ified" don't
2-
// trigger the lint.
3-
41
#![deny(clippy::doc_markdown)]
2+
#![allow(clippy::doc_lazy_continuation)]
3+
4+
mod issue13097 {
5+
// This test checks that words starting with capital letters and ending with "ified" don't
6+
// trigger the lint.
7+
pub enum OutputFormat {
8+
/// `HumaNified`
9+
//~^ ERROR: item in documentation is missing backticks
10+
Plain,
11+
// Should not warn!
12+
/// JSONified console output
13+
Json,
14+
}
15+
}
516

17+
#[rustfmt::skip]
618
pub enum OutputFormat {
7-
/// `HumaNified`
8-
//~^ ERROR: item in documentation is missing backticks
19+
/**
20+
* `HumaNified`
21+
//~^ ERROR: item in documentation is missing backticks
22+
* Before \u{08888} `HumaNified` \{u08888} After
23+
//~^ ERROR: item in documentation is missing backticks
24+
* meow meow \[`meow_meow`\] meow meow?
25+
//~^ ERROR: item in documentation is missing backticks
26+
* \u{08888} `meow_meow` \[meow meow] meow?
27+
//~^ ERROR: item in documentation is missing backticks
28+
* Above
29+
* \u{08888}
30+
* \[hi\](<https://example.com>) `HumaNified` \[example](<https://example.com>)
31+
//~^ ERROR: item in documentation is missing backticks
32+
* \u{08888}
33+
* Below
34+
*/
935
Plain,
1036
// Should not warn!
1137
/// JSONified console output

‎tests/ui/doc/doc_markdown-issue_13097.rs

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,37 @@
1-
// This test checks that words starting with capital letters and ending with "ified" don't
2-
// trigger the lint.
3-
41
#![deny(clippy::doc_markdown)]
2+
#![allow(clippy::doc_lazy_continuation)]
3+
4+
mod issue13097 {
5+
// This test checks that words starting with capital letters and ending with "ified" don't
6+
// trigger the lint.
7+
pub enum OutputFormat {
8+
/// HumaNified
9+
//~^ ERROR: item in documentation is missing backticks
10+
Plain,
11+
// Should not warn!
12+
/// JSONified console output
13+
Json,
14+
}
15+
}
516

17+
#[rustfmt::skip]
618
pub enum OutputFormat {
7-
/// HumaNified
8-
//~^ ERROR: item in documentation is missing backticks
19+
/**
20+
* HumaNified
21+
//~^ ERROR: item in documentation is missing backticks
22+
* Before \u{08888} HumaNified \{u08888} After
23+
//~^ ERROR: item in documentation is missing backticks
24+
* meow meow \[meow_meow\] meow meow?
25+
//~^ ERROR: item in documentation is missing backticks
26+
* \u{08888} meow_meow \[meow meow] meow?
27+
//~^ ERROR: item in documentation is missing backticks
28+
* Above
29+
* \u{08888}
30+
* \[hi\](<https://example.com>) HumaNified \[example](<https://example.com>)
31+
//~^ ERROR: item in documentation is missing backticks
32+
* \u{08888}
33+
* Below
34+
*/
935
Plain,
1036
// Should not warn!
1137
/// JSONified console output

‎tests/ui/doc/doc_markdown-issue_13097.stderr

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,79 @@
11
error: item in documentation is missing backticks
2-
--> tests/ui/doc/doc_markdown-issue_13097.rs:7:9
2+
--> tests/ui/doc/doc_markdown-issue_13097.rs:8:13
33
|
4-
LL | /// HumaNified
5-
| ^^^^^^^^^^
4+
LL | /// HumaNified
5+
| ^^^^^^^^^^
66
|
77
note: the lint level is defined here
8-
--> tests/ui/doc/doc_markdown-issue_13097.rs:4:9
8+
--> tests/ui/doc/doc_markdown-issue_13097.rs:1:9
99
|
1010
LL | #![deny(clippy::doc_markdown)]
1111
| ^^^^^^^^^^^^^^^^^^^^
1212
help: try
1313
|
14-
LL - /// HumaNified
15-
LL + /// `HumaNified`
14+
LL - /// HumaNified
15+
LL + /// `HumaNified`
1616
|
1717

18-
error: aborting due to 1 previous error
18+
error: item in documentation is missing backticks
19+
--> tests/ui/doc/doc_markdown-issue_13097.rs:20:8
20+
|
21+
LL | * HumaNified
22+
| ^^^^^^^^^^
23+
|
24+
help: try
25+
|
26+
LL - * HumaNified
27+
LL + * `HumaNified`
28+
|
29+
30+
error: item in documentation is missing backticks
31+
--> tests/ui/doc/doc_markdown-issue_13097.rs:22:25
32+
|
33+
LL | * Before \u{08888} HumaNified \{u08888} After
34+
| ^^^^^^^^^^
35+
|
36+
help: try
37+
|
38+
LL - * Before \u{08888} HumaNified \{u08888} After
39+
LL + * Before \u{08888} `HumaNified` \{u08888} After
40+
|
41+
42+
error: item in documentation is missing backticks
43+
--> tests/ui/doc/doc_markdown-issue_13097.rs:24:20
44+
|
45+
LL | * meow meow \[meow_meow\] meow meow?
46+
| ^^^^^^^^^
47+
|
48+
help: try
49+
|
50+
LL - * meow meow \[meow_meow\] meow meow?
51+
LL + * meow meow \[`meow_meow`\] meow meow?
52+
|
53+
54+
error: item in documentation is missing backticks
55+
--> tests/ui/doc/doc_markdown-issue_13097.rs:26:18
56+
|
57+
LL | * \u{08888} meow_meow \[meow meow] meow?
58+
| ^^^^^^^^^
59+
|
60+
help: try
61+
|
62+
LL - * \u{08888} meow_meow \[meow meow] meow?
63+
LL + * \u{08888} `meow_meow` \[meow meow] meow?
64+
|
65+
66+
error: item in documentation is missing backticks
67+
--> tests/ui/doc/doc_markdown-issue_13097.rs:30:38
68+
|
69+
LL | * \[hi\](<https://example.com>) HumaNified \[example](<https://example.com>)
70+
| ^^^^^^^^^^
71+
|
72+
help: try
73+
|
74+
LL - * \[hi\](<https://example.com>) HumaNified \[example](<https://example.com>)
75+
LL + * \[hi\](<https://example.com>) `HumaNified` \[example](<https://example.com>)
76+
|
77+
78+
error: aborting due to 6 previous errors
1979

0 commit comments

Comments
 (0)