error when a closure is used as def body

# User-Facing Changes

`def` now errors instead of silently ignoring closure blocks:

```nushell
> def foo [] {|bar| }
Error: nu::parser::parse_mismatch

  × Parse mismatch during operation.
   ╭─[entry #1:1:12]
 1 │ def foo [] {|bar| }
   ·            ────┬───
   ·                ╰── expected definition body { ... }
   ╰────
```
This commit is contained in:
Solomon Victorino 2024-11-12 11:47:31 -07:00
parent dff6268d66
commit f844a3dbd8
16 changed files with 47 additions and 33 deletions

View file

@ -18,7 +18,7 @@ impl Command for Def {
.input_output_types(vec![(Type::Nothing, Type::Nothing)]) .input_output_types(vec![(Type::Nothing, Type::Nothing)])
.required("def_name", SyntaxShape::String, "Command name.") .required("def_name", SyntaxShape::String, "Command name.")
.required("params", SyntaxShape::Signature, "Parameters.") .required("params", SyntaxShape::Signature, "Parameters.")
.required("block", SyntaxShape::Closure(None), "Body of the definition.") .required("block", SyntaxShape::Block(false), "Body of the definition.")
.switch("env", "keep the environment defined inside the command", None) .switch("env", "keep the environment defined inside the command", None)
.switch("wrapped", "treat unknown flags and arguments as strings (requires ...rest-like parameter in signature)", None) .switch("wrapped", "treat unknown flags and arguments as strings (requires ...rest-like parameter in signature)", None)
.category(Category::Core) .category(Category::Core)

View file

@ -18,7 +18,7 @@ impl Command for ExportDef {
.input_output_types(vec![(Type::Nothing, Type::Nothing)]) .input_output_types(vec![(Type::Nothing, Type::Nothing)])
.required("def_name", SyntaxShape::String, "Command name.") .required("def_name", SyntaxShape::String, "Command name.")
.required("params", SyntaxShape::Signature, "Parameters.") .required("params", SyntaxShape::Signature, "Parameters.")
.required("block", SyntaxShape::Block, "Body of the definition.") .required("block", SyntaxShape::Block(false), "Body of the definition.")
.switch("env", "keep the environment defined inside the command", None) .switch("env", "keep the environment defined inside the command", None)
.switch("wrapped", "treat unknown flags and arguments as strings (requires ...rest-like parameter in signature)", None) .switch("wrapped", "treat unknown flags and arguments as strings (requires ...rest-like parameter in signature)", None)
.category(Category::Core) .category(Category::Core)

View file

@ -20,7 +20,7 @@ impl Command for ExportModule {
.required("module", SyntaxShape::String, "Module name or module path.") .required("module", SyntaxShape::String, "Module name or module path.")
.optional( .optional(
"block", "block",
SyntaxShape::Block, SyntaxShape::Block(true),
"Body of the module if 'module' parameter is not a path.", "Body of the module if 'module' parameter is not a path.",
) )
.category(Category::Core) .category(Category::Core)

View file

@ -27,7 +27,7 @@ impl Command for For {
SyntaxShape::Keyword(b"in".to_vec(), Box::new(SyntaxShape::Any)), SyntaxShape::Keyword(b"in".to_vec(), Box::new(SyntaxShape::Any)),
"Range of the loop.", "Range of the loop.",
) )
.required("block", SyntaxShape::Block, "The block to run.") .required("block", SyntaxShape::Block(true), "The block to run.")
.creates_scope() .creates_scope()
.category(Category::Core) .category(Category::Core)
} }

View file

