about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJason Newcomb <jsnewcomb@pm.me>2025-05-21 22:01:41 +0000
committerGitHub <noreply@github.com>2025-05-21 22:01:41 +0000
commita6e40fa9e90f6230fca2bc83c01dfaf02d840e84 (patch)
treeb067eff5643838a6360dd2d54b4e299edceae4e5
parent3da4c1033aa95d0c1a4310240295d49c6ef0d8fc (diff)
parentacff5d36cc4553bfb59a76d41e51020116ce4c6c (diff)
downloadrust-a6e40fa9e90f6230fca2bc83c01dfaf02d840e84.tar.gz
rust-a6e40fa9e90f6230fca2bc83c01dfaf02d840e84.zip
[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%
-rw-r--r--clippy_lints/src/doc/markdown.rs56
-rw-r--r--clippy_lints/src/doc/mod.rs9
-rw-r--r--tests/ui/doc/doc_markdown-issue_13097.fixed36
-rw-r--r--tests/ui/doc/doc_markdown-issue_13097.rs36
-rw-r--r--tests/ui/doc/doc_markdown-issue_13097.stderr74
5 files changed, 179 insertions, 32 deletions
diff --git a/clippy_lints/src/doc/markdown.rs b/clippy_lints/src/doc/markdown.rs
index 7a1c7c675d2..69c3b9150c3 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::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;
         }
 
+        // We'll use this offset to calculate the span to lint.
+        let fragment_offset = word.as_ptr() as usize - text.as_ptr() as usize;
+
         // 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(),
+        check_word(
+            cx,
+            word,
+            fragments,
+            &fragment_range,
+            fragment_offset,
+            code_level,
+            blockquote_level,
         );
-
-        check_word(cx, word, span, 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,
diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs
index 87da380e954..c1491d96ceb 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();
diff --git a/tests/ui/doc/doc_markdown-issue_13097.fixed b/tests/ui/doc/doc_markdown-issue_13097.fixed
index fb0f40b34a4..e0136584f3d 100644
--- a/tests/ui/doc/doc_markdown-issue_13097.fixed
+++ b/tests/ui/doc/doc_markdown-issue_13097.fixed
@@ -1,11 +1,37 @@
-// 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)]
+
+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
+    /**
+     * `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
diff --git a/tests/ui/doc/doc_markdown-issue_13097.rs b/tests/ui/doc/doc_markdown-issue_13097.rs
index 8c1e1a3cd6c..2e89fe6c56b 100644
--- a/tests/ui/doc/doc_markdown-issue_13097.rs
+++ b/tests/ui/doc/doc_markdown-issue_13097.rs
@@ -1,11 +1,37 @@
-// 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)]
+
+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
+    /**
+     * 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
diff --git a/tests/ui/doc/doc_markdown-issue_13097.stderr b/tests/ui/doc/doc_markdown-issue_13097.stderr
index 65b8f2ed80b..cea788301d4 100644
--- a/tests/ui/doc/doc_markdown-issue_13097.stderr
+++ b/tests/ui/doc/doc_markdown-issue_13097.stderr
@@ -1,19 +1,79 @@
 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
-   |         ^^^^^^^^^^
+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)]
    |         ^^^^^^^^^^^^^^^^^^^^
 help: try
    |
-LL -     /// HumaNified
-LL +     /// `HumaNified`
+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