From 36c30ade3aef0dee7606305b4908a3a0456d17d9 Mon Sep 17 00:00:00 2001 From: Bahex Date: Tue, 13 May 2025 22:25:07 +0300 Subject: [PATCH] fix parsing of bare word string interpolations that start with a sub expression (#15735) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - fixes #15731 # Description Existing bare word string interpolation only works if the string doesn't start with a subxpression. ```nushell echo fork(2) # => fork2 echo (2)fork # => Error: nu::parser::unclosed_delimiter # => # => × Unclosed delimiter. # => ╭─[entry #25:1:13] # => 1 │ echo (2)fork # => ╰──── ``` This PR lifts that restriction. ```nushell echo fork(2) # => fork2 echo (2)fork # => 2fork ``` This was first brought to my attention on discord with the following command failing to parse. ```nushell docker run -u (id -u):(id -g) ``` It now works. # User-Facing Changes # Tests + Formatting No existing test broke or required tweaking. Additional tests covering this case was added. - :green_circle: toolkit fmt - :green_circle: toolkit clippy - :green_circle: toolkit test - :green_circle: toolkit test stdlib # After Submitting --------- Co-authored-by: Bahex <17417311+Bahex@users.noreply.github.com> --- crates/nu-parser/src/parser.rs | 34 ++++++++++--- crates/nu-parser/tests/test_parser.rs | 72 +++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 7 deletions(-) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index a458fb85ed..0be38f800c 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -455,8 +455,10 @@ fn parse_external_arg(working_set: &mut StateWorkingSet, span: Span) -> External fn parse_regular_external_arg(working_set: &mut StateWorkingSet, span: Span) -> Expression { let contents = working_set.get_span_contents(span); - if contents.starts_with(b"$") || contents.starts_with(b"(") { + if contents.starts_with(b"$") { parse_dollar_expr(working_set, span) + } else if contents.starts_with(b"(") { + parse_paren_expr(working_set, span, &SyntaxShape::Any) } else if contents.starts_with(b"[") { parse_list_expression(working_set, span, &SyntaxShape::Any) } else { @@ -1977,15 +1979,33 @@ pub fn parse_paren_expr( let starting_error_count = working_set.parse_errors.len(); if let Some(expr) = parse_range(working_set, span) { - expr - } else { - working_set.parse_errors.truncate(starting_error_count); + return expr; + } - if matches!(shape, SyntaxShape::Signature) { - parse_signature(working_set, span) + working_set.parse_errors.truncate(starting_error_count); + + if matches!(shape, SyntaxShape::Signature) { + return parse_signature(working_set, span); + } + + let fcp_expr = parse_full_cell_path(working_set, None, span); + let fcp_error_count = working_set.parse_errors.len(); + if fcp_error_count > starting_error_count { + let malformed_subexpr = working_set.parse_errors[starting_error_count..] + .iter() + .any(|e| match e { + ParseError::Unclosed(right, _) if right == ")" => true, + ParseError::Unbalanced(left, right, _) if left == "(" && right == ")" => true, + _ => false, + }); + if malformed_subexpr { + working_set.parse_errors.truncate(starting_error_count); + parse_string(working_set, span) } else { - parse_full_cell_path(working_set, None, span) + fcp_expr } + } else { + fcp_expr } } diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index 5e204b38a2..4dd7715067 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -1576,6 +1576,78 @@ mod string { assert!(matches!(subexprs[3], &Expr::FullCellPath(..))); } + #[test] + pub fn parse_string_interpolation_bare_starting_subexpr() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let block = parse( + &mut working_set, + None, + b"\"\" ++ (1 + 3)foo(7 - 5)bar", + true, + ); + + assert!(working_set.parse_errors.is_empty()); + + assert_eq!(block.len(), 1); + let pipeline = &block.pipelines[0]; + assert_eq!(pipeline.len(), 1); + let element = &pipeline.elements[0]; + assert!(element.redirection.is_none()); + + let subexprs: Vec<&Expr> = match &element.expr.expr { + Expr::BinaryOp(_, _, rhs) => match &rhs.expr { + Expr::StringInterpolation(expressions) => { + expressions.iter().map(|e| &e.expr).collect() + } + _ => panic!("Expected an `Expr::StringInterpolation`"), + }, + _ => panic!("Expected an `Expr::BinaryOp`"), + }; + + assert_eq!(subexprs.len(), 4); + + assert!(matches!(subexprs[0], &Expr::FullCellPath(..))); + assert_eq!(subexprs[1], &Expr::String("foo".to_string())); + assert!(matches!(subexprs[2], &Expr::FullCellPath(..))); + assert_eq!(subexprs[3], &Expr::String("bar".to_string())); + } + + #[test] + pub fn parse_string_interpolation_bare_starting_subexpr_external_arg() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let block = parse(&mut working_set, None, b"^echo ($nu.home-path)/path", true); + + assert!(working_set.parse_errors.is_empty()); + + assert_eq!(block.len(), 1); + let pipeline = &block.pipelines[0]; + assert_eq!(pipeline.len(), 1); + let element = &pipeline.elements[0]; + assert!(element.redirection.is_none()); + + let subexprs: Vec<&Expr> = match &element.expr.expr { + Expr::ExternalCall(_, args) => match &args[0] { + ExternalArgument::Regular(expression) => match &expression.expr { + Expr::StringInterpolation(expressions) => { + expressions.iter().map(|e| &e.expr).collect() + } + _ => panic!("Expected an `ExternalArgument::Regular`"), + }, + _ => panic!("Expected an `Expr::StringInterpolation`"), + }, + _ => panic!("Expected an `Expr::BinaryOp`"), + }; + + assert_eq!(subexprs.len(), 2); + + assert!(matches!(subexprs[0], &Expr::FullCellPath(..))); + assert_eq!(subexprs[1], &Expr::String("/path".to_string())); + } + #[test] pub fn parse_nested_expressions() { let engine_state = EngineState::new();