Split doctests into two categories: mergeable ones and standalone ones

GuillaumeGomez · GuillaumeGomez · commit 6ae35248358f · 2024-08-13T20:14:53.000+02:00
diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs
@@ -206,7 +206,7 @@ pub(crate) fn run(dcx: DiagCtxtHandle<'_>, options: RustdocOptions) -> Result<()
         test_args,
         nocapture,
         opts,
-        rustdoc_options,
+        &rustdoc_options,
         &unused_extern_reports,
         standalone_tests,
         mergeable_tests,
@@ -259,39 +259,43 @@ pub(crate) fn run_tests(
     mut test_args: Vec<String>,
     nocapture: bool,
     opts: GlobalTestOptions,
-    rustdoc_options: RustdocOptions,
+    rustdoc_options: &Arc<RustdocOptions>,
     unused_extern_reports: &Arc<Mutex<Vec<UnusedExterns>>>,
     mut standalone_tests: Vec<test::TestDescAndFn>,
-    mut mergeable_tests: FxHashMap<Edition, Vec<(DocTest, ScrapedDoctest)>>,
+    mergeable_tests: FxHashMap<Edition, Vec<(DocTest, ScrapedDoctest)>>,
 ) {
     test_args.insert(0, "rustdoctest".to_string());
     if nocapture {
         test_args.push("--nocapture".to_string());
     }
 
     let mut nb_errors = 0;
+    let target_str = rustdoc_options.target.to_string();
 
     for (edition, mut doctests) in mergeable_tests {
         if doctests.is_empty() {
             continue;
         }
         doctests.sort_by(|(_, a), (_, b)| a.name.cmp(&b.name));
-        let outdir = Arc::clone(&doctests[0].outdir);
 
         let mut tests_runner = runner::DocTestRunner::new();
 
         let rustdoc_test_options = IndividualTestOptions::new(
             &rustdoc_options,
-            format!("merged_doctest"),
-            PathBuf::from(r"doctest.rs"),
+            &format!("merged_doctest_{edition}"),
+            PathBuf::from(format!("doctest_{edition}.rs")),
         );
 
         for (doctest, scraped_test) in &doctests {
-            tests_runner.add_test(doctest, scraped_test);
+            tests_runner.add_test(doctest, scraped_test, &target_str);
         }
-        if let Ok(success) =
-            tests_runner.run_tests(rustdoc_test_options, edition, &opts, &test_args, &outdir)
-        {
+        if let Ok(success) = tests_runner.run_tests(
+            rustdoc_test_options,
+            edition,
+            &opts,
+            &test_args,
+            rustdoc_options,
+        ) {
             if !success {
                 nb_errors += 1;
             }
@@ -311,7 +315,7 @@ pub(crate) fn run_tests(
                     doctest,
                     scraped_test,
                     opts.clone(),
-                    rustdoc_test_options.clone(),
+                    Arc::clone(&rustdoc_options),
                     unused_extern_reports.clone(),
                 ));
             }
@@ -406,7 +410,7 @@ impl DirState {
 // We could unify this struct the one in rustc but they have different
 // ownership semantics, so doing so would create wasteful allocations.
 #[derive(serde::Serialize, serde::Deserialize)]
-struct UnusedExterns {
+pub(crate) struct UnusedExterns {
     /// Lint level of the unused_crate_dependencies lint
     lint_level: String,
     /// List of unused externs by their names.
@@ -642,12 +646,11 @@ fn make_maybe_absolute_path(path: PathBuf) -> PathBuf {
 }
 struct IndividualTestOptions {
     outdir: DirState,
-    test_id: String,
     path: PathBuf,
 }
 
 impl IndividualTestOptions {
-    fn new(options: &RustdocOptions, test_id: String, test_path: PathBuf) -> Self {
+    fn new(options: &RustdocOptions, test_id: &str, test_path: PathBuf) -> Self {
         let outdir = if let Some(ref path) = options.persist_doctests {
             let mut path = path.clone();
             path.push(&test_id);
@@ -662,15 +665,14 @@ impl IndividualTestOptions {
             DirState::Temp(get_doctest_dir().expect("rustdoc needs a tempdir"))
         };
 
-        Self { outdir, test_id, path: test_path }
+        Self { outdir, path: test_path }
     }
 }
 
 /// A doctest scraped from the code, ready to be turned into a runnable test.
-struct ScrapedDoctest {
+pub(crate) struct ScrapedDoctest {
     filename: FileName,
     line: usize,
-    logical_path: Vec<String>,
     langstr: LangString,
     text: String,
     name: String,
@@ -692,7 +694,7 @@ impl ScrapedDoctest {
         let name =
             format!("{} - {item_path}(line {line})", filename.prefer_remapped_unconditionaly());
 
-        Self { filename, line, logical_path, langstr, text, name }
+        Self { filename, line, langstr, text, name }
     }
     fn edition(&self, opts: &RustdocOptions) -> Edition {
         self.langstr.edition.unwrap_or(opts.edition)
@@ -701,6 +703,19 @@ impl ScrapedDoctest {
     fn no_run(&self, opts: &RustdocOptions) -> bool {
         self.langstr.no_run || opts.no_run
     }
+    fn path(&self) -> PathBuf {
+        match &self.filename {
+            FileName::Real(path) => {
+                if let Some(local_path) = path.local_path() {
+                    local_path.to_path_buf()
+                } else {
+                    // Somehow we got the filename from the metadata of another crate, should never happen
+                    unreachable!("doctest from a different crate");
+                }
+            }
+            _ => PathBuf::from(r"doctest.rs"),
+        }
+    }
 }
 
 pub(crate) trait DoctestVisitor {
@@ -757,7 +772,7 @@ impl CreateRunnableDoctests {
 
         let edition = scraped_test.edition(&self.rustdoc_options);
         let doctest =
-            DocTest::new(&scraped_test.text, Some(&self.opts.crate_name), edition, test_id);
+            DocTest::new(&scraped_test.text, Some(&self.opts.crate_name), edition, Some(test_id));
         let is_standalone = scraped_test.langstr.compile_fail
             || scraped_test.langstr.test_harness
             || self.rustdoc_options.nocapture
@@ -784,7 +799,7 @@ impl CreateRunnableDoctests {
             test,
             scraped_test,
             self.opts.clone(),
-            self.rustdoc_options.clone(),
+            Arc::clone(&self.rustdoc_options),
             self.unused_extern_reports.clone(),
         )
     }
@@ -794,32 +809,20 @@ fn generate_test_desc_and_fn(
     test: DocTest,
     scraped_test: ScrapedDoctest,
     opts: GlobalTestOptions,
-    rustdoc_options: IndividualTestOptions,
+    rustdoc_options: Arc<RustdocOptions>,
     unused_externs: Arc<Mutex<Vec<UnusedExterns>>>,
 ) -> test::TestDescAndFn {
     let target_str = rustdoc_options.target.to_string();
+    let rustdoc_test_options = IndividualTestOptions::new(
+        &rustdoc_options,
+        test.test_id.as_deref().unwrap_or_else(|| "<doctest>"),
+        scraped_test.path(),
+    );
 
-    let path = match &scraped_test.filename {
-        FileName::Real(path) => {
-            if let Some(local_path) = path.local_path() {
-                local_path.to_path_buf()
-            } else {
-                // Somehow we got the filename from the metadata of another crate, should never happen
-                unreachable!("doctest from a different crate");
-            }
-        }
-        _ => PathBuf::from(r"doctest.rs"),
-    };
-
-    let name = &test.name;
-    let rustdoc_test_options =
-        IndividualTestOptions::new(&rustdoc_options, test.test_id.clone(), path);
-    // let rustdoc_options_clone = rustdoc_options.clone();
-
-    debug!("creating test {name}: {}", scraped_test.text);
+    debug!("creating test {}: {}", scraped_test.name, scraped_test.text);
     test::TestDescAndFn {
         desc: test::TestDesc {
-            name: test::DynTestName(name),
+            name: test::DynTestName(scraped_test.name.clone()),
             ignore: match scraped_test.langstr.ignore {
                 Ignore::All => true,
                 Ignore::None => false,
diff --git a/src/librustdoc/doctest/markdown.rs b/src/librustdoc/doctest/markdown.rs
@@ -1,6 +1,7 @@
 //! Doctest functionality used only for doctests in `.md` Markdown files.
 
 use std::fs::read_to_string;
+use std::sync::{Arc, Mutex};
 
 use rustc_span::FileName;
 use tempfile::tempdir;
@@ -114,6 +115,16 @@ pub(crate) fn test(options: Options) -> Result<(), String> {
 
     let mut collector = CreateRunnableDoctests::new(options.clone(), opts);
     md_collector.tests.into_iter().for_each(|t| collector.add_test(t));
-    crate::doctest::run_tests(options.test_args, options.nocapture, collector.standalone_tests);
+    let CreateRunnableDoctests { opts, rustdoc_options, standalone_tests, mergeable_tests, .. } =
+        collector;
+    crate::doctest::run_tests(
+        options.test_args,
+        options.nocapture,
+        opts,
+        &rustdoc_options,
+        &Arc::new(Mutex::new(Vec::new())),
+        standalone_tests,
+        mergeable_tests,
+    );
     Ok(())
 }
diff --git a/src/librustdoc/doctest/runner.rs b/src/librustdoc/doctest/runner.rs
@@ -2,13 +2,12 @@ use rustc_data_structures::fx::FxHashSet;
 use rustc_span::edition::Edition;
 
 use std::fmt::Write;
-use std::sync::{Arc, Mutex};
 
 use crate::doctest::{
-    run_test, DirState, DocTest, GlobalTestOptions, IndividualTestOptions, RunnableDoctest,
-    RustdocOptions, ScrapedDoctest, TestFailure, UnusedExterns,
+    run_test, DocTest, GlobalTestOptions, IndividualTestOptions, RunnableDoctest, RustdocOptions,
+    ScrapedDoctest, TestFailure, UnusedExterns,
 };
-use crate::html::markdown::LangString;
+use crate::html::markdown::{Ignore, LangString};
 
 /// Convenient type to merge compatible doctests into one.
 pub(crate) struct DocTestRunner {
@@ -17,7 +16,6 @@ pub(crate) struct DocTestRunner {
     output: String,
     supports_color: bool,
     nb_tests: usize,
-    doctests: Vec<DocTest>,
 }
 
 impl DocTestRunner {
@@ -28,12 +26,21 @@ impl DocTestRunner {
             output: String::new(),
             supports_color: true,
             nb_tests: 0,
-            doctests: Vec::with_capacity(10),
         }
     }
 
-    pub(crate) fn add_test(&mut self, doctest: &DocTest, scraped_test: &ScrapedDoctest) {
-        if !doctest.ignore {
+    pub(crate) fn add_test(
+        &mut self,
+        doctest: &DocTest,
+        scraped_test: &ScrapedDoctest,
+        target_str: &str,
+    ) {
+        let ignore = match scraped_test.langstr.ignore {
+            Ignore::All => true,
+            Ignore::None => false,
+            Ignore::Some(ref ignores) => ignores.iter().any(|s| target_str.contains(s)),
+        };
+        if !ignore {
             for line in doctest.crate_attrs.split('\n') {
                 self.crate_attrs.insert(line.to_string());
             }
@@ -43,11 +50,16 @@ impl DocTestRunner {
         }
         self.ids.push_str(&format!(
             "{}::TEST",
-            generate_mergeable_doctest(doctest, scraped_test, self.nb_tests, &mut self.output),
+            generate_mergeable_doctest(
+                doctest,
+                scraped_test,
+                ignore,
+                self.nb_tests,
+                &mut self.output
+            ),
         ));
         self.supports_color &= doctest.supports_color;
         self.nb_tests += 1;
-        self.doctests.push(doctest);
     }
 
     pub(crate) fn run_tests(
@@ -56,9 +68,7 @@ impl DocTestRunner {
         edition: Edition,
         opts: &GlobalTestOptions,
         test_args: &[String],
-        outdir: &Arc<DirState>,
         rustdoc_options: &RustdocOptions,
-        unused_externs: Arc<Mutex<Vec<UnusedExterns>>>,
     ) -> Result<bool, ()> {
         let mut code = "\
 #![allow(unused_extern_crates)]
@@ -73,7 +83,19 @@ impl DocTestRunner {
             code.push('\n');
         }
 
-        DocTest::push_attrs(&mut code, opts, &mut 0);
+        if opts.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.push_str("#![allow(unused)]\n");
+        }
+
+        // Next, any attributes that came from the crate root via #![doc(test(attr(...)))].
+        for attr in &opts.attrs {
+            code.push_str(&format!("#![{attr}]\n"));
+        }
+
         code.push_str("extern crate test;\n");
 
         let test_args =
@@ -91,7 +113,6 @@ test::test_main(&[{test_args}], vec![{ids}], None);
             ids = self.ids,
         )
         .expect("failed to generate test code");
-        // let out_dir = build_test_dir(outdir, true, "");
         let runnable_test = RunnableDoctest {
             full_test_code: code,
             full_test_line_offset: 0,
@@ -102,7 +123,8 @@ test::test_main(&[{test_args}], vec![{ids}], None);
             edition,
             no_run: false,
         };
-        let ret = run_test(runnable_test, rustdoc_options, self.supports_color, unused_externs);
+        let ret =
+            run_test(runnable_test, rustdoc_options, self.supports_color, |_: UnusedExterns| {});
         if let Err(TestFailure::CompileError) = ret { Err(()) } else { Ok(ret.is_ok()) }
     }
 }
@@ -111,12 +133,13 @@ test::test_main(&[{test_args}], vec![{ids}], None);
 fn generate_mergeable_doctest(
     doctest: &DocTest,
     scraped_test: &ScrapedDoctest,
+    ignore: bool,
     id: usize,
     output: &mut String,
 ) -> String {
     let test_id = format!("__doctest_{id}");
 
-    if doctest.ignore {
+    if ignore {
         // We generate nothing else.
         writeln!(output, "mod {test_id} {{\n").unwrap();
     } else {
@@ -166,8 +189,7 @@ pub const TEST: test::TestDescAndFn = test::TestDescAndFn {{
 }};
 }}",
         test_name = scraped_test.name,
-        ignore = scraped_test.langstr.ignore,
-        file = scraped_test.file,
+        file = scraped_test.path(),
         line = scraped_test.line,
         no_run = scraped_test.langstr.no_run,
         should_panic = if !scraped_test.langstr.no_run && scraped_test.langstr.should_panic {
@@ -177,7 +199,7 @@ pub const TEST: test::TestDescAndFn = test::TestDescAndFn {{
         },
         // Setting `no_run` to `true` in `TestDesc` still makes the test run, so we simply
         // don't give it the function to run.
-        runner = if scraped_test.langstr.no_run || scraped_test.langstr.ignore {
+        runner = if ignore || scraped_test.langstr.no_run {
             "Ok::<(), String>(())"
         } else {
             "self::main()"
diff --git a/src/librustdoc/doctest/tests.rs b/src/librustdoc/doctest/tests.rs
@@ -10,9 +10,10 @@ fn make_test(
     opts: &GlobalTestOptions,
     test_id: Option<&str>,
 ) -> (String, usize) {
-    let doctest = DocTest::new(test_code, crate_name, DEFAULT_EDITION);
+    let doctest =
+        DocTest::new(test_code, crate_name, DEFAULT_EDITION, test_id.map(|s| s.to_string()));
     let (code, line_offset) =
-        doctest.generate_unique_doctest(test_code, dont_insert_main, opts, test_id, crate_name);
+        doctest.generate_unique_doctest(test_code, dont_insert_main, opts, crate_name);
     (code, line_offset)
 }
 
