Add Span merging functions (#12511)

# Description
This PR adds a few functions to `Span` for merging spans together:
- `Span::append`: merges two spans that are known to be in order.
- `Span::concat`: returns a span that encompasses all the spans in a
slice. The spans must be in order.
- `Span::merge`: merges two spans (no order necessary).
- `Span::merge_many`: merges an iterator of spans into a single span (no
order necessary).

These are meant to replace the free-standing `nu_protocol::span`
function.

The spans in a `LiteCommand` (the `parts`) should always be in order
based on the lite parser and lexer. So, the parser code sees the most
usage of `Span::append` and `Span::concat` where the order is known. In
other code areas, `Span::merge` and `Span::merge_many` are used since
the order between spans is often not known.

crates/nu-protocol/src/ast/import_pattern.rs

@@ -1,6 +1,6 @@
 use serde::{Deserialize, Serialize};
 
-use crate::{span, ModuleId, Span, VarId};
+use crate::{ModuleId, Span, VarId};
 use std::collections::HashSet;
 
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
@@ -12,17 +12,22 @@ pub enum ImportPatternMember {
 
 impl ImportPatternMember {
     pub fn span(&self) -> Span {
-        let mut spans = vec![];
         match self {
-            ImportPatternMember::Glob { span } => spans.push(*span),
-            ImportPatternMember::Name { name: _, span } => spans.push(*span),
+            ImportPatternMember::Glob { span } | ImportPatternMember::Name { span, .. } => *span,
             ImportPatternMember::List { names } => {
-                for (_, span) in names {
-                    spans.push(*span);
-                }
+                let first = names
+                    .first()
+                    .map(|&(_, span)| span)
+                    .unwrap_or(Span::unknown());
+
+                let last = names
+                    .last()
+                    .map(|&(_, span)| span)
+                    .unwrap_or(Span::unknown());
+
+                Span::append(first, last)
             }
         }
-        span(&spans)
     }
 }
 
@@ -59,13 +64,13 @@ impl ImportPattern {
     }
 
     pub fn span(&self) -> Span {
-        let mut spans = vec![self.head.span];
-
-        for member in &self.members {
-            spans.push(member.span());
-        }
-
-        span(&spans)
+        Span::append(
+            self.head.span,
+            self.members
+                .last()
+                .map(ImportPatternMember::span)
+                .unwrap_or(self.head.span),
+        )
     }
 
     pub fn with_hidden(self, hidden: HashSet<Vec<u8>>) -> Self {

crates/nu-protocol/src/span.rs

@@ -1,7 +1,6 @@
-use std::ops::Deref;
-
 use miette::SourceSpan;
 use serde::{Deserialize, Serialize};
+use std::ops::Deref;
 
 /// A spanned area of interest, generic over what kind of thing is of interest
 #[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq, Eq)]
@@ -74,77 +73,123 @@ impl<T> IntoSpanned for T {
 /// Spans are a global offset across all seen files, which are cached in the engine's state. The start and
 /// end offset together make the inclusive start/exclusive end pair for where to underline to highlight
 /// a given point of interest.
-#[non_exhaustive]
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
 pub struct Span {
     pub start: usize,
     pub end: usize,
 }
 
-impl From<Span> for SourceSpan {
-    fn from(s: Span) -> Self {
-        Self::new(s.start.into(), s.end - s.start)
-    }
-}
-
 impl Span {
-    pub fn new(start: usize, end: usize) -> Span {
+    pub fn new(start: usize, end: usize) -> Self {
         debug_assert!(
             end >= start,
             "Can't create a Span whose end < start, start={start}, end={end}"
         );
 
-        Span { start, end }
+        Self { start, end }
     }
 
-    pub const fn unknown() -> Span {
-        Span { start: 0, end: 0 }
+    pub const fn unknown() -> Self {
+        Self { start: 0, end: 0 }
     }
 
     /// Note: Only use this for test data, *not* live data, as it will point into unknown source
     /// when used in errors.
-    pub const fn test_data() -> Span {
+    pub const fn test_data() -> Self {
         Self::unknown()
     }
 
-    pub fn offset(&self, offset: usize) -> Span {
-        Span::new(self.start - offset, self.end - offset)
+    pub fn offset(&self, offset: usize) -> Self {
+        Self::new(self.start - offset, self.end - offset)
     }
 
     pub fn contains(&self, pos: usize) -> bool {
-        pos >= self.start && pos < self.end
+        self.start <= pos && pos < self.end
     }
 
-    pub fn contains_span(&self, span: Span) -> bool {
-        span.start >= self.start && span.end <= self.end
+    pub fn contains_span(&self, span: Self) -> bool {
+        self.start <= span.start && span.end <= self.end
     }
 
-    /// Point to the space just past this span, useful for missing
-    /// values
-    pub fn past(&self) -> Span {
-        Span {
+    /// Point to the space just past this span, useful for missing values
+    pub fn past(&self) -> Self {
+        Self {
             start: self.end,
             end: self.end,
         }
     }
+
+    /// Returns the minimal [`Span`] that encompasses both of the given spans.
+    ///
+    /// The two `Spans` can overlap in the middle,
+    /// but must otherwise be in order by satisfying:
+    /// - `self.start <= after.start`
+    /// - `self.end <= after.end`
+    ///
+    /// If this is not guaranteed to be the case, use [`Span::merge`] instead.
+    pub fn append(self, after: Self) -> Self {
+        debug_assert!(
+            self.start <= after.start && self.end <= after.end,
+            "Can't merge two Spans that are not in order"
+        );
+        Self {
+            start: self.start,
+            end: after.end,
+        }
+    }
+
+    /// Returns the minimal [`Span`] that encompasses both of the given spans.
+    ///
+    /// The spans need not be in order or have any relationship.
+    ///
+    /// [`Span::append`] is slightly more efficient if the spans are known to be in order.
+    pub fn merge(self, other: Self) -> Self {
+        Self {
+            start: usize::min(self.start, other.start),
+            end: usize::max(self.end, other.end),
+        }
+    }
+
+    /// Returns the minimal [`Span`] that encompasses all of the spans in the given slice.
+    ///
+    /// The spans are assumed to be in order, that is, all consecutive spans must satisfy:
+    /// - `spans[i].start <= spans[i + 1].start`
+    /// - `spans[i].end <= spans[i + 1].end`
+    ///
+    /// (Two consecutive spans can overlap as long as the above is true.)
+    ///
+    /// Use [`Span::merge_many`] if the spans are not known to be in order.
+    pub fn concat(spans: &[Self]) -> Self {
+        // TODO: enable assert below
+        // debug_assert!(!spans.is_empty());
+        debug_assert!(spans.windows(2).all(|spans| {
+            let &[a, b] = spans else {
+                return false;
+            };
+            a.start <= b.start && a.end <= b.end
+        }));
+        Self {
+            start: spans.first().map(|s| s.start).unwrap_or(0),
+            end: spans.last().map(|s| s.end).unwrap_or(0),
+        }
+    }
+
+    /// Returns the minimal [`Span`] that encompasses all of the spans in the given iterator.
+    ///
+    /// The spans need not be in order or have any relationship.
+    ///
+    /// [`Span::concat`] is more efficient if the spans are known to be in order.
+    pub fn merge_many(spans: impl IntoIterator<Item = Self>) -> Self {
+        spans
+            .into_iter()
+            .reduce(Self::merge)
+            .unwrap_or(Self::unknown())
+    }
 }
 
-/// Used when you have a slice of spans of at least size 1
-pub fn span(spans: &[Span]) -> Span {
-    let length = spans.len();
-
-    //TODO debug_assert!(length > 0, "expect spans > 0");
-    if length == 0 {
-        Span::unknown()
-    } else if length == 1 {
-        spans[0]
-    } else {
-        let end = spans
-            .iter()
-            .map(|s| s.end)
-            .max()
-            .expect("Must be an end. Length > 0");
-        Span::new(spans[0].start, end)
+impl From<Span> for SourceSpan {
+    fn from(s: Span) -> Self {
+        Self::new(s.start.into(), s.end - s.start)
     }
 }