Skip to content

Commit

Permalink
Add OneOf shape to fix else (#7385)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
sophiajt committed Dec 7, 2022
1 parent eaec480 commit fa15a28
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 2 deletions.
8 changes: 7 additions & 1 deletion crates/nu-command/src/core_commands/if_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 25 additions & 1 deletion crates/nu-parser/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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,
)),
);
}
}
Expand Down
9 changes: 9 additions & 0 deletions crates/nu-protocol/src/syntax_shape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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"),
}
}
Expand Down
16 changes: 16 additions & 0 deletions src/tests/test_conditionals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
}

0 comments on commit fa15a28

Please sign in to comment.