rustdoc: Properly detect syntactically invalid doctests

That is, even in the presence of parse error recovery.

Author: fmease

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
     /// ```

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;

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::{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;
@@ -445,40 +444,33 @@ fn parse_source(
     parent_dcx: Option<DiagCtxtHandle<'_>>,
     span: Span,
 ) -> Result<ParseSourceInfo, ()> {
-    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() };
 
     let wrapped_source = format!("{DOCTEST_CODE_WRAPPER}{source}\n}}");
 
-    let filename = FileName::anon_source_code(&wrapped_source);
-
-    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);
+    // 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() };
 
     // 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())));
 
     // 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());
+            reset_error_count(&psess);
+            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();
@@ -531,7 +523,12 @@ fn parse_source(
     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 =>
+                && let Some(ref body) = fn_item.body
+                // 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.
+                && psess.dcx().has_errors().is_none() =>
         {
             for attr in &item.attrs {
                 if attr.style == AttrStyle::Outer || attr.has_any_name(not_crate_attrs) {
@@ -587,14 +584,9 @@ fn parse_source(
                             }
                         }
                     }
-                    StmtKind::Expr(ref expr) => {
-                        if matches!(expr.kind, ast::ExprKind::Err(_)) {
-                            reset_error_count(&psess);
-                            return Err(());
-                        }
-                        has_non_items = true;
+                    StmtKind::Let(_) | StmtKind::Expr(_) | StmtKind::Semi(_) | StmtKind::Empty => {
+                        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

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

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`.

tests/rustdoc-ui/doctest/recovered-syntax-error.rs

@@ -0,0 +1,9 @@
+// issue: <https://github.com/rust-lang/rust/issues/147999>
+//@ 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)]
+//! ```

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
+