Update std::error::Report based on feedback

library/std/src/error.rs

@@ -25,7 +25,7 @@ use crate::backtrace::Backtrace;
 use crate::borrow::Cow;
 use crate::cell;
 use crate::char;
-use crate::fmt::{self, Debug, Display};
+use crate::fmt::{self, Debug, Display, Write};
 use crate::mem::transmute;
 use crate::num;
 use crate::str;
@@ -63,7 +63,7 @@ pub trait Error: Debug + Display {
     ///
     /// #[derive(Debug)]
     /// struct SuperError {
-    ///     side: SuperErrorSideKick,
+    ///     source: SuperErrorSideKick,
     /// }
     ///
     /// impl fmt::Display for SuperError {
@@ -74,7 +74,7 @@ pub trait Error: Debug + Display {
     ///
     /// impl Error for SuperError {
     ///     fn source(&self) -> Option<&(dyn Error + 'static)> {
-    ///         Some(&self.side)
+    ///         Some(&self.source)
     ///     }
     /// }
     ///
@@ -90,7 +90,7 @@ pub trait Error: Debug + Display {
     /// impl Error for SuperErrorSideKick {}
     ///
     /// fn get_super_error() -> Result<(), SuperError> {
-    ///     Err(SuperError { side: SuperErrorSideKick })
+    ///     Err(SuperError { source: SuperErrorSideKick })
     /// }
     ///
     /// fn main() {
@@ -836,10 +836,6 @@ impl dyn Error + Send + Sync {
 ///
 /// impl<'a> Error for SuperError<'a> {}
 ///
-/// // Note that the error doesn't need to be `Send` or `Sync`.
-/// impl<'a> !Send for SuperError<'a> {}
-/// impl<'a> !Sync for SuperError<'a> {}
-///
 /// fn main() {
 ///     let msg = String::from("Huzzah!");
 ///     let error = SuperError { side: &msg };
@@ -883,6 +879,19 @@ where
         self
     }
 
+    fn backtrace(&self) -> Option<&Backtrace> {
+        // have to grab the backtrace on the first error directly since that error may not be
+        // 'static
+        let backtrace = self.error.backtrace();
+        let backtrace = backtrace.or_else(|| {
+            self.error
+                .source()
+                .map(|source| source.chain().find_map(|source| source.backtrace()))
+                .flatten()
+        });
+        backtrace
+    }
+
     /// Format the report as a single line.
     #[unstable(feature = "error_reporter", issue = "90172")]
     fn fmt_singleline(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -911,17 +920,17 @@ where
 
             for (ind, error) in cause.chain().enumerate() {
                 writeln!(f)?;
-
-                if multiple {
-                    write!(f, "{: >4}: {}", ind, Indented { source: error })?;
-                } else {
-                    write!(f, "    {}", error)?;
-                }
+                let mut indented = Indented {
+                    inner: f,
+                    number: if multiple { Some(ind) } else { None },
+                    started: false,
+                };
+                write!(indented, "{}", error)?;
             }
         }
 
         if self.show_backtrace {
-            let backtrace = error.backtrace();
+            let backtrace = self.backtrace();
 
             if let Some(backtrace) = backtrace {
                 let backtrace = backtrace.to_string();
@@ -968,23 +977,34 @@ where
 }
 
 /// Wrapper type for indenting the inner source.
-struct Indented<D> {
-    source: D,
+struct Indented<'a, D> {
+    inner: &'a mut D,
+    number: Option<usize>,
+    started: bool,
 }
 
-impl<D> fmt::Display for Indented<D>
+impl<T> Write for Indented<'_, T>
 where
-    D: fmt::Display,
+    T: Write,
 {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let source = self.source.to_string();
-
-        for (ind, line) in source.trim().split('\n').filter(|l| !l.is_empty()).enumerate() {
-            if ind > 0 {
-                write!(f, "\n      {}", line)?;
-            } else {
-                write!(f, "{}", line)?;
+    fn write_str(&mut self, s: &str) -> fmt::Result {
+        for (i, line) in s.split('\n').enumerate() {
+            if !self.started {
+                self.started = true;
+                match self.number {
+                    Some(number) => write!(self.inner, "{: >5}: ", number)?,
+                    None => self.inner.write_str("    ")?,
                 }
+            } else if i > 0 {
+                self.inner.write_char('\n')?;
+                if self.number.is_some() {
+                    self.inner.write_str("       ")?;
+                } else {
+                    self.inner.write_str("    ")?;
+                }
+            }
+
+            self.inner.write_str(line)?;
         }
 
         Ok(())

library/std/src/error/tests.rs

@@ -36,13 +36,12 @@ fn downcasting() {
     }
 }
 
-use crate::backtrace;
-use crate::env;
+use crate::backtrace::Backtrace;
 use crate::error::Report;
 
 #[derive(Debug)]
 struct SuperError {
-    side: SuperErrorSideKick,
+    source: SuperErrorSideKick,
 }
 
 impl fmt::Display for SuperError {
@@ -53,7 +52,7 @@ impl fmt::Display for SuperError {
 
 impl Error for SuperError {
     fn source(&self) -> Option<&(dyn Error + 'static)> {
-        Some(&self.side)
+        Some(&self.source)
     }
 }
 
@@ -70,7 +69,7 @@ impl Error for SuperErrorSideKick {}
 
 #[test]
 fn single_line_formatting() {
-    let error = SuperError { side: SuperErrorSideKick };
+    let error = SuperError { source: SuperErrorSideKick };
     let report = Report::new(&error);
     let actual = report.to_string();
     let expected = String::from("SuperError is here!: SuperErrorSideKick is here!");
@@ -80,7 +79,7 @@ fn single_line_formatting() {
 
 #[test]
 fn multi_line_formatting() {
-    let error = SuperError { side: SuperErrorSideKick };
+    let error = SuperError { source: SuperErrorSideKick };
     let report = Report::new(&error).pretty(true);
     let actual = report.to_string();
     let expected =
@@ -108,50 +107,57 @@ fn error_with_no_sources_formats_multi_line_correctly() {
 }
 
 #[test]
-fn error_with_backtrace_outputs_correctly() {
-    use backtrace::Backtrace;
+fn error_with_backtrace_outputs_correctly_with_one_source() {
+    let trace = Backtrace::force_capture();
+    let expected = format!("The source of the error
 
-    env::remove_var("RUST_BACKTRACE");
+Caused by:
+    Error with backtrace
 
-    #[derive(Debug)]
-    struct ErrorWithBacktrace<'a> {
-        msg: &'a str,
-        trace: Backtrace,
-    }
+Stack backtrace:
+{}", trace);
+    let error = GenericError::new("Error with backtrace");
+    let mut error = GenericError::new_with_source("The source of the error", error);
+    error.backtrace = Some(trace);
+    let report = Report::new(error).pretty(true).show_backtrace(true);
 
-    impl<'a> fmt::Display for ErrorWithBacktrace<'a> {
-        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-            write!(f, "Error with backtrace: {}", self.msg)
-        }
-    }
 
-    impl<'a> Error for ErrorWithBacktrace<'a> {
-        fn backtrace(&self) -> Option<&Backtrace> {
-            Some(&self.trace)
-        }
-    }
+    println!("Error: {}", report);
+    assert_eq!(expected.trim_end(), report.to_string());
+}
 
-    let msg = String::from("The source of the error");
-    let report = Report::new(ErrorWithBacktrace { msg: &msg, trace: Backtrace::capture() })
-        .pretty(true)
-        .show_backtrace(true);
+#[test]
+fn error_with_backtrace_outputs_correctly_with_two_sources() {
+    let trace = Backtrace::force_capture();
+    let expected = format!("Error with two sources
 
-    let expected = String::from(
-        "Error with backtrace: The source of the error\n\nStack backtrace:\ndisabled backtrace",
-    );
+Caused by:
+    0: The source of the error
+    1: Error with backtrace
 
-    assert_eq!(expected, report.to_string());
+Stack backtrace:
+{}", trace);
+    let mut error = GenericError::new("Error with backtrace");
+    error.backtrace = Some(trace);
+    let error = GenericError::new_with_source("The source of the error", error);
+    let error = GenericError::new_with_source("Error with two sources", error);
+    let report = Report::new(error).pretty(true).show_backtrace(true);
+
+
+    println!("Error: {}", report);
+    assert_eq!(expected.trim_end(), report.to_string());
 }
 
 #[derive(Debug)]
 struct GenericError<D> {
     message: D,
+    backtrace: Option<Backtrace>,
     source: Option<Box<dyn Error + 'static>>,
 }
 
 impl<D> GenericError<D> {
     fn new(message: D) -> GenericError<D> {
-        Self { message, source: None }
+        Self { message, backtrace: None, source: None }
     }
 
     fn new_with_source<E>(message: D, source: E) -> GenericError<D>
@@ -160,7 +166,7 @@ impl<D> GenericError<D> {
     {
         let source: Box<dyn Error + 'static> = Box::new(source);
         let source = Some(source);
-        GenericError { message, source }
+        GenericError { message, backtrace: None, source }
     }
 }
 
@@ -180,6 +186,10 @@ where
     fn source(&self) -> Option<&(dyn Error + 'static)> {
         self.source.as_deref()
     }
+
+    fn backtrace(&self) -> Option<&Backtrace> {
+        self.backtrace.as_ref()
+    }
 }
 
 #[test]
@@ -297,8 +307,12 @@ The message
 
 
 Caused by:
-   0: The message
-   1: The message"#;
+    0: 
+       The message
+       
+    1: 
+       The message
+       "#;
 
     let actual = report.to_string();
     assert_eq!(expected, actual);
@@ -326,3 +340,59 @@ Caused by:
     let actual = report.to_string();
     assert_eq!(expected, actual);
 }
+
+#[test]
+fn empty_lines_mid_message() {
+    #[derive(Debug)]
+    struct MyMessage;
+
+    impl fmt::Display for MyMessage {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            f.write_str("line 1\n\nline 2")
+        }
+    }
+
+    let error = GenericError::new(MyMessage);
+    let error = GenericError::new_with_source(MyMessage, error);
+    let error = GenericError::new_with_source(MyMessage, error);
+    let report = Report::new(error).pretty(true);
+    let expected = r#"line 1
+
+line 2
+
+Caused by:
+    0: line 1
+       
+       line 2
+    1: line 1
+       
+       line 2"#;
+
+    let actual = report.to_string();
+    assert_eq!(expected, actual);
+}
+
+#[test]
+fn only_one_source() {
+    #[derive(Debug)]
+    struct MyMessage;
+
+    impl fmt::Display for MyMessage {
+        fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+            f.write_str("line 1\nline 2")
+        }
+    }
+
+    let error = GenericError::new(MyMessage);
+    let error = GenericError::new_with_source(MyMessage, error);
+    let report = Report::new(error).pretty(true);
+    let expected = r#"line 1
+line 2
+
+Caused by:
+    line 1
+    line 2"#;
+
+    let actual = report.to_string();
+    assert_eq!(expected, actual);
+}