From 803c24f9cee58cc9b325e495312e3e16a9d0f8d5 Mon Sep 17 00:00:00 2001 From: Luis <53346834+luismeyer95@users.noreply.github.com> Date: Sat, 31 May 2025 08:59:01 +0200 Subject: [PATCH] fix(parser): don't parse closure in block position (fixes #15417) (#15680) Hello! This is my 1st contribution and an attempt at fixing #15417. # Description When parsing a brace expression, check if the shape is a block or match block before attempting to parse it as a closure. Results: - `if true {|| print hi}` now produces a `nu::parser` error instead of executing and outputting `hi`. The `nu::parser` error is the same one produced by running `|| print hi` (`nu::parser::shell_oror`) - `match true {|| print hi}` now fails with a `nu::parser` error instead of passing parsing and failing with `nu::compile::invalid_keyword_call` My understanding reading the code/docs is that the shape is a contextual constraint that needs to be satisfied for parsing to succeed, in this case the `if` placing a `Block` shape constraint on next tokens. So it would need to be checked in priority (if not `Any`) to understand how the next tokens should be parsed. Is that correct? Or is there a reason I'm not aware of to ignore the shape and attempt to parse as closure like it's currently the case when the parser sees `|` or `||` as next tokens? # User-Facing Changes No change in behaviour, but this PR fixes parsing to fail on some incorrect syntax which could be considered a breaking change. # Tests + Formatting - Added corresponding tests - `toolkit check pr` passed --- crates/nu-parser/src/parser.rs | 4 ++++ crates/nu-parser/tests/test_parser.rs | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index ef72e8a104..185142d139 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -2057,6 +2057,10 @@ pub fn parse_brace_expr( } else if matches!(second_token_contents, Some(TokenContents::Pipe)) || matches!(second_token_contents, Some(TokenContents::PipePipe)) { + if matches!(shape, SyntaxShape::Block) { + working_set.error(ParseError::Mismatch("block".into(), "closure".into(), span)); + return Expression::garbage(working_set, span); + } parse_closure_expression(working_set, shape, span) } else if matches!(third_token, Some(b":")) { parse_full_cell_path(working_set, None, span) diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index 4dd7715067..8b4fd5e95b 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -2796,6 +2796,26 @@ mod input_types { } } + #[test] + fn closure_in_block_position_errors_correctly() { + let mut engine_state = EngineState::new(); + add_declarations(&mut engine_state); + + let mut working_set = StateWorkingSet::new(&engine_state); + let inputs = [r#"if true { || print hi }"#, r#"if true { |x| $x }"#]; + + for input in inputs { + parse(&mut working_set, None, input.as_bytes(), true); + assert!( + matches!( + working_set.parse_errors.first(), + Some(ParseError::Mismatch(_, _, _)) + ), + "testing: {input}" + ); + } + } + #[test] fn else_errors_correctly() { let mut engine_state = EngineState::new();