From fa15a2856a12e40dff52a396c6bd31cdf1c845c5 Mon Sep 17 00:00:00 2001
From: JT <547158+jntrnr@users.noreply.github.com>
Date: Thu, 8 Dec 2022 08:58:54 +1300
Subject: [PATCH] Add OneOf shape to fix `else` (#7385)

# Description

fixes #7384

This is a stop-gap fix until we remove type-directed parsing. With this,
you can create a `OneOf` shape that can be one of a list of syntax
shapes. This gives you a little more control than you get with `Any`,
allowing you to add `Block` without breaking other parsing rules.

# User-Facing Changes

`else` block will no longer capture variables as it will now use a block
instead of a closure.

# Tests + Formatting

Don't forget to add tests that cover your changes.

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

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

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
---
 crates/nu-command/src/core_commands/if_.rs |  8 ++++++-
 crates/nu-parser/src/parser.rs             | 26 +++++++++++++++++++++-
 crates/nu-protocol/src/syntax_shape.rs     |  9 ++++++++
 src/tests/test_conditionals.rs             | 16 +++++++++++++
 4 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/crates/nu-command/src/core_commands/if_.rs b/crates/nu-command/src/core_commands/if_.rs
index 54e4629db..160e7a82f 100644
--- a/crates/nu-command/src/core_commands/if_.rs
+++ b/crates/nu-command/src/core_commands/if_.rs
@@ -28,7 +28,13 @@ impl Command for If {
             )
             .optional(
                 "else_expression",
-                SyntaxShape::Keyword(b"else".to_vec(), Box::new(SyntaxShape::Expression)),
+                SyntaxShape::Keyword(
+                    b"else".to_vec(),
+                    Box::new(SyntaxShape::OneOf(vec![
+                        SyntaxShape::Block,
+                        SyntaxShape::Expression,
+                    ])),
+                ),
                 "expression or block to run if check fails",
             )
             .category(Category::Core)
diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs
index bf5d1f638..8dfd5d550 100644
--- a/crates/nu-parser/src/parser.rs
+++ b/crates/nu-parser/src/parser.rs
@@ -644,6 +644,27 @@ pub fn parse_multispan_value(
 
             (arg, error)
         }
+        SyntaxShape::OneOf(shapes) => {
+            for shape in shapes.iter() {
+                if let (s, None) = parse_multispan_value(
+                    working_set,
+                    spans,
+                    spans_idx,
+                    shape,
+                    expand_aliases_denylist,
+                ) {
+                    return (s, None);
+                }
+            }
+            let span = spans[*spans_idx];
+            (
+                Expression::garbage(span),
+                Some(ParseError::Expected(
+                    format!("one of a list of accepted shapes: {:?}", shapes),
+                    span,
+                )),
+            )
+        }
         SyntaxShape::Expression => {
             trace!("parsing: expression");
 
@@ -4252,7 +4273,10 @@ pub fn parse_value(
             } else {
                 return (
                     Expression::garbage(span),
-                    Some(ParseError::Expected("non-block value".into(), span)),
+                    Some(ParseError::Expected(
+                        format!("non-block value: {}", shape),
+                        span,
+                    )),
                 );
             }
         }
diff --git a/crates/nu-protocol/src/syntax_shape.rs b/crates/nu-protocol/src/syntax_shape.rs
index 495ff9e5f..26a32fe46 100644
--- a/crates/nu-protocol/src/syntax_shape.rs
+++ b/crates/nu-protocol/src/syntax_shape.rs
@@ -101,6 +101,9 @@ pub enum SyntaxShape {
     /// A custom shape with custom completion logic
     Custom(Box<SyntaxShape>, DeclId),
 
+    /// One of a list of possible items, checked in order
+    OneOf(Vec<SyntaxShape>),
+
     /// Nothing
     Nothing,
 }
@@ -132,6 +135,7 @@ impl SyntaxShape {
             SyntaxShape::Keyword(_, expr) => expr.to_type(),
             SyntaxShape::MathExpression => Type::Any,
             SyntaxShape::Number => Type::Number,
+            SyntaxShape::OneOf(_) => Type::Any,
             SyntaxShape::Operator => Type::Any,
             SyntaxShape::Range => Type::Any,
             SyntaxShape::Record => Type::Record(vec![]), // FIXME: What role should fields play in the Record type?
@@ -191,6 +195,11 @@ impl Display for SyntaxShape {
             SyntaxShape::Boolean => write!(f, "bool"),
             SyntaxShape::Error => write!(f, "error"),
             SyntaxShape::Custom(x, _) => write!(f, "custom<{}>", x),
+            SyntaxShape::OneOf(list) => {
+                let arg_vec: Vec<_> = list.iter().map(|x| x.to_string()).collect();
+                let arg_string = arg_vec.join(", ");
+                write!(f, "one_of({})", arg_string)
+            }
             SyntaxShape::Nothing => write!(f, "nothing"),
         }
     }
diff --git a/src/tests/test_conditionals.rs b/src/tests/test_conditionals.rs
index 285485c92..20307ffe6 100644
--- a/src/tests/test_conditionals.rs
+++ b/src/tests/test_conditionals.rs
@@ -59,3 +59,19 @@ fn if_elseif3() -> TestResult {
 fn if_elseif4() -> TestResult {
     run_test("if 2 > 3 { 5 } else if 6 < 7 { 4 } else { 8 } ", "4")
 }
+
+#[test]
+fn mutation_in_else() -> TestResult {
+    run_test(
+        "mut x = 100; if 2 > 3 { $x = 200 } else { $x = 300 }; $x ",
+        "300",
+    )
+}
+
+#[test]
+fn mutation_in_else2() -> TestResult {
+    run_test(
+        "mut x = 100; if 2 > 3 { $x = 200 } else if true { $x = 400 } else { $x = 300 }; $x ",
+        "400",
+    )
+}