From 3ee462944689cd1569240080f6c6da57453025d2 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Thu, 30 May 2024 21:05:12 -0700 Subject: [PATCH 01/17] rustdoc: Rename `Tester` to `DoctestVisitor` The new name more accurately captures what it is. --- src/librustdoc/doctest.rs | 16 ++++++++-------- src/librustdoc/html/markdown.rs | 8 ++++---- .../passes/check_doc_test_visibility.rs | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 0d4bad6921db9..f6f2b0439133d 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -983,12 +983,12 @@ impl IndividualTestOptions { } } -pub(crate) trait Tester { - fn add_test(&mut self, test: String, config: LangString, line: usize); +pub(crate) trait DoctestVisitor { + fn visit_test(&mut self, test: String, config: LangString, line: usize); fn get_line(&self) -> usize { 0 } - fn register_header(&mut self, _name: &str, _level: u32) {} + fn visit_header(&mut self, _name: &str, _level: u32) {} } pub(crate) struct Collector { @@ -1091,8 +1091,8 @@ impl Collector { } } -impl Tester for Collector { - fn add_test(&mut self, test: String, config: LangString, line: usize) { +impl DoctestVisitor for Collector { + fn visit_test(&mut self, test: String, config: LangString, line: usize) { let filename = self.get_filename(); let name = self.generate_name(line, &filename); let crate_name = self.crate_name.clone(); @@ -1242,7 +1242,7 @@ impl Tester for Collector { } } - fn register_header(&mut self, name: &str, level: u32) { + fn visit_header(&mut self, name: &str, level: u32) { if self.use_headers { // We use these headings as test names, so it's good if // they're valid identifiers. @@ -1287,8 +1287,8 @@ impl Tester for Collector { } #[cfg(test)] // used in tests -impl Tester for Vec { - fn add_test(&mut self, _test: String, _config: LangString, line: usize) { +impl DoctestVisitor for Vec { + fn visit_test(&mut self, _test: String, _config: LangString, line: usize) { self.push(line); } } diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 11cc81700ff58..c8973679132c5 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -710,7 +710,7 @@ impl<'a, I: Iterator>> Iterator for Footnotes<'a, I> { } } -pub(crate) fn find_testable_code( +pub(crate) fn find_testable_code( doc: &str, tests: &mut T, error_codes: ErrorCodes, @@ -720,7 +720,7 @@ pub(crate) fn find_testable_code( find_codes(doc, tests, error_codes, enable_per_target_ignores, extra_info, false) } -pub(crate) fn find_codes( +pub(crate) fn find_codes( doc: &str, tests: &mut T, error_codes: ErrorCodes, @@ -773,7 +773,7 @@ pub(crate) fn find_codes( nb_lines -= 1; } let line = tests.get_line() + nb_lines + 1; - tests.add_test(text, block_info, line); + tests.visit_test(text, block_info, line); prev_offset = offset.start; } Event::Start(Tag::Heading(level, _, _)) => { @@ -781,7 +781,7 @@ pub(crate) fn find_codes( } Event::Text(ref s) if register_header.is_some() => { let level = register_header.unwrap(); - tests.register_header(s, level); + tests.visit_header(s, level); register_header = None; } _ => {} diff --git a/src/librustdoc/passes/check_doc_test_visibility.rs b/src/librustdoc/passes/check_doc_test_visibility.rs index d53eac0bccb1f..a4cdb488f05f8 100644 --- a/src/librustdoc/passes/check_doc_test_visibility.rs +++ b/src/librustdoc/passes/check_doc_test_visibility.rs @@ -44,8 +44,8 @@ pub(crate) struct Tests { pub(crate) found_tests: usize, } -impl crate::doctest::Tester for Tests { - fn add_test(&mut self, _: String, config: LangString, _: usize) { +impl crate::doctest::DoctestVisitor for Tests { + fn visit_test(&mut self, _: String, config: LangString, _: usize) { if config.rust && config.ignore == Ignore::None { self.found_tests += 1; } From f9e12ef946808f470f9d1cd97387a9381d13ad83 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Thu, 30 May 2024 22:10:14 -0700 Subject: [PATCH 02/17] rustdoc: Use `write_all` to ensure all content is written --- src/librustdoc/doctest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index f6f2b0439133d..08bc0f0008453 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -80,7 +80,7 @@ pub(crate) fn generate_args_file(file_path: &Path, options: &RustdocOptions) -> let content = content.join("\n"); - file.write(content.as_bytes()) + file.write_all(content.as_bytes()) .map_err(|error| format!("failed to write arguments to temporary file: {error:?}"))?; Ok(()) } From 516010bd0f7ba82f29fb1cf56fdc990c9386f39c Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Thu, 30 May 2024 22:36:52 -0700 Subject: [PATCH 03/17] Start moving format-specific code into doctest submodule --- src/librustdoc/doctest.rs | 129 ++--------------------------- src/librustdoc/doctest/markdown.rs | 1 + src/librustdoc/doctest/rust.rs | 126 ++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 120 deletions(-) create mode 100644 src/librustdoc/doctest/markdown.rs create mode 100644 src/librustdoc/doctest/rust.rs diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 08bc0f0008453..58f65450e69c7 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -1,20 +1,19 @@ +mod markdown; +mod rust; + use rustc_ast as ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::Lrc; use rustc_errors::emitter::stderr_destination; use rustc_errors::{ColorConfig, ErrorGuaranteed, FatalError}; -use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID, LOCAL_CRATE}; -use rustc_hir::{self as hir, intravisit, CRATE_HIR_ID}; +use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE}; +use rustc_hir::CRATE_HIR_ID; use rustc_interface::interface; -use rustc_middle::hir::map::Map; -use rustc_middle::hir::nested_filter; -use rustc_middle::ty::TyCtxt; use rustc_parse::new_parser_from_source_str; use rustc_parse::parser::attr::InnerAttrPolicy; -use rustc_resolve::rustdoc::span_of_fragments; use rustc_session::config::{self, CrateType, ErrorOutputType}; +use rustc_session::lint; use rustc_session::parse::ParseSess; -use rustc_session::{lint, Session}; use rustc_span::edition::Edition; use rustc_span::source_map::SourceMap; use rustc_span::symbol::sym; @@ -33,11 +32,12 @@ use std::sync::{Arc, Mutex}; use tempfile::{Builder as TempFileBuilder, TempDir}; -use crate::clean::{types::AttributesExt, Attributes}; use crate::config::Options as RustdocOptions; -use crate::html::markdown::{self, ErrorCodes, Ignore, LangString}; +use crate::html::markdown::{ErrorCodes, Ignore, LangString}; use crate::lint::init_lints; +use self::rust::HirCollector; + /// Options that apply to all doctests in a crate or Markdown file (for `rustdoc foo.md`). #[derive(Clone, Default)] pub(crate) struct GlobalTestOptions { @@ -1293,116 +1293,5 @@ impl DoctestVisitor for Vec { } } -struct HirCollector<'a, 'hir, 'tcx> { - sess: &'a Session, - collector: &'a mut Collector, - map: Map<'hir>, - codes: ErrorCodes, - tcx: TyCtxt<'tcx>, -} - -impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> { - fn visit_testable( - &mut self, - name: String, - def_id: LocalDefId, - sp: Span, - nested: F, - ) { - let ast_attrs = self.tcx.hir().attrs(self.tcx.local_def_id_to_hir_id(def_id)); - if let Some(ref cfg) = ast_attrs.cfg(self.tcx, &FxHashSet::default()) { - if !cfg.matches(&self.sess.psess, Some(self.tcx.features())) { - return; - } - } - - let has_name = !name.is_empty(); - if has_name { - self.collector.names.push(name); - } - - // The collapse-docs pass won't combine sugared/raw doc attributes, or included files with - // anything else, this will combine them for us. - let attrs = Attributes::from_ast(ast_attrs); - if let Some(doc) = attrs.opt_doc_value() { - // Use the outermost invocation, so that doctest names come from where the docs were written. - let span = ast_attrs - .iter() - .find(|attr| attr.doc_str().is_some()) - .map(|attr| attr.span.ctxt().outer_expn().expansion_cause().unwrap_or(attr.span)) - .unwrap_or(DUMMY_SP); - self.collector.set_position(span); - markdown::find_testable_code( - &doc, - self.collector, - self.codes, - self.collector.enable_per_target_ignores, - Some(&crate::html::markdown::ExtraInfo::new( - self.tcx, - def_id.to_def_id(), - span_of_fragments(&attrs.doc_strings).unwrap_or(sp), - )), - ); - } - - nested(self); - - if has_name { - self.collector.names.pop(); - } - } -} - -impl<'a, 'hir, 'tcx> intravisit::Visitor<'hir> for HirCollector<'a, 'hir, 'tcx> { - type NestedFilter = nested_filter::All; - - fn nested_visit_map(&mut self) -> Self::Map { - self.map - } - - fn visit_item(&mut self, item: &'hir hir::Item<'_>) { - let name = match &item.kind { - hir::ItemKind::Impl(impl_) => { - rustc_hir_pretty::id_to_string(&self.map, impl_.self_ty.hir_id) - } - _ => item.ident.to_string(), - }; - - self.visit_testable(name, item.owner_id.def_id, item.span, |this| { - intravisit::walk_item(this, item); - }); - } - - fn visit_trait_item(&mut self, item: &'hir hir::TraitItem<'_>) { - self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| { - intravisit::walk_trait_item(this, item); - }); - } - - fn visit_impl_item(&mut self, item: &'hir hir::ImplItem<'_>) { - self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| { - intravisit::walk_impl_item(this, item); - }); - } - - fn visit_foreign_item(&mut self, item: &'hir hir::ForeignItem<'_>) { - self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| { - intravisit::walk_foreign_item(this, item); - }); - } - - fn visit_variant(&mut self, v: &'hir hir::Variant<'_>) { - self.visit_testable(v.ident.to_string(), v.def_id, v.span, |this| { - intravisit::walk_variant(this, v); - }); - } - - fn visit_field_def(&mut self, f: &'hir hir::FieldDef<'_>) { - self.visit_testable(f.ident.to_string(), f.def_id, f.span, |this| { - intravisit::walk_field_def(this, f); - }); - } -} - #[cfg(test)] mod tests; diff --git a/src/librustdoc/doctest/markdown.rs b/src/librustdoc/doctest/markdown.rs new file mode 100644 index 0000000000000..a55b2941b87f8 --- /dev/null +++ b/src/librustdoc/doctest/markdown.rs @@ -0,0 +1 @@ +//! Doctest functionality used only for doctests in `.md` Markdown files. diff --git a/src/librustdoc/doctest/rust.rs b/src/librustdoc/doctest/rust.rs new file mode 100644 index 0000000000000..e4527de77a023 --- /dev/null +++ b/src/librustdoc/doctest/rust.rs @@ -0,0 +1,126 @@ +//! Doctest functionality used only for doctests in `.rs` source files. + +use rustc_data_structures::fx::FxHashSet; +use rustc_hir::def_id::LocalDefId; +use rustc_hir::{self as hir, intravisit}; +use rustc_middle::hir::map::Map; +use rustc_middle::hir::nested_filter; +use rustc_middle::ty::TyCtxt; +use rustc_resolve::rustdoc::span_of_fragments; +use rustc_session::Session; +use rustc_span::{Span, DUMMY_SP}; + +use super::Collector; +use crate::clean::{types::AttributesExt, Attributes}; +use crate::html::markdown::{self, ErrorCodes}; + +pub(super) struct HirCollector<'a, 'hir, 'tcx> { + pub(super) sess: &'a Session, + pub(super) collector: &'a mut Collector, + pub(super) map: Map<'hir>, + pub(super) codes: ErrorCodes, + pub(super) tcx: TyCtxt<'tcx>, +} + +impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> { + pub(super) fn visit_testable( + &mut self, + name: String, + def_id: LocalDefId, + sp: Span, + nested: F, + ) { + let ast_attrs = self.tcx.hir().attrs(self.tcx.local_def_id_to_hir_id(def_id)); + if let Some(ref cfg) = ast_attrs.cfg(self.tcx, &FxHashSet::default()) { + if !cfg.matches(&self.sess.psess, Some(self.tcx.features())) { + return; + } + } + + let has_name = !name.is_empty(); + if has_name { + self.collector.names.push(name); + } + + // The collapse-docs pass won't combine sugared/raw doc attributes, or included files with + // anything else, this will combine them for us. + let attrs = Attributes::from_ast(ast_attrs); + if let Some(doc) = attrs.opt_doc_value() { + // Use the outermost invocation, so that doctest names come from where the docs were written. + let span = ast_attrs + .iter() + .find(|attr| attr.doc_str().is_some()) + .map(|attr| attr.span.ctxt().outer_expn().expansion_cause().unwrap_or(attr.span)) + .unwrap_or(DUMMY_SP); + self.collector.set_position(span); + markdown::find_testable_code( + &doc, + self.collector, + self.codes, + self.collector.enable_per_target_ignores, + Some(&crate::html::markdown::ExtraInfo::new( + self.tcx, + def_id.to_def_id(), + span_of_fragments(&attrs.doc_strings).unwrap_or(sp), + )), + ); + } + + nested(self); + + if has_name { + self.collector.names.pop(); + } + } +} + +impl<'a, 'hir, 'tcx> intravisit::Visitor<'hir> for HirCollector<'a, 'hir, 'tcx> { + type NestedFilter = nested_filter::All; + + fn nested_visit_map(&mut self) -> Self::Map { + self.map + } + + fn visit_item(&mut self, item: &'hir hir::Item<'_>) { + let name = match &item.kind { + hir::ItemKind::Impl(impl_) => { + rustc_hir_pretty::id_to_string(&self.map, impl_.self_ty.hir_id) + } + _ => item.ident.to_string(), + }; + + self.visit_testable(name, item.owner_id.def_id, item.span, |this| { + intravisit::walk_item(this, item); + }); + } + + fn visit_trait_item(&mut self, item: &'hir hir::TraitItem<'_>) { + self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| { + intravisit::walk_trait_item(this, item); + }); + } + + fn visit_impl_item(&mut self, item: &'hir hir::ImplItem<'_>) { + self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| { + intravisit::walk_impl_item(this, item); + }); + } + + fn visit_foreign_item(&mut self, item: &'hir hir::ForeignItem<'_>) { + self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| { + intravisit::walk_foreign_item(this, item); + }); + } + + fn visit_variant(&mut self, v: &'hir hir::Variant<'_>) { + self.visit_testable(v.ident.to_string(), v.def_id, v.span, |this| { + intravisit::walk_variant(this, v); + }); + } + + fn visit_field_def(&mut self, f: &'hir hir::FieldDef<'_>) { + self.visit_testable(f.ident.to_string(), f.def_id, f.span, |this| { + intravisit::walk_field_def(this, f); + }); + } +} From 16db1a1bd0ca3f82a470363b8e6f95a8d03f63c5 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Thu, 30 May 2024 22:42:32 -0700 Subject: [PATCH 04/17] Move Markdown-specific doctest code into submodule --- src/librustdoc/doctest.rs | 2 ++ src/librustdoc/doctest/markdown.rs | 46 ++++++++++++++++++++++++++++ src/librustdoc/lib.rs | 2 +- src/librustdoc/markdown.rs | 48 ++---------------------------- 4 files changed, 51 insertions(+), 47 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 58f65450e69c7..22b5fa0951a4a 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -1,6 +1,8 @@ mod markdown; mod rust; +pub(crate) use markdown::test as test_markdown; + use rustc_ast as ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::Lrc; diff --git a/src/librustdoc/doctest/markdown.rs b/src/librustdoc/doctest/markdown.rs index a55b2941b87f8..13239b9a5172a 100644 --- a/src/librustdoc/doctest/markdown.rs +++ b/src/librustdoc/doctest/markdown.rs @@ -1 +1,47 @@ //! Doctest functionality used only for doctests in `.md` Markdown files. + +use std::fs::read_to_string; + +use rustc_span::DUMMY_SP; +use tempfile::tempdir; + +use super::{generate_args_file, Collector, GlobalTestOptions}; +use crate::config::Options; +use crate::html::markdown::{find_testable_code, ErrorCodes}; + +/// Runs any tests/code examples in the markdown file `input`. +pub(crate) fn test(options: Options) -> Result<(), String> { + use rustc_session::config::Input; + let input_str = match &options.input { + Input::File(path) => { + read_to_string(&path).map_err(|err| format!("{}: {err}", path.display()))? + } + Input::Str { name: _, input } => input.clone(), + }; + + let mut opts = GlobalTestOptions::default(); + opts.no_crate_inject = true; + + let temp_dir = + tempdir().map_err(|error| format!("failed to create temporary directory: {error:?}"))?; + let file_path = temp_dir.path().join("rustdoc-cfgs"); + generate_args_file(&file_path, &options)?; + + let mut collector = Collector::new( + options.input.filestem().to_string(), + options.clone(), + true, + opts, + None, + options.input.opt_path().map(ToOwned::to_owned), + options.enable_per_target_ignores, + file_path, + ); + collector.set_position(DUMMY_SP); + let codes = ErrorCodes::from(options.unstable_features.is_nightly_build()); + + find_testable_code(&input_str, &mut collector, codes, options.enable_per_target_ignores, None); + + crate::doctest::run_tests(options.test_args, options.nocapture, collector.tests); + Ok(()) +} diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index c0d2f9cfaf95d..3b6bddf263a9f 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -728,7 +728,7 @@ fn main_args( core::new_dcx(options.error_format, None, options.diagnostic_width, &options.unstable_opts); match (options.should_test, options.markdown_input()) { - (true, Some(_)) => return wrap_return(&diag, markdown::test(options)), + (true, Some(_)) => return wrap_return(&diag, doctest::test_markdown(options)), (true, None) => return doctest::run(&diag, options), (false, Some(input)) => { let input = input.to_owned(); diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index bcc5a37618a4c..a98f81d011e84 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -3,18 +3,12 @@ use std::fs::{create_dir_all, read_to_string, File}; use std::io::prelude::*; use std::path::Path; -use tempfile::tempdir; - use rustc_span::edition::Edition; -use rustc_span::DUMMY_SP; -use crate::config::{Options, RenderOptions}; -use crate::doctest::{generate_args_file, Collector, GlobalTestOptions}; +use crate::config::RenderOptions; use crate::html::escape::Escape; use crate::html::markdown; -use crate::html::markdown::{ - find_testable_code, ErrorCodes, HeadingOffset, IdMap, Markdown, MarkdownWithToc, -}; +use crate::html::markdown::{ErrorCodes, HeadingOffset, IdMap, Markdown, MarkdownWithToc}; /// Separate any lines at the start of the file that begin with `# ` or `%`. fn extract_leading_metadata(s: &str) -> (Vec<&str>, &str) { @@ -137,41 +131,3 @@ pub(crate) fn render>( Ok(_) => Ok(()), } } - -/// Runs any tests/code examples in the markdown file `input`. -pub(crate) fn test(options: Options) -> Result<(), String> { - use rustc_session::config::Input; - let input_str = match &options.input { - Input::File(path) => { - read_to_string(&path).map_err(|err| format!("{}: {err}", path.display()))? - } - Input::Str { name: _, input } => input.clone(), - }; - - let mut opts = GlobalTestOptions::default(); - opts.no_crate_inject = true; - - let temp_dir = - tempdir().map_err(|error| format!("failed to create temporary directory: {error:?}"))?; - let file_path = temp_dir.path().join("rustdoc-cfgs"); - generate_args_file(&file_path, &options)?; - - let mut collector = Collector::new( - options.input.filestem().to_string(), - options.clone(), - true, - opts, - None, - options.input.opt_path().map(ToOwned::to_owned), - options.enable_per_target_ignores, - file_path, - ); - collector.set_position(DUMMY_SP); - let codes = ErrorCodes::from(options.unstable_features.is_nightly_build()); - - // For markdown files, custom code classes will be disabled until the feature is enabled by default. - find_testable_code(&input_str, &mut collector, codes, options.enable_per_target_ignores, None); - - crate::doctest::run_tests(options.test_args, options.nocapture, collector.tests); - Ok(()) -} From 85499ebf13d0c3c731ac5d69f02ed8e7eb2735bb Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 31 May 2024 00:31:26 -0700 Subject: [PATCH 05/17] Separate doctest collection from running --- src/librustdoc/doctest.rs | 182 ++++++----------------------- src/librustdoc/doctest/markdown.rs | 110 ++++++++++++++--- src/librustdoc/doctest/rust.rs | 133 ++++++++++++++++----- 3 files changed, 237 insertions(+), 188 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 22b5fa0951a4a..8088b57dd76be 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -8,7 +8,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::Lrc; use rustc_errors::emitter::stderr_destination; use rustc_errors::{ColorConfig, ErrorGuaranteed, FatalError}; -use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE}; +use rustc_hir::def_id::LOCAL_CRATE; use rustc_hir::CRATE_HIR_ID; use rustc_interface::interface; use rustc_parse::new_parser_from_source_str; @@ -19,10 +19,9 @@ use rustc_session::parse::ParseSess; use rustc_span::edition::Edition; use rustc_span::source_map::SourceMap; use rustc_span::symbol::sym; -use rustc_span::{BytePos, FileName, Pos, Span, DUMMY_SP}; +use rustc_span::FileName; use rustc_target::spec::{Target, TargetTriple}; -use std::env; use std::fs::File; use std::io::{self, Write}; use std::panic; @@ -38,7 +37,8 @@ use crate::config::Options as RustdocOptions; use crate::html::markdown::{ErrorCodes, Ignore, LangString}; use crate::lint::init_lints; -use self::rust::HirCollector; +use self::markdown::MdDoctest; +use self::rust::{HirCollector, RustDoctest}; /// Options that apply to all doctests in a crate or Markdown file (for `rustdoc foo.md`). #[derive(Clone, Default)] @@ -182,29 +182,19 @@ pub(crate) fn run( let mut collector = Collector::new( tcx.crate_name(LOCAL_CRATE).to_string(), options, - false, opts, - Some(compiler.sess.psess.clone_source_map()), - None, - enable_per_target_ignores, file_path, ); - let mut hir_collector = HirCollector { - sess: &compiler.sess, - collector: &mut collector, - map: tcx.hir(), - codes: ErrorCodes::from( - compiler.sess.opts.unstable_features.is_nightly_build(), - ), + let hir_collector = HirCollector::new( + &compiler.sess, + tcx.hir(), + ErrorCodes::from(compiler.sess.opts.unstable_features.is_nightly_build()), + enable_per_target_ignores, tcx, - }; - hir_collector.visit_testable( - "".to_string(), - CRATE_DEF_ID, - tcx.hir().span(CRATE_HIR_ID), - |this| tcx.hir().walk_toplevel_module(this), ); + let tests = hir_collector.collect_crate(); + tests.into_iter().for_each(|t| collector.add_test(ScrapedDoctest::Rust(t))); collector }); @@ -985,6 +975,12 @@ impl IndividualTestOptions { } } +/// A doctest scraped from the code, ready to be turned into a runnable test. +enum ScrapedDoctest { + Rust(RustDoctest), + Markdown(MdDoctest), +} + pub(crate) trait DoctestVisitor { fn visit_test(&mut self, test: String, config: LangString, line: usize); fn get_line(&self) -> usize { @@ -996,36 +992,9 @@ pub(crate) trait DoctestVisitor { pub(crate) struct Collector { pub(crate) tests: Vec, - // The name of the test displayed to the user, separated by `::`. - // - // In tests from Rust source, this is the path to the item - // e.g., `["std", "vec", "Vec", "push"]`. - // - // In tests from a markdown file, this is the titles of all headers (h1~h6) - // of the sections that contain the code block, e.g., if the markdown file is - // written as: - // - // ``````markdown - // # Title - // - // ## Subtitle - // - // ```rust - // assert!(true); - // ``` - // `````` - // - // the `names` vector of that test will be `["Title", "Subtitle"]`. - names: Vec, - rustdoc_options: RustdocOptions, - use_headers: bool, - enable_per_target_ignores: bool, crate_name: String, opts: GlobalTestOptions, - position: Span, - source_map: Option>, - filename: Option, visited_tests: FxHashMap<(String, usize), usize>, unused_extern_reports: Arc>>, compiling_test_count: AtomicUsize, @@ -1036,24 +1005,14 @@ impl Collector { pub(crate) fn new( crate_name: String, rustdoc_options: RustdocOptions, - use_headers: bool, opts: GlobalTestOptions, - source_map: Option>, - filename: Option, - enable_per_target_ignores: bool, arg_file: PathBuf, ) -> Collector { Collector { tests: Vec::new(), - names: Vec::new(), rustdoc_options, - use_headers, - enable_per_target_ignores, crate_name, opts, - position: DUMMY_SP, - source_map, - filename, visited_tests: FxHashMap::default(), unused_extern_reports: Default::default(), compiling_test_count: AtomicUsize::new(0), @@ -1061,8 +1020,8 @@ impl Collector { } } - fn generate_name(&self, line: usize, filename: &FileName) -> String { - let mut item_path = self.names.join("::"); + fn generate_name(&self, filename: &FileName, line: usize, logical_path: &[String]) -> String { + let mut item_path = logical_path.join("::"); item_path.retain(|c| c != ' '); if !item_path.is_empty() { item_path.push(' '); @@ -1070,40 +1029,24 @@ impl Collector { format!("{} - {item_path}(line {line})", filename.prefer_local()) } - pub(crate) fn set_position(&mut self, position: Span) { - self.position = position; - } - - fn get_filename(&self) -> FileName { - if let Some(ref source_map) = self.source_map { - let filename = source_map.span_to_filename(self.position); - if let FileName::Real(ref filename) = filename - && let Ok(cur_dir) = env::current_dir() - && let Some(local_path) = filename.local_path() - && let Ok(path) = local_path.strip_prefix(&cur_dir) - { - return path.to_owned().into(); + fn add_test(&mut self, test: ScrapedDoctest) { + let (filename, line, logical_path, langstr, text) = match test { + ScrapedDoctest::Rust(RustDoctest { filename, line, logical_path, langstr, text }) => { + (filename, line, logical_path, langstr, text) } - filename - } else if let Some(ref filename) = self.filename { - filename.clone().into() - } else { - FileName::Custom("input".to_owned()) - } - } -} + ScrapedDoctest::Markdown(MdDoctest { filename, line, logical_path, langstr, text }) => { + (filename, line, logical_path, langstr, text) + } + }; -impl DoctestVisitor for Collector { - fn visit_test(&mut self, test: String, config: LangString, line: usize) { - let filename = self.get_filename(); - let name = self.generate_name(line, &filename); + let name = self.generate_name(&filename, line, &logical_path); let crate_name = self.crate_name.clone(); let opts = self.opts.clone(); - let edition = config.edition.unwrap_or(self.rustdoc_options.edition); + let edition = langstr.edition.unwrap_or(self.rustdoc_options.edition); let target_str = self.rustdoc_options.target.to_string(); let unused_externs = self.unused_extern_reports.clone(); - let no_run = config.no_run || self.rustdoc_options.no_run; - if !config.compile_fail { + let no_run = langstr.no_run || self.rustdoc_options.no_run; + if !langstr.compile_fail { self.compiling_test_count.fetch_add(1, Ordering::SeqCst); } @@ -1140,11 +1083,11 @@ impl DoctestVisitor for Collector { let rustdoc_test_options = IndividualTestOptions::new(&self.rustdoc_options, &self.arg_file, test_id); - debug!("creating test {name}: {test}"); + debug!("creating test {name}: {text}"); self.tests.push(test::TestDescAndFn { desc: test::TestDesc { name: test::DynTestName(name), - ignore: match config.ignore { + ignore: match langstr.ignore { Ignore::All => true, Ignore::None => false, Ignore::Some(ref ignores) => ignores.iter().any(|s| target_str.contains(s)), @@ -1157,7 +1100,7 @@ impl DoctestVisitor for Collector { end_col: 0, // compiler failures are test failures should_panic: test::ShouldPanic::No, - compile_fail: config.compile_fail, + compile_fail: langstr.compile_fail, no_run, test_type: test::TestType::DocTest, }, @@ -1166,11 +1109,11 @@ impl DoctestVisitor for Collector { unused_externs.lock().unwrap().push(uext); }; let res = run_test( - &test, + &text, &crate_name, line, rustdoc_test_options, - config, + langstr, no_run, &opts, edition, @@ -1233,59 +1176,6 @@ impl DoctestVisitor for Collector { })), }); } - - fn get_line(&self) -> usize { - if let Some(ref source_map) = self.source_map { - let line = self.position.lo().to_usize(); - let line = source_map.lookup_char_pos(BytePos(line as u32)).line; - if line > 0 { line - 1 } else { line } - } else { - 0 - } - } - - fn visit_header(&mut self, name: &str, level: u32) { - if self.use_headers { - // We use these headings as test names, so it's good if - // they're valid identifiers. - let name = name - .chars() - .enumerate() - .map(|(i, c)| { - if (i == 0 && rustc_lexer::is_id_start(c)) - || (i != 0 && rustc_lexer::is_id_continue(c)) - { - c - } else { - '_' - } - }) - .collect::(); - - // Here we try to efficiently assemble the header titles into the - // test name in the form of `h1::h2::h3::h4::h5::h6`. - // - // Suppose that originally `self.names` contains `[h1, h2, h3]`... - let level = level as usize; - if level <= self.names.len() { - // ... Consider `level == 2`. All headers in the lower levels - // are irrelevant in this new level. So we should reset - // `self.names` to contain headers until

, and replace that - // slot with the new name: `[h1, name]`. - self.names.truncate(level); - self.names[level - 1] = name; - } else { - // ... On the other hand, consider `level == 5`. This means we - // need to extend `self.names` to contain five headers. We fill - // in the missing level (

) with `_`. Thus `self.names` will - // become `[h1, h2, h3, "_", name]`. - if level - 1 > self.names.len() { - self.names.resize(level - 1, "_".to_owned()); - } - self.names.push(name); - } - } - } } #[cfg(test)] // used in tests diff --git a/src/librustdoc/doctest/markdown.rs b/src/librustdoc/doctest/markdown.rs index 13239b9a5172a..6f2769b768101 100644 --- a/src/librustdoc/doctest/markdown.rs +++ b/src/librustdoc/doctest/markdown.rs @@ -2,12 +2,84 @@ use std::fs::read_to_string; -use rustc_span::DUMMY_SP; +use rustc_span::FileName; use tempfile::tempdir; -use super::{generate_args_file, Collector, GlobalTestOptions}; +use super::{generate_args_file, Collector, DoctestVisitor, GlobalTestOptions, ScrapedDoctest}; use crate::config::Options; -use crate::html::markdown::{find_testable_code, ErrorCodes}; +use crate::html::markdown::{find_testable_code, ErrorCodes, LangString}; + +pub(super) struct MdDoctest { + pub(super) filename: FileName, + pub(super) line: usize, + pub(super) logical_path: Vec, + pub(super) langstr: LangString, + pub(super) text: String, +} + +struct MdCollector { + tests: Vec, + cur_path: Vec, + filename: FileName, +} + +impl DoctestVisitor for MdCollector { + fn visit_test(&mut self, test: String, config: LangString, line: usize) { + let filename = self.filename.clone(); + self.tests.push(MdDoctest { + filename, + line, + logical_path: self.cur_path.clone(), + langstr: config, + text: test, + }); + } + + fn get_line(&self) -> usize { + 0 + } + + fn visit_header(&mut self, name: &str, level: u32) { + // We use these headings as test names, so it's good if + // they're valid identifiers. + let name = name + .chars() + .enumerate() + .map(|(i, c)| { + if (i == 0 && rustc_lexer::is_id_start(c)) + || (i != 0 && rustc_lexer::is_id_continue(c)) + { + c + } else { + '_' + } + }) + .collect::(); + + // Here we try to efficiently assemble the header titles into the + // test name in the form of `h1::h2::h3::h4::h5::h6`. + // + // Suppose that originally `self.cur_path` contains `[h1, h2, h3]`... + let level = level as usize; + if level <= self.cur_path.len() { + // ... Consider `level == 2`. All headers in the lower levels + // are irrelevant in this new level. So we should reset + // `self.names` to contain headers until

, and replace that + // slot with the new name: `[h1, name]`. + self.cur_path.truncate(level); + self.cur_path[level - 1] = name; + } else { + // ... On the other hand, consider `level == 5`. This means we + // need to extend `self.names` to contain five headers. We fill + // in the missing level (

) with `_`. Thus `self.names` will + // become `[h1, h2, h3, "_", name]`. + if level - 1 > self.cur_path.len() { + self.cur_path.resize(level - 1, "_".to_owned()); + } + self.cur_path.push(name); + } + } +} /// Runs any tests/code examples in the markdown file `input`. pub(crate) fn test(options: Options) -> Result<(), String> { @@ -27,21 +99,29 @@ pub(crate) fn test(options: Options) -> Result<(), String> { let file_path = temp_dir.path().join("rustdoc-cfgs"); generate_args_file(&file_path, &options)?; - let mut collector = Collector::new( - options.input.filestem().to_string(), - options.clone(), - true, - opts, - None, - options.input.opt_path().map(ToOwned::to_owned), - options.enable_per_target_ignores, - file_path, - ); - collector.set_position(DUMMY_SP); + let mut md_collector = MdCollector { + tests: vec![], + cur_path: vec![], + filename: options + .input + .opt_path() + .map(ToOwned::to_owned) + .map(FileName::from) + .unwrap_or(FileName::Custom("input".to_owned())), + }; let codes = ErrorCodes::from(options.unstable_features.is_nightly_build()); - find_testable_code(&input_str, &mut collector, codes, options.enable_per_target_ignores, None); + find_testable_code( + &input_str, + &mut md_collector, + codes, + options.enable_per_target_ignores, + None, + ); + let mut collector = + Collector::new(options.input.filestem().to_string(), options.clone(), opts, file_path); + md_collector.tests.into_iter().for_each(|t| collector.add_test(ScrapedDoctest::Markdown(t))); crate::doctest::run_tests(options.test_args, options.nocapture, collector.tests); Ok(()) } diff --git a/src/librustdoc/doctest/rust.rs b/src/librustdoc/doctest/rust.rs index e4527de77a023..9e62ed34a586f 100644 --- a/src/librustdoc/doctest/rust.rs +++ b/src/librustdoc/doctest/rust.rs @@ -1,29 +1,108 @@ //! Doctest functionality used only for doctests in `.rs` source files. -use rustc_data_structures::fx::FxHashSet; -use rustc_hir::def_id::LocalDefId; -use rustc_hir::{self as hir, intravisit}; +use std::env; + +use rustc_data_structures::{fx::FxHashSet, sync::Lrc}; +use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID}; +use rustc_hir::{self as hir, intravisit, CRATE_HIR_ID}; use rustc_middle::hir::map::Map; use rustc_middle::hir::nested_filter; use rustc_middle::ty::TyCtxt; use rustc_resolve::rustdoc::span_of_fragments; use rustc_session::Session; -use rustc_span::{Span, DUMMY_SP}; +use rustc_span::source_map::SourceMap; +use rustc_span::{BytePos, FileName, Pos, Span, DUMMY_SP}; -use super::Collector; +use super::DoctestVisitor; use crate::clean::{types::AttributesExt, Attributes}; -use crate::html::markdown::{self, ErrorCodes}; - -pub(super) struct HirCollector<'a, 'hir, 'tcx> { - pub(super) sess: &'a Session, - pub(super) collector: &'a mut Collector, - pub(super) map: Map<'hir>, - pub(super) codes: ErrorCodes, - pub(super) tcx: TyCtxt<'tcx>, +use crate::html::markdown::{self, ErrorCodes, LangString}; + +pub(super) struct RustDoctest { + pub(super) filename: FileName, + pub(super) line: usize, + pub(super) logical_path: Vec, + pub(super) langstr: LangString, + pub(super) text: String, +} + +struct RustCollector { + source_map: Lrc, + tests: Vec, + cur_path: Vec, + position: Span, +} + +impl RustCollector { + fn get_filename(&self) -> FileName { + let filename = self.source_map.span_to_filename(self.position); + if let FileName::Real(ref filename) = filename + && let Ok(cur_dir) = env::current_dir() + && let Some(local_path) = filename.local_path() + && let Ok(path) = local_path.strip_prefix(&cur_dir) + { + return path.to_owned().into(); + } + filename + } +} + +impl DoctestVisitor for RustCollector { + fn visit_test(&mut self, test: String, config: LangString, line: usize) { + self.tests.push(RustDoctest { + filename: self.get_filename(), + line, + logical_path: self.cur_path.clone(), + langstr: config, + text: test, + }); + } + + fn get_line(&self) -> usize { + let line = self.position.lo().to_usize(); + let line = self.source_map.lookup_char_pos(BytePos(line as u32)).line; + if line > 0 { line - 1 } else { line } + } + + fn visit_header(&mut self, _name: &str, _level: u32) {} +} + +pub(super) struct HirCollector<'a, 'tcx> { + sess: &'a Session, + map: Map<'tcx>, + codes: ErrorCodes, + tcx: TyCtxt<'tcx>, + enable_per_target_ignores: bool, + collector: RustCollector, +} + +impl<'a, 'tcx> HirCollector<'a, 'tcx> { + pub fn new( + sess: &'a Session, + map: Map<'tcx>, + codes: ErrorCodes, + enable_per_target_ignores: bool, + tcx: TyCtxt<'tcx>, + ) -> Self { + let collector = RustCollector { + source_map: sess.psess.clone_source_map(), + cur_path: vec![], + position: DUMMY_SP, + tests: vec![], + }; + Self { sess, map, codes, enable_per_target_ignores, tcx, collector } + } + + pub fn collect_crate(mut self) -> Vec { + let tcx = self.tcx; + self.visit_testable("".to_string(), CRATE_DEF_ID, tcx.hir().span(CRATE_HIR_ID), |this| { + tcx.hir().walk_toplevel_module(this) + }); + self.collector.tests + } } -impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> { - pub(super) fn visit_testable( +impl<'a, 'tcx> HirCollector<'a, 'tcx> { + fn visit_testable( &mut self, name: String, def_id: LocalDefId, @@ -39,7 +118,7 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> { let has_name = !name.is_empty(); if has_name { - self.collector.names.push(name); + self.collector.cur_path.push(name); } // The collapse-docs pass won't combine sugared/raw doc attributes, or included files with @@ -52,12 +131,12 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> { .find(|attr| attr.doc_str().is_some()) .map(|attr| attr.span.ctxt().outer_expn().expansion_cause().unwrap_or(attr.span)) .unwrap_or(DUMMY_SP); - self.collector.set_position(span); + self.collector.position = span; markdown::find_testable_code( &doc, - self.collector, + &mut self.collector, self.codes, - self.collector.enable_per_target_ignores, + self.enable_per_target_ignores, Some(&crate::html::markdown::ExtraInfo::new( self.tcx, def_id.to_def_id(), @@ -69,19 +148,19 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> { nested(self); if has_name { - self.collector.names.pop(); + self.collector.cur_path.pop(); } } } -impl<'a, 'hir, 'tcx> intravisit::Visitor<'hir> for HirCollector<'a, 'hir, 'tcx> { +impl<'a, 'tcx> intravisit::Visitor<'tcx> for HirCollector<'a, 'tcx> { type NestedFilter = nested_filter::All; fn nested_visit_map(&mut self) -> Self::Map { self.map } - fn visit_item(&mut self, item: &'hir hir::Item<'_>) { + fn visit_item(&mut self, item: &'tcx hir::Item<'_>) { let name = match &item.kind { hir::ItemKind::Impl(impl_) => { rustc_hir_pretty::id_to_string(&self.map, impl_.self_ty.hir_id) @@ -94,31 +173,31 @@ impl<'a, 'hir, 'tcx> intravisit::Visitor<'hir> for HirCollector<'a, 'hir, 'tcx> }); } - fn visit_trait_item(&mut self, item: &'hir hir::TraitItem<'_>) { + fn visit_trait_item(&mut self, item: &'tcx hir::TraitItem<'_>) { self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| { intravisit::walk_trait_item(this, item); }); } - fn visit_impl_item(&mut self, item: &'hir hir::ImplItem<'_>) { + fn visit_impl_item(&mut self, item: &'tcx hir::ImplItem<'_>) { self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| { intravisit::walk_impl_item(this, item); }); } - fn visit_foreign_item(&mut self, item: &'hir hir::ForeignItem<'_>) { + fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem<'_>) { self.visit_testable(item.ident.to_string(), item.owner_id.def_id, item.span, |this| { intravisit::walk_foreign_item(this, item); }); } - fn visit_variant(&mut self, v: &'hir hir::Variant<'_>) { + fn visit_variant(&mut self, v: &'tcx hir::Variant<'_>) { self.visit_testable(v.ident.to_string(), v.def_id, v.span, |this| { intravisit::walk_variant(this, v); }); } - fn visit_field_def(&mut self, f: &'hir hir::FieldDef<'_>) { + fn visit_field_def(&mut self, f: &'tcx hir::FieldDef<'_>) { self.visit_testable(f.ident.to_string(), f.def_id, f.span, |this| { intravisit::walk_field_def(this, f); }); From b7dd401a78598615a7733bfe615cbcbd9393e44a Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 31 May 2024 00:50:41 -0700 Subject: [PATCH 06/17] rustdoc: Extract actual doctest running logic into function --- src/librustdoc/doctest.rs | 189 ++++++++++++++++++----------- src/librustdoc/doctest/markdown.rs | 12 +- 2 files changed, 126 insertions(+), 75 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 8088b57dd76be..61195f914ebf4 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -179,7 +179,7 @@ pub(crate) fn run( let opts = scrape_test_config(crate_attrs); let enable_per_target_ignores = options.enable_per_target_ignores; - let mut collector = Collector::new( + let mut collector = CreateRunnableDoctests::new( tcx.crate_name(LOCAL_CRATE).to_string(), options, opts, @@ -989,7 +989,7 @@ pub(crate) trait DoctestVisitor { fn visit_header(&mut self, _name: &str, _level: u32) {} } -pub(crate) struct Collector { +pub(crate) struct CreateRunnableDoctests { pub(crate) tests: Vec, rustdoc_options: RustdocOptions, @@ -1001,14 +1001,14 @@ pub(crate) struct Collector { arg_file: PathBuf, } -impl Collector { +impl CreateRunnableDoctests { pub(crate) fn new( crate_name: String, rustdoc_options: RustdocOptions, opts: GlobalTestOptions, arg_file: PathBuf, - ) -> Collector { - Collector { + ) -> CreateRunnableDoctests { + CreateRunnableDoctests { tests: Vec::new(), rustdoc_options, crate_name, @@ -1105,77 +1105,122 @@ impl Collector { test_type: test::TestType::DocTest, }, testfn: test::DynTestFn(Box::new(move || { - let report_unused_externs = |uext| { - unused_externs.lock().unwrap().push(uext); - }; - let res = run_test( - &text, - &crate_name, - line, - rustdoc_test_options, - langstr, - no_run, - &opts, - edition, - path, - report_unused_externs, - ); - - if let Err(err) = res { - match err { - TestFailure::CompileError => { - eprint!("Couldn't compile the test."); - } - TestFailure::UnexpectedCompilePass => { - eprint!("Test compiled successfully, but it's marked `compile_fail`."); - } - TestFailure::UnexpectedRunPass => { - eprint!("Test executable succeeded, but it's marked `should_panic`."); - } - TestFailure::MissingErrorCodes(codes) => { - eprint!("Some expected error codes were not found: {codes:?}"); - } - TestFailure::ExecutionError(err) => { - eprint!("Couldn't run the test: {err}"); - if err.kind() == io::ErrorKind::PermissionDenied { - eprint!(" - maybe your tempdir is mounted with noexec?"); - } - } - TestFailure::ExecutionFailure(out) => { - eprintln!("Test executable failed ({reason}).", reason = out.status); - - // FIXME(#12309): An unfortunate side-effect of capturing the test - // executable's output is that the relative ordering between the test's - // stdout and stderr is lost. However, this is better than the - // alternative: if the test executable inherited the parent's I/O - // handles the output wouldn't be captured at all, even on success. - // - // The ordering could be preserved if the test process' stderr was - // redirected to stdout, but that functionality does not exist in the - // standard library, so it may not be portable enough. - let stdout = str::from_utf8(&out.stdout).unwrap_or_default(); - let stderr = str::from_utf8(&out.stderr).unwrap_or_default(); - - if !stdout.is_empty() || !stderr.is_empty() { - eprintln!(); - - if !stdout.is_empty() { - eprintln!("stdout:\n{stdout}"); - } - - if !stderr.is_empty() { - eprintln!("stderr:\n{stderr}"); - } - } - } + doctest_run_fn( + RunnableDoctest { + crate_name, + line, + rustdoc_test_options, + langstr, + no_run, + opts, + edition, + path, + text, + }, + unused_externs, + ) + })), + }); + } +} + +/// A doctest that is ready to run. +struct RunnableDoctest { + crate_name: String, + line: usize, + rustdoc_test_options: IndividualTestOptions, + langstr: LangString, + no_run: bool, + opts: GlobalTestOptions, + edition: Edition, + path: PathBuf, + text: String, +} + +fn doctest_run_fn( + test: RunnableDoctest, + unused_externs: Arc>>, +) -> Result<(), String> { + let RunnableDoctest { + crate_name, + line, + rustdoc_test_options, + langstr, + no_run, + opts, + edition, + path, + text, + } = test; + + let report_unused_externs = |uext| { + unused_externs.lock().unwrap().push(uext); + }; + let res = run_test( + &text, + &crate_name, + line, + rustdoc_test_options, + langstr, + no_run, + &opts, + edition, + path, + report_unused_externs, + ); + + if let Err(err) = res { + match err { + TestFailure::CompileError => { + eprint!("Couldn't compile the test."); + } + TestFailure::UnexpectedCompilePass => { + eprint!("Test compiled successfully, but it's marked `compile_fail`."); + } + TestFailure::UnexpectedRunPass => { + eprint!("Test executable succeeded, but it's marked `should_panic`."); + } + TestFailure::MissingErrorCodes(codes) => { + eprint!("Some expected error codes were not found: {codes:?}"); + } + TestFailure::ExecutionError(err) => { + eprint!("Couldn't run the test: {err}"); + if err.kind() == io::ErrorKind::PermissionDenied { + eprint!(" - maybe your tempdir is mounted with noexec?"); + } + } + TestFailure::ExecutionFailure(out) => { + eprintln!("Test executable failed ({reason}).", reason = out.status); + + // FIXME(#12309): An unfortunate side-effect of capturing the test + // executable's output is that the relative ordering between the test's + // stdout and stderr is lost. However, this is better than the + // alternative: if the test executable inherited the parent's I/O + // handles the output wouldn't be captured at all, even on success. + // + // The ordering could be preserved if the test process' stderr was + // redirected to stdout, but that functionality does not exist in the + // standard library, so it may not be portable enough. + let stdout = str::from_utf8(&out.stdout).unwrap_or_default(); + let stderr = str::from_utf8(&out.stderr).unwrap_or_default(); + + if !stdout.is_empty() || !stderr.is_empty() { + eprintln!(); + + if !stdout.is_empty() { + eprintln!("stdout:\n{stdout}"); } - panic::resume_unwind(Box::new(())); + if !stderr.is_empty() { + eprintln!("stderr:\n{stderr}"); + } } - Ok(()) - })), - }); + } + } + + panic::resume_unwind(Box::new(())); } + Ok(()) } #[cfg(test)] // used in tests diff --git a/src/librustdoc/doctest/markdown.rs b/src/librustdoc/doctest/markdown.rs index 6f2769b768101..7a52a8cb18d02 100644 --- a/src/librustdoc/doctest/markdown.rs +++ b/src/librustdoc/doctest/markdown.rs @@ -5,7 +5,9 @@ use std::fs::read_to_string; use rustc_span::FileName; use tempfile::tempdir; -use super::{generate_args_file, Collector, DoctestVisitor, GlobalTestOptions, ScrapedDoctest}; +use super::{ + generate_args_file, CreateRunnableDoctests, DoctestVisitor, GlobalTestOptions, ScrapedDoctest, +}; use crate::config::Options; use crate::html::markdown::{find_testable_code, ErrorCodes, LangString}; @@ -119,8 +121,12 @@ pub(crate) fn test(options: Options) -> Result<(), String> { None, ); - let mut collector = - Collector::new(options.input.filestem().to_string(), options.clone(), opts, file_path); + let mut collector = CreateRunnableDoctests::new( + options.input.filestem().to_string(), + options.clone(), + opts, + file_path, + ); md_collector.tests.into_iter().for_each(|t| collector.add_test(ScrapedDoctest::Markdown(t))); crate::doctest::run_tests(options.test_args, options.nocapture, collector.tests); Ok(()) From 279b4d22f70f8bdb84dad547cf0dbefd587c4bf3 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 31 May 2024 11:19:24 -0700 Subject: [PATCH 07/17] Merge `RustDoctest` and `MdDoctest` into one type --- src/librustdoc/doctest.rs | 89 ++++++++++++------------------ src/librustdoc/doctest/markdown.rs | 14 +---- src/librustdoc/doctest/rust.rs | 16 ++---- 3 files changed, 42 insertions(+), 77 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 61195f914ebf4..5026abb3bffb3 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -37,8 +37,7 @@ use crate::config::Options as RustdocOptions; use crate::html::markdown::{ErrorCodes, Ignore, LangString}; use crate::lint::init_lints; -use self::markdown::MdDoctest; -use self::rust::{HirCollector, RustDoctest}; +use self::rust::HirCollector; /// Options that apply to all doctests in a crate or Markdown file (for `rustdoc foo.md`). #[derive(Clone, Default)] @@ -194,7 +193,7 @@ pub(crate) fn run( tcx, ); let tests = hir_collector.collect_crate(); - tests.into_iter().for_each(|t| collector.add_test(ScrapedDoctest::Rust(t))); + tests.into_iter().for_each(|t| collector.add_test(t)); collector }); @@ -976,9 +975,12 @@ impl IndividualTestOptions { } /// A doctest scraped from the code, ready to be turned into a runnable test. -enum ScrapedDoctest { - Rust(RustDoctest), - Markdown(MdDoctest), +struct ScrapedDoctest { + filename: FileName, + line: usize, + logical_path: Vec, + langstr: LangString, + text: String, } pub(crate) trait DoctestVisitor { @@ -1030,27 +1032,18 @@ impl CreateRunnableDoctests { } fn add_test(&mut self, test: ScrapedDoctest) { - let (filename, line, logical_path, langstr, text) = match test { - ScrapedDoctest::Rust(RustDoctest { filename, line, logical_path, langstr, text }) => { - (filename, line, logical_path, langstr, text) - } - ScrapedDoctest::Markdown(MdDoctest { filename, line, logical_path, langstr, text }) => { - (filename, line, logical_path, langstr, text) - } - }; - - let name = self.generate_name(&filename, line, &logical_path); + let name = self.generate_name(&test.filename, test.line, &test.logical_path); let crate_name = self.crate_name.clone(); let opts = self.opts.clone(); - let edition = langstr.edition.unwrap_or(self.rustdoc_options.edition); + let edition = test.langstr.edition.unwrap_or(self.rustdoc_options.edition); let target_str = self.rustdoc_options.target.to_string(); let unused_externs = self.unused_extern_reports.clone(); - let no_run = langstr.no_run || self.rustdoc_options.no_run; - if !langstr.compile_fail { + let no_run = test.langstr.no_run || self.rustdoc_options.no_run; + if !test.langstr.compile_fail { self.compiling_test_count.fetch_add(1, Ordering::SeqCst); } - let path = match &filename { + let path = match &test.filename { FileName::Real(path) => { if let Some(local_path) = path.local_path() { local_path.to_path_buf() @@ -1063,7 +1056,8 @@ impl CreateRunnableDoctests { }; // For example `module/file.rs` would become `module_file_rs` - let file = filename + let file = test + .filename .prefer_local() .to_string_lossy() .chars() @@ -1072,22 +1066,25 @@ impl CreateRunnableDoctests { let test_id = format!( "{file}_{line}_{number}", file = file, - line = line, + line = test.line, number = { // Increases the current test number, if this file already // exists or it creates a new entry with a test number of 0. - self.visited_tests.entry((file.clone(), line)).and_modify(|v| *v += 1).or_insert(0) + self.visited_tests + .entry((file.clone(), test.line)) + .and_modify(|v| *v += 1) + .or_insert(0) }, ); let rustdoc_test_options = IndividualTestOptions::new(&self.rustdoc_options, &self.arg_file, test_id); - debug!("creating test {name}: {text}"); + debug!("creating test {name}: {}", test.text); self.tests.push(test::TestDescAndFn { desc: test::TestDesc { name: test::DynTestName(name), - ignore: match langstr.ignore { + ignore: match test.langstr.ignore { Ignore::All => true, Ignore::None => false, Ignore::Some(ref ignores) => ignores.iter().any(|s| target_str.contains(s)), @@ -1100,7 +1097,7 @@ impl CreateRunnableDoctests { end_col: 0, // compiler failures are test failures should_panic: test::ShouldPanic::No, - compile_fail: langstr.compile_fail, + compile_fail: test.langstr.compile_fail, no_run, test_type: test::TestType::DocTest, }, @@ -1108,14 +1105,12 @@ impl CreateRunnableDoctests { doctest_run_fn( RunnableDoctest { crate_name, - line, rustdoc_test_options, - langstr, no_run, opts, edition, path, - text, + scraped_test: test, }, unused_externs, ) @@ -1127,45 +1122,31 @@ impl CreateRunnableDoctests { /// A doctest that is ready to run. struct RunnableDoctest { crate_name: String, - line: usize, rustdoc_test_options: IndividualTestOptions, - langstr: LangString, no_run: bool, opts: GlobalTestOptions, edition: Edition, path: PathBuf, - text: String, + scraped_test: ScrapedDoctest, } fn doctest_run_fn( - test: RunnableDoctest, + runnable_test: RunnableDoctest, unused_externs: Arc>>, ) -> Result<(), String> { - let RunnableDoctest { - crate_name, - line, - rustdoc_test_options, - langstr, - no_run, - opts, - edition, - path, - text, - } = test; - let report_unused_externs = |uext| { unused_externs.lock().unwrap().push(uext); }; let res = run_test( - &text, - &crate_name, - line, - rustdoc_test_options, - langstr, - no_run, - &opts, - edition, - path, + &runnable_test.scraped_test.text, + &runnable_test.crate_name, + runnable_test.scraped_test.line, + runnable_test.rustdoc_test_options, + runnable_test.scraped_test.langstr, + runnable_test.no_run, + &runnable_test.opts, + runnable_test.edition, + runnable_test.path, report_unused_externs, ); diff --git a/src/librustdoc/doctest/markdown.rs b/src/librustdoc/doctest/markdown.rs index 7a52a8cb18d02..5d0dc5926e88b 100644 --- a/src/librustdoc/doctest/markdown.rs +++ b/src/librustdoc/doctest/markdown.rs @@ -11,16 +11,8 @@ use super::{ use crate::config::Options; use crate::html::markdown::{find_testable_code, ErrorCodes, LangString}; -pub(super) struct MdDoctest { - pub(super) filename: FileName, - pub(super) line: usize, - pub(super) logical_path: Vec, - pub(super) langstr: LangString, - pub(super) text: String, -} - struct MdCollector { - tests: Vec, + tests: Vec, cur_path: Vec, filename: FileName, } @@ -28,7 +20,7 @@ struct MdCollector { impl DoctestVisitor for MdCollector { fn visit_test(&mut self, test: String, config: LangString, line: usize) { let filename = self.filename.clone(); - self.tests.push(MdDoctest { + self.tests.push(ScrapedDoctest { filename, line, logical_path: self.cur_path.clone(), @@ -127,7 +119,7 @@ pub(crate) fn test(options: Options) -> Result<(), String> { opts, file_path, ); - md_collector.tests.into_iter().for_each(|t| collector.add_test(ScrapedDoctest::Markdown(t))); + md_collector.tests.into_iter().for_each(|t| collector.add_test(t)); crate::doctest::run_tests(options.test_args, options.nocapture, collector.tests); Ok(()) } diff --git a/src/librustdoc/doctest/rust.rs b/src/librustdoc/doctest/rust.rs index 9e62ed34a586f..458c90881c935 100644 --- a/src/librustdoc/doctest/rust.rs +++ b/src/librustdoc/doctest/rust.rs @@ -13,21 +13,13 @@ use rustc_session::Session; use rustc_span::source_map::SourceMap; use rustc_span::{BytePos, FileName, Pos, Span, DUMMY_SP}; -use super::DoctestVisitor; +use super::{DoctestVisitor, ScrapedDoctest}; use crate::clean::{types::AttributesExt, Attributes}; use crate::html::markdown::{self, ErrorCodes, LangString}; -pub(super) struct RustDoctest { - pub(super) filename: FileName, - pub(super) line: usize, - pub(super) logical_path: Vec, - pub(super) langstr: LangString, - pub(super) text: String, -} - struct RustCollector { source_map: Lrc, - tests: Vec, + tests: Vec, cur_path: Vec, position: Span, } @@ -48,7 +40,7 @@ impl RustCollector { impl DoctestVisitor for RustCollector { fn visit_test(&mut self, test: String, config: LangString, line: usize) { - self.tests.push(RustDoctest { + self.tests.push(ScrapedDoctest { filename: self.get_filename(), line, logical_path: self.cur_path.clone(), @@ -92,7 +84,7 @@ impl<'a, 'tcx> HirCollector<'a, 'tcx> { Self { sess, map, codes, enable_per_target_ignores, tcx, collector } } - pub fn collect_crate(mut self) -> Vec { + pub fn collect_crate(mut self) -> Vec { let tcx = self.tcx; self.visit_testable("".to_string(), CRATE_DEF_ID, tcx.hir().span(CRATE_HIR_ID), |this| { tcx.hir().walk_toplevel_module(this) From 46d2aa5a8f8d09d4789d2af01a6acdb3ba9e41a4 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 31 May 2024 11:40:01 -0700 Subject: [PATCH 08/17] Remove global options from `IndividualTestOptions` --- src/librustdoc/doctest.rs | 61 ++++++++++++++------------------------- 1 file changed, 21 insertions(+), 40 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 5026abb3bffb3..f1a4df52a59b9 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -365,7 +365,8 @@ fn run_test( test: &str, crate_name: &str, line: usize, - rustdoc_options: IndividualTestOptions, + rustdoc_options: &RustdocOptions, + test_options: IndividualTestOptions, mut lang_string: LangString, no_run: bool, opts: &GlobalTestOptions, @@ -379,12 +380,12 @@ fn run_test( lang_string.test_harness, opts, edition, - Some(&rustdoc_options.test_id), + Some(&test_options.test_id), ); // Make sure we emit well-formed executable names for our target. let rust_out = add_exe_suffix("rust_out".to_owned(), &rustdoc_options.target); - let output_file = rustdoc_options.outdir.path().join(rust_out); + let output_file = test_options.outdir.path().join(rust_out); let rustc_binary = rustdoc_options .test_builder @@ -392,7 +393,7 @@ fn run_test( .unwrap_or_else(|| rustc_interface::util::rustc_path().expect("found rustc")); let mut compiler = wrapped_rustc_command(&rustdoc_options.test_builder_wrappers, rustc_binary); - compiler.arg(&format!("@{}", rustdoc_options.arg_file.display())); + compiler.arg(&format!("@{}", test_options.arg_file.display())); if let Some(sysroot) = &rustdoc_options.maybe_sysroot { compiler.arg(format!("--sysroot={}", sysroot.display())); @@ -405,20 +406,22 @@ fn run_test( if lang_string.test_harness { compiler.arg("--test"); } - if rustdoc_options.is_json_unused_externs_enabled && !lang_string.compile_fail { + if rustdoc_options.json_unused_externs.is_enabled() && !lang_string.compile_fail { compiler.arg("--error-format=json"); compiler.arg("--json").arg("unused-externs"); compiler.arg("-W").arg("unused_crate_dependencies"); compiler.arg("-Z").arg("unstable-options"); } - if no_run && !lang_string.compile_fail && rustdoc_options.should_persist_doctests { + if no_run && !lang_string.compile_fail && rustdoc_options.persist_doctests.is_none() { + // FIXME: why does this code check if it *shouldn't* persist doctests + // -- shouldn't it be the negation? compiler.arg("--emit=metadata"); } - compiler.arg("--target").arg(match rustdoc_options.target { + compiler.arg("--target").arg(match &rustdoc_options.target { TargetTriple::TargetTriple(s) => s, TargetTriple::TargetJson { path_for_rustdoc, .. } => { - path_for_rustdoc.to_str().expect("target path must be valid unicode").to_string() + path_for_rustdoc.to_str().expect("target path must be valid unicode") } }); if let ErrorOutputType::HumanReadable(kind) = rustdoc_options.error_format { @@ -511,15 +514,15 @@ fn run_test( let mut cmd; let output_file = make_maybe_absolute_path(output_file); - if let Some(tool) = rustdoc_options.runtool { + if let Some(tool) = &rustdoc_options.runtool { let tool = make_maybe_absolute_path(tool.into()); cmd = Command::new(tool); - cmd.args(rustdoc_options.runtool_args); + cmd.args(&rustdoc_options.runtool_args); cmd.arg(output_file); } else { cmd = Command::new(output_file); } - if let Some(run_directory) = rustdoc_options.test_run_directory { + if let Some(run_directory) = &rustdoc_options.test_run_directory { cmd.current_dir(run_directory); } @@ -923,20 +926,9 @@ fn partition_source(s: &str, edition: Edition) -> (String, String, String) { } pub(crate) struct IndividualTestOptions { - test_builder: Option, - test_builder_wrappers: Vec, - is_json_unused_externs_enabled: bool, - should_persist_doctests: bool, - error_format: ErrorOutputType, - test_run_directory: Option, - nocapture: bool, arg_file: PathBuf, outdir: DirState, - runtool: Option, - runtool_args: Vec, - target: TargetTriple, test_id: String, - maybe_sysroot: Option, } impl IndividualTestOptions { @@ -955,22 +947,7 @@ impl IndividualTestOptions { DirState::Temp(get_doctest_dir().expect("rustdoc needs a tempdir")) }; - Self { - test_builder: options.test_builder.clone(), - test_builder_wrappers: options.test_builder_wrappers.clone(), - is_json_unused_externs_enabled: options.json_unused_externs.is_enabled(), - should_persist_doctests: options.persist_doctests.is_none(), - error_format: options.error_format, - test_run_directory: options.test_run_directory.clone(), - nocapture: options.nocapture, - arg_file: arg_file.into(), - outdir, - runtool: options.runtool.clone(), - runtool_args: options.runtool_args.clone(), - target: options.target.clone(), - test_id, - maybe_sysroot: options.maybe_sysroot.clone(), - } + Self { arg_file: arg_file.into(), outdir, test_id } } } @@ -994,7 +971,7 @@ pub(crate) trait DoctestVisitor { pub(crate) struct CreateRunnableDoctests { pub(crate) tests: Vec, - rustdoc_options: RustdocOptions, + rustdoc_options: Arc, crate_name: String, opts: GlobalTestOptions, visited_tests: FxHashMap<(String, usize), usize>, @@ -1012,7 +989,7 @@ impl CreateRunnableDoctests { ) -> CreateRunnableDoctests { CreateRunnableDoctests { tests: Vec::new(), - rustdoc_options, + rustdoc_options: Arc::new(rustdoc_options), crate_name, opts, visited_tests: FxHashMap::default(), @@ -1077,6 +1054,7 @@ impl CreateRunnableDoctests { }, ); + let rustdoc_options = self.rustdoc_options.clone(); let rustdoc_test_options = IndividualTestOptions::new(&self.rustdoc_options, &self.arg_file, test_id); @@ -1112,6 +1090,7 @@ impl CreateRunnableDoctests { path, scraped_test: test, }, + rustdoc_options, unused_externs, ) })), @@ -1132,6 +1111,7 @@ struct RunnableDoctest { fn doctest_run_fn( runnable_test: RunnableDoctest, + rustdoc_options: Arc, unused_externs: Arc>>, ) -> Result<(), String> { let report_unused_externs = |uext| { @@ -1141,6 +1121,7 @@ fn doctest_run_fn( &runnable_test.scraped_test.text, &runnable_test.crate_name, runnable_test.scraped_test.line, + &rustdoc_options, runnable_test.rustdoc_test_options, runnable_test.scraped_test.langstr, runnable_test.no_run, From 790b7e9cbfb6aa591520856b018402d62eceb690 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 31 May 2024 16:56:04 -0700 Subject: [PATCH 09/17] rustdoc: Remove `DoctestVisitor::get_line` This was used to get the line number of the first line from the current docstring, which was then used together with an offset within the docstring. It's simpler to just pass the offset to the visitor and have it do the math because it's clearer and this calculation only needs to be done in one place (the Rust doctest visitor). --- src/librustdoc/doctest.rs | 11 ++++----- src/librustdoc/doctest/markdown.rs | 10 ++++---- src/librustdoc/doctest/rust.rs | 17 ++++++------- src/librustdoc/html/markdown.rs | 24 ++++++++++++++++++- .../passes/check_doc_test_visibility.rs | 4 ++-- 5 files changed, 42 insertions(+), 24 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index f1a4df52a59b9..dcb350748b64b 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -34,7 +34,7 @@ use std::sync::{Arc, Mutex}; use tempfile::{Builder as TempFileBuilder, TempDir}; use crate::config::Options as RustdocOptions; -use crate::html::markdown::{ErrorCodes, Ignore, LangString}; +use crate::html::markdown::{ErrorCodes, Ignore, LangString, MdRelLine}; use crate::lint::init_lints; use self::rust::HirCollector; @@ -961,10 +961,7 @@ struct ScrapedDoctest { } pub(crate) trait DoctestVisitor { - fn visit_test(&mut self, test: String, config: LangString, line: usize); - fn get_line(&self) -> usize { - 0 - } + fn visit_test(&mut self, test: String, config: LangString, rel_line: MdRelLine); fn visit_header(&mut self, _name: &str, _level: u32) {} } @@ -1187,8 +1184,8 @@ fn doctest_run_fn( #[cfg(test)] // used in tests impl DoctestVisitor for Vec { - fn visit_test(&mut self, _test: String, _config: LangString, line: usize) { - self.push(line); + fn visit_test(&mut self, _test: String, _config: LangString, rel_line: MdRelLine) { + self.push(1 + rel_line.offset()); } } diff --git a/src/librustdoc/doctest/markdown.rs b/src/librustdoc/doctest/markdown.rs index 5d0dc5926e88b..5fc5f59036bf8 100644 --- a/src/librustdoc/doctest/markdown.rs +++ b/src/librustdoc/doctest/markdown.rs @@ -9,7 +9,7 @@ use super::{ generate_args_file, CreateRunnableDoctests, DoctestVisitor, GlobalTestOptions, ScrapedDoctest, }; use crate::config::Options; -use crate::html::markdown::{find_testable_code, ErrorCodes, LangString}; +use crate::html::markdown::{find_testable_code, ErrorCodes, LangString, MdRelLine}; struct MdCollector { tests: Vec, @@ -18,8 +18,10 @@ struct MdCollector { } impl DoctestVisitor for MdCollector { - fn visit_test(&mut self, test: String, config: LangString, line: usize) { + fn visit_test(&mut self, test: String, config: LangString, rel_line: MdRelLine) { let filename = self.filename.clone(); + // First line of Markdown is line 1. + let line = 1 + rel_line.offset(); self.tests.push(ScrapedDoctest { filename, line, @@ -29,10 +31,6 @@ impl DoctestVisitor for MdCollector { }); } - fn get_line(&self) -> usize { - 0 - } - fn visit_header(&mut self, name: &str, level: u32) { // We use these headings as test names, so it's good if // they're valid identifiers. diff --git a/src/librustdoc/doctest/rust.rs b/src/librustdoc/doctest/rust.rs index 458c90881c935..e6bef395fa9ce 100644 --- a/src/librustdoc/doctest/rust.rs +++ b/src/librustdoc/doctest/rust.rs @@ -15,7 +15,7 @@ use rustc_span::{BytePos, FileName, Pos, Span, DUMMY_SP}; use super::{DoctestVisitor, ScrapedDoctest}; use crate::clean::{types::AttributesExt, Attributes}; -use crate::html::markdown::{self, ErrorCodes, LangString}; +use crate::html::markdown::{self, ErrorCodes, LangString, MdRelLine}; struct RustCollector { source_map: Lrc, @@ -36,10 +36,17 @@ impl RustCollector { } filename } + + fn get_base_line(&self) -> usize { + let sp_lo = self.position.lo().to_usize(); + let loc = self.source_map.lookup_char_pos(BytePos(sp_lo as u32)); + loc.line + } } impl DoctestVisitor for RustCollector { - fn visit_test(&mut self, test: String, config: LangString, line: usize) { + fn visit_test(&mut self, test: String, config: LangString, rel_line: MdRelLine) { + let line = self.get_base_line() + rel_line.offset(); self.tests.push(ScrapedDoctest { filename: self.get_filename(), line, @@ -49,12 +56,6 @@ impl DoctestVisitor for RustCollector { }); } - fn get_line(&self) -> usize { - let line = self.position.lo().to_usize(); - let line = self.source_map.lookup_char_pos(BytePos(line as u32)).line; - if line > 0 { line - 1 } else { line } - } - fn visit_header(&mut self, _name: &str, _level: u32) {} } diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index c8973679132c5..17aa0ecf23dc8 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -710,6 +710,28 @@ impl<'a, I: Iterator>> Iterator for Footnotes<'a, I> { } } +/// A newtype that represents a relative line number in Markdown. +/// +/// In other words, this represents an offset from the first line of Markdown +/// in a doc comment or other source. If the first Markdown line appears on line 32, +/// and the `MdRelLine` is 3, then the absolute line for this one is 35. I.e., it's +/// a zero-based offset. +pub(crate) struct MdRelLine { + offset: usize, +} + +impl MdRelLine { + /// See struct docs. + pub(crate) const fn new(offset: usize) -> Self { + Self { offset } + } + + /// See struct docs. + pub(crate) const fn offset(self) -> usize { + self.offset + } +} + pub(crate) fn find_testable_code( doc: &str, tests: &mut T, @@ -772,7 +794,7 @@ pub(crate) fn find_codes( if nb_lines != 0 && !&doc[prev_offset..offset.start].ends_with('\n') { nb_lines -= 1; } - let line = tests.get_line() + nb_lines + 1; + let line = MdRelLine::new(nb_lines); tests.visit_test(text, block_info, line); prev_offset = offset.start; } diff --git a/src/librustdoc/passes/check_doc_test_visibility.rs b/src/librustdoc/passes/check_doc_test_visibility.rs index a4cdb488f05f8..0437f5e5fd818 100644 --- a/src/librustdoc/passes/check_doc_test_visibility.rs +++ b/src/librustdoc/passes/check_doc_test_visibility.rs @@ -10,7 +10,7 @@ use crate::clean; use crate::clean::utils::inherits_doc_hidden; use crate::clean::*; use crate::core::DocContext; -use crate::html::markdown::{find_testable_code, ErrorCodes, Ignore, LangString}; +use crate::html::markdown::{find_testable_code, ErrorCodes, Ignore, LangString, MdRelLine}; use crate::visit::DocVisitor; use rustc_hir as hir; use rustc_middle::lint::LintLevelSource; @@ -45,7 +45,7 @@ pub(crate) struct Tests { } impl crate::doctest::DoctestVisitor for Tests { - fn visit_test(&mut self, _: String, config: LangString, _: usize) { + fn visit_test(&mut self, _: String, config: LangString, _: MdRelLine) { if config.rust && config.ignore == Ignore::None { self.found_tests += 1; } From e9e26345475f08e4c5d9be597eddcc9b437b9a1a Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 3 Jun 2024 20:32:28 -0700 Subject: [PATCH 10/17] Make two fields computed on-demand --- src/librustdoc/doctest.rs | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index dcb350748b64b..1d027fbd15d88 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -960,6 +960,16 @@ struct ScrapedDoctest { text: String, } +impl ScrapedDoctest { + fn edition(&self, opts: &RustdocOptions) -> Edition { + self.langstr.edition.unwrap_or(opts.edition) + } + + fn no_run(&self, opts: &RustdocOptions) -> bool { + self.langstr.no_run || opts.no_run + } +} + pub(crate) trait DoctestVisitor { fn visit_test(&mut self, test: String, config: LangString, rel_line: MdRelLine); fn visit_header(&mut self, _name: &str, _level: u32) {} @@ -1009,10 +1019,8 @@ impl CreateRunnableDoctests { let name = self.generate_name(&test.filename, test.line, &test.logical_path); let crate_name = self.crate_name.clone(); let opts = self.opts.clone(); - let edition = test.langstr.edition.unwrap_or(self.rustdoc_options.edition); let target_str = self.rustdoc_options.target.to_string(); let unused_externs = self.unused_extern_reports.clone(); - let no_run = test.langstr.no_run || self.rustdoc_options.no_run; if !test.langstr.compile_fail { self.compiling_test_count.fetch_add(1, Ordering::SeqCst); } @@ -1073,7 +1081,7 @@ impl CreateRunnableDoctests { // compiler failures are test failures should_panic: test::ShouldPanic::No, compile_fail: test.langstr.compile_fail, - no_run, + no_run: test.no_run(&rustdoc_options), test_type: test::TestType::DocTest, }, testfn: test::DynTestFn(Box::new(move || { @@ -1081,9 +1089,7 @@ impl CreateRunnableDoctests { RunnableDoctest { crate_name, rustdoc_test_options, - no_run, opts, - edition, path, scraped_test: test, }, @@ -1099,9 +1105,7 @@ impl CreateRunnableDoctests { struct RunnableDoctest { crate_name: String, rustdoc_test_options: IndividualTestOptions, - no_run: bool, opts: GlobalTestOptions, - edition: Edition, path: PathBuf, scraped_test: ScrapedDoctest, } @@ -1114,6 +1118,8 @@ fn doctest_run_fn( let report_unused_externs = |uext| { unused_externs.lock().unwrap().push(uext); }; + let no_run = runnable_test.scraped_test.no_run(&rustdoc_options); + let edition = runnable_test.scraped_test.edition(&rustdoc_options); let res = run_test( &runnable_test.scraped_test.text, &runnable_test.crate_name, @@ -1121,9 +1127,9 @@ fn doctest_run_fn( &rustdoc_options, runnable_test.rustdoc_test_options, runnable_test.scraped_test.langstr, - runnable_test.no_run, + no_run, &runnable_test.opts, - runnable_test.edition, + edition, runnable_test.path, report_unused_externs, ); From a429afacbc4c6a753af6b526e74f19b7caedaaad Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 3 Jun 2024 22:49:54 -0700 Subject: [PATCH 11/17] Remove `RunnableDoctest` It should instead be the actual input to the running logic. Currently it's not actually quite runnable since it's still missing some information. --- src/librustdoc/doctest.rs | 41 ++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 1d027fbd15d88..043d70dc6aac6 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -1086,13 +1086,11 @@ impl CreateRunnableDoctests { }, testfn: test::DynTestFn(Box::new(move || { doctest_run_fn( - RunnableDoctest { - crate_name, - rustdoc_test_options, - opts, - path, - scraped_test: test, - }, + crate_name, + rustdoc_test_options, + opts, + path, + test, rustdoc_options, unused_externs, ) @@ -1101,36 +1099,31 @@ impl CreateRunnableDoctests { } } -/// A doctest that is ready to run. -struct RunnableDoctest { +fn doctest_run_fn( crate_name: String, - rustdoc_test_options: IndividualTestOptions, - opts: GlobalTestOptions, + test_opts: IndividualTestOptions, + global_opts: GlobalTestOptions, path: PathBuf, scraped_test: ScrapedDoctest, -} - -fn doctest_run_fn( - runnable_test: RunnableDoctest, rustdoc_options: Arc, unused_externs: Arc>>, ) -> Result<(), String> { let report_unused_externs = |uext| { unused_externs.lock().unwrap().push(uext); }; - let no_run = runnable_test.scraped_test.no_run(&rustdoc_options); - let edition = runnable_test.scraped_test.edition(&rustdoc_options); + let no_run = scraped_test.no_run(&rustdoc_options); + let edition = scraped_test.edition(&rustdoc_options); let res = run_test( - &runnable_test.scraped_test.text, - &runnable_test.crate_name, - runnable_test.scraped_test.line, + &scraped_test.text, + &crate_name, + scraped_test.line, &rustdoc_options, - runnable_test.rustdoc_test_options, - runnable_test.scraped_test.langstr, + test_opts, + scraped_test.langstr, no_run, - &runnable_test.opts, + &global_opts, edition, - runnable_test.path, + path, report_unused_externs, ); From 366000dc07340abfeba6de1d9e7db0913304fa59 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 3 Jun 2024 23:16:40 -0700 Subject: [PATCH 12/17] Move some arguments to fields and reorganize fields I moved some local arguments and options to either the local options struct or, if it made sense, the global options struct. --- src/librustdoc/doctest.rs | 74 ++++++++++++------------------ src/librustdoc/doctest/markdown.rs | 26 ++++++----- src/librustdoc/html/markdown.rs | 12 ++++- 3 files changed, 54 insertions(+), 58 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 043d70dc6aac6..5e3dc4200b7b7 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -40,8 +40,10 @@ use crate::lint::init_lints; use self::rust::HirCollector; /// Options that apply to all doctests in a crate or Markdown file (for `rustdoc foo.md`). -#[derive(Clone, Default)] +#[derive(Clone)] pub(crate) struct GlobalTestOptions { + /// Name of the crate (for regular `rustdoc`) or Markdown file (for `rustdoc foo.md`). + pub(crate) crate_name: String, /// Whether to disable the default `extern crate my_crate;` when creating doctests. pub(crate) no_crate_inject: bool, /// Whether inserting extra indent spaces in code block, @@ -49,6 +51,8 @@ pub(crate) struct GlobalTestOptions { pub(crate) insert_indent_space: bool, /// Additional crate-level attributes to add to doctests. pub(crate) attrs: Vec, + /// Path to file containing arguments for the invocation of rustc. + pub(crate) args_file: PathBuf, } pub(crate) fn generate_args_file(file_path: &Path, options: &RustdocOptions) -> Result<(), String> { @@ -167,24 +171,19 @@ pub(crate) fn run( Ok(temp_dir) => temp_dir, Err(error) => return crate::wrap_return(dcx, Err(error)), }; - let file_path = temp_dir.path().join("rustdoc-cfgs"); - crate::wrap_return(dcx, generate_args_file(&file_path, &options))?; + let args_path = temp_dir.path().join("rustdoc-cfgs"); + crate::wrap_return(dcx, generate_args_file(&args_path, &options))?; let (tests, unused_extern_reports, compiling_test_count) = interface::run_compiler(config, |compiler| { compiler.enter(|queries| { let collector = queries.global_ctxt()?.enter(|tcx| { + let crate_name = tcx.crate_name(LOCAL_CRATE).to_string(); let crate_attrs = tcx.hir().attrs(CRATE_HIR_ID); - - let opts = scrape_test_config(crate_attrs); + let opts = scrape_test_config(crate_name, crate_attrs, args_path); let enable_per_target_ignores = options.enable_per_target_ignores; - let mut collector = CreateRunnableDoctests::new( - tcx.crate_name(LOCAL_CRATE).to_string(), - options, - opts, - file_path, - ); + let mut collector = CreateRunnableDoctests::new(options, opts); let hir_collector = HirCollector::new( &compiler.sess, tcx.hir(), @@ -264,11 +263,20 @@ pub(crate) fn run_tests( } // Look for `#![doc(test(no_crate_inject))]`, used by crates in the std facade. -fn scrape_test_config(attrs: &[ast::Attribute]) -> GlobalTestOptions { +fn scrape_test_config( + crate_name: String, + attrs: &[ast::Attribute], + args_file: PathBuf, +) -> GlobalTestOptions { use rustc_ast_pretty::pprust; - let mut opts = - GlobalTestOptions { no_crate_inject: false, attrs: Vec::new(), insert_indent_space: false }; + let mut opts = GlobalTestOptions { + crate_name, + no_crate_inject: false, + attrs: Vec::new(), + insert_indent_space: false, + args_file, + }; let test_attrs: Vec<_> = attrs .iter() @@ -363,7 +371,6 @@ fn wrapped_rustc_command(rustc_wrappers: &[PathBuf], rustc_binary: &Path) -> Com fn run_test( test: &str, - crate_name: &str, line: usize, rustdoc_options: &RustdocOptions, test_options: IndividualTestOptions, @@ -371,12 +378,11 @@ fn run_test( no_run: bool, opts: &GlobalTestOptions, edition: Edition, - path: PathBuf, report_unused_externs: impl Fn(UnusedExterns), ) -> Result<(), TestFailure> { let (test, line_offset, supports_color) = make_test( test, - Some(crate_name), + Some(&opts.crate_name), lang_string.test_harness, opts, edition, @@ -393,14 +399,14 @@ fn run_test( .unwrap_or_else(|| rustc_interface::util::rustc_path().expect("found rustc")); let mut compiler = wrapped_rustc_command(&rustdoc_options.test_builder_wrappers, rustc_binary); - compiler.arg(&format!("@{}", test_options.arg_file.display())); + compiler.arg(&format!("@{}", opts.args_file.display())); if let Some(sysroot) = &rustdoc_options.maybe_sysroot { compiler.arg(format!("--sysroot={}", sysroot.display())); } compiler.arg("--edition").arg(&edition.to_string()); - compiler.env("UNSTABLE_RUSTDOC_TEST_PATH", path); + compiler.env("UNSTABLE_RUSTDOC_TEST_PATH", &test_options.path); compiler.env("UNSTABLE_RUSTDOC_TEST_LINE", format!("{}", line as isize - line_offset as isize)); compiler.arg("-o").arg(&output_file); if lang_string.test_harness { @@ -926,13 +932,13 @@ fn partition_source(s: &str, edition: Edition) -> (String, String, String) { } pub(crate) struct IndividualTestOptions { - arg_file: PathBuf, outdir: DirState, test_id: String, + path: PathBuf, } impl IndividualTestOptions { - fn new(options: &RustdocOptions, arg_file: &Path, test_id: String) -> Self { + fn new(options: &RustdocOptions, test_id: String, test_path: PathBuf) -> Self { let outdir = if let Some(ref path) = options.persist_doctests { let mut path = path.clone(); path.push(&test_id); @@ -947,7 +953,7 @@ impl IndividualTestOptions { DirState::Temp(get_doctest_dir().expect("rustdoc needs a tempdir")) }; - Self { arg_file: arg_file.into(), outdir, test_id } + Self { outdir, test_id, path: test_path } } } @@ -979,30 +985,24 @@ pub(crate) struct CreateRunnableDoctests { pub(crate) tests: Vec, rustdoc_options: Arc, - crate_name: String, opts: GlobalTestOptions, visited_tests: FxHashMap<(String, usize), usize>, unused_extern_reports: Arc>>, compiling_test_count: AtomicUsize, - arg_file: PathBuf, } impl CreateRunnableDoctests { pub(crate) fn new( - crate_name: String, rustdoc_options: RustdocOptions, opts: GlobalTestOptions, - arg_file: PathBuf, ) -> CreateRunnableDoctests { CreateRunnableDoctests { tests: Vec::new(), rustdoc_options: Arc::new(rustdoc_options), - crate_name, opts, visited_tests: FxHashMap::default(), unused_extern_reports: Default::default(), compiling_test_count: AtomicUsize::new(0), - arg_file, } } @@ -1017,7 +1017,6 @@ impl CreateRunnableDoctests { fn add_test(&mut self, test: ScrapedDoctest) { let name = self.generate_name(&test.filename, test.line, &test.logical_path); - let crate_name = self.crate_name.clone(); let opts = self.opts.clone(); let target_str = self.rustdoc_options.target.to_string(); let unused_externs = self.unused_extern_reports.clone(); @@ -1060,8 +1059,7 @@ impl CreateRunnableDoctests { ); let rustdoc_options = self.rustdoc_options.clone(); - let rustdoc_test_options = - IndividualTestOptions::new(&self.rustdoc_options, &self.arg_file, test_id); + let rustdoc_test_options = IndividualTestOptions::new(&self.rustdoc_options, test_id, path); debug!("creating test {name}: {}", test.text); self.tests.push(test::TestDescAndFn { @@ -1085,25 +1083,15 @@ impl CreateRunnableDoctests { test_type: test::TestType::DocTest, }, testfn: test::DynTestFn(Box::new(move || { - doctest_run_fn( - crate_name, - rustdoc_test_options, - opts, - path, - test, - rustdoc_options, - unused_externs, - ) + doctest_run_fn(rustdoc_test_options, opts, test, rustdoc_options, unused_externs) })), }); } } fn doctest_run_fn( - crate_name: String, test_opts: IndividualTestOptions, global_opts: GlobalTestOptions, - path: PathBuf, scraped_test: ScrapedDoctest, rustdoc_options: Arc, unused_externs: Arc>>, @@ -1115,7 +1103,6 @@ fn doctest_run_fn( let edition = scraped_test.edition(&rustdoc_options); let res = run_test( &scraped_test.text, - &crate_name, scraped_test.line, &rustdoc_options, test_opts, @@ -1123,7 +1110,6 @@ fn doctest_run_fn( no_run, &global_opts, edition, - path, report_unused_externs, ); diff --git a/src/librustdoc/doctest/markdown.rs b/src/librustdoc/doctest/markdown.rs index 5fc5f59036bf8..b8ab7adb36e8c 100644 --- a/src/librustdoc/doctest/markdown.rs +++ b/src/librustdoc/doctest/markdown.rs @@ -73,7 +73,7 @@ impl DoctestVisitor for MdCollector { } } -/// Runs any tests/code examples in the markdown file `input`. +/// Runs any tests/code examples in the markdown file `options.input`. pub(crate) fn test(options: Options) -> Result<(), String> { use rustc_session::config::Input; let input_str = match &options.input { @@ -83,13 +83,20 @@ pub(crate) fn test(options: Options) -> Result<(), String> { Input::Str { name: _, input } => input.clone(), }; - let mut opts = GlobalTestOptions::default(); - opts.no_crate_inject = true; - + // Obviously not a real crate name, but close enough for purposes of doctests. + let crate_name = options.input.filestem().to_string(); let temp_dir = tempdir().map_err(|error| format!("failed to create temporary directory: {error:?}"))?; - let file_path = temp_dir.path().join("rustdoc-cfgs"); - generate_args_file(&file_path, &options)?; + let args_file = temp_dir.path().join("rustdoc-cfgs"); + generate_args_file(&args_file, &options)?; + + let opts = GlobalTestOptions { + crate_name, + no_crate_inject: true, + insert_indent_space: false, + attrs: vec![], + args_file, + }; let mut md_collector = MdCollector { tests: vec![], @@ -111,12 +118,7 @@ pub(crate) fn test(options: Options) -> Result<(), String> { None, ); - let mut collector = CreateRunnableDoctests::new( - options.input.filestem().to_string(), - options.clone(), - opts, - file_path, - ); + 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.tests); Ok(()) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 17aa0ecf23dc8..bae929c64eab2 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -39,6 +39,7 @@ use std::collections::VecDeque; use std::fmt::Write; use std::iter::Peekable; use std::ops::{ControlFlow, Range}; +use std::path::PathBuf; use std::str::{self, CharIndices}; use std::sync::OnceLock; @@ -287,8 +288,15 @@ impl<'a, I: Iterator>> Iterator for CodeBlocks<'_, 'a, I> { .collect::(); let krate = krate.as_ref().map(|s| s.as_str()); - let mut opts: GlobalTestOptions = Default::default(); - opts.insert_indent_space = true; + // FIXME: separate out the code to make a code block into runnable code + // from the complicated doctest logic + let opts = GlobalTestOptions { + crate_name: krate.map(String::from).unwrap_or_default(), + no_crate_inject: false, + insert_indent_space: true, + attrs: vec![], + args_file: PathBuf::new(), + }; let (test, _, _) = doctest::make_test(&test, krate, false, &opts, edition, None); let channel = if test.contains("#![feature(") { "&version=nightly" } else { "" }; From 0dc72d95514d3d11eca2e430bbd206ceb0c8b135 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 3 Jun 2024 23:47:12 -0700 Subject: [PATCH 13/17] Make doctests before running them; reintroduce `RunnableDoctest` --- src/librustdoc/doctest.rs | 96 ++++++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 42 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 5e3dc4200b7b7..c7622d5b53944 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -369,29 +369,25 @@ fn wrapped_rustc_command(rustc_wrappers: &[PathBuf], rustc_binary: &Path) -> Com command } +struct RunnableDoctest { + full_test_code: String, + full_test_line_offset: usize, + test_opts: IndividualTestOptions, + global_opts: GlobalTestOptions, + scraped_test: ScrapedDoctest, +} + fn run_test( - test: &str, - line: usize, + doctest: RunnableDoctest, rustdoc_options: &RustdocOptions, - test_options: IndividualTestOptions, - mut lang_string: LangString, - no_run: bool, - opts: &GlobalTestOptions, - edition: Edition, + supports_color: bool, report_unused_externs: impl Fn(UnusedExterns), ) -> Result<(), TestFailure> { - let (test, line_offset, supports_color) = make_test( - test, - Some(&opts.crate_name), - lang_string.test_harness, - opts, - edition, - Some(&test_options.test_id), - ); - + let scraped_test = &doctest.scraped_test; + let langstr = &scraped_test.langstr; // Make sure we emit well-formed executable names for our target. let rust_out = add_exe_suffix("rust_out".to_owned(), &rustdoc_options.target); - let output_file = test_options.outdir.path().join(rust_out); + let output_file = doctest.test_opts.outdir.path().join(rust_out); let rustc_binary = rustdoc_options .test_builder @@ -399,27 +395,33 @@ fn run_test( .unwrap_or_else(|| rustc_interface::util::rustc_path().expect("found rustc")); let mut compiler = wrapped_rustc_command(&rustdoc_options.test_builder_wrappers, rustc_binary); - compiler.arg(&format!("@{}", opts.args_file.display())); + compiler.arg(&format!("@{}", doctest.global_opts.args_file.display())); if let Some(sysroot) = &rustdoc_options.maybe_sysroot { compiler.arg(format!("--sysroot={}", sysroot.display())); } - compiler.arg("--edition").arg(&edition.to_string()); - compiler.env("UNSTABLE_RUSTDOC_TEST_PATH", &test_options.path); - compiler.env("UNSTABLE_RUSTDOC_TEST_LINE", format!("{}", line as isize - line_offset as isize)); + compiler.arg("--edition").arg(&scraped_test.edition(rustdoc_options).to_string()); + compiler.env("UNSTABLE_RUSTDOC_TEST_PATH", &doctest.test_opts.path); + compiler.env( + "UNSTABLE_RUSTDOC_TEST_LINE", + format!("{}", scraped_test.line as isize - doctest.full_test_line_offset as isize), + ); compiler.arg("-o").arg(&output_file); - if lang_string.test_harness { + if langstr.test_harness { compiler.arg("--test"); } - if rustdoc_options.json_unused_externs.is_enabled() && !lang_string.compile_fail { + if rustdoc_options.json_unused_externs.is_enabled() && !langstr.compile_fail { compiler.arg("--error-format=json"); compiler.arg("--json").arg("unused-externs"); compiler.arg("-W").arg("unused_crate_dependencies"); compiler.arg("-Z").arg("unstable-options"); } - if no_run && !lang_string.compile_fail && rustdoc_options.persist_doctests.is_none() { + if scraped_test.no_run(rustdoc_options) + && !langstr.compile_fail + && rustdoc_options.persist_doctests.is_none() + { // FIXME: why does this code check if it *shouldn't* persist doctests // -- shouldn't it be the negation? compiler.arg("--emit=metadata"); @@ -459,7 +461,7 @@ fn run_test( let mut child = compiler.spawn().expect("Failed to spawn rustc process"); { let stdin = child.stdin.as_mut().expect("Failed to open stdin"); - stdin.write_all(test.as_bytes()).expect("could write out test sources"); + stdin.write_all(doctest.full_test_code.as_bytes()).expect("could write out test sources"); } let output = child.wait_with_output().expect("Failed to read stdout"); @@ -490,20 +492,26 @@ fn run_test( } let _bomb = Bomb(&out); - match (output.status.success(), lang_string.compile_fail) { + match (output.status.success(), langstr.compile_fail) { (true, true) => { return Err(TestFailure::UnexpectedCompilePass); } (true, false) => {} (false, true) => { - if !lang_string.error_codes.is_empty() { + if !langstr.error_codes.is_empty() { // We used to check if the output contained "error[{}]: " but since we added the // colored output, we can't anymore because of the color escape characters before // the ":". - lang_string.error_codes.retain(|err| !out.contains(&format!("error[{err}]"))); - - if !lang_string.error_codes.is_empty() { - return Err(TestFailure::MissingErrorCodes(lang_string.error_codes)); + let missing_codes: Vec = scraped_test + .langstr + .error_codes + .iter() + .filter(|err| !out.contains(&format!("error[{err}]"))) + .cloned() + .collect(); + + if !missing_codes.is_empty() { + return Err(TestFailure::MissingErrorCodes(missing_codes)); } } } @@ -512,7 +520,7 @@ fn run_test( } } - if no_run { + if scraped_test.no_run(rustdoc_options) { return Ok(()); } @@ -544,9 +552,9 @@ fn run_test( match result { Err(e) => return Err(TestFailure::ExecutionError(e)), Ok(out) => { - if lang_string.should_panic && out.status.success() { + if langstr.should_panic && out.status.success() { return Err(TestFailure::UnexpectedRunPass); - } else if !lang_string.should_panic && !out.status.success() { + } else if !langstr.should_panic && !out.status.success() { return Err(TestFailure::ExecutionFailure(out)); } } @@ -1099,19 +1107,23 @@ fn doctest_run_fn( let report_unused_externs = |uext| { unused_externs.lock().unwrap().push(uext); }; - let no_run = scraped_test.no_run(&rustdoc_options); let edition = scraped_test.edition(&rustdoc_options); - let res = run_test( + let (full_test_code, full_test_line_offset, supports_color) = make_test( &scraped_test.text, - scraped_test.line, - &rustdoc_options, - test_opts, - scraped_test.langstr, - no_run, + Some(&global_opts.crate_name), + scraped_test.langstr.test_harness, &global_opts, edition, - report_unused_externs, + Some(&test_opts.test_id), ); + let runnable_test = RunnableDoctest { + full_test_code, + full_test_line_offset, + test_opts, + global_opts, + scraped_test, + }; + let res = run_test(runnable_test, &rustdoc_options, supports_color, report_unused_externs); if let Err(err) = res { match err { From d06a05e26289ce55114462786faeaf0e7a559cf5 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 3 Jun 2024 23:58:42 -0700 Subject: [PATCH 14/17] Move logic for "making" doctests to submodule This code turns the raw code given by the user into something actually runnable, e.g. by adding a `main` function if it doesn't already exist. I also made a couple other items private that didn't need to be crate-public. --- src/librustdoc/doctest.rs | 381 +-------------------------------- src/librustdoc/doctest/make.rs | 381 +++++++++++++++++++++++++++++++++ 2 files changed, 387 insertions(+), 375 deletions(-) create mode 100644 src/librustdoc/doctest/make.rs diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index c7622d5b53944..a732e645b6baa 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -1,23 +1,19 @@ +mod make; mod markdown; mod rust; +pub(crate) use make::make_test; pub(crate) use markdown::test as test_markdown; use rustc_ast as ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_data_structures::sync::Lrc; -use rustc_errors::emitter::stderr_destination; use rustc_errors::{ColorConfig, ErrorGuaranteed, FatalError}; use rustc_hir::def_id::LOCAL_CRATE; use rustc_hir::CRATE_HIR_ID; use rustc_interface::interface; -use rustc_parse::new_parser_from_source_str; -use rustc_parse::parser::attr::InnerAttrPolicy; use rustc_session::config::{self, CrateType, ErrorOutputType}; use rustc_session::lint; -use rustc_session::parse::ParseSess; use rustc_span::edition::Edition; -use rustc_span::source_map::SourceMap; use rustc_span::symbol::sym; use rustc_span::FileName; use rustc_target::spec::{Target, TargetTriple}; @@ -577,369 +573,7 @@ fn make_maybe_absolute_path(path: PathBuf) -> PathBuf { } } -/// Transforms a test into code that can be compiled into a Rust binary, and returns the number of -/// lines before the test code begins as well as if the output stream supports colors or not. -pub(crate) fn make_test( - s: &str, - crate_name: Option<&str>, - dont_insert_main: bool, - opts: &GlobalTestOptions, - edition: Edition, - test_id: Option<&str>, -) -> (String, usize, bool) { - let (crate_attrs, everything_else, crates) = partition_source(s, edition); - let everything_else = everything_else.trim(); - let mut line_offset = 0; - let mut prog = String::new(); - let mut supports_color = false; - - 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. - prog.push_str("#![allow(unused)]\n"); - line_offset += 1; - } - - // Next, any attributes that came from the crate root via #![doc(test(attr(...)))]. - for attr in &opts.attrs { - prog.push_str(&format!("#![{attr}]\n")); - line_offset += 1; - } - - // Now push any outer attributes from the example, assuming they - // are intended to be crate attributes. - prog.push_str(&crate_attrs); - prog.push_str(&crates); - - // Uses librustc_ast to parse the doctest and find if there's a main fn and the extern - // crate already is included. - let result = rustc_driver::catch_fatal_errors(|| { - rustc_span::create_session_if_not_set_then(edition, |_| { - use rustc_errors::emitter::{Emitter, HumanEmitter}; - use rustc_errors::DiagCtxt; - use rustc_parse::parser::ForceCollect; - use rustc_span::source_map::FilePathMapping; - - let filename = FileName::anon_source_code(s); - let source = crates + everything_else; - - // Any errors in parsing should also appear when the doctest is compiled for real, so just - // send all the errors that librustc_ast emits directly into a `Sink` instead of stderr. - let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); - let fallback_bundle = rustc_errors::fallback_fluent_bundle( - rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), - false, - ); - supports_color = - HumanEmitter::new(stderr_destination(ColorConfig::Auto), fallback_bundle.clone()) - .supports_color(); - - let emitter = HumanEmitter::new(Box::new(io::sink()), fallback_bundle); - - // FIXME(misdreavus): pass `-Z treat-err-as-bug` to the doctest parser - let dcx = DiagCtxt::new(Box::new(emitter)).disable_warnings(); - let psess = ParseSess::with_dcx(dcx, sm); - - let mut found_main = false; - let mut found_extern_crate = crate_name.is_none(); - let mut found_macro = false; - - let mut parser = match new_parser_from_source_str(&psess, filename, source) { - Ok(p) => p, - Err(errs) => { - errs.into_iter().for_each(|err| err.cancel()); - return (found_main, found_extern_crate, found_macro); - } - }; - - loop { - match parser.parse_item(ForceCollect::No) { - Ok(Some(item)) => { - if !found_main - && let ast::ItemKind::Fn(..) = item.kind - && item.ident.name == sym::main - { - found_main = true; - } - - if !found_extern_crate - && let ast::ItemKind::ExternCrate(original) = item.kind - { - // This code will never be reached if `crate_name` is none because - // `found_extern_crate` is initialized to `true` if it is none. - let crate_name = crate_name.unwrap(); - - match original { - Some(name) => found_extern_crate = name.as_str() == crate_name, - None => found_extern_crate = item.ident.as_str() == crate_name, - } - } - - if !found_macro && let ast::ItemKind::MacCall(..) = item.kind { - found_macro = true; - } - - if found_main && found_extern_crate { - break; - } - } - Ok(None) => break, - Err(e) => { - e.cancel(); - break; - } - } - - // The supplied item is only used for diagnostics, - // which are swallowed here anyway. - parser.maybe_consume_incorrect_semicolon(None); - } - - // Reset errors so that they won't be reported as compiler bugs when dropping the - // dcx. Any errors in the tests will be reported when the test file is compiled, - // Note that we still need to cancel the errors above otherwise `Diag` will panic on - // drop. - psess.dcx.reset_err_count(); - - (found_main, found_extern_crate, found_macro) - }) - }); - let Ok((already_has_main, already_has_extern_crate, found_macro)) = result else { - // If the parser panicked due to a fatal error, pass the test code through unchanged. - // The error will be reported during compilation. - return (s.to_owned(), 0, false); - }; - - // If a doctest's `fn main` is being masked by a wrapper macro, the parsing loop above won't - // see it. In that case, run the old text-based scan to see if they at least have a main - // function written inside a macro invocation. See - // https://github.com/rust-lang/rust/issues/56898 - let already_has_main = if found_macro && !already_has_main { - s.lines() - .map(|line| { - let comment = line.find("//"); - if let Some(comment_begins) = comment { &line[0..comment_begins] } else { line } - }) - .any(|code| code.contains("fn main")) - } else { - already_has_main - }; - - // Don't inject `extern crate std` because it's already injected by the - // compiler. - if !already_has_extern_crate && !opts.no_crate_inject && crate_name != Some("std") { - if let Some(crate_name) = crate_name { - // Don't inject `extern crate` if the crate is never used. - // NOTE: this is terribly inaccurate because it doesn't actually - // parse the source, but only has false positives, not false - // negatives. - if s.contains(crate_name) { - // rustdoc implicitly inserts an `extern crate` item for the own crate - // which may be unused, so we need to allow the lint. - prog.push_str("#[allow(unused_extern_crates)]\n"); - - prog.push_str(&format!("extern crate r#{crate_name};\n")); - line_offset += 1; - } - } - } - - // FIXME: This code cannot yet handle no_std test cases yet - if dont_insert_main || already_has_main || prog.contains("![no_std]") { - prog.push_str(everything_else); - } else { - let returns_result = everything_else.trim_end().ends_with("(())"); - // Give each doctest main function a unique name. - // This is for example needed for the tooling around `-C instrument-coverage`. - let inner_fn_name = if let Some(test_id) = test_id { - format!("_doctest_main_{test_id}") - } else { - "_inner".into() - }; - let inner_attr = if test_id.is_some() { "#[allow(non_snake_case)] " } else { "" }; - let (main_pre, main_post) = if returns_result { - ( - format!( - "fn main() {{ {inner_attr}fn {inner_fn_name}() -> Result<(), impl core::fmt::Debug> {{\n", - ), - format!("\n}} {inner_fn_name}().unwrap() }}"), - ) - } else if test_id.is_some() { - ( - format!("fn main() {{ {inner_attr}fn {inner_fn_name}() {{\n",), - format!("\n}} {inner_fn_name}() }}"), - ) - } else { - ("fn main() {\n".into(), "\n}".into()) - }; - // Note on newlines: We insert a line/newline *before*, and *after* - // the doctest and adjust the `line_offset` accordingly. - // In the case of `-C instrument-coverage`, this means that the generated - // inner `main` function spans from the doctest opening codeblock to the - // closing one. For example - // /// ``` <- start of the inner main - // /// <- code under doctest - // /// ``` <- end of the inner main - line_offset += 1; - - // add extra 4 spaces for each line to offset the code block - let content = if opts.insert_indent_space { - everything_else - .lines() - .map(|line| format!(" {}", line)) - .collect::>() - .join("\n") - } else { - everything_else.to_string() - }; - prog.extend([&main_pre, content.as_str(), &main_post].iter().cloned()); - } - - debug!("final doctest:\n{prog}"); - - (prog, line_offset, supports_color) -} - -fn check_if_attr_is_complete(source: &str, edition: Edition) -> bool { - if source.is_empty() { - // Empty content so nothing to check in here... - return true; - } - rustc_driver::catch_fatal_errors(|| { - rustc_span::create_session_if_not_set_then(edition, |_| { - use rustc_errors::emitter::HumanEmitter; - use rustc_errors::DiagCtxt; - use rustc_span::source_map::FilePathMapping; - - let filename = FileName::anon_source_code(source); - // Any errors in parsing should also appear when the doctest is compiled for real, so just - // send all the errors that librustc_ast emits directly into a `Sink` instead of stderr. - let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); - let fallback_bundle = rustc_errors::fallback_fluent_bundle( - rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), - false, - ); - - let emitter = HumanEmitter::new(Box::new(io::sink()), fallback_bundle); - - let dcx = DiagCtxt::new(Box::new(emitter)).disable_warnings(); - let psess = ParseSess::with_dcx(dcx, sm); - let mut parser = match new_parser_from_source_str(&psess, filename, source.to_owned()) { - Ok(p) => p, - Err(errs) => { - errs.into_iter().for_each(|err| err.cancel()); - // If there is an unclosed delimiter, an error will be returned by the - // tokentrees. - return false; - } - }; - // If a parsing error happened, it's very likely that the attribute is incomplete. - if let Err(e) = parser.parse_attribute(InnerAttrPolicy::Permitted) { - e.cancel(); - return false; - } - true - }) - }) - .unwrap_or(false) -} - -fn partition_source(s: &str, edition: Edition) -> (String, String, String) { - #[derive(Copy, Clone, PartialEq)] - enum PartitionState { - Attrs, - Crates, - Other, - } - let mut state = PartitionState::Attrs; - let mut before = String::new(); - let mut crates = String::new(); - let mut after = String::new(); - - let mut mod_attr_pending = String::new(); - - for line in s.lines() { - let trimline = line.trim(); - - // FIXME(misdreavus): if a doc comment is placed on an extern crate statement, it will be - // shunted into "everything else" - match state { - PartitionState::Attrs => { - state = if trimline.starts_with("#![") { - if !check_if_attr_is_complete(line, edition) { - mod_attr_pending = line.to_owned(); - } else { - mod_attr_pending.clear(); - } - PartitionState::Attrs - } else if trimline.chars().all(|c| c.is_whitespace()) - || (trimline.starts_with("//") && !trimline.starts_with("///")) - { - PartitionState::Attrs - } else if trimline.starts_with("extern crate") - || trimline.starts_with("#[macro_use] extern crate") - { - PartitionState::Crates - } else { - // First we check if the previous attribute was "complete"... - if !mod_attr_pending.is_empty() { - // If not, then we append the new line into the pending attribute to check - // if this time it's complete... - mod_attr_pending.push_str(line); - if !trimline.is_empty() - && check_if_attr_is_complete(&mod_attr_pending, edition) - { - // If it's complete, then we can clear the pending content. - mod_attr_pending.clear(); - } - // In any case, this is considered as `PartitionState::Attrs` so it's - // prepended before rustdoc's inserts. - PartitionState::Attrs - } else { - PartitionState::Other - } - }; - } - PartitionState::Crates => { - state = if trimline.starts_with("extern crate") - || trimline.starts_with("#[macro_use] extern crate") - || trimline.chars().all(|c| c.is_whitespace()) - || (trimline.starts_with("//") && !trimline.starts_with("///")) - { - PartitionState::Crates - } else { - PartitionState::Other - }; - } - PartitionState::Other => {} - } - - match state { - PartitionState::Attrs => { - before.push_str(line); - before.push('\n'); - } - PartitionState::Crates => { - crates.push_str(line); - crates.push('\n'); - } - PartitionState::Other => { - after.push_str(line); - after.push('\n'); - } - } - } - - debug!("before:\n{before}"); - debug!("crates:\n{crates}"); - debug!("after:\n{after}"); - - (before, after, crates) -} - -pub(crate) struct IndividualTestOptions { +struct IndividualTestOptions { outdir: DirState, test_id: String, path: PathBuf, @@ -989,8 +623,8 @@ pub(crate) trait DoctestVisitor { fn visit_header(&mut self, _name: &str, _level: u32) {} } -pub(crate) struct CreateRunnableDoctests { - pub(crate) tests: Vec, +struct CreateRunnableDoctests { + tests: Vec, rustdoc_options: Arc, opts: GlobalTestOptions, @@ -1000,10 +634,7 @@ pub(crate) struct CreateRunnableDoctests { } impl CreateRunnableDoctests { - pub(crate) fn new( - rustdoc_options: RustdocOptions, - opts: GlobalTestOptions, - ) -> CreateRunnableDoctests { + fn new(rustdoc_options: RustdocOptions, opts: GlobalTestOptions) -> CreateRunnableDoctests { CreateRunnableDoctests { tests: Vec::new(), rustdoc_options: Arc::new(rustdoc_options), diff --git a/src/librustdoc/doctest/make.rs b/src/librustdoc/doctest/make.rs new file mode 100644 index 0000000000000..1eafad9cfc6e7 --- /dev/null +++ b/src/librustdoc/doctest/make.rs @@ -0,0 +1,381 @@ +//! Logic for transforming the raw code given by the user into something actually +//! runnable, e.g. by adding a `main` function if it doesn't already exist. + +use std::io; + +use rustc_ast as ast; +use rustc_data_structures::sync::Lrc; +use rustc_errors::emitter::stderr_destination; +use rustc_errors::ColorConfig; +use rustc_parse::new_parser_from_source_str; +use rustc_parse::parser::attr::InnerAttrPolicy; +use rustc_session::parse::ParseSess; +use rustc_span::edition::Edition; +use rustc_span::source_map::SourceMap; +use rustc_span::symbol::sym; +use rustc_span::FileName; + +use super::GlobalTestOptions; + +/// Transforms a test into code that can be compiled into a Rust binary, and returns the number of +/// lines before the test code begins as well as if the output stream supports colors or not. +pub(crate) fn make_test( + s: &str, + crate_name: Option<&str>, + dont_insert_main: bool, + opts: &GlobalTestOptions, + edition: Edition, + test_id: Option<&str>, +) -> (String, usize, bool) { + let (crate_attrs, everything_else, crates) = partition_source(s, edition); + let everything_else = everything_else.trim(); + let mut line_offset = 0; + let mut prog = String::new(); + let mut supports_color = false; + + 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. + prog.push_str("#![allow(unused)]\n"); + line_offset += 1; + } + + // Next, any attributes that came from the crate root via #![doc(test(attr(...)))]. + for attr in &opts.attrs { + prog.push_str(&format!("#![{attr}]\n")); + line_offset += 1; + } + + // Now push any outer attributes from the example, assuming they + // are intended to be crate attributes. + prog.push_str(&crate_attrs); + prog.push_str(&crates); + + // Uses librustc_ast to parse the doctest and find if there's a main fn and the extern + // crate already is included. + let result = rustc_driver::catch_fatal_errors(|| { + rustc_span::create_session_if_not_set_then(edition, |_| { + use rustc_errors::emitter::{Emitter, HumanEmitter}; + use rustc_errors::DiagCtxt; + use rustc_parse::parser::ForceCollect; + use rustc_span::source_map::FilePathMapping; + + let filename = FileName::anon_source_code(s); + let source = crates + everything_else; + + // Any errors in parsing should also appear when the doctest is compiled for real, so just + // send all the errors that librustc_ast emits directly into a `Sink` instead of stderr. + let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); + let fallback_bundle = rustc_errors::fallback_fluent_bundle( + rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), + false, + ); + supports_color = + HumanEmitter::new(stderr_destination(ColorConfig::Auto), fallback_bundle.clone()) + .supports_color(); + + let emitter = HumanEmitter::new(Box::new(io::sink()), fallback_bundle); + + // FIXME(misdreavus): pass `-Z treat-err-as-bug` to the doctest parser + let dcx = DiagCtxt::new(Box::new(emitter)).disable_warnings(); + let psess = ParseSess::with_dcx(dcx, sm); + + let mut found_main = false; + let mut found_extern_crate = crate_name.is_none(); + let mut found_macro = false; + + let mut parser = match new_parser_from_source_str(&psess, filename, source) { + Ok(p) => p, + Err(errs) => { + errs.into_iter().for_each(|err| err.cancel()); + return (found_main, found_extern_crate, found_macro); + } + }; + + loop { + match parser.parse_item(ForceCollect::No) { + Ok(Some(item)) => { + if !found_main + && let ast::ItemKind::Fn(..) = item.kind + && item.ident.name == sym::main + { + found_main = true; + } + + if !found_extern_crate + && let ast::ItemKind::ExternCrate(original) = item.kind + { + // This code will never be reached if `crate_name` is none because + // `found_extern_crate` is initialized to `true` if it is none. + let crate_name = crate_name.unwrap(); + + match original { + Some(name) => found_extern_crate = name.as_str() == crate_name, + None => found_extern_crate = item.ident.as_str() == crate_name, + } + } + + if !found_macro && let ast::ItemKind::MacCall(..) = item.kind { + found_macro = true; + } + + if found_main && found_extern_crate { + break; + } + } + Ok(None) => break, + Err(e) => { + e.cancel(); + break; + } + } + + // The supplied item is only used for diagnostics, + // which are swallowed here anyway. + parser.maybe_consume_incorrect_semicolon(None); + } + + // Reset errors so that they won't be reported as compiler bugs when dropping the + // dcx. Any errors in the tests will be reported when the test file is compiled, + // Note that we still need to cancel the errors above otherwise `Diag` will panic on + // drop. + psess.dcx.reset_err_count(); + + (found_main, found_extern_crate, found_macro) + }) + }); + let Ok((already_has_main, already_has_extern_crate, found_macro)) = result else { + // If the parser panicked due to a fatal error, pass the test code through unchanged. + // The error will be reported during compilation. + return (s.to_owned(), 0, false); + }; + + // If a doctest's `fn main` is being masked by a wrapper macro, the parsing loop above won't + // see it. In that case, run the old text-based scan to see if they at least have a main + // function written inside a macro invocation. See + // https://github.com/rust-lang/rust/issues/56898 + let already_has_main = if found_macro && !already_has_main { + s.lines() + .map(|line| { + let comment = line.find("//"); + if let Some(comment_begins) = comment { &line[0..comment_begins] } else { line } + }) + .any(|code| code.contains("fn main")) + } else { + already_has_main + }; + + // Don't inject `extern crate std` because it's already injected by the + // compiler. + if !already_has_extern_crate && !opts.no_crate_inject && crate_name != Some("std") { + if let Some(crate_name) = crate_name { + // Don't inject `extern crate` if the crate is never used. + // NOTE: this is terribly inaccurate because it doesn't actually + // parse the source, but only has false positives, not false + // negatives. + if s.contains(crate_name) { + // rustdoc implicitly inserts an `extern crate` item for the own crate + // which may be unused, so we need to allow the lint. + prog.push_str("#[allow(unused_extern_crates)]\n"); + + prog.push_str(&format!("extern crate r#{crate_name};\n")); + line_offset += 1; + } + } + } + + // FIXME: This code cannot yet handle no_std test cases yet + if dont_insert_main || already_has_main || prog.contains("![no_std]") { + prog.push_str(everything_else); + } else { + let returns_result = everything_else.trim_end().ends_with("(())"); + // Give each doctest main function a unique name. + // This is for example needed for the tooling around `-C instrument-coverage`. + let inner_fn_name = if let Some(test_id) = test_id { + format!("_doctest_main_{test_id}") + } else { + "_inner".into() + }; + let inner_attr = if test_id.is_some() { "#[allow(non_snake_case)] " } else { "" }; + let (main_pre, main_post) = if returns_result { + ( + format!( + "fn main() {{ {inner_attr}fn {inner_fn_name}() -> Result<(), impl core::fmt::Debug> {{\n", + ), + format!("\n}} {inner_fn_name}().unwrap() }}"), + ) + } else if test_id.is_some() { + ( + format!("fn main() {{ {inner_attr}fn {inner_fn_name}() {{\n",), + format!("\n}} {inner_fn_name}() }}"), + ) + } else { + ("fn main() {\n".into(), "\n}".into()) + }; + // Note on newlines: We insert a line/newline *before*, and *after* + // the doctest and adjust the `line_offset` accordingly. + // In the case of `-C instrument-coverage`, this means that the generated + // inner `main` function spans from the doctest opening codeblock to the + // closing one. For example + // /// ``` <- start of the inner main + // /// <- code under doctest + // /// ``` <- end of the inner main + line_offset += 1; + + // add extra 4 spaces for each line to offset the code block + let content = if opts.insert_indent_space { + everything_else + .lines() + .map(|line| format!(" {}", line)) + .collect::>() + .join("\n") + } else { + everything_else.to_string() + }; + prog.extend([&main_pre, content.as_str(), &main_post].iter().cloned()); + } + + debug!("final doctest:\n{prog}"); + + (prog, line_offset, supports_color) +} + +fn check_if_attr_is_complete(source: &str, edition: Edition) -> bool { + if source.is_empty() { + // Empty content so nothing to check in here... + return true; + } + rustc_driver::catch_fatal_errors(|| { + rustc_span::create_session_if_not_set_then(edition, |_| { + use rustc_errors::emitter::HumanEmitter; + use rustc_errors::DiagCtxt; + use rustc_span::source_map::FilePathMapping; + + let filename = FileName::anon_source_code(source); + // Any errors in parsing should also appear when the doctest is compiled for real, so just + // send all the errors that librustc_ast emits directly into a `Sink` instead of stderr. + let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); + let fallback_bundle = rustc_errors::fallback_fluent_bundle( + rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), + false, + ); + + let emitter = HumanEmitter::new(Box::new(io::sink()), fallback_bundle); + + let dcx = DiagCtxt::new(Box::new(emitter)).disable_warnings(); + let psess = ParseSess::with_dcx(dcx, sm); + let mut parser = + match new_parser_from_source_str(&psess, filename, source.to_owned()) { + Ok(p) => p, + Err(errs) => { + errs.into_iter().for_each(|err| err.cancel()); + // If there is an unclosed delimiter, an error will be returned by the + // tokentrees. + return false; + } + }; + // If a parsing error happened, it's very likely that the attribute is incomplete. + if let Err(e) = parser.parse_attribute(InnerAttrPolicy::Permitted) { + e.cancel(); + return false; + } + true + }) + }) + .unwrap_or(false) +} + +fn partition_source(s: &str, edition: Edition) -> (String, String, String) { + #[derive(Copy, Clone, PartialEq)] + enum PartitionState { + Attrs, + Crates, + Other, + } + let mut state = PartitionState::Attrs; + let mut before = String::new(); + let mut crates = String::new(); + let mut after = String::new(); + + let mut mod_attr_pending = String::new(); + + for line in s.lines() { + let trimline = line.trim(); + + // FIXME(misdreavus): if a doc comment is placed on an extern crate statement, it will be + // shunted into "everything else" + match state { + PartitionState::Attrs => { + state = if trimline.starts_with("#![") { + if !check_if_attr_is_complete(line, edition) { + mod_attr_pending = line.to_owned(); + } else { + mod_attr_pending.clear(); + } + PartitionState::Attrs + } else if trimline.chars().all(|c| c.is_whitespace()) + || (trimline.starts_with("//") && !trimline.starts_with("///")) + { + PartitionState::Attrs + } else if trimline.starts_with("extern crate") + || trimline.starts_with("#[macro_use] extern crate") + { + PartitionState::Crates + } else { + // First we check if the previous attribute was "complete"... + if !mod_attr_pending.is_empty() { + // If not, then we append the new line into the pending attribute to check + // if this time it's complete... + mod_attr_pending.push_str(line); + if !trimline.is_empty() + && check_if_attr_is_complete(&mod_attr_pending, edition) + { + // If it's complete, then we can clear the pending content. + mod_attr_pending.clear(); + } + // In any case, this is considered as `PartitionState::Attrs` so it's + // prepended before rustdoc's inserts. + PartitionState::Attrs + } else { + PartitionState::Other + } + }; + } + PartitionState::Crates => { + state = if trimline.starts_with("extern crate") + || trimline.starts_with("#[macro_use] extern crate") + || trimline.chars().all(|c| c.is_whitespace()) + || (trimline.starts_with("//") && !trimline.starts_with("///")) + { + PartitionState::Crates + } else { + PartitionState::Other + }; + } + PartitionState::Other => {} + } + + match state { + PartitionState::Attrs => { + before.push_str(line); + before.push('\n'); + } + PartitionState::Crates => { + crates.push_str(line); + crates.push('\n'); + } + PartitionState::Other => { + after.push_str(line); + after.push('\n'); + } + } + } + + debug!("before:\n{before}"); + debug!("crates:\n{crates}"); + debug!("after:\n{after}"); + + (before, after, crates) +} From 815c447680a984c953d9c00a774aa151651f075f Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Tue, 4 Jun 2024 00:16:44 -0700 Subject: [PATCH 15/17] Parse full doctest source; extract helper for parsing code It doesn't really make sense to skip part of the source when we're parsing it, so parse the whole doctest. This simplifies things too. --- src/librustdoc/doctest/make.rs | 194 ++++++++++++++++++--------------- 1 file changed, 104 insertions(+), 90 deletions(-) diff --git a/src/librustdoc/doctest/make.rs b/src/librustdoc/doctest/make.rs index 1eafad9cfc6e7..90efcbdd8b1b4 100644 --- a/src/librustdoc/doctest/make.rs +++ b/src/librustdoc/doctest/make.rs @@ -6,7 +6,7 @@ use std::io; use rustc_ast as ast; use rustc_data_structures::sync::Lrc; use rustc_errors::emitter::stderr_destination; -use rustc_errors::ColorConfig; +use rustc_errors::{ColorConfig, FatalError}; use rustc_parse::new_parser_from_source_str; use rustc_parse::parser::attr::InnerAttrPolicy; use rustc_session::parse::ParseSess; @@ -55,6 +55,95 @@ pub(crate) fn make_test( // Uses librustc_ast to parse the doctest and find if there's a main fn and the extern // crate already is included. + let Ok((already_has_main, already_has_extern_crate)) = + check_for_main_and_extern_crate(crate_name, s.to_owned(), edition, &mut supports_color) + else { + // If the parser panicked due to a fatal error, pass the test code through unchanged. + // The error will be reported during compilation. + return (s.to_owned(), 0, false); + }; + + // Don't inject `extern crate std` because it's already injected by the + // compiler. + if !already_has_extern_crate && !opts.no_crate_inject && crate_name != Some("std") { + if let Some(crate_name) = crate_name { + // Don't inject `extern crate` if the crate is never used. + // NOTE: this is terribly inaccurate because it doesn't actually + // parse the source, but only has false positives, not false + // negatives. + if s.contains(crate_name) { + // rustdoc implicitly inserts an `extern crate` item for the own crate + // which may be unused, so we need to allow the lint. + prog.push_str("#[allow(unused_extern_crates)]\n"); + + prog.push_str(&format!("extern crate r#{crate_name};\n")); + line_offset += 1; + } + } + } + + // FIXME: This code cannot yet handle no_std test cases yet + if dont_insert_main || already_has_main || prog.contains("![no_std]") { + prog.push_str(everything_else); + } else { + let returns_result = everything_else.trim_end().ends_with("(())"); + // Give each doctest main function a unique name. + // This is for example needed for the tooling around `-C instrument-coverage`. + let inner_fn_name = if let Some(test_id) = test_id { + format!("_doctest_main_{test_id}") + } else { + "_inner".into() + }; + let inner_attr = if test_id.is_some() { "#[allow(non_snake_case)] " } else { "" }; + let (main_pre, main_post) = if returns_result { + ( + format!( + "fn main() {{ {inner_attr}fn {inner_fn_name}() -> Result<(), impl core::fmt::Debug> {{\n", + ), + format!("\n}} {inner_fn_name}().unwrap() }}"), + ) + } else if test_id.is_some() { + ( + format!("fn main() {{ {inner_attr}fn {inner_fn_name}() {{\n",), + format!("\n}} {inner_fn_name}() }}"), + ) + } else { + ("fn main() {\n".into(), "\n}".into()) + }; + // Note on newlines: We insert a line/newline *before*, and *after* + // the doctest and adjust the `line_offset` accordingly. + // In the case of `-C instrument-coverage`, this means that the generated + // inner `main` function spans from the doctest opening codeblock to the + // closing one. For example + // /// ``` <- start of the inner main + // /// <- code under doctest + // /// ``` <- end of the inner main + line_offset += 1; + + // add extra 4 spaces for each line to offset the code block + let content = if opts.insert_indent_space { + everything_else + .lines() + .map(|line| format!(" {}", line)) + .collect::>() + .join("\n") + } else { + everything_else.to_string() + }; + prog.extend([&main_pre, content.as_str(), &main_post].iter().cloned()); + } + + debug!("final doctest:\n{prog}"); + + (prog, line_offset, supports_color) +} + +fn check_for_main_and_extern_crate( + crate_name: Option<&str>, + source: String, + edition: Edition, + supports_color: &mut bool, +) -> Result<(bool, bool), FatalError> { let result = rustc_driver::catch_fatal_errors(|| { rustc_span::create_session_if_not_set_then(edition, |_| { use rustc_errors::emitter::{Emitter, HumanEmitter}; @@ -62,8 +151,7 @@ pub(crate) fn make_test( use rustc_parse::parser::ForceCollect; use rustc_span::source_map::FilePathMapping; - let filename = FileName::anon_source_code(s); - let source = crates + everything_else; + let filename = FileName::anon_source_code(&source); // Any errors in parsing should also appear when the doctest is compiled for real, so just // send all the errors that librustc_ast emits directly into a `Sink` instead of stderr. @@ -72,7 +160,7 @@ pub(crate) fn make_test( rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false, ); - supports_color = + *supports_color = HumanEmitter::new(stderr_destination(ColorConfig::Auto), fallback_bundle.clone()) .supports_color(); @@ -86,13 +174,14 @@ pub(crate) fn make_test( let mut found_extern_crate = crate_name.is_none(); let mut found_macro = false; - let mut parser = match new_parser_from_source_str(&psess, filename, source) { - Ok(p) => p, - Err(errs) => { - errs.into_iter().for_each(|err| err.cancel()); - return (found_main, found_extern_crate, found_macro); - } - }; + let mut parser = + match new_parser_from_source_str(&psess, filename, source.clone()) { + Ok(p) => p, + Err(errs) => { + errs.into_iter().for_each(|err| err.cancel()); + return (found_main, found_extern_crate, found_macro); + } + }; loop { match parser.parse_item(ForceCollect::No) { @@ -146,18 +235,15 @@ pub(crate) fn make_test( (found_main, found_extern_crate, found_macro) }) }); - let Ok((already_has_main, already_has_extern_crate, found_macro)) = result else { - // If the parser panicked due to a fatal error, pass the test code through unchanged. - // The error will be reported during compilation. - return (s.to_owned(), 0, false); - }; + let (already_has_main, already_has_extern_crate, found_macro) = result?; // If a doctest's `fn main` is being masked by a wrapper macro, the parsing loop above won't // see it. In that case, run the old text-based scan to see if they at least have a main // function written inside a macro invocation. See // https://github.com/rust-lang/rust/issues/56898 let already_has_main = if found_macro && !already_has_main { - s.lines() + source + .lines() .map(|line| { let comment = line.find("//"); if let Some(comment_begins) = comment { &line[0..comment_begins] } else { line } @@ -167,79 +253,7 @@ pub(crate) fn make_test( already_has_main }; - // Don't inject `extern crate std` because it's already injected by the - // compiler. - if !already_has_extern_crate && !opts.no_crate_inject && crate_name != Some("std") { - if let Some(crate_name) = crate_name { - // Don't inject `extern crate` if the crate is never used. - // NOTE: this is terribly inaccurate because it doesn't actually - // parse the source, but only has false positives, not false - // negatives. - if s.contains(crate_name) { - // rustdoc implicitly inserts an `extern crate` item for the own crate - // which may be unused, so we need to allow the lint. - prog.push_str("#[allow(unused_extern_crates)]\n"); - - prog.push_str(&format!("extern crate r#{crate_name};\n")); - line_offset += 1; - } - } - } - - // FIXME: This code cannot yet handle no_std test cases yet - if dont_insert_main || already_has_main || prog.contains("![no_std]") { - prog.push_str(everything_else); - } else { - let returns_result = everything_else.trim_end().ends_with("(())"); - // Give each doctest main function a unique name. - // This is for example needed for the tooling around `-C instrument-coverage`. - let inner_fn_name = if let Some(test_id) = test_id { - format!("_doctest_main_{test_id}") - } else { - "_inner".into() - }; - let inner_attr = if test_id.is_some() { "#[allow(non_snake_case)] " } else { "" }; - let (main_pre, main_post) = if returns_result { - ( - format!( - "fn main() {{ {inner_attr}fn {inner_fn_name}() -> Result<(), impl core::fmt::Debug> {{\n", - ), - format!("\n}} {inner_fn_name}().unwrap() }}"), - ) - } else if test_id.is_some() { - ( - format!("fn main() {{ {inner_attr}fn {inner_fn_name}() {{\n",), - format!("\n}} {inner_fn_name}() }}"), - ) - } else { - ("fn main() {\n".into(), "\n}".into()) - }; - // Note on newlines: We insert a line/newline *before*, and *after* - // the doctest and adjust the `line_offset` accordingly. - // In the case of `-C instrument-coverage`, this means that the generated - // inner `main` function spans from the doctest opening codeblock to the - // closing one. For example - // /// ``` <- start of the inner main - // /// <- code under doctest - // /// ``` <- end of the inner main - line_offset += 1; - - // add extra 4 spaces for each line to offset the code block - let content = if opts.insert_indent_space { - everything_else - .lines() - .map(|line| format!(" {}", line)) - .collect::>() - .join("\n") - } else { - everything_else.to_string() - }; - prog.extend([&main_pre, content.as_str(), &main_post].iter().cloned()); - } - - debug!("final doctest:\n{prog}"); - - (prog, line_offset, supports_color) + Ok((already_has_main, already_has_extern_crate)) } fn check_if_attr_is_complete(source: &str, edition: Edition) -> bool { From 3670ad59ade784226515327f2d776f02a6613f2c Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Tue, 4 Jun 2024 01:05:50 -0700 Subject: [PATCH 16/17] Fix broken rustdoc unit tests --- src/librustdoc/doctest/tests.rs | 59 +++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/src/librustdoc/doctest/tests.rs b/src/librustdoc/doctest/tests.rs index 9629acb31eb68..9124ec63267c3 100644 --- a/src/librustdoc/doctest/tests.rs +++ b/src/librustdoc/doctest/tests.rs @@ -1,10 +1,23 @@ +use std::path::PathBuf; + use super::{make_test, GlobalTestOptions}; use rustc_span::edition::DEFAULT_EDITION; +/// Default [`GlobalTestOptions`] for these unit tests. +fn default_global_opts(crate_name: impl Into) -> GlobalTestOptions { + GlobalTestOptions { + crate_name: crate_name.into(), + no_crate_inject: false, + insert_indent_space: false, + attrs: vec![], + args_file: PathBuf::new(), + } +} + #[test] fn make_test_basic() { //basic use: wraps with `fn main`, adds `#![allow(unused)]` - let opts = GlobalTestOptions::default(); + let opts = default_global_opts(""); let input = "assert_eq!(2+2, 4);"; let expected = "#![allow(unused)] fn main() { @@ -19,7 +32,7 @@ assert_eq!(2+2, 4); fn make_test_crate_name_no_use() { // If you give a crate name but *don't* use it within the test, it won't bother inserting // the `extern crate` statement. - let opts = GlobalTestOptions::default(); + let opts = default_global_opts("asdf"); let input = "assert_eq!(2+2, 4);"; let expected = "#![allow(unused)] fn main() { @@ -34,7 +47,7 @@ assert_eq!(2+2, 4); fn make_test_crate_name() { // If you give a crate name and use it within the test, it will insert an `extern crate` // statement before `fn main`. - let opts = GlobalTestOptions::default(); + let opts = default_global_opts("asdf"); let input = "use asdf::qwop; assert_eq!(2+2, 4);"; let expected = "#![allow(unused)] @@ -53,8 +66,7 @@ assert_eq!(2+2, 4); fn make_test_no_crate_inject() { // Even if you do use the crate within the test, setting `opts.no_crate_inject` will skip // adding it anyway. - let opts = - GlobalTestOptions { no_crate_inject: true, attrs: vec![], insert_indent_space: false }; + let opts = GlobalTestOptions { no_crate_inject: true, ..default_global_opts("asdf") }; let input = "use asdf::qwop; assert_eq!(2+2, 4);"; let expected = "#![allow(unused)] @@ -72,7 +84,7 @@ fn make_test_ignore_std() { // Even if you include a crate name, and use it in the doctest, we still won't include an // `extern crate` statement if the crate is "std" -- that's included already by the // compiler! - let opts = GlobalTestOptions::default(); + let opts = default_global_opts("std"); let input = "use std::*; assert_eq!(2+2, 4);"; let expected = "#![allow(unused)] @@ -89,7 +101,7 @@ assert_eq!(2+2, 4); fn make_test_manual_extern_crate() { // When you manually include an `extern crate` statement in your doctest, `make_test` // assumes you've included one for your own crate too. - let opts = GlobalTestOptions::default(); + let opts = default_global_opts("asdf"); let input = "extern crate asdf; use asdf::qwop; assert_eq!(2+2, 4);"; @@ -106,7 +118,7 @@ assert_eq!(2+2, 4); #[test] fn make_test_manual_extern_crate_with_macro_use() { - let opts = GlobalTestOptions::default(); + let opts = default_global_opts("asdf"); let input = "#[macro_use] extern crate asdf; use asdf::qwop; assert_eq!(2+2, 4);"; @@ -125,7 +137,7 @@ assert_eq!(2+2, 4); fn make_test_opts_attrs() { // If you supplied some doctest attributes with `#![doc(test(attr(...)))]`, it will use // those instead of the stock `#![allow(unused)]`. - let mut opts = GlobalTestOptions::default(); + let mut opts = default_global_opts("asdf"); opts.attrs.push("feature(sick_rad)".to_string()); let input = "use asdf::qwop; assert_eq!(2+2, 4);"; @@ -159,7 +171,7 @@ assert_eq!(2+2, 4); fn make_test_crate_attrs() { // Including inner attributes in your doctest will apply them to the whole "crate", pasting // them outside the generated main function. - let opts = GlobalTestOptions::default(); + let opts = default_global_opts(""); let input = "#![feature(sick_rad)] assert_eq!(2+2, 4);"; let expected = "#![allow(unused)] @@ -175,7 +187,7 @@ assert_eq!(2+2, 4); #[test] fn make_test_with_main() { // Including your own `fn main` wrapper lets the test use it verbatim. - let opts = GlobalTestOptions::default(); + let opts = default_global_opts(""); let input = "fn main() { assert_eq!(2+2, 4); }"; @@ -191,7 +203,7 @@ fn main() { #[test] fn make_test_fake_main() { // ... but putting it in a comment will still provide a wrapper. - let opts = GlobalTestOptions::default(); + let opts = default_global_opts(""); let input = "//Ceci n'est pas une `fn main` assert_eq!(2+2, 4);"; let expected = "#![allow(unused)] @@ -207,7 +219,7 @@ assert_eq!(2+2, 4); #[test] fn make_test_dont_insert_main() { // Even with that, if you set `dont_insert_main`, it won't create the `fn main` wrapper. - let opts = GlobalTestOptions::default(); + let opts = default_global_opts(""); let input = "//Ceci n'est pas une `fn main` assert_eq!(2+2, 4);"; let expected = "#![allow(unused)] @@ -219,8 +231,8 @@ assert_eq!(2+2, 4);" } #[test] -fn make_test_issues_21299_33731() { - let opts = GlobalTestOptions::default(); +fn make_test_issues_21299() { + let opts = default_global_opts(""); let input = "// fn main assert_eq!(2+2, 4);"; @@ -234,6 +246,11 @@ assert_eq!(2+2, 4); let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None); assert_eq!((output, len), (expected, 2)); +} + +#[test] +fn make_test_issues_33731() { + let opts = default_global_opts("asdf"); let input = "extern crate hella_qwop; assert_eq!(asdf::foo, 4);"; @@ -253,7 +270,7 @@ assert_eq!(asdf::foo, 4); #[test] fn make_test_main_in_macro() { - let opts = GlobalTestOptions::default(); + let opts = default_global_opts("my_crate"); let input = "#[macro_use] extern crate my_crate; test_wrapper! { fn main() {} @@ -272,7 +289,7 @@ test_wrapper! { #[test] fn make_test_returns_result() { // creates an inner function and unwraps it - let opts = GlobalTestOptions::default(); + let opts = default_global_opts(""); let input = "use std::io; let mut input = String::new(); io::stdin().read_line(&mut input)?; @@ -292,7 +309,7 @@ Ok::<(), io:Error>(()) #[test] fn make_test_named_wrapper() { // creates an inner function with a specific name - let opts = GlobalTestOptions::default(); + let opts = default_global_opts(""); let input = "assert_eq!(2+2, 4);"; let expected = "#![allow(unused)] fn main() { #[allow(non_snake_case)] fn _doctest_main__some_unique_name() { @@ -307,8 +324,7 @@ assert_eq!(2+2, 4); #[test] fn make_test_insert_extra_space() { // will insert indent spaces in the code block if `insert_indent_space` is true - let opts = - GlobalTestOptions { no_crate_inject: false, attrs: vec![], insert_indent_space: true }; + let opts = GlobalTestOptions { insert_indent_space: true, ..default_global_opts("") }; let input = "use std::*; assert_eq!(2+2, 4); eprintln!(\"hello anan\"); @@ -327,8 +343,7 @@ fn main() { #[test] fn make_test_insert_extra_space_fn_main() { // if input already has a fn main, it should insert a space before it - let opts = - GlobalTestOptions { no_crate_inject: false, attrs: vec![], insert_indent_space: true }; + let opts = GlobalTestOptions { insert_indent_space: true, ..default_global_opts("") }; let input = "use std::*; fn main() { assert_eq!(2+2, 4); From 6aab04e9b7d89af51eb96fcc35d7b3dab96f940a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 7 Jun 2024 18:10:31 +0200 Subject: [PATCH 17/17] run fmt --- src/librustdoc/doctest/make.rs | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/librustdoc/doctest/make.rs b/src/librustdoc/doctest/make.rs index 90efcbdd8b1b4..599611407ed8a 100644 --- a/src/librustdoc/doctest/make.rs +++ b/src/librustdoc/doctest/make.rs @@ -174,14 +174,13 @@ fn check_for_main_and_extern_crate( let mut found_extern_crate = crate_name.is_none(); let mut found_macro = false; - let mut parser = - match new_parser_from_source_str(&psess, filename, source.clone()) { - Ok(p) => p, - Err(errs) => { - errs.into_iter().for_each(|err| err.cancel()); - return (found_main, found_extern_crate, found_macro); - } - }; + let mut parser = match new_parser_from_source_str(&psess, filename, source.clone()) { + Ok(p) => p, + Err(errs) => { + errs.into_iter().for_each(|err| err.cancel()); + return (found_main, found_extern_crate, found_macro); + } + }; loop { match parser.parse_item(ForceCollect::No) { @@ -280,16 +279,15 @@ fn check_if_attr_is_complete(source: &str, edition: Edition) -> bool { let dcx = DiagCtxt::new(Box::new(emitter)).disable_warnings(); let psess = ParseSess::with_dcx(dcx, sm); - let mut parser = - match new_parser_from_source_str(&psess, filename, source.to_owned()) { - Ok(p) => p, - Err(errs) => { - errs.into_iter().for_each(|err| err.cancel()); - // If there is an unclosed delimiter, an error will be returned by the - // tokentrees. - return false; - } - }; + let mut parser = match new_parser_from_source_str(&psess, filename, source.to_owned()) { + Ok(p) => p, + Err(errs) => { + errs.into_iter().for_each(|err| err.cancel()); + // If there is an unclosed delimiter, an error will be returned by the + // tokentrees. + return false; + } + }; // If a parsing error happened, it's very likely that the attribute is incomplete. if let Err(e) = parser.parse_attribute(InnerAttrPolicy::Permitted) { e.cancel();