rust-lang/rust
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

[Perf] Optimize documentation lints **a lot** (1/2) (18% -> 10%) (#14693)

Browse Source 
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%
This commit is contained in:
Jason Newcomb
2025-05-21 22:01:41 +00:00
committed by GitHub
parent 3da4c1033a acff5d36cc
commit a6e40fa9e9
5 changed files with 180 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
clippy_lints/src/doc/markdown.rs
View File

@@ -6,13 +6,15 @@ use rustc_lint::LateContext;
use rustc_span::{BytePos, Pos, Span};
use url::Url;
use crate::doc::DOC_MARKDOWN;
use crate::doc::{DOC_MARKDOWN, Fragments};
use std::ops::Range;
pub fn check(
cx: &LateContext<'_>,
valid_idents: &FxHashSet<String>,
text: &str,
span: Span,
fragments: &Fragments<'_>,
fragment_range: Range<usize>,
code_level: isize,
blockquote_level: isize,
) {
@@ -64,20 +66,31 @@ pub fn check(
close_parens += 1;
}
// Adjust for the current word
let offset = word.as_ptr() as usize - text.as_ptr() as usize;
let span = Span::new(
span.lo() + BytePos::from_usize(offset),
span.lo() + BytePos::from_usize(offset + word.len()),
span.ctxt(),
span.parent(),
);
// We'll use this offset to calculate the span to lint.
let fragment_offset = word.as_ptr() as usize - text.as_ptr() as usize;
check_word(cx, word, span, code_level, blockquote_level);
// Adjust for the current word
check_word(
cx,
word,
fragments,
&fragment_range,
fragment_offset,
code_level,
blockquote_level,
);
}
}
fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, blockquote_level: isize) {
fn check_word(
cx: &LateContext<'_>,
word: &str,
fragments: &Fragments<'_>,
range: &Range<usize>,
fragment_offset: usize,
code_level: isize,
blockquote_level: isize,
) {
/// Checks if a string is upper-camel-case, i.e., starts with an uppercase and
/// contains at least two uppercase letters (`Clippy` is ok) and one lower-case
/// letter (`NASA` is ok).
@@ -117,6 +130,16 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, b
// try to get around the fact that `foo::bar` parses as a valid URL
&& !url.cannot_be_a_base()
{
let Some(fragment_span) = fragments.span(cx, range.clone()) else {
return;
};
let span = Span::new(
fragment_span.lo() + BytePos::from_usize(fragment_offset),
fragment_span.lo() + BytePos::from_usize(fragment_offset + word.len()),
fragment_span.ctxt(),
fragment_span.parent(),
);
span_lint_and_sugg(
cx,
DOC_MARKDOWN,
@@ -137,6 +160,17 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, b
}
if has_underscore(word) || word.contains("::") || is_camel_case(word) || word.ends_with("()") {
let Some(fragment_span) = fragments.span(cx, range.clone()) else {
return;
};
let span = Span::new(
fragment_span.lo() + BytePos::from_usize(fragment_offset),
fragment_span.lo() + BytePos::from_usize(fragment_offset + word.len()),
fragment_span.ctxt(),
fragment_span.parent(),
);
span_lint_and_then(
cx,
DOC_MARKDOWN,

9
clippy_lints/src/doc/mod.rs
View File

@@ -730,7 +730,10 @@ struct Fragments<'a> {
}
impl Fragments<'_> {
fn span(self, cx: &LateContext<'_>, range: Range<usize>) -> Option<Span> {
/// get the span for the markdown range. Note that this function is not cheap, use it with
/// caution.
#[must_use]
fn span(&self, cx: &LateContext<'_>, range: Range<usize>) -> Option<Span> {
source_span_for_markdown_range(cx.tcx, self.doc, &range, self.fragments)
}
}
@@ -1068,9 +1071,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
);
} else {
for (text, range, assoc_code_level) in text_to_check {
if let Some(span) = fragments.span(cx, range) {
markdown::check(cx, valid_idents, &text, span, assoc_code_level, blockquote_level);
}
markdown::check(cx, valid_idents, &text, &fragments, range, assoc_code_level, blockquote_level);
}
}
text_to_check = Vec::new();

34
tests/ui/doc/doc_markdown-issue_13097.fixed
View File

@@ -1,13 +1,39 @@
// This test checks that words starting with capital letters and ending with "ified" don't
// trigger the lint.
#![deny(clippy::doc_markdown)]
#![allow(clippy::doc_lazy_continuation)]
pub enum OutputFormat {
mod issue13097 {
// This test checks that words starting with capital letters and ending with "ified" don't
// trigger the lint.
pub enum OutputFormat {
/// `HumaNified`
//~^ ERROR: item in documentation is missing backticks
Plain,
// Should not warn!
/// JSONified console output
Json,
}
}
#[rustfmt::skip]
pub enum OutputFormat {
/**
* `HumaNified`
//~^ ERROR: item in documentation is missing backticks
* Before \u{08888} `HumaNified` \{u08888} After
//~^ ERROR: item in documentation is missing backticks
* meow meow \[`meow_meow`\] meow meow?
//~^ ERROR: item in documentation is missing backticks
* \u{08888} `meow_meow` \[meow meow] meow?
//~^ ERROR: item in documentation is missing backticks
* Above
* \u{08888}
* \[hi\](<https://example.com>) `HumaNified` \[example](<https://example.com>)
//~^ ERROR: item in documentation is missing backticks
* \u{08888}
* Below
*/
Plain,
// Should not warn!
/// JSONified console output
Json,
}

34
tests/ui/doc/doc_markdown-issue_13097.rs
View File

@@ -1,13 +1,39 @@
// This test checks that words starting with capital letters and ending with "ified" don't
// trigger the lint.
#![deny(clippy::doc_markdown)]
#![allow(clippy::doc_lazy_continuation)]
pub enum OutputFormat {
mod issue13097 {
// This test checks that words starting with capital letters and ending with "ified" don't
// trigger the lint.
pub enum OutputFormat {
/// HumaNified
//~^ ERROR: item in documentation is missing backticks
Plain,
// Should not warn!
/// JSONified console output
Json,
}
}
#[rustfmt::skip]
pub enum OutputFormat {
/**
* HumaNified
//~^ ERROR: item in documentation is missing backticks
* Before \u{08888} HumaNified \{u08888} After
//~^ ERROR: item in documentation is missing backticks
* meow meow \[meow_meow\] meow meow?
//~^ ERROR: item in documentation is missing backticks
* \u{08888} meow_meow \[meow meow] meow?
//~^ ERROR: item in documentation is missing backticks
* Above
* \u{08888}
* \[hi\](<https://example.com>) HumaNified \[example](<https://example.com>)
//~^ ERROR: item in documentation is missing backticks
* \u{08888}
* Below
*/
Plain,
// Should not warn!
/// JSONified console output
Json,
}

66
tests/ui/doc/doc_markdown-issue_13097.stderr
View File

@@ -1,11 +1,11 @@
error: item in documentation is missing backticks
--> tests/ui/doc/doc_markdown-issue_13097.rs:7:9
--> tests/ui/doc/doc_markdown-issue_13097.rs:8:13
|
LL | /// HumaNified
| ^^^^^^^^^^
|
note: the lint level is defined here
--> tests/ui/doc/doc_markdown-issue_13097.rs:4:9
--> tests/ui/doc/doc_markdown-issue_13097.rs:1:9
|
LL | #![deny(clippy::doc_markdown)]
| ^^^^^^^^^^^^^^^^^^^^
@@ -15,5 +15,65 @@ LL - /// HumaNified
LL + /// `HumaNified`
|
error: aborting due to 1 previous error
error: item in documentation is missing backticks
--> tests/ui/doc/doc_markdown-issue_13097.rs:20:8
|
LL | * HumaNified
| ^^^^^^^^^^
|
help: try
|
LL - * HumaNified
LL + * `HumaNified`
|
error: item in documentation is missing backticks
--> tests/ui/doc/doc_markdown-issue_13097.rs:22:25
|
LL | * Before \u{08888} HumaNified \{u08888} After
| ^^^^^^^^^^
|
help: try
|
LL - * Before \u{08888} HumaNified \{u08888} After
LL + * Before \u{08888} `HumaNified` \{u08888} After
|
error: item in documentation is missing backticks
--> tests/ui/doc/doc_markdown-issue_13097.rs:24:20
|
LL | * meow meow \[meow_meow\] meow meow?
| ^^^^^^^^^
|
help: try
|
LL - * meow meow \[meow_meow\] meow meow?
LL + * meow meow \[`meow_meow`\] meow meow?
|
error: item in documentation is missing backticks
--> tests/ui/doc/doc_markdown-issue_13097.rs:26:18
|
LL | * \u{08888} meow_meow \[meow meow] meow?
| ^^^^^^^^^
|
help: try
|
LL - * \u{08888} meow_meow \[meow meow] meow?
LL + * \u{08888} `meow_meow` \[meow meow] meow?
|
error: item in documentation is missing backticks
--> tests/ui/doc/doc_markdown-issue_13097.rs:30:38
|
LL | * \[hi\](<https://example.com>) HumaNified \[example](<https://example.com>)
| ^^^^^^^^^^
|
help: try
|
LL - * \[hi\](<https://example.com>) HumaNified \[example](<https://example.com>)
LL + * \[hi\](<https://example.com>) `HumaNified` \[example](<https://example.com>)
|
error: aborting due to 6 previous errors