Improve doc comment code language tag parsing, don't use a full parser (#15967)

Now only tokenizes the contents of code blocks instead of spawning a
thread for a full parser

The language tag handling now correctly picks up things like `rust,
ignore`

`no_test` (#10491) is no longer specially handled,
rustdoc considers `rust,no_test` to be a regular test

changelog: [`needless_doctest_main`], [`test_attr_in_doctest`]: now
handles whitespace in language tags

Author: dswij

clippy_lints/src/doc/mod.rs

@@ -22,7 +22,6 @@ use rustc_resolve::rustdoc::{
 };
 use rustc_session::impl_lint_pass;
 use rustc_span::Span;
-use rustc_span::edition::Edition;
 use std::ops::Range;
 use url::Url;
 
@@ -36,6 +35,7 @@ mod markdown;
 mod missing_headers;
 mod needless_doctest_main;
 mod suspicious_doc_comments;
+mod test_attr_in_doctest;
 mod too_long_first_doc_paragraph;
 
 declare_clippy_lint! {
@@ -900,8 +900,6 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
     ))
 }
 
-const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail"];
-
 enum Container {
     Blockquote,
     List(usize),
@@ -966,6 +964,70 @@ fn check_for_code_clusters<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a
     }
 }
 
+#[derive(Clone, Copy)]
+#[expect(clippy::struct_excessive_bools)]
+struct CodeTags {
+    no_run: bool,
+    ignore: bool,
+    compile_fail: bool,
+
+    rust: bool,
+}
+
+impl Default for CodeTags {
+    fn default() -> Self {
+        Self {
+            no_run: false,
+            ignore: false,
+            compile_fail: false,
+
+            rust: true,
+        }
+    }
+}
+
+impl CodeTags {
+    /// Based on <https://github.com/rust-lang/rust/blob/1.90.0/src/librustdoc/html/markdown.rs#L1169>
+    fn parse(lang: &str) -> Self {
+        let mut tags = Self::default();
+
+        let mut seen_rust_tags = false;
+        let mut seen_other_tags = false;
+        for item in lang.split([',', ' ', '\t']) {
+            match item.trim() {
+                "" => {},
+                "rust" => {
+                    tags.rust = true;
+                    seen_rust_tags = true;
+                },
+                "ignore" => {
+                    tags.ignore = true;
+                    seen_rust_tags = !seen_other_tags;
+                },
+                "no_run" => {
+                    tags.no_run = true;
+                    seen_rust_tags = !seen_other_tags;
+                },
+                "should_panic" => seen_rust_tags = !seen_other_tags,
+                "compile_fail" => {
+                    tags.compile_fail = true;
+                    seen_rust_tags = !seen_other_tags || seen_rust_tags;
+                },
+                "test_harness" | "standalone_crate" => {
+                    seen_rust_tags = !seen_other_tags || seen_rust_tags;
+                },
+                _ if item.starts_with("ignore-") => seen_rust_tags = true,
+                _ if item.starts_with("edition") => {},
+                _ => seen_other_tags = true,
+            }
+        }
+
+        tags.rust &= seen_rust_tags || !seen_other_tags;
+
+        tags
+    }
+}
+
 /// Checks parsed documentation.
 /// This walks the "events" (think sections of markdown) produced by `pulldown_cmark`,
 /// so lints here will generally access that information.
