Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 30, 2025
+198 −298
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 80 additions & 37 deletions clippy_lints/src/doc/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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! {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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)
Expand All @@ -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() {
Expand Down
203 changes: 53 additions & 150 deletions clippy_lints/src/doc/needless_doctest_main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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");
}
}
}
Loading
Loading