Extract an iterator that cleans redundant nested error text

src/lib.rs

@@ -272,6 +272,8 @@ mod error_chain;
 pub use crate::error_chain::*;
 
 mod report;
+#[cfg(feature = "std")]
+pub use report::CleanedErrorText;
 pub use report::{Report, __InternalExtractErrorType};
 
 doc_comment::doc_comment! {

src/report.rs

@@ -272,33 +272,23 @@ impl<'a> ReportFormatter<'a> {
     fn cleaned_error_trace(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
         const NOTE: char = '*';
 
-        let mut original_messages = ChainCompat::new(self.0).map(ToString::to_string);
-        let mut prev = original_messages.next();
-
-        let mut cleaned_messages = vec![];
         let mut any_cleaned = false;
         let mut any_removed = false;
-        for msg in original_messages {
-            if let Some(mut prev) = prev {
-                let cleaned = prev.trim_end_matches(&msg).trim_end().trim_end_matches(':');
-                if cleaned.is_empty() {
+        let cleaned_messages: Vec<_> = CleanedErrorText::new(self.0)
+            .flat_map(|(_, mut msg, cleaned)| {
+                if msg.is_empty() {
                     any_removed = true;
-                    // Do not add this to the output list
-                } else if cleaned != prev {
-                    any_cleaned = true;
-                    let cleaned_len = cleaned.len();
-                    prev.truncate(cleaned_len);
-                    prev.push(' ');
-                    prev.push(NOTE);
-                    cleaned_messages.push(prev);
+                    None
                 } else {
-                    cleaned_messages.push(prev);
+                    if cleaned {
+                        any_cleaned = true;
+                        msg.push(' ');
+                        msg.push(NOTE);
+                    }
+                    Some(msg)
                 }
-            }
-
-            prev = Some(msg);
-        }
-        cleaned_messages.extend(prev);
+            })
+            .collect();
 
         let mut visible_messages = cleaned_messages.iter();
 
@@ -357,6 +347,94 @@ fn trace_cleaning_enabled() -> bool {
     !DISABLED.get(|| env::var_os(SNAFU_RAW_ERROR_MESSAGES).map_or(false, |v| v == "1"))
 }
 
+/// An iterator over an Error and its sources that removes duplicated
+/// text from the error display strings.
+///
+/// It's common for errors with a `source` to have a `Display`
+/// implementation that includes their source text as well:
+///
+/// ```text
+/// Outer error text: Middle error text: Inner error text
+/// ```
+///
+/// This works for smaller errors without much detail, but can be
+/// annoying when trying to format the error in a more structured way,
+/// such as line-by-line:
+///
+/// ```text
+/// 1. Outer error text: Middle error text: Inner error text
+/// 2. Middle error text: Inner error text
+/// 3. Inner error text
+/// ```
+///
+/// This iterator compares each pair of errors in the source chain,
+/// removing the source error's text from the containing error's text:
+///
+/// ```text
+/// 1. Outer error text
+/// 2. Middle error text
+/// 3. Inner error text
+/// ```
+#[cfg(feature = "std")]
+pub struct CleanedErrorText<'a>(Option<CleanedErrorTextStep<'a>>);
+
+#[cfg(feature = "std")]
+impl<'a> CleanedErrorText<'a> {
+    /// Constructs the iterator.
+    pub fn new(error: &'a dyn crate::Error) -> Self {
+        Self(Some(CleanedErrorTextStep::new(error)))
+    }
+}
+
+#[cfg(feature = "std")]
+impl<'a> Iterator for CleanedErrorText<'a> {
+    /// The original error, the display string and if it has been cleaned
+    type Item = (&'a dyn crate::Error, String, bool);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        use std::mem;
+
+        let mut step = self.0.take()?;
+        let mut error_text = mem::replace(&mut step.error_text, Default::default());
+
+        match step.error.source() {
+            Some(next_error) => {
+                let next_error_text = next_error.to_string();
+
+                let cleaned_text = error_text
+                    .trim_end_matches(&next_error_text)
+                    .trim_end()
+                    .trim_end_matches(':');
+                let cleaned = cleaned_text.len() != error_text.len();
+                let cleaned_len = cleaned_text.len();
+                error_text.truncate(cleaned_len);
+
+                self.0 = Some(CleanedErrorTextStep {
+                    error: next_error,
+                    error_text: next_error_text,
+                });
+
+                Some((step.error, error_text, cleaned))
+            }
+            None => Some((step.error, error_text, false)),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+struct CleanedErrorTextStep<'a> {
+    error: &'a dyn crate::Error,
+    error_text: String,
+}
+
+#[cfg(feature = "std")]
+impl<'a> CleanedErrorTextStep<'a> {
+    fn new(error: &'a dyn crate::Error) -> Self {
+        let error_text = error.to_string();
+        Self { error, error_text }
+    }
+}
+
 #[doc(hidden)]
 pub trait __InternalExtractErrorType {
     type Err;

tests/report.rs

@@ -1,4 +1,4 @@
-use snafu::{prelude::*, IntoError, Report};
+use snafu::{prelude::*, CleanedErrorText, IntoError, Report};
 
 macro_rules! assert_contains {
     (needle: $needle:expr, haystack: $haystack:expr) => {
@@ -162,3 +162,59 @@ struct TestFunctionError;
 fn procedural_macro_works_with_test_functions() -> Result<(), TestFunctionError> {
     Ok(())
 }
+
+#[track_caller]
+fn assert_cleaning_step(iter: &mut CleanedErrorText, text: &str, removed_text: &str) {
+    let (error, actual_text, actual_cleaned) =
+        iter.next().expect("Iterator unexpectedly exhausted");
+    let actual_original_text = error.to_string();
+
+    let original_text = [text, removed_text].concat();
+    let cleaned = !removed_text.is_empty();
+
+    assert_eq!(original_text, actual_original_text);
+    assert_eq!(text, actual_text);
+    assert_eq!(cleaned, actual_cleaned);
+}
+
+#[test]
+fn cleaning_a_leaf_error_changes_nothing() {
+    #[derive(Debug, Snafu)]
+    #[snafu(display("But I am only C"))]
+    struct C;
+
+    let c = C;
+    let mut iter = CleanedErrorText::new(&c);
+
+    assert_cleaning_step(&mut iter, "But I am only C", "");
+    assert!(iter.next().is_none());
+}
+
+#[test]
+fn cleaning_nested_errors_removes_duplication() {
+    #[derive(Debug, Snafu)]
+    #[snafu(display("This is A: {source}"))]
+    struct A {
+        source: B,
+    }
+
+    #[derive(Debug, Snafu)]
+    #[snafu(display("And this is B: {source}"))]
+    struct B {
+        source: C,
+    }
+
+    #[derive(Debug, Snafu)]
+    #[snafu(display("But I am only C"))]
+    struct C;
+
+    let a = A {
+        source: B { source: C },
+    };
+    let mut iter = CleanedErrorText::new(&a);
+
+    assert_cleaning_step(&mut iter, "This is A", ": And this is B: But I am only C");
+    assert_cleaning_step(&mut iter, "And this is B", ": But I am only C");
+    assert_cleaning_step(&mut iter, "But I am only C", "");
+    assert!(iter.next().is_none());
+}