@@ -981,14 +1043,10 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
 ) -> DocHeaders {
     // true if a safety header was found
     let mut headers = DocHeaders::default();
-    let mut in_code = false;
+    let mut code = None;
     let mut in_link = None;
     let mut in_heading = false;
     let mut in_footnote_definition = false;
-    let mut is_rust = false;
-    let mut no_test = false;
-    let mut ignore = false;
-    let mut edition = None;
     let mut ticks_unbalanced = false;
     let mut text_to_check: Vec<(CowStr<'_>, Range<usize>, isize)> = Vec::new();
     let mut paragraph_range = 0..0;
@@ -1048,31 +1106,12 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                 containers.pop();
             },
             Start(CodeBlock(ref kind)) => {
-                in_code = true;
-                if let CodeBlockKind::Fenced(lang) = kind {
-                    for item in lang.split(',') {
-                        if item == "ignore" {
-                            is_rust = false;
-                            break;
-                        } else if item == "no_test" {
-                            no_test = true;
-                        } else if item == "no_run" || item == "compile_fail" {
-                            ignore = true;
-                        }
-                        if let Some(stripped) = item.strip_prefix("edition") {
-                            is_rust = true;
-                            edition = stripped.parse::<Edition>().ok();
-                        } else if item.is_empty() || RUST_CODE.contains(&item) {
-                            is_rust = true;
-                        }
-                    }
-                }
-            },
-            End(TagEnd::CodeBlock) => {
-                in_code = false;
-                is_rust = false;
-                ignore = false;
+                code = Some(match kind {
+                    CodeBlockKind::Indented => CodeTags::default(),
+                    CodeBlockKind::Fenced(lang) => CodeTags::parse(lang),
+                });
             },
+            End(TagEnd::CodeBlock) => code = None,
             Start(Link { dest_url, .. }) => in_link = Some(dest_url),
             End(TagEnd::Link) => in_link = None,
             Start(Heading { .. } | Paragraph | Item) => {
@@ -1182,7 +1221,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                 paragraph_range.end = range.end;
                 let range_ = range.clone();
                 ticks_unbalanced |= text.contains('`')
-                    && !in_code
+                    && code.is_none()
                     && doc[range.clone()].bytes().enumerate().any(|(i, c)| {
                         // scan the markdown source code bytes for backquotes that aren't preceded by backslashes
                         // - use bytes, instead of chars, to avoid utf8 decoding overhead (special chars are ascii)
@@ -1205,10 +1244,14 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                 headers.safety |= in_heading && trimmed_text == "Implementation Safety";
                 headers.errors |= in_heading && trimmed_text == "Errors";
                 headers.panics |= in_heading && trimmed_text == "Panics";
-                if in_code {
-                    if is_rust && !no_test {
-                        let edition = edition.unwrap_or_else(|| cx.tcx.sess.edition());
-                        needless_doctest_main::check(cx, &text, edition, range.clone(), fragments, ignore);
+
+                if let Some(tags) = code {
+                    if tags.rust && !tags.compile_fail && !tags.ignore {
+                        needless_doctest_main::check(cx, &text, range.start, fragments);
+
+                        if !tags.no_run {
+                            test_attr_in_doctest::check(cx, &text, range.start, fragments);
+                        }
                     }
                 } else {
                     if in_link.is_some() {

clippy_lints/src/doc/needless_doctest_main.rs

@@ -1,160 +1,63 @@
-use std::ops::Range;
-use std::sync::Arc;
-use std::{io, thread};
-
-use crate::doc::{NEEDLESS_DOCTEST_MAIN, TEST_ATTR_IN_DOCTEST};
+use super::Fragments;
+use crate::doc::NEEDLESS_DOCTEST_MAIN;
 use clippy_utils::diagnostics::span_lint;
-use rustc_ast::{CoroutineKind, Fn, FnRetTy, Item, ItemKind};
-use rustc_errors::emitter::HumanEmitter;
-use rustc_errors::{Diag, DiagCtxt};
+use clippy_utils::tokenize_with_text;
+use rustc_lexer::TokenKind;
 use rustc_lint::LateContext;
-use rustc_parse::lexer::StripTokens;
-use rustc_parse::new_parser_from_source_str;
-use rustc_parse::parser::ForceCollect;
-use rustc_session::parse::ParseSess;
-use rustc_span::edition::Edition;
-use rustc_span::source_map::{FilePathMapping, SourceMap};
-use rustc_span::{FileName, Ident, Pos, sym};
-
-use super::Fragments;
-
-fn get_test_spans(item: &Item, ident: Ident, test_attr_spans: &mut Vec<Range<usize>>) {
-    test_attr_spans.extend(
-        item.attrs
-            .iter()
-            .find(|attr| attr.has_name(sym::test))
-            .map(|attr| attr.span.lo().to_usize()..ident.span.hi().to_usize()),
-    );
-}
-
-pub fn check(
-    cx: &LateContext<'_>,
-    text: &str,
-    edition: Edition,
-    range: Range<usize>,
-    fragments: Fragments<'_>,
-    ignore: bool,
-) {
-    // return whether the code contains a needless `fn main` plus a vector of byte position ranges
-    // of all `#[test]` attributes in not ignored code examples
-    fn check_code_sample(code: String, edition: Edition, ignore: bool) -> (bool, Vec<Range<usize>>) {
-        rustc_driver::catch_fatal_errors(|| {
-            rustc_span::create_session_globals_then(edition, &[], None, || {
-                let mut test_attr_spans = vec![];
-                let filename = FileName::anon_source_code(&code);
-
-                let translator = rustc_driver::default_translator();
-                let emitter = HumanEmitter::new(Box::new(io::sink()), translator);
-                let dcx = DiagCtxt::new(Box::new(emitter)).disable_warnings();
-                #[expect(clippy::arc_with_non_send_sync)] // `Arc` is expected by with_dcx
-                let sm = Arc::new(SourceMap::new(FilePathMapping::empty()));
-                let psess = ParseSess::with_dcx(dcx, sm);
-
-                let mut parser =
-                    match new_parser_from_source_str(&psess, filename, code, StripTokens::ShebangAndFrontmatter) {
-                        Ok(p) => p,
-                        Err(errs) => {
-                            errs.into_iter().for_each(Diag::cancel);
-                            return (false, test_attr_spans);
-                        },
-                    };
-
-                let mut relevant_main_found = false;
-                let mut eligible = true;
-                loop {
-                    match parser.parse_item(ForceCollect::No) {
-                        Ok(Some(item)) => match &item.kind {
-                            ItemKind::Fn(box Fn {
-                                ident,
-                                sig,
-                                body: Some(block),
-                                ..
-                            }) if ident.name == sym::main => {
-                                if !ignore {
-                                    get_test_spans(&item, *ident, &mut test_attr_spans);
-                                }
-
-                                let is_async = matches!(sig.header.coroutine_kind, Some(CoroutineKind::Async { .. }));
-                                let returns_nothing = match &sig.decl.output {
-                                    FnRetTy::Default(..) => true,
-                                    FnRetTy::Ty(ty) if ty.kind.is_unit() => true,
-                                    FnRetTy::Ty(_) => false,
-                                };
-
-                                if returns_nothing && !is_async && !block.stmts.is_empty() {
-                                    // This main function should be linted, but only if there are no other functions
-                                    relevant_main_found = true;
-                                } else {
-                                    // This main function should not be linted, we're done
-                                    eligible = false;
-                                }
-                            },
-                            // Another function was found; this case is ignored for needless_doctest_main
-                            ItemKind::Fn(fn_) => {
-                                eligible = false;
-                                if ignore {
-                                    // If ignore is active invalidating one lint,
-                                    // and we already found another function thus
-                                    // invalidating the other one, we have no
-                                    // business continuing.
-                                    return (false, test_attr_spans);
-                                }
-                                get_test_spans(&item, fn_.ident, &mut test_attr_spans);
-                            },
-                            // Tests with one of these items are ignored
-                            ItemKind::Static(..)
-                            | ItemKind::Const(..)
-                            | ItemKind::ExternCrate(..)
-                            | ItemKind::ForeignMod(..) => {
-                                eligible = false;
-                            },
-                            _ => {},
-                        },
-                        Ok(None) => break,
-                        Err(e) => {
-                            // See issue #15041. When calling `.cancel()` on the `Diag`, Clippy will unexpectedly panic
-                            // when the `Diag` is unwinded. Meanwhile, we can just call `.emit()`, since the `DiagCtxt`
-                            // is just a sink, nothing will be printed.
-                            e.emit();
-                            return (false, test_attr_spans);
-                        },
-                    }
-                }
-
-                (relevant_main_found & eligible, test_attr_spans)
-            })
-        })
-        .ok()
-        .unwrap_or_default()
+use rustc_span::InnerSpan;
+
+fn returns_unit<'a>(mut tokens: impl Iterator<Item = (TokenKind, &'a str, InnerSpan)>) -> bool {
+    let mut next = || tokens.next().map_or(TokenKind::Whitespace, |(kind, ..)| kind);
+
+    match next() {
+        // {
+        TokenKind::OpenBrace => true,
+        // - > ( ) {
+        TokenKind::Minus => {
+            next() == TokenKind::Gt
+                && next() == TokenKind::OpenParen
+                && next() == TokenKind::CloseParen
+                && next() == TokenKind::OpenBrace
+        },
+        _ => false,
     }
+}
 
-    let trailing_whitespace = text.len() - text.trim_end().len();
-
-    // We currently only test for "fn main". Checking for the real
-    // entrypoint (with tcx.entry_fn(())) in each block would be unnecessarily
-    // expensive, as those are probably intended and relevant. Same goes for
-    // macros and other weird ways of declaring a main function.
-    //
-    // Also, as we only check for attribute names and don't do macro expansion,
-    // we can check only for #[test]
-
-    if !((text.contains("main") && text.contains("fn")) || text.contains("#[test]")) {
+pub fn check(cx: &LateContext<'_>, text: &str, offset: usize, fragments: Fragments<'_>) {
+    if !text.contains("main") {
         return;
     }
 
-    // Because of the global session, we need to create a new session in a different thread with
-    // the edition we need.
-    let text = text.to_owned();
-    let (has_main, test_attr_spans) = thread::spawn(move || check_code_sample(text, edition, ignore))
-        .join()
-        .expect("thread::spawn failed");
-    if has_main && let Some(span) = fragments.span(cx, range.start..range.end - trailing_whitespace) {
-        span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest");
-    }
-    for span in test_attr_spans {
-        let span = (range.start + span.start)..(range.start + span.end);
-        if let Some(span) = fragments.span(cx, span) {
-            span_lint(cx, TEST_ATTR_IN_DOCTEST, span, "unit tests in doctest are not executed");
+    let mut tokens = tokenize_with_text(text).filter(|&(kind, ..)| {
+        !matches!(
+            kind,
+            TokenKind::Whitespace | TokenKind::BlockComment { .. } | TokenKind::LineComment { .. }
+        )
+    });
+    if let Some((TokenKind::Ident, "fn", fn_span)) = tokens.next()
+        && let Some((TokenKind::Ident, "main", main_span)) = tokens.next()
+        && let Some((TokenKind::OpenParen, ..)) = tokens.next()
+        && let Some((TokenKind::CloseParen, ..)) = tokens.next()
+        && returns_unit(&mut tokens)
+    {
+        let mut depth = 1;
+        for (kind, ..) in &mut tokens {
+            match kind {
+                TokenKind::OpenBrace => depth += 1,
+                TokenKind::CloseBrace => {
+                    depth -= 1;
+                    if depth == 0 {
+                        break;
+                    }
+                },
+                _ => {},
+            }
+        }
+
+        if tokens.next().is_none()
+            && let Some(span) = fragments.span(cx, fn_span.start + offset..main_span.end + offset)
+        {
+            span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest");
         }
     }
 }