diff options
| author | blyxyas <blyxyas@gmail.com> | 2025-04-25 20:25:51 +0200 |
|---|---|---|
| committer | blyxyas <blyxyas@gmail.com> | 2025-04-25 21:14:37 +0200 |
| commit | 565cf5a89e0d197d52cd428b0d78556eb88fc9a0 (patch) | |
| tree | 9d77a5f7e5890dfc450acb3eccc2c900de137d5e /clippy_lints | |
| parent | 34f81f96e9f43ba5c033f146eabbd5eb0d48638f (diff) | |
| download | rust-565cf5a89e0d197d52cd428b0d78556eb88fc9a0.tar.gz rust-565cf5a89e0d197d52cd428b0d78556eb88fc9a0.zip | |
Optimize documentation lints **a lot** (1/2)
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 18 lines of documentation on `tokio`, which ends up being about 1800 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 issue number 1034 is merged will be open. Note that not all crates were affected by this crate equally, those with more docs are affected far more than those light ones.
Diffstat (limited to 'clippy_lints')
| -rw-r--r-- | clippy_lints/src/doc/markdown.rs | 70 | ||||
| -rw-r--r-- | clippy_lints/src/doc/mod.rs | 9 |
2 files changed, 60 insertions, 19 deletions
diff --git a/clippy_lints/src/doc/markdown.rs b/clippy_lints/src/doc/markdown.rs index 7a1c7c675d2..e40b4e20ea4 100644 --- a/clippy_lints/src/doc/markdown.rs +++ b/clippy_lints/src/doc/markdown.rs @@ -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::{ControlFlow, 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,23 +66,38 @@ 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 + if check_word( + cx, + word, + fragments, + &fragment_range, + fragment_offset, + code_level, + blockquote_level, + ) + .is_break() + { + return; + } } } -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, +) -> ControlFlow<()> { /// 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). + /// letter (`NASA` is ok).[ /// Plurals are also excluded (`IDs` is ok). fn is_camel_case(s: &str) -> bool { if s.starts_with(|c: char| c.is_ascii_digit() | c.is_ascii_lowercase()) { @@ -117,6 +134,17 @@ 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 ControlFlow::Break(()); + }; + + 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, @@ -126,17 +154,28 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, b format!("<{word}>"), Applicability::MachineApplicable, ); - return; + return ControlFlow::Continue(()); } // We assume that mixed-case words are not meant to be put inside backticks. (Issue #2343) // // We also assume that backticks are not necessary if inside a quote. (Issue #10262) if code_level > 0 || blockquote_level > 0 || (has_underscore(word) && has_hyphen(word)) { - return; + return ControlFlow::Break(()); } if has_underscore(word) || word.contains("::") || is_camel_case(word) || word.ends_with("()") { + let Some(fragment_span) = fragments.span(cx, range.clone()) else { + return ControlFlow::Break(()); + }; + + 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, @@ -149,4 +188,5 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, b }, ); } + ControlFlow::Continue(()) } diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index ab77edf1147..5a6c0059e7c 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -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(); |
