diff --git a/compiler/rustc_attr_parsing/src/attributes/mod.rs b/compiler/rustc_attr_parsing/src/attributes/mod.rs index 639c75d7c5e47..65fb238bf7cec 100644 --- a/compiler/rustc_attr_parsing/src/attributes/mod.rs +++ b/compiler/rustc_attr_parsing/src/attributes/mod.rs @@ -232,7 +232,7 @@ pub(crate) enum AttributeOrder { /// /// Attributes are processed from bottom to top, so this raises a warning/error on all the attributes /// further above the lowest one: - /// ``` + /// ```ignore (illustrative) /// #[stable(since="1.0")] //~ WARNING duplicated attribute /// #[stable(since="2.0")] /// ``` @@ -243,7 +243,7 @@ pub(crate) enum AttributeOrder { /// /// Attributes are processed from bottom to top, so this raises a warning/error on all the attributes /// below the highest one: - /// ``` + /// ```ignore (illustrative) /// #[path="foo.rs"] /// #[path="bar.rs"] //~ WARNING duplicated attribute /// ``` diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 9e32a85e361a5..0fe71a0b9b0a3 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -205,11 +205,6 @@ pub trait Emitter { true } - /// Checks if we can use colors in the current output stream. - fn supports_color(&self) -> bool { - false - } - fn source_map(&self) -> Option<&SourceMap>; fn translator(&self) -> &Translator; diff --git a/src/librustdoc/doctest/make.rs b/src/librustdoc/doctest/make.rs index 6c177f942fb05..941aafff19edc 100644 --- a/src/librustdoc/doctest/make.rs +++ b/src/librustdoc/doctest/make.rs @@ -2,19 +2,18 @@ //! runnable, e.g. by adding a `main` function if it doesn't already exist. use std::fmt::{self, Write as _}; -use std::io; use std::sync::Arc; use rustc_ast::token::{Delimiter, TokenKind}; use rustc_ast::tokenstream::TokenTree; use rustc_ast::{self as ast, AttrStyle, HasAttrs, StmtKind}; -use rustc_errors::emitter::stderr_destination; -use rustc_errors::{AutoStream, ColorConfig, DiagCtxtHandle}; +use rustc_errors::emitter::SilentEmitter; +use rustc_errors::{Diag, DiagCtxt, DiagCtxtHandle}; use rustc_parse::lexer::StripTokens; use rustc_parse::new_parser_from_source_str; use rustc_session::parse::ParseSess; use rustc_span::edition::{DEFAULT_EDITION, Edition}; -use rustc_span::source_map::SourceMap; +use rustc_span::source_map::{FilePathMapping, SourceMap}; use rustc_span::symbol::sym; use rustc_span::{DUMMY_SP, FileName, Span, kw}; use tracing::debug; @@ -140,16 +139,21 @@ impl<'a> BuildDocTestBuilder<'a> { maybe_crate_attrs, })) = result else { - // If the AST returned an error, we don't want this doctest to be merged with the - // others. - return DocTestBuilder::invalid( - Vec::new(), - String::new(), - String::new(), - String::new(), - source.to_string(), + // If we failed to parse the doctest, create a dummy doctest. + return DocTestBuilder { + supports_color: false, + has_main_fn: false, + global_crate_attrs: Vec::new(), + crate_attrs: String::new(), + maybe_crate_attrs: String::new(), + crates: String::new(), + everything_else: source.to_string(), + already_has_extern_crate: false, test_id, - ); + failed_to_parse: true, + // We certainly don't want to merge such a doctest with others. + can_be_merged: false, + }; }; debug!("crate_attrs:\n{crate_attrs}{maybe_crate_attrs}"); @@ -173,7 +177,7 @@ impl<'a> BuildDocTestBuilder<'a> { everything_else, already_has_extern_crate, test_id, - invalid_ast: false, + failed_to_parse: false, can_be_merged, } } @@ -193,7 +197,7 @@ pub(crate) struct DocTestBuilder { pub(crate) crates: String, pub(crate) everything_else: String, pub(crate) test_id: Option, - pub(crate) invalid_ast: bool, + pub(crate) failed_to_parse: bool, pub(crate) can_be_merged: bool, } @@ -272,29 +276,6 @@ impl std::string::ToString for DocTestWrapResult { } impl DocTestBuilder { - fn invalid( - global_crate_attrs: Vec, - crate_attrs: String, - maybe_crate_attrs: String, - crates: String, - everything_else: String, - test_id: Option, - ) -> Self { - Self { - supports_color: false, - has_main_fn: false, - global_crate_attrs, - crate_attrs, - maybe_crate_attrs, - crates, - everything_else, - already_has_extern_crate: false, - test_id, - invalid_ast: true, - can_be_merged: false, - } - } - /// Transforms a test into code that can be compiled into a Rust binary, and returns the number of /// lines before the test code begins. pub(crate) fn generate_unique_doctest( @@ -304,10 +285,10 @@ impl DocTestBuilder { opts: &GlobalTestOptions, crate_name: Option<&str>, ) -> (DocTestWrapResult, usize) { - if self.invalid_ast { - // If the AST failed to compile, no need to go generate a complete doctest, the error - // will be better this way. - debug!("invalid AST:\n{test_code}"); + if self.failed_to_parse { + // If we failed to parse the doctest, there's no need to go generate a complete doctest, + // the diagnostics will be better this way. + debug!("failed to parse doctest:\n{test_code}"); return (DocTestWrapResult::SyntaxError(test_code.to_string()), 0); } let mut line_offset = 0; @@ -429,56 +410,40 @@ impl DocTestBuilder { } } -fn reset_error_count(psess: &ParseSess) { - // 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(); -} - -const DOCTEST_CODE_WRAPPER: &str = "fn f(){"; - fn parse_source( source: &str, crate_name: &Option<&str>, parent_dcx: Option>, span: Span, ) -> Result { - use rustc_errors::DiagCtxt; - use rustc_errors::emitter::{Emitter, HumanEmitter}; - use rustc_span::source_map::FilePathMapping; - let mut info = ParseSourceInfo { already_has_extern_crate: crate_name.is_none(), ..Default::default() }; + const DOCTEST_CODE_WRAPPER: &str = "fn f(){"; let wrapped_source = format!("{DOCTEST_CODE_WRAPPER}{source}\n}}"); - let filename = FileName::anon_source_code(&wrapped_source); + // Any parse errors should also appear when the doctest is compiled for real, + // so just suppress them here. + let emitter = SilentEmitter { translator: rustc_driver::default_translator() }; - let sm = Arc::new(SourceMap::new(FilePathMapping::empty())); - let translator = rustc_driver::default_translator(); - info.supports_color = - HumanEmitter::new(stderr_destination(ColorConfig::Auto), translator.clone()) - .supports_color(); - // Any errors in parsing should also appear when the doctest is compiled for real, so just - // send all the errors that the parser emits directly into a `Sink` instead of stderr. - let emitter = HumanEmitter::new(AutoStream::never(Box::new(io::sink())), translator); - - // 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 psess = ParseSess::with_dcx(dcx, Arc::new(SourceMap::new(FilePathMapping::empty()))); + // Reset errors at the end so that we don't panic on delayed bugs. + let _defer = rustc_data_structures::defer(|| psess.dcx().reset_err_count()); // Don't strip any tokens; it wouldn't matter anyway because the source is wrapped in a function. - let mut parser = - match new_parser_from_source_str(&psess, filename, wrapped_source, StripTokens::Nothing) { - Ok(p) => p, - Err(errs) => { - errs.into_iter().for_each(|err| err.cancel()); - reset_error_count(&psess); - return Err(()); - } - }; + let mut parser = match new_parser_from_source_str( + &psess, + FileName::anon_source_code(&wrapped_source), + wrapped_source, + StripTokens::Nothing, + ) { + Ok(p) => p, + Err(errs) => { + errs.into_iter().for_each(|err| err.cancel()); + return Err(()); + } + }; fn push_to_s(s: &mut String, source: &str, span: rustc_span::Span, prev_span_hi: &mut usize) { let extra_len = DOCTEST_CODE_WRAPPER.len(); @@ -524,124 +489,120 @@ fn parse_source( is_extern_crate } - let mut prev_span_hi = 0; let not_crate_attrs = &[sym::forbid, sym::allow, sym::warn, sym::deny, sym::expect]; - let parsed = parser.parse_item(rustc_parse::parser::ForceCollect::No); + let mut prev_span_hi = 0; - let result = match parsed { - Ok(Some(ref item)) - if let ast::ItemKind::Fn(ref fn_item) = item.kind - && let Some(ref body) = fn_item.body => - { - for attr in &item.attrs { - if attr.style == AttrStyle::Outer || attr.has_any_name(not_crate_attrs) { - // There is one exception to these attributes: - // `#![allow(internal_features)]`. If this attribute is used, we need to - // consider it only as a crate-level attribute. - if attr.has_name(sym::allow) - && let Some(list) = attr.meta_item_list() - && list.iter().any(|sub_attr| sub_attr.has_name(sym::internal_features)) - { - push_to_s(&mut info.crate_attrs, source, attr.span, &mut prev_span_hi); - } else { - push_to_s( - &mut info.maybe_crate_attrs, - source, - attr.span, - &mut prev_span_hi, - ); - } - } else { - push_to_s(&mut info.crate_attrs, source, attr.span, &mut prev_span_hi); - } + let item = parser.parse_item(rustc_parse::parser::ForceCollect::No).map_err(Diag::cancel)?; + + // The parser might've recovered from some syntax errors and thus returned `Ok(_)`. + // However, since we've suppressed diagnostic emission above, we must ensure that + // we mark the doctest as syntactically invalid. Otherwise, we can end up generating + // a runnable doctest that's free of any errors even though the source was malformed. + if psess.dcx().has_errors().is_some() { + return Err(()); + } + + let Some(box ast::Item { + attrs, + kind: ast::ItemKind::Fn(box ast::Fn { body: Some(body), .. }), + .. + }) = item + else { + // We know that this *has* to be a function with a body + // because we've wrapped the whole source in one above. + unreachable!() + }; + + for attr in attrs { + if attr.style == AttrStyle::Outer || attr.has_any_name(not_crate_attrs) { + // There is one exception to these attributes: + // `#![allow(internal_features)]`. If this attribute is used, we need to + // consider it only as a crate-level attribute. + if attr.has_name(sym::allow) + && let Some(list) = attr.meta_item_list() + && list.iter().any(|sub_attr| sub_attr.has_name(sym::internal_features)) + { + push_to_s(&mut info.crate_attrs, source, attr.span, &mut prev_span_hi); + } else { + push_to_s(&mut info.maybe_crate_attrs, source, attr.span, &mut prev_span_hi); } - let mut has_non_items = false; - for stmt in &body.stmts { - let mut is_extern_crate = false; - match stmt.kind { - StmtKind::Item(ref item) => { - is_extern_crate = check_item(item, &mut info, crate_name); - } - // We assume that the macro calls will expand to item(s) even though they could - // expand to statements and expressions. - StmtKind::MacCall(ref mac_call) => { - if !info.has_main_fn { - // For backward compatibility, we look for the token sequence `fn main(…)` - // in the macro input (!) to crudely detect main functions "masked by a - // wrapper macro". For the record, this is a horrible heuristic! - // See . - let mut iter = mac_call.mac.args.tokens.iter(); - while let Some(token) = iter.next() { - if let TokenTree::Token(token, _) = token - && let TokenKind::Ident(kw::Fn, _) = token.kind - && let Some(TokenTree::Token(ident, _)) = iter.peek() - && let TokenKind::Ident(sym::main, _) = ident.kind - && let Some(TokenTree::Delimited(.., Delimiter::Parenthesis, _)) = { - iter.next(); - iter.peek() - } - { - info.has_main_fn = true; - break; - } + } else { + push_to_s(&mut info.crate_attrs, source, attr.span, &mut prev_span_hi); + } + } + + let mut has_non_items = false; + for stmt in &body.stmts { + let mut is_extern_crate = false; + match stmt.kind { + StmtKind::Item(ref item) => { + is_extern_crate = check_item(item, &mut info, crate_name); + } + // We assume that the macro calls will expand to item(s) even though they could + // expand to statements and expressions. + StmtKind::MacCall(ref mac_call) => { + if !info.has_main_fn { + // For backward compatibility, we look for the token sequence `fn main(…)` + // in the macro input (!) to crudely detect main functions "masked by a + // wrapper macro". For the record, this is a horrible heuristic! + // See . + let mut iter = mac_call.mac.args.tokens.iter(); + while let Some(token) = iter.next() { + if let TokenTree::Token(token, _) = token + && let TokenKind::Ident(kw::Fn, _) = token.kind + && let Some(TokenTree::Token(ident, _)) = iter.peek() + && let TokenKind::Ident(sym::main, _) = ident.kind + && let Some(TokenTree::Delimited(.., Delimiter::Parenthesis, _)) = { + iter.next(); + iter.peek() } + { + info.has_main_fn = true; + break; } } - StmtKind::Expr(ref expr) => { - if matches!(expr.kind, ast::ExprKind::Err(_)) { - reset_error_count(&psess); - return Err(()); - } - has_non_items = true; - } - StmtKind::Let(_) | StmtKind::Semi(_) | StmtKind::Empty => has_non_items = true, - } - - // Weirdly enough, the `Stmt` span doesn't include its attributes, so we need to - // tweak the span to include the attributes as well. - let mut span = stmt.span; - if let Some(attr) = - stmt.kind.attrs().iter().find(|attr| attr.style == AttrStyle::Outer) - { - span = span.with_lo(attr.span.lo()); - } - if info.everything_else.is_empty() - && (!info.maybe_crate_attrs.is_empty() || !info.crate_attrs.is_empty()) - { - // To keep the doctest code "as close as possible" to the original, we insert - // all the code located between this new span and the previous span which - // might contain code comments and backlines. - push_to_s(&mut info.crates, source, span.shrink_to_lo(), &mut prev_span_hi); - } - if !is_extern_crate { - push_to_s(&mut info.everything_else, source, span, &mut prev_span_hi); - } else { - push_to_s(&mut info.crates, source, span, &mut prev_span_hi); } } - if has_non_items { - if info.has_main_fn - && let Some(dcx) = parent_dcx - && !span.is_dummy() - { - dcx.span_warn( - span, - "the `main` function of this doctest won't be run as it contains \ - expressions at the top level, meaning that the whole doctest code will be \ - wrapped in a function", - ); - } - info.has_main_fn = false; + StmtKind::Let(_) | StmtKind::Expr(_) | StmtKind::Semi(_) | StmtKind::Empty => { + has_non_items = true } - Ok(info) } - Err(e) => { - e.cancel(); - Err(()) + + // Weirdly enough, the `Stmt` span doesn't include its attributes, so we need to + // tweak the span to include the attributes as well. + let mut span = stmt.span; + if let Some(attr) = stmt.kind.attrs().iter().find(|attr| attr.style == AttrStyle::Outer) { + span = span.with_lo(attr.span.lo()); } - _ => Err(()), - }; + if info.everything_else.is_empty() + && (!info.maybe_crate_attrs.is_empty() || !info.crate_attrs.is_empty()) + { + // To keep the doctest code "as close as possible" to the original, we insert + // all the code located between this new span and the previous span which + // might contain code comments and backlines. + push_to_s(&mut info.crates, source, span.shrink_to_lo(), &mut prev_span_hi); + } + if !is_extern_crate { + push_to_s(&mut info.everything_else, source, span, &mut prev_span_hi); + } else { + push_to_s(&mut info.crates, source, span, &mut prev_span_hi); + } + } + + if has_non_items { + if info.has_main_fn + && let Some(dcx) = parent_dcx + && !span.is_dummy() + { + dcx.span_warn( + span, + "the `main` function of this doctest won't be run as it contains \ + expressions at the top level, meaning that the whole doctest code will be \ + wrapped in a function", + ); + } + info.has_main_fn = false; + } - reset_error_count(&psess); - result + Ok(info) } diff --git a/tests/rustdoc-ui/doctest/doctest-output-include-fail.stdout b/tests/rustdoc-ui/doctest/doctest-output-include-fail.stdout index ceaf60b120152..82c527ef56988 100644 --- a/tests/rustdoc-ui/doctest/doctest-output-include-fail.stdout +++ b/tests/rustdoc-ui/doctest/doctest-output-include-fail.stdout @@ -13,7 +13,15 @@ LL | let x = 234 // no semicolon here! oh no! LL | } | - unexpected token -error: aborting due to 1 previous error +warning: unused variable: `x` + --> $DIR/doctest-output-include-fail.md:5:9 + | +LL | let x = 234 // no semicolon here! oh no! + | ^ help: if this is intentional, prefix it with an underscore: `_x` + | + = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default + +error: aborting due to 1 previous error; 1 warning emitted Couldn't compile the test. @@ -22,4 +30,3 @@ failures: test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME -all doctests ran in $TIME; merged doctests compilation took $TIME diff --git a/tests/rustdoc-ui/doctest/nocapture-fail.stderr b/tests/rustdoc-ui/doctest/nocapture-fail.stderr index c6a5785a24fe4..a9920a93dd45d 100644 --- a/tests/rustdoc-ui/doctest/nocapture-fail.stderr +++ b/tests/rustdoc-ui/doctest/nocapture-fail.stderr @@ -14,5 +14,12 @@ LL | Input: 123 LL ~ } } | -error: aborting due to 1 previous error +error[E0601]: `main` function not found in crate `rust_out` + --> $DIR/nocapture-fail.rs:10:2 + | +LL | } + | ^ consider adding a `main` function to `$DIR/nocapture-fail.rs` + +error: aborting due to 2 previous errors +For more information about this error, try `rustc --explain E0601`. diff --git a/tests/rustdoc-ui/doctest/recovered-syntax-error.rs b/tests/rustdoc-ui/doctest/recovered-syntax-error.rs new file mode 100644 index 0000000000000..1b34d69c2ea24 --- /dev/null +++ b/tests/rustdoc-ui/doctest/recovered-syntax-error.rs @@ -0,0 +1,9 @@ +// issue: +//@ compile-flags: --test +//@ normalize-stdout: "tests/rustdoc-ui/doctest" -> "$$DIR" +//@ normalize-stdout: "finished in \d+\.\d+s" -> "finished in $$TIME" +//@ failure-status: 101 + +//! ``` +//! #[derive(Clone)] +//! ``` diff --git a/tests/rustdoc-ui/doctest/recovered-syntax-error.stdout b/tests/rustdoc-ui/doctest/recovered-syntax-error.stdout new file mode 100644 index 0000000000000..3080bb5f788a5 --- /dev/null +++ b/tests/rustdoc-ui/doctest/recovered-syntax-error.stdout @@ -0,0 +1,22 @@ + +running 1 test +test $DIR/recovered-syntax-error.rs - (line 7) ... FAILED + +failures: + +---- $DIR/recovered-syntax-error.rs - (line 7) stdout ---- +error: expected item after attributes + --> $DIR/recovered-syntax-error.rs:8:1 + | +LL | #[derive(Clone)] + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +Couldn't compile the test. + +failures: + $DIR/recovered-syntax-error.rs - (line 7) + +test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME +