Cell paths: make optional path members short-circuit (#8554)

This is a follow-up to https://github.com/nushell/nushell/pull/8379 and
https://github.com/nushell/nushell/discussions/8502.

This PR makes it so that the new `?` syntax for marking a path member as
optional short-circuits, as voted on in the
[8502](https://github.com/nushell/nushell/discussions/8502) poll.
Previously, `{ foo: 123 }.bar?.baz` would raise an error:

```
> { foo: 123 }.bar?.baz
  × Data cannot be accessed with a cell path
   ╭─[entry #15:1:1]
 1 │ { foo: 123 }.bar?.baz
   ·                   ─┬─
   ·                    ╰── nothing doesn't support cell paths
   ╰────
   ```

Here's what was happening:

1. The `bar?` path member access returns `nothing` because there is no field named `bar` on the record
2. The `baz` path member access fails when trying to access a `baz` field on that `nothing` value

After this change, `{ foo: 123 }.bar?.baz` returns `nothing`; the failed `bar?` access immediately returns `nothing` and the `baz` access never runs.

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

@@ -742,7 +742,7 @@ impl Value {
                             if let Some(item) = val.get(*count) {
                                 current = item.clone();
                             } else if *optional {
-                                current = Value::nothing(*origin_span);
+                                return Ok(Value::nothing(*origin_span)); // short-circuit
                             } else if val.is_empty() {
                                 err_or_null!(
                                     ShellError::AccessEmptyContent { span: *origin_span },
@@ -762,7 +762,7 @@ impl Value {
                             if let Some(item) = val.get(*count) {
                                 current = Value::int(*item as i64, *origin_span);
                             } else if *optional {
-                                current = Value::nothing(*origin_span);
+                                return Ok(Value::nothing(*origin_span)); // short-circuit
                             } else if val.is_empty() {
                                 err_or_null!(
                                     ShellError::AccessEmptyContent { span: *origin_span },
@@ -782,7 +782,7 @@ impl Value {
                             if let Some(item) = val.clone().into_range_iter(None)?.nth(*count) {
                                 current = item.clone();
                             } else if *optional {
-                                current = Value::nothing(*origin_span);
+                                return Ok(Value::nothing(*origin_span)); // short-circuit
                             } else {
                                 err_or_null!(
                                     ShellError::AccessBeyondEndOfStream { span: *origin_span },
@@ -795,7 +795,8 @@ impl Value {
                                 Ok(val) => val,
                                 Err(err) => {
                                     if *optional {
-                                        Value::nothing(*origin_span)
+                                        return Ok(Value::nothing(*origin_span));
+                                    // short-circuit
                                     } else {
                                         return Err(err);
                                     }
@@ -803,7 +804,7 @@ impl Value {
                             };
                         }
                         Value::Nothing { .. } if *optional => {
-                            current = Value::nothing(*origin_span);
+                            return Ok(Value::nothing(*origin_span)); // short-circuit
                         }
                         // Records (and tables) are the only built-in which support column names,
                         // so only use this message for them.
@@ -843,7 +844,7 @@ impl Value {
                         }) {
                             current = found.1.clone();
                         } else if *optional {
-                            current = Value::nothing(*origin_span);
+                            return Ok(Value::nothing(*origin_span)); // short-circuit
                         } else {
                             if from_user_input {
                                 if let Some(suggestion) = did_you_mean(&cols, column_name) {
@@ -869,7 +870,7 @@ impl Value {
                         if columns.contains(&column_name.as_str()) {
                             current = val.get_column_value(column_name)?;
                         } else if *optional {
-                            current = Value::nothing(*origin_span);
+                            return Ok(Value::nothing(*origin_span)); // short-circuit
                         } else {
                             if from_user_input {
                                 if let Some(suggestion) = did_you_mean(&columns, column_name) {
@@ -934,7 +935,7 @@ impl Value {
                         current = val.follow_path_string(column_name.clone(), *origin_span)?;
                     }
                     Value::Nothing { .. } if *optional => {
-                        current = Value::nothing(*origin_span);
+                        return Ok(Value::nothing(*origin_span)); // short-circuit
                     }
                     Value::Error { error } => err_or_null!(*error.to_owned(), *origin_span),
                     x => {

src/tests/test_cell_path.rs

@@ -48,8 +48,10 @@ fn record_single_field_optional() -> TestResult {
 }
 
 #[test]
-fn record_single_field_optional_does_not_short_circuit() -> TestResult {
-    fail_test("{foo: 'bar'}.foobar?.baz", "nothing")
+fn record_single_field_optional_short_circuits() -> TestResult {
+    // Check that we return null as soon as the `.foobar?` access
+    // fails instead of erroring on the `.baz` access
+    run_test("{foo: 'bar'}.foobar?.baz  | to nuon", "null")
 }
 
 #[test]
@@ -138,3 +140,12 @@ fn do_not_delve_too_deep_in_nested_lists() -> TestResult {
 fn cell_path_literals() -> TestResult {
     run_test("let cell_path = $.a.b; {a: {b: 3}} | get $cell_path", "3")
 }
+
+// Test whether cell path access short-circuits properly
+#[test]
+fn deeply_nested_cell_path_short_circuits() -> TestResult {
+    run_test(
+        "{foo: [{bar: 'baz'}]}.foo.3?.bar.asdfdafg.234.foobar  | to nuon",
+        "null",
+    )
+}