@ -24,7 +24,7 @@ impl Command for If {
.required("cond", SyntaxShape::MathExpression, "Condition to check.") .required("cond", SyntaxShape::MathExpression, "Condition to check.")
.required( .required(
"then_block", "then_block",
SyntaxShape::Block, SyntaxShape::Block(true),
"Block to run if check succeeds.", "Block to run if check succeeds.",
) )
.optional( .optional(
@ -32,7 +32,7 @@ impl Command for If {
SyntaxShape::Keyword( SyntaxShape::Keyword(
b"else".to_vec(), b"else".to_vec(),
Box::new(SyntaxShape::OneOf(vec![ Box::new(SyntaxShape::OneOf(vec![
SyntaxShape::Block, SyntaxShape::Block(true),
SyntaxShape::Expression, SyntaxShape::Expression,
])), ])),
), ),

View file

@ -17,7 +17,7 @@ impl Command for Loop {
Signature::build("loop") Signature::build("loop")
.input_output_types(vec![(Type::Nothing, Type::Nothing)]) .input_output_types(vec![(Type::Nothing, Type::Nothing)])
.allow_variants_without_examples(true) .allow_variants_without_examples(true)
.required("block", SyntaxShape::Block, "Block to loop.") .required("block", SyntaxShape::Block(true), "Block to loop.")
.category(Category::Core) .category(Category::Core)
} }

View file

@ -20,7 +20,7 @@ impl Command for Module {
.required("module", SyntaxShape::String, "Module name or module path.") .required("module", SyntaxShape::String, "Module name or module path.")
.optional( .optional(
"block", "block",
SyntaxShape::Block, SyntaxShape::Block(true),
"Body of the module if 'module' parameter is not a module path.", "Body of the module if 'module' parameter is not a module path.",
) )
.category(Category::Core) .category(Category::Core)

View file

@ -16,7 +16,7 @@ impl Command for Try {
fn signature(&self) -> nu_protocol::Signature { fn signature(&self) -> nu_protocol::Signature {
Signature::build("try") Signature::build("try")
.input_output_types(vec![(Type::Any, Type::Any)]) .input_output_types(vec![(Type::Any, Type::Any)])
.required("try_block", SyntaxShape::Block, "Block to run.") .required("try_block", SyntaxShape::Block(true), "Block to run.")
.optional( .optional(
"catch_closure", "catch_closure",
SyntaxShape::Keyword( SyntaxShape::Keyword(

View file

@ -20,7 +20,7 @@ impl Command for While {
.required("cond", SyntaxShape::MathExpression, "Condition to check.") .required("cond", SyntaxShape::MathExpression, "Condition to check.")
.required( .required(
"block", "block",
SyntaxShape::Block, SyntaxShape::Block(true),
"Block to loop if check succeeds.", "Block to loop if check succeeds.",
) )
.category(Category::Core) .category(Category::Core)

View file

@ -14,7 +14,7 @@ impl Command for ExportEnv {
.input_output_types(vec![(Type::Nothing, Type::Nothing)]) .input_output_types(vec![(Type::Nothing, Type::Nothing)])
.required( .required(
"block", "block",
SyntaxShape::Block, SyntaxShape::Block(true),
"The block to run to set the environment.", "The block to run to set the environment.",
) )
.category(Category::Env) .category(Category::Env)

View file

@ -462,10 +462,9 @@ pub fn parse_def(
let block = working_set.get_block_mut(*block_id); let block = working_set.get_block_mut(*block_id);
block.signature = Box::new(sig.clone()); block.signature = Box::new(sig.clone());
} }
_ => working_set.error(ParseError::Expected( _ => {
"definition body closure { ... }", working_set.error(ParseError::Expected("definition body { ... }", arg.span))
arg.span, }
)),
} }
} }

View file

@ -799,7 +799,7 @@ fn parse_oneof(
| Some(ParseError::ExpectedWithStringMsg(_, error_span)) => { | Some(ParseError::ExpectedWithStringMsg(_, error_span)) => {
matches!( matches!(
shape, shape,
SyntaxShape::Block | SyntaxShape::Closure(_) | SyntaxShape::Expression SyntaxShape::Block(_) | SyntaxShape::Closure(_) | SyntaxShape::Expression
) && *error_span != spans[*spans_idx] ) && *error_span != spans[*spans_idx]
} }
_ => true, _ => true,
@ -1888,8 +1888,8 @@ pub fn parse_brace_expr(
// If we're empty, that means an empty record or closure // If we're empty, that means an empty record or closure
if matches!(shape, SyntaxShape::Closure(_)) { if matches!(shape, SyntaxShape::Closure(_)) {
parse_closure_expression(working_set, shape, span) parse_closure_expression(working_set, shape, span)
} else if matches!(shape, SyntaxShape::Block) { } else if let SyntaxShape::Block(capture_mutable) = shape {
parse_block_expression(working_set, span) parse_block_expression(working_set, capture_mutable, span)
} else if matches!(shape, SyntaxShape::MatchBlock) { } else if matches!(shape, SyntaxShape::MatchBlock) {
parse_match_block_expression(working_set, span) parse_match_block_expression(working_set, span)
} else { } else {
@ -1903,8 +1903,8 @@ pub fn parse_brace_expr(
parse_full_cell_path(working_set, None, span) parse_full_cell_path(working_set, None, span)
} else if matches!(shape, SyntaxShape::Closure(_)) { } else if matches!(shape, SyntaxShape::Closure(_)) {
parse_closure_expression(working_set, shape, span) parse_closure_expression(working_set, shape, span)
} else if matches!(shape, SyntaxShape::Block) { } else if let SyntaxShape::Block(capture_mutable) = shape {
parse_block_expression(working_set, span) parse_block_expression(working_set, capture_mutable, span)
} else if matches!(shape, SyntaxShape::MatchBlock) { } else if matches!(shape, SyntaxShape::MatchBlock) {
parse_match_block_expression(working_set, span) parse_match_block_expression(working_set, span)
} else if second_token.is_some_and(|c| { } else if second_token.is_some_and(|c| {
@ -4405,7 +4405,11 @@ fn table_type(head: &[Expression], rows: &[Vec<Expression>]) -> (Type, Vec<Parse
(Type::Table(ty.into()), errors) (Type::Table(ty.into()), errors)
} }
pub fn parse_block_expression(working_set: &mut StateWorkingSet, span: Span) -> Expression { pub fn parse_block_expression(
working_set: &mut StateWorkingSet,
capture_mutable: &bool,
span: Span,
) -> Expression {
trace!("parsing: block expression"); trace!("parsing: block expression");
let bytes = working_set.get_span_contents(span); let bytes = working_set.get_span_contents(span);
@ -4460,7 +4464,15 @@ pub fn parse_block_expression(working_set: &mut StateWorkingSet, span: Span) ->
let block_id = working_set.add_block(Arc::new(output)); let block_id = working_set.add_block(Arc::new(output));
Expression::new(working_set, Expr::Block(block_id), span, Type::Block) Expression::new(
working_set,
match capture_mutable {
true => Expr::Block(block_id),
false => Expr::Closure(block_id),
},
span,
Type::Block,
)
} }
pub fn parse_match_block_expression(working_set: &mut StateWorkingSet, span: Span) -> Expression { pub fn parse_match_block_expression(working_set: &mut StateWorkingSet, span: Span) -> Expression {
@ -4645,7 +4657,7 @@ pub fn parse_match_block_expression(working_set: &mut StateWorkingSet, span: Spa
working_set, working_set,
&[output[position].span], &[output[position].span],
&mut 0, &mut 0,
&SyntaxShape::OneOf(vec![SyntaxShape::Block, SyntaxShape::Expression]), &SyntaxShape::OneOf(vec![SyntaxShape::Block(true), SyntaxShape::Expression]),
); );
position += 1; position += 1;
working_set.exit_scope(); working_set.exit_scope();
@ -4921,7 +4933,7 @@ pub fn parse_value(
// Be sure to return ParseError::Expected(..) if invoked for one of these shapes, but lex // Be sure to return ParseError::Expected(..) if invoked for one of these shapes, but lex
// stream doesn't start with '{'} -- parsing in SyntaxShape::Any arm depends on this error variant. // stream doesn't start with '{'} -- parsing in SyntaxShape::Any arm depends on this error variant.
SyntaxShape::Block | SyntaxShape::Closure(..) | SyntaxShape::Record(_) => { SyntaxShape::Block(_) | SyntaxShape::Closure(..) | SyntaxShape::Record(_) => {
working_set.error(ParseError::Expected("block, closure or record", span)); working_set.error(ParseError::Expected("block, closure or record", span));
Expression::garbage(working_set, span) Expression::garbage(working_set, span)

View file

@ -1991,7 +1991,7 @@ mod input_types {
.input_output_types(vec![(Type::Nothing, Type::Nothing)]) .input_output_types(vec![(Type::Nothing, Type::Nothing)])
.required("def_name", SyntaxShape::String, "definition name") .required("def_name", SyntaxShape::String, "definition name")
.required("params", SyntaxShape::Signature, "parameters") .required("params", SyntaxShape::Signature, "parameters")
.required("body", SyntaxShape::Closure(None), "body of the definition") .required("body", SyntaxShape::Block(false), "body of the definition")
.category(Category::Core) .category(Category::Core)
} }
@ -2228,7 +2228,7 @@ mod input_types {
.required("cond", SyntaxShape::MathExpression, "condition to check") .required("cond", SyntaxShape::MathExpression, "condition to check")
.required( .required(
"then_block", "then_block",
SyntaxShape::Block, SyntaxShape::Block(true),
"block to run if check succeeds", "block to run if check succeeds",
) )
.optional( .optional(
@ -2236,7 +2236,7 @@ mod input_types {
SyntaxShape::Keyword( SyntaxShape::Keyword(
b"else".to_vec(), b"else".to_vec(),
Box::new(SyntaxShape::OneOf(vec![ Box::new(SyntaxShape::OneOf(vec![
SyntaxShape::Block, SyntaxShape::Block(true),
SyntaxShape::Expression, SyntaxShape::Expression,
])), ])),
), ),

View file

@ -18,7 +18,8 @@ pub enum SyntaxShape {
Binary, Binary,
/// A block is allowed, eg `{start this thing}` /// A block is allowed, eg `{start this thing}`
Block, /// `bool`: capture mutable variables
Block(bool),
/// A boolean value, eg `true` or `false` /// A boolean value, eg `true` or `false`
Boolean, Boolean,
@ -143,7 +144,7 @@ impl SyntaxShape {
match self { match self {
SyntaxShape::Any => Type::Any, SyntaxShape::Any => Type::Any,
SyntaxShape::Block => Type::Block, SyntaxShape::Block(_) => Type::Block,
SyntaxShape::Closure(_) => Type::Closure, SyntaxShape::Closure(_) => Type::Closure,
SyntaxShape::Binary => Type::Binary, SyntaxShape::Binary => Type::Binary,
SyntaxShape::CellPath => Type::Any, SyntaxShape::CellPath => Type::Any,
@ -209,7 +210,7 @@ impl Display for SyntaxShape {
SyntaxShape::Directory => write!(f, "directory"), SyntaxShape::Directory => write!(f, "directory"),
SyntaxShape::GlobPattern => write!(f, "glob"), SyntaxShape::GlobPattern => write!(f, "glob"),
SyntaxShape::ImportPattern => write!(f, "import"), SyntaxShape::ImportPattern => write!(f, "import"),
SyntaxShape::Block => write!(f, "block"), SyntaxShape::Block(_) => write!(f, "block"),
SyntaxShape::Closure(args) => { SyntaxShape::Closure(args) => {
if let Some(args) = args { if let Some(args) = args {
let arg_vec: Vec<_> = args.iter().map(|x| x.to_string()).collect(); let arg_vec: Vec<_> = args.iter().map(|x| x.to_string()).collect();

View file

@ -109,7 +109,7 @@ impl Type {
Type::Range => SyntaxShape::Range, Type::Range => SyntaxShape::Range,
Type::Bool => SyntaxShape::Boolean, Type::Bool => SyntaxShape::Boolean,
Type::String => SyntaxShape::String, Type::String => SyntaxShape::String,
Type::Block => SyntaxShape::Block, // FIXME needs more accuracy Type::Block => SyntaxShape::Block(true), // FIXME needs more accuracy
Type::Closure => SyntaxShape::Closure(None), // FIXME needs more accuracy Type::Closure => SyntaxShape::Closure(None), // FIXME needs more accuracy
Type::CellPath => SyntaxShape::CellPath, Type::CellPath => SyntaxShape::CellPath,
Type::Duration => SyntaxShape::Duration, Type::Duration => SyntaxShape::Duration,

View file

@ -931,8 +931,10 @@ fn record_missing_value() -> TestResult {
} }
#[test] #[test]
fn def_requires_body_closure() -> TestResult { fn def_requires_body_block() -> TestResult {
fail_test("def a [] (echo 4)", "expected definition body closure") fail_test("def a [] (echo 4)", "expected definition body")?;
fail_test("def a [] {|| }", "expected definition body")?;
fail_test("def a [] {|b: int| echo $b }", "expected definition body")
} }
#[test] #[test]