Collect and use #![doc(test(attr(..)))] at module level too

Author: Urgau

src/librustdoc/doctest.rs

@@ -45,8 +45,6 @@ pub(crate) struct GlobalTestOptions {
     /// Whether inserting extra indent spaces in code block,
     /// default is `false`, only `true` for generating code link of Rust playground
     pub(crate) insert_indent_space: bool,
-    /// Additional crate-level attributes to add to doctests.
-    pub(crate) attrs: Vec<String>,
     /// Path to file containing arguments for the invocation of rustc.
     pub(crate) args_file: PathBuf,
 }
@@ -396,12 +394,9 @@ fn scrape_test_config(
     attrs: &[hir::Attribute],
     args_file: PathBuf,
 ) -> GlobalTestOptions {
-    use rustc_ast_pretty::pprust;
-
     let mut opts = GlobalTestOptions {
         crate_name,
         no_crate_inject: false,
-        attrs: Vec::new(),
         insert_indent_space: false,
         args_file,
     };
@@ -418,13 +413,7 @@ fn scrape_test_config(
         if attr.has_name(sym::no_crate_inject) {
             opts.no_crate_inject = true;
         }
-        if attr.has_name(sym::attr)
-            && let Some(l) = attr.meta_item_list()
-        {
-            for item in l {
-                opts.attrs.push(pprust::meta_list_item_to_string(item));
-            }
-        }
+        // NOTE: `test(attr(..))` is handled when discovering the individual tests
     }
 
     opts
@@ -872,6 +861,7 @@ pub(crate) struct ScrapedDocTest {
     langstr: LangString,
     text: String,
     name: String,
+    global_crate_attrs: Vec<String>,
 }
 
 impl ScrapedDocTest {
@@ -881,6 +871,7 @@ impl ScrapedDocTest {
         logical_path: Vec<String>,
         langstr: LangString,
         text: String,
+        global_crate_attrs: Vec<String>,
     ) -> Self {
         let mut item_path = logical_path.join("::");
         item_path.retain(|c| c != ' ');
@@ -890,7 +881,7 @@ impl ScrapedDocTest {
         let name =
             format!("{} - {item_path}(line {line})", filename.prefer_remapped_unconditionaly());
 
-        Self { filename, line, langstr, text, name }
+        Self { filename, line, langstr, text, name, global_crate_attrs }
     }
     fn edition(&self, opts: &RustdocOptions) -> Edition {
         self.langstr.edition.unwrap_or(opts.edition)
@@ -974,6 +965,7 @@ impl CreateRunnableDocTests {
             &scraped_test.text,
             Some(&self.opts.crate_name),
             edition,
+            scraped_test.global_crate_attrs.clone(),
             self.can_merge_doctests,
             Some(test_id),
             Some(&scraped_test.langstr),

src/librustdoc/doctest/extracted.rs

@@ -35,12 +35,14 @@ impl ExtractedDocTests {
     ) {
         let edition = scraped_test.edition(options);
 
-        let ScrapedDocTest { filename, line, langstr, text, name } = scraped_test;
+        let ScrapedDocTest { filename, line, langstr, text, name, global_crate_attrs } =
+            scraped_test;
 
         let doctest = DocTestBuilder::new(
             &text,
             Some(&opts.crate_name),
             edition,
+            global_crate_attrs,
             false,
             None,
             Some(&langstr),

src/librustdoc/doctest/make.rs

@@ -41,6 +41,7 @@ pub(crate) struct DocTestBuilder {
     pub(crate) supports_color: bool,
     pub(crate) already_has_extern_crate: bool,
     pub(crate) has_main_fn: bool,
+    pub(crate) global_crate_attrs: Vec<String>,
     pub(crate) crate_attrs: String,
     /// If this is a merged doctest, it will be put into `everything_else`, otherwise it will
     /// put into `crate_attrs`.
@@ -57,6 +58,7 @@ impl DocTestBuilder {
         source: &str,
         crate_name: Option<&str>,
         edition: Edition,
+        global_crate_attrs: Vec<String>,
         can_merge_doctests: bool,
         // If `test_id` is `None`, it means we're generating code for a code example "run" link.
         test_id: Option<String>,
@@ -88,6 +90,7 @@ impl DocTestBuilder {
             // If the AST returned an error, we don't want this doctest to be merged with the
             // others.
             return Self::invalid(
+                Vec::new(),
                 String::new(),
                 String::new(),
                 String::new(),
@@ -104,12 +107,16 @@ impl DocTestBuilder {
         let can_be_merged = can_merge_doctests
             && !has_global_allocator
             && crate_attrs.is_empty()
+            // FIXME: We can probably merge tests which have the same global crate attrs,
+            // like we already do for the edition
+            && global_crate_attrs.is_empty()
             // If this is a merged doctest and a defined macro uses `$crate`, then the path will
             // not work, so better not put it into merged doctests.
             && !(has_macro_def && everything_else.contains("$crate"));
         Self {
             supports_color,
             has_main_fn,
+            global_crate_attrs,
             crate_attrs,
             maybe_crate_attrs,
             crates,
@@ -122,6 +129,7 @@ impl DocTestBuilder {
     }
 
     fn invalid(
+        global_crate_attrs: Vec<String>,
         crate_attrs: String,
         maybe_crate_attrs: String,
         crates: String,
@@ -131,6 +139,7 @@ impl DocTestBuilder {
         Self {
             supports_color: false,
             has_main_fn: false,
+            global_crate_attrs,
             crate_attrs,
             maybe_crate_attrs,
             crates,
@@ -160,7 +169,8 @@ impl DocTestBuilder {
         let mut line_offset = 0;
         let mut prog = String::new();
         let everything_else = self.everything_else.trim();
-        if opts.attrs.is_empty() {
+
+        if self.global_crate_attrs.is_empty() {
             // If there aren't any attributes supplied by #![doc(test(attr(...)))], then allow some
             // lints that are commonly triggered in doctests. The crate-level test attributes are
             // commonly used to make tests fail in case they trigger warnings, so having this there in
@@ -169,8 +179,8 @@ impl DocTestBuilder {
             line_offset += 1;
         }
 
-        // Next, any attributes that came from the crate root via #![doc(test(attr(...)))].
-        for attr in &opts.attrs {
+        // Next, any attributes that came from #![doc(test(attr(...)))].
+        for attr in &self.global_crate_attrs {
             prog.push_str(&format!("#![{attr}]\n"));
             line_offset += 1;
         }

src/librustdoc/doctest/markdown.rs

@@ -24,7 +24,14 @@ impl DocTestVisitor for MdCollector {
         let filename = self.filename.clone();
         // First line of Markdown is line 1.
         let line = 1 + rel_line.offset();
-        self.tests.push(ScrapedDocTest::new(filename, line, self.cur_path.clone(), config, test));
+        self.tests.push(ScrapedDocTest::new(
+            filename,
+            line,
+            self.cur_path.clone(),
+            config,
+            test,
+            Vec::new(),
+        ));
     }
 
     fn visit_header(&mut self, name: &str, level: u32) {
@@ -89,7 +96,6 @@ pub(crate) fn test(input: &Input, options: Options) -> Result<(), String> {
         crate_name,
         no_crate_inject: true,
         insert_indent_space: false,
-        attrs: vec![],
         args_file,
     };
 

src/librustdoc/doctest/runner.rs

@@ -12,6 +12,7 @@ use crate::html::markdown::{Ignore, LangString};
 /// Convenient type to merge compatible doctests into one.
 pub(crate) struct DocTestRunner {
     crate_attrs: FxIndexSet<String>,
+    global_crate_attrs: FxIndexSet<String>,
     ids: String,
     output: String,
     output_merged_tests: String,
@@ -23,6 +24,7 @@ impl DocTestRunner {
     pub(crate) fn new() -> Self {
         Self {
             crate_attrs: FxIndexSet::default(),
+            global_crate_attrs: FxIndexSet::default(),
             ids: String::new(),
             output: String::new(),
             output_merged_tests: String::new(),
@@ -46,6 +48,9 @@ impl DocTestRunner {
             for line in doctest.crate_attrs.split('\n') {
                 self.crate_attrs.insert(line.to_string());
             }
+            for line in &doctest.global_crate_attrs {
+                self.global_crate_attrs.insert(line.to_string());
+            }
         }
         self.ids.push_str(&format!(
             "tests.push({}::TEST);\n",
@@ -85,16 +90,16 @@ impl DocTestRunner {
             code_prefix.push('\n');
         }
 
-        if opts.attrs.is_empty() {
+        if self.global_crate_attrs.is_empty() {
             // If there aren't any attributes supplied by #![doc(test(attr(...)))], then allow some
             // lints that are commonly triggered in doctests. The crate-level test attributes are
             // commonly used to make tests fail in case they trigger warnings, so having this there in
             // that case may cause some tests to pass when they shouldn't have.
             code_prefix.push_str("#![allow(unused)]\n");
         }
 
-        // Next, any attributes that came from the crate root via #![doc(test(attr(...)))].
-        for attr in &opts.attrs {
+        // Next, any attributes that came from #![doc(test(attr(...)))].
+        for attr in &self.global_crate_attrs {
             code_prefix.push_str(&format!("#![{attr}]\n"));
         }
 

src/librustdoc/doctest/rust.rs

@@ -3,14 +3,15 @@
 use std::env;
 use std::sync::Arc;
 
+use rustc_ast_pretty::pprust;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir::def_id::{CRATE_DEF_ID, LocalDefId};
 use rustc_hir::{self as hir, CRATE_HIR_ID, intravisit};
 use rustc_middle::hir::nested_filter;
 use rustc_middle::ty::TyCtxt;
 use rustc_resolve::rustdoc::span_of_fragments;
 use rustc_span::source_map::SourceMap;
-use rustc_span::{BytePos, DUMMY_SP, FileName, Pos, Span};
+use rustc_span::{BytePos, DUMMY_SP, FileName, Pos, Span, sym};
 
 use super::{DocTestVisitor, ScrapedDocTest};
 use crate::clean::{Attributes, extract_cfg_from_attrs};
@@ -21,6 +22,7 @@ struct RustCollector {
     tests: Vec<ScrapedDocTest>,
     cur_path: Vec<String>,
     position: Span,
+    global_crate_attrs: Vec<String>,
 }
 
 impl RustCollector {
@@ -54,6 +56,7 @@ impl DocTestVisitor for RustCollector {
             self.cur_path.clone(),
             config,
             test,
+            self.global_crate_attrs.clone(),
         ));
     }
 
@@ -73,6 +76,7 @@ impl<'tcx> HirCollector<'tcx> {
             cur_path: vec![],
             position: DUMMY_SP,
             tests: vec![],
+            global_crate_attrs: Vec::new(),
         };
         Self { codes, tcx, collector }
     }
@@ -149,6 +153,46 @@ impl<'tcx> intravisit::Visitor<'tcx> for HirCollector<'tcx> {
         self.tcx
     }
 
+    fn visit_mod(&mut self, m: &'tcx hir::Mod<'tcx>, _s: Span, hir_id: hir::HirId) {
+        let attrs = self.tcx.hir_attrs(hir_id);
+
+        if !attrs.is_empty() {
+            // Try collecting `#![doc(test(attr(...)))]` from the attribute module
+            //
+            // We do it in 2 steps since the first `meta_item_list` returns owned
+            // `MetaItemInner` instead of reference to them.
+            let test_attrs: Vec<_> = attrs
+                .iter()
+                .filter(|a| a.has_name(sym::doc))
+                .flat_map(|a| a.meta_item_list().unwrap_or_default())
+                .filter(|a| a.has_name(sym::test))
+                .collect();
+            let additional_global_crate_attrs: Vec<_> = test_attrs
+                .iter()
+                .flat_map(|a| a.meta_item_list().unwrap_or_default())
+                .filter(|a| a.has_name(sym::attr))
+                .flat_map(|a| a.meta_item_list().unwrap_or_default())
+                .map(|i| pprust::meta_list_item_to_string(i))
+                .collect();
+
+            if additional_global_crate_attrs.is_empty() {
+                intravisit::walk_mod(self, m)
+            } else {
+                // Add the additional attributes to the global_crate_attrs vector
+                let old_len = self.collector.global_crate_attrs.len();
+                self.collector.global_crate_attrs.extend(additional_global_crate_attrs);
+
+                let r = intravisit::walk_mod(self, m);
+
+                // Restore global_crate_attrs to it's previous size/content
+                self.collector.global_crate_attrs.truncate(old_len);
+                r
+            }
+        } else {
+            intravisit::walk_mod(self, m)
+        }
+    }
+
     fn visit_item(&mut self, item: &'tcx hir::Item<'_>) {
         let name = match &item.kind {
             hir::ItemKind::Impl(impl_) => {

src/librustdoc/html/markdown.rs

@@ -300,10 +300,10 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
                 crate_name: krate.map(String::from).unwrap_or_default(),
                 no_crate_inject: false,
                 insert_indent_space: true,
-                attrs: vec![],
                 args_file: PathBuf::new(),
             };
-            let doctest = doctest::DocTestBuilder::new(&test, krate, edition, false, None, None);
+            let doctest =
+                doctest::DocTestBuilder::new(&test, krate, edition, Vec::new(), false, None, None);
             let (test, _) = doctest.generate_unique_doctest(&test, false, &opts, krate);
             let channel = if test.contains("#![feature(") { "&amp;version=nightly" } else { "" };
 

tests/rustdoc-ui/doctest/dead-code-module-2.rs

@@ -0,0 +1,27 @@
+// Same test as dead-code-module but with 2 doc(test(attr())) at different levels.
+
+//@ edition: 2024
+//@ compile-flags:--test
+//@ normalize-stdout: "tests/rustdoc-ui/doctest" -> "$$DIR"
+//@ normalize-stdout: "finished in \d+\.\d+s" -> "finished in $$TIME"
+//@ failure-status: 101
+
+#![doc(test(attr(allow(unused_variables))))]
+
+mod my_mod {
+    #![doc(test(attr(deny(warnings))))]
+
+    /// Example
+    ///
+    /// ```rust,no_run
+    /// trait T { fn f(); }
+    /// ```
+    pub fn f() {}
+}
+
+/// Example
+///
+/// ```rust,no_run
+/// trait OnlyWarning { fn no_deny_warnings(); }
+/// ```
+pub fn g() {}

tests/rustdoc-ui/doctest/dead-code-module-2.stdout

@@ -0,0 +1,30 @@
+
+running 2 tests
+test $DIR/dead-code-module-2.rs - my_mod::f (line 16) - compile ... FAILED
+test $DIR/dead-code-module-2.rs - g (line 24) - compile ... ok
+
+failures:
+
+---- $DIR/dead-code-module-2.rs - my_mod::f (line 16) stdout ----
+error: trait `T` is never used
+  --> $DIR/dead-code-module-2.rs:17:7
+   |
+LL | trait T { fn f(); }
+   |       ^
+   |
+note: the lint level is defined here
+  --> $DIR/dead-code-module-2.rs:15:9
+   |
+LL | #![deny(warnings)]
+   |         ^^^^^^^^
+   = note: `#[deny(dead_code)]` implied by `#[deny(warnings)]`
+
+error: aborting due to 1 previous error
+
+Couldn't compile the test.
+
+failures:
+    $DIR/dead-code-module-2.rs - my_mod::f (line 16)
+
+test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
+