Protocol: debug_assert!() Span to reflect a valid slice (#6806)

Also enforce this by #[non_exhaustive] span such that going forward we
cannot, in debug builds (1), construct invalid spans.

The motivation for this stems from #6431 where I've seen crashes due to
invalid slice indexing.

My hope is this will mitigate such senarios

1. https://github.com/nushell/nushell/pull/6431#issuecomment-1278147241

# Description

(description of your pull request here)

# Tests

Make sure you've done the following:

- [ ] Add tests that cover your changes, either in the command examples,
the crate/tests folder, or in the /tests folder.
- [ ] Try to think about corner cases and various ways how your changes
could break. Cover them with tests.
- [ ] If adding tests is not possible, please document in the PR body a
minimal example with steps on how to reproduce so one can verify your
change works.

Make sure you've run and fixed any issues with these commands:

- [x] `cargo fmt --all -- --check` to check standard code formatting
(`cargo fmt --all` applies these changes)
- [ ] `cargo clippy --workspace --features=extra -- -D warnings -D
clippy::unwrap_used -A clippy::needless_collect` to check that you're
using the standard code style
- [ ] `cargo test --workspace --features=extra` to check that all the
tests pass

# Documentation

- [ ] If your PR touches a user-facing nushell feature then make sure
that there is an entry in the documentation
(https://github.com/nushell/nushell.github.io) for the feature, and
update it if necessary.

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

@@ -32,7 +32,7 @@ impl ImportPattern {
             head: ImportPatternHead {
                 name: vec![],
                 id: None,
-                span: Span { start: 0, end: 0 },
+                span: Span::unknown(),
             },
             members: vec![],
             hidden: HashSet::new(),

crates/nu-protocol/src/config.rs

@@ -981,7 +981,7 @@ fn create_hooks(value: &Value) -> Result<Hooks, ShellError> {
             _ => Err(ShellError::UnsupportedConfigValue(
                 "record for 'hooks' config".into(),
                 "non-record value".into(),
-                Span { start: 0, end: 0 },
+                Span::unknown(),
             )),
         },
     }

crates/nu-protocol/src/engine/engine_state.rs

@@ -755,10 +755,7 @@ impl EngineState {
     pub fn get_file_source(&self, file_id: usize) -> String {
         for file in self.files.iter().enumerate() {
             if file.0 == file_id {
-                let contents = self.get_span_contents(&Span {
-                    start: file.1 .1,
-                    end: file.1 .2,
-                });
+                let contents = self.get_span_contents(&Span::new(file.1 .1, file.1 .2));
                 let output = String::from_utf8_lossy(contents).to_string();
 
                 return output;
@@ -1318,10 +1315,9 @@ impl<'a> StateWorkingSet<'a> {
     pub fn get_file_source(&self, file_id: usize) -> String {
         for file in self.files().enumerate() {
             if file.0 == file_id {
-                let output = String::from_utf8_lossy(self.get_span_contents(Span {
-                    start: file.1 .1,
-                    end: file.1 .2,
-                }))
+                let output = String::from_utf8_lossy(
+                    self.get_span_contents(Span::new(file.1 .1, file.1 .2)),
+                )
                 .to_string();
 
                 return output;
@@ -2041,10 +2037,7 @@ impl<'a> miette::SourceCode for &StateWorkingSet<'a> {
                     let found_file = "Found matching file";
                     dbg!(found_file);
                 }
-                let our_span = Span {
-                    start: *start,
-                    end: *end,
-                };
+                let our_span = Span::new(*start, *end);
                 // We need to move to a local span because we're only reading
                 // the specific file contents via self.get_span_contents.
                 let local_span = (span.offset() - *start, span.len()).into();

crates/nu-protocol/src/pipeline_data.rs

@@ -308,7 +308,7 @@ impl PipelineData {
             }
             PipelineData::ListStream(stream, ..) => Ok(stream.map(f).into_pipeline_data(ctrlc)),
             PipelineData::ExternalStream { stdout: None, .. } => {
-                Ok(PipelineData::new(Span { start: 0, end: 0 }))
+                Ok(PipelineData::new(Span::unknown()))
             }
             PipelineData::ExternalStream {
                 stdout: Some(stream),
@@ -366,7 +366,7 @@ impl PipelineData {
                 Ok(stream.flat_map(f).into_pipeline_data(ctrlc))
             }
             PipelineData::ExternalStream { stdout: None, .. } => {
-                Ok(PipelineData::new(Span { start: 0, end: 0 }))
+                Ok(PipelineData::new(Span::unknown()))
             }
             PipelineData::ExternalStream {
                 stdout: Some(stream),
@@ -419,7 +419,7 @@ impl PipelineData {
             }
             PipelineData::ListStream(stream, ..) => Ok(stream.filter(f).into_pipeline_data(ctrlc)),
             PipelineData::ExternalStream { stdout: None, .. } => {
-                Ok(PipelineData::new(Span { start: 0, end: 0 }))
+                Ok(PipelineData::new(Span::unknown()))
             }
             PipelineData::ExternalStream {
                 stdout: Some(stream),

crates/nu-protocol/src/span.rs

@@ -14,6 +14,7 @@ where
 /// 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,
@@ -28,24 +29,28 @@ impl From<Span> for SourceSpan {
 
 impl Span {
     pub fn new(start: usize, end: usize) -> Span {
+        debug_assert!(
+            end >= start,
+            "Can't create a Span whose end < start, start={}, end={}",
+            start,
+            end
+        );
+
         Span { start, end }
     }
 
-    pub fn unknown() -> Self {
-        Self::new(0, 0)
+    pub const fn unknown() -> Span {
+        Span { 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 fn test_data() -> Span {
-        Span { start: 0, end: 0 }
+    pub const fn test_data() -> Span {
+        Self::unknown()
     }
 
     pub fn offset(&self, offset: usize) -> Span {
-        Span {
-            start: self.start - offset,
-            end: self.end - offset,
-        }
+        Span::new(self.start - offset, self.end - offset)
     }
 
     pub fn contains(&self, pos: usize) -> bool {
@@ -70,15 +75,17 @@ impl Span {
 pub fn span(spans: &[Span]) -> Span {
     let length = spans.len();
 
+    //TODO debug_assert!(length > 0, "expect spans > 0");
     if length == 0 {
-        // TODO: do this for now, but we might also want to protect against this case
-        Span { start: 0, end: 0 }
+        Span::unknown()
     } else if length == 1 {
         spans[0]
     } else {
-        Span {
-            start: spans[0].start,
-            end: spans[length - 1].end,
-        }
+        let end = spans
+            .iter()
+            .map(|s| s.end)
+            .max()
+            .expect("Must be an end. Length > 0");
+        Span::new(spans[0].start, end)
     }
 }

crates/nu-protocol/src/value/mod.rs

@@ -1391,7 +1391,7 @@ impl Value {
 impl Default for Value {
     fn default() -> Self {
         Value::Nothing {
-            span: Span { start: 0, end: 0 },
+            span: Span::unknown(),
         }
     }
 }