From 9a5a2c4f3f8fb327f45764a39115d65af5f9a4fb Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mike.gerwitz@ryansg.com>
Date: Fri, 29 Apr 2022 11:57:50 -0400
Subject: [PATCH] tamer: diagnose::report: Avoid re-resolving adjacent
 identical spans

This does what the original proof-of-concept implementation did---skip a
span that was just processed, since it'll be squashed into the previous
anyway.  These duplicate spans originate from the diagnostic system when
producing supplemental help information.

DEV-12151
---
 tamer/src/diagnose/report.rs | 67 ++++++++++++++++++++++++++++--------
 1 file changed, 52 insertions(+), 15 deletions(-)

diff --git a/tamer/src/diagnose/report.rs b/tamer/src/diagnose/report.rs
index 83f3759e..6f62825e 100644
--- a/tamer/src/diagnose/report.rs
+++ b/tamer/src/diagnose/report.rs
@@ -26,7 +26,8 @@
 
 use super::{
     resolver::{
-        Column, ResolvedSpanData, SourceLine, SpanResolver, SpanResolverError,
+        Column, ResolvedSpan, ResolvedSpanData, SourceLine, SpanResolver,
+        SpanResolverError,
     },
     AnnotatedSpan, Diagnostic, Label, Level,
 };
@@ -89,18 +90,8 @@ impl<R: SpanResolver> Reporter for VisualReporter<R> {
         &mut self,
         diagnostic: &'d D,
     ) -> Report<'d, D> {
-        // TODO: Avoid duplicate lookups of the same span,
-        //   or at least adjacent ones.
-        let mspans = diagnostic.describe().into_iter().map(
-            |AnnotatedSpan(span, level, olabel)| {
-                let slabel = olabel.map(|label| SpanLabel(level, label));
-
-                match self.resolver.resolve(span) {
-                    Ok(rspan) => MaybeResolvedSpan::Resolved(rspan, slabel),
-                    Err(e) => MaybeResolvedSpan::Unresolved(span, slabel, e),
-                }
-            },
-        );
+        let mspans =
+            describe_resolved(|span| self.resolver.resolve(span), diagnostic);
 
         let mut report = Report::empty(Message(diagnostic));
         report.extend(mspans.map(Into::into));
@@ -113,6 +104,46 @@ impl<R: SpanResolver> Reporter for VisualReporter<R> {
     }
 }
 
+/// Request a diagnostic description and immediately resolve the provided
+///   [`AnnotatedSpan`]s into [`MaybeResolvedSpan`]s.
+///
+/// Adjacent identical [`Span`]s are elided such that the first one is
+///   resolved but the second one produces [`MaybeResolvedSpan::Elided`];
+///     this avoids the expensive column resolution and source line
+///     allocation for a span that was just processed and whose section will
+///     be squashed anyway
+///       (see [`Section::maybe_squash_into`]).
+fn describe_resolved<'d, D, F>(
+    mut resolve: F,
+    diagnostic: &'d D,
+) -> impl Iterator<Item = MaybeResolvedSpan<'d, ResolvedSpan>>
+where
+    D: Diagnostic,
+    F: FnMut(Span) -> Result<ResolvedSpan, SpanResolverError>,
+{
+    diagnostic.describe().into_iter().scan(
+        UNKNOWN_SPAN,
+        move |prev_span, AnnotatedSpan(span, level, olabel)| {
+            let slabel = olabel.map(|label| SpanLabel(level, label));
+
+            // Avoid re-resolving
+            //   (and allocating memory for the source lines of)
+            //   a span that was just resolved,
+            //     which will just be squashed with the previous anyway.
+            if *prev_span == span {
+                return Some(MaybeResolvedSpan::Elided(span, slabel));
+            }
+
+            *prev_span = span;
+
+            Some(match resolve(span) {
+                Ok(rspan) => MaybeResolvedSpan::Resolved(rspan, slabel),
+                Err(e) => MaybeResolvedSpan::Unresolved(span, slabel, e),
+            })
+        },
+    )
+}
+
 /// A [`Span`] that may have been resolved.
 ///
 /// The span will remain unresolved if an error occurred,
@@ -128,6 +159,7 @@ impl<R: SpanResolver> Reporter for VisualReporter<R> {
 #[derive(Debug, PartialEq, Eq)]
 enum MaybeResolvedSpan<'d, S: ResolvedSpanData> {
     Resolved(S, Option<SpanLabel<'d>>),
+    Elided(Span, Option<SpanLabel<'d>>),
     Unresolved(Span, Option<SpanLabel<'d>>, SpanResolverError),
 }
 
@@ -372,7 +404,8 @@ where
 
                 (span, level)
             }
-            MaybeResolvedSpan::Unresolved(span, olabel, _) => {
+            MaybeResolvedSpan::Elided(span, olabel)
+            | MaybeResolvedSpan::Unresolved(span, olabel, _) => {
                 let level =
                     olabel.as_ref().map(SpanLabel::level).unwrap_or_default();
 
@@ -447,7 +480,11 @@ where
                 ),
             ),
 
-            MaybeResolvedSpan::Unresolved(span, _, _) => {
+            // Note that elided will be squashed anyway,
+            //   so the fact that this results in an unresolved heading is
+            //   okay.
+            MaybeResolvedSpan::Elided(span, _)
+            | MaybeResolvedSpan::Unresolved(span, _, _) => {
                 SpanHeading(span.context(), HeadingLineNum::Unresolved(*span))
             }
         }