From e39dcab60e8a8108eb07d3dbf086a7029d915017 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Wed, 5 Jun 2024 19:10:22 -0700 Subject: [PATCH] wip: fix run-external quoting, parsing for external call --- crates/nu-cli/src/syntax_highlight.rs | 2 +- crates/nu-command/src/filesystem/util.rs | 2 +- crates/nu-command/src/system/run_external.rs | 115 +++------ crates/nu-parser/src/flatten.rs | 26 +- crates/nu-parser/src/parser.rs | 196 +++++++++++---- crates/nu-parser/tests/test_parser.rs | 245 ++++++++++++++++--- crates/nu-protocol/src/ast/expr.rs | 8 +- crates/nu-protocol/src/ast/expression.rs | 4 +- crates/nu-protocol/src/debugger/profiler.rs | 2 +- crates/nu-protocol/src/eval_base.rs | 2 +- 10 files changed, 412 insertions(+), 190 deletions(-) diff --git a/crates/nu-cli/src/syntax_highlight.rs b/crates/nu-cli/src/syntax_highlight.rs index e296943af6..ec921b48d6 100644 --- a/crates/nu-cli/src/syntax_highlight.rs +++ b/crates/nu-cli/src/syntax_highlight.rs @@ -452,7 +452,7 @@ fn find_matching_block_end_in_expr( } } - Expr::StringInterpolation(exprs) => exprs.iter().find_map(|expr| { + Expr::StringInterpolation(exprs, _) => exprs.iter().find_map(|expr| { find_matching_block_end_in_expr( line, working_set, diff --git a/crates/nu-command/src/filesystem/util.rs b/crates/nu-command/src/filesystem/util.rs index 1b755875bd..c1dc925968 100644 --- a/crates/nu-command/src/filesystem/util.rs +++ b/crates/nu-command/src/filesystem/util.rs @@ -115,7 +115,7 @@ pub fn get_rest_for_glob_pattern( Value::String { val, .. } if matches!( &expr.expr, - Expr::FullCellPath(_) | Expr::StringInterpolation(_) + Expr::FullCellPath(_) | Expr::StringInterpolation(_, _) ) => { // should not expand if given input type is not glob. diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index 614befe352..0e848887df 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -9,7 +9,6 @@ use nu_protocol::{ use nu_system::ForegroundChild; use nu_utils::IgnoreCaseExt; use std::{ - borrow::Cow, io::Write, path::{Path, PathBuf}, process::Stdio, @@ -32,8 +31,16 @@ impl Command for External { fn signature(&self) -> nu_protocol::Signature { Signature::build(self.name()) .input_output_types(vec![(Type::Any, Type::Any)]) - .required("command", SyntaxShape::String, "External command to run.") - .rest("args", SyntaxShape::Any, "Arguments for external command.") + .required( + "command", + SyntaxShape::OneOf(vec![SyntaxShape::GlobPattern, SyntaxShape::String]), + "External command to run.", + ) + .rest( + "args", + SyntaxShape::OneOf(vec![SyntaxShape::GlobPattern, SyntaxShape::Any]), + "Arguments for external command.", + ) .category(Category::System) } @@ -216,19 +223,6 @@ impl Command for External { } } -/// Removes surrounding quotes from a string. Doesn't remove quotes from raw -/// strings. Returns the original string if it doesn't have matching quotes. -fn remove_quotes(s: &str) -> &str { - let quoted_by_double_quotes = s.len() >= 2 && s.starts_with('"') && s.ends_with('"'); - let quoted_by_single_quotes = s.len() >= 2 && s.starts_with('\'') && s.ends_with('\''); - let quoted_by_backticks = s.len() >= 2 && s.starts_with('`') && s.ends_with('`'); - if quoted_by_double_quotes || quoted_by_single_quotes || quoted_by_backticks { - &s[1..s.len() - 1] - } else { - s - } -} - /// Evaluate all arguments from a call, performing expansions when necessary. pub fn eval_arguments_from_call( engine_state: &EngineState, @@ -240,14 +234,13 @@ pub fn eval_arguments_from_call( let mut args: Vec> = vec![]; for (expr, spread) in call.rest_iter(1) { if is_bare_string(expr) { - // If `expr` is a bare string, perform tilde-expansion, - // glob-expansion, and inner-quotes-removal, in that order. + // If `expr` is a bare string, perform glob expansion. for arg in eval_argument(engine_state, stack, expr, spread)? { - let tilde_expanded = expand_tilde(&arg); - for glob_expanded in expand_glob(&tilde_expanded, &cwd, expr.span, ctrlc)? { - let inner_quotes_removed = remove_inner_quotes(&glob_expanded); - args.push(inner_quotes_removed.into_owned().into_spanned(expr.span)); - } + args.extend( + expand_glob(&arg, &cwd, expr.span, ctrlc)? + .into_iter() + .map(|s| s.into_spanned(expr.span)), + ); } } else { for arg in eval_argument(engine_state, stack, expr, spread)? { @@ -271,12 +264,6 @@ fn eval_argument( expr: &Expression, spread: bool, ) -> Result, ShellError> { - // Remove quotes from string literals. - let mut expr = expr.clone(); - if let Expr::String(s) = &expr.expr { - expr.expr = Expr::String(remove_quotes(s).into()); - } - let eval = get_eval_expression(engine_state); match eval(engine_state, stack, &expr)? { Value::List { vals, .. } => { @@ -291,6 +278,13 @@ fn eval_argument( }) } } + Value::Glob { val, .. } => { + if spread { + Err(ShellError::CannotSpreadAsList { span: expr.span }) + } else { + Ok(vec![expand_tilde(&val)]) + } + } value => { if spread { Err(ShellError::CannotSpreadAsList { span: expr.span }) @@ -304,14 +298,12 @@ fn eval_argument( /// Returns whether an expression is considered a bare string. /// /// Bare strings are defined as string literals that are either unquoted or -/// quoted by backticks. Raw strings or string interpolations don't count. +/// quoted by backticks. Raw strings or non-bare string interpolations don't count. fn is_bare_string(expr: &Expression) -> bool { - let Expr::String(s) = &expr.expr else { - return false; - }; - let quoted_by_double_quotes = s.len() >= 2 && s.starts_with('"') && s.ends_with('"'); - let quoted_by_single_quotes = s.len() >= 2 && s.starts_with('\'') && s.ends_with('\''); - !quoted_by_double_quotes && !quoted_by_single_quotes + matches!( + expr.expr, + Expr::GlobPattern(_, false) | Expr::StringInterpolation(_, false) + ) } /// Performs tilde expansion on `arg`. Returns the original string if `arg` @@ -396,27 +388,6 @@ fn expand_glob( } } -/// Transforms `--option="value"` into `--option=value`. `value` can be quoted -/// with double quotes, single quotes, or backticks. Only removes the outermost -/// pair of quotes after the equal sign. -fn remove_inner_quotes(arg: &str) -> Cow<'_, str> { - // Check that `arg` is a long option. - if !arg.starts_with("--") { - return Cow::Borrowed(arg); - } - // Split `arg` on the first `=`. - let Some((option, value)) = arg.split_once('=') else { - return Cow::Borrowed(arg); - }; - // Check that `option` doesn't contain quotes. - if option.contains('"') || option.contains('\'') || option.contains('`') { - return Cow::Borrowed(arg); - } - // Remove the outermost pair of quotes from `value`. - let value = remove_quotes(value); - Cow::Owned(format!("{option}={value}")) -} - /// Write `PipelineData` into `writer`. If `PipelineData` is not binary, it is /// first rendered using the `table` command. /// @@ -651,17 +622,6 @@ mod test { use nu_protocol::ast::ListItem; use nu_test_support::{fs::Stub, playground::Playground}; - #[test] - fn test_remove_quotes() { - assert_eq!(remove_quotes(r#""#), r#""#); - assert_eq!(remove_quotes(r#"'"#), r#"'"#); - assert_eq!(remove_quotes(r#"''"#), r#""#); - assert_eq!(remove_quotes(r#""foo""#), r#"foo"#); - assert_eq!(remove_quotes(r#"`foo '"' bar`"#), r#"foo '"' bar"#); - assert_eq!(remove_quotes(r#"'foo' bar"#), r#"'foo' bar"#); - assert_eq!(remove_quotes(r#"r#'foo'#"#), r#"r#'foo'#"#); - } - #[test] fn test_eval_argument() { fn expression(expr: Expr) -> Expression { @@ -741,25 +701,6 @@ mod test { }) } - #[test] - fn test_remove_inner_quotes() { - let actual = remove_inner_quotes(r#"--option=value"#); - let expected = r#"--option=value"#; - assert_eq!(actual, expected); - - let actual = remove_inner_quotes(r#"--option="value""#); - let expected = r#"--option=value"#; - assert_eq!(actual, expected); - - let actual = remove_inner_quotes(r#"--option='value'"#); - let expected = r#"--option=value"#; - assert_eq!(actual, expected); - - let actual = remove_inner_quotes(r#"--option "value""#); - let expected = r#"--option "value""#; - assert_eq!(actual, expected); - } - #[test] fn test_write_pipeline_data() { let engine_state = EngineState::new(); diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index 92b424783a..4a797f3501 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -277,7 +277,7 @@ fn flatten_expression_into( output[arg_start..].sort(); } Expr::ExternalCall(head, args) => { - if let Expr::String(..) = &head.expr { + if let Expr::String(..) | Expr::GlobPattern(..) = &head.expr { output.push((head.span, FlatShape::External)); } else { flatten_expression_into(working_set, head, output); @@ -410,24 +410,22 @@ fn flatten_expression_into( output.push((Span::new(last_end, outer_span.end), FlatShape::List)); } } - Expr::StringInterpolation(exprs) => { + Expr::StringInterpolation(exprs, quoted) => { let mut flattened = vec![]; for expr in exprs { flatten_expression_into(working_set, expr, &mut flattened); } - if let Some(first) = flattened.first() { - if first.0.start != expr.span.start { - // If we aren't a bare word interpolation, also highlight the outer quotes - output.push(( - Span::new(expr.span.start, expr.span.start + 2), - FlatShape::StringInterpolation, - )); - flattened.push(( - Span::new(expr.span.end - 1, expr.span.end), - FlatShape::StringInterpolation, - )); - } + if *quoted { + // If we aren't a bare word interpolation, also highlight the outer quotes + output.push(( + Span::new(expr.span.start, expr.span.start + 2), + FlatShape::StringInterpolation, + )); + flattened.push(( + Span::new(expr.span.end - 1, expr.span.end), + FlatShape::StringInterpolation, + )); } output.extend(flattened); } diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 149bce8958..6d76c59423 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -16,7 +16,6 @@ use nu_protocol::{ IN_VARIABLE_ID, }; use std::{ - borrow::Cow, collections::{HashMap, HashSet}, num::ParseIntError, str, @@ -222,6 +221,152 @@ pub(crate) fn check_call( } } +/// Parses a string in the arg or head position of an external call. +/// +/// If the string begins with `r#`, it is parsed as a raw string. If it doesn't contain any quotes +/// or parentheses, it is parsed as a glob pattern so that tilde and glob expansion can be handled +/// by `run-external`. Otherwise, we use a custom state machine to put together an interpolated +/// string, where each balanced pair of quotes is parsed as a separate part of the string, and then +/// concatenated together. +/// +/// For example, `-foo="bar\nbaz"` becomes `$"-foo=bar\nbaz"` +fn parse_external_string(working_set: &mut StateWorkingSet, span: Span) -> Expression { + let contents = &working_set.get_span_contents(span); + + if contents.starts_with(b"r#") { + parse_raw_string(working_set, span) + } else if contents + .iter() + .any(|b| matches!(b, b'"' | b'\'' | b'`' | b'(' | b')')) + { + enum State { + Bare { + from: usize, + }, + Quote { + from: usize, + quote_char: u8, + escaped: bool, + depth: i32, + }, + } + let make_span = |from: usize, index: usize| Span { + start: span.start + from, + end: span.start + index, + }; + let mut spans = vec![]; + let mut state = State::Bare { from: 0 }; + for (index, &ch) in contents.iter().enumerate() { + match &mut state { + State::Bare { from } => match ch { + b'"' | b'\'' | b'`' => { + // Push bare string + if index != *from { + spans.push(make_span(*from, index)); + } + // then transition to other state + state = State::Quote { + from: index, + quote_char: ch, + escaped: false, + depth: 1, + }; + } + // Continue to consume + _ => (), + }, + State::Quote { + from, + quote_char, + escaped, + depth, + } => match ch { + b'"' if !*escaped => { + // Count if there are more than `depth` quotes remaining + if contents[index..] + .iter() + .filter(|b| *b == quote_char) + .count() as i32 + > *depth + { + // Increment depth to be greedy + *depth += 1; + } else { + // Decrement depth + *depth -= 1; + } + if *depth == 0 { + // End of string + spans.push(make_span(*from, index + 1)); + // go back to Bare state + state = State::Bare { from: index + 1 }; + } + } + b'\\' if !*escaped && *quote_char == b'"' => { + // The next token is escaped so it doesn't count (only for double quote) + *escaped = true; + } + _ => { + *escaped = false; + } + }, + } + } + + // Add the final span + match state { + State::Bare { from } | State::Quote { from, .. } => { + if from < contents.len() { + spans.push(make_span(from, contents.len())); + } + } + } + + // Log the spans that will be parsed + if log::log_enabled!(log::Level::Trace) { + let contents = spans + .iter() + .map(|span| String::from_utf8_lossy(working_set.get_span_contents(*span))) + .collect::>(); + + trace!("parsing: external string, parts: {contents:?}") + } + + // Parse each as its own string + let exprs: Vec = spans + .into_iter() + .map(|span| parse_string(working_set, span)) + .collect(); + + if exprs + .iter() + .all(|expr| matches!(expr.expr, Expr::String(..))) + { + // If the exprs are all strings anyway, just collapse into a single string. + let string = exprs + .into_iter() + .map(|expr| { + let Expr::String(contents) = expr.expr else { + unreachable!("already checked that this was a String") + }; + contents + }) + .collect::(); + Expression::new(working_set, Expr::String(string), span, Type::String) + } else { + // Make a string interpolation out of the expressions. + Expression::new( + working_set, + Expr::StringInterpolation(exprs, false), + span, + Type::String, + ) + } + } else { + parse_glob_pattern(working_set, span) + } +} + fn parse_external_arg(working_set: &mut StateWorkingSet, span: Span) -> ExternalArgument { let contents = working_set.get_span_contents(span); @@ -229,8 +374,6 @@ fn parse_external_arg(working_set: &mut StateWorkingSet, span: Span) -> External ExternalArgument::Regular(parse_dollar_expr(working_set, span)) } else if contents.starts_with(b"[") { ExternalArgument::Regular(parse_list_expression(working_set, span, &SyntaxShape::Any)) - } else if contents.starts_with(b"r#") { - ExternalArgument::Regular(parse_raw_string(working_set, span)) } else if contents.len() > 3 && contents.starts_with(b"...") && (contents[3] == b'$' || contents[3] == b'[' || contents[3] == b'(') @@ -241,18 +384,7 @@ fn parse_external_arg(working_set: &mut StateWorkingSet, span: Span) -> External &SyntaxShape::List(Box::new(SyntaxShape::Any)), )) } else { - // Eval stage trims the quotes, so we don't have to do the same thing when parsing. - let (contents, err) = unescape_string_preserving_quotes(contents, span); - if let Some(err) = err { - working_set.error(err); - } - - ExternalArgument::Regular(Expression::new( - working_set, - Expr::String(contents), - span, - Type::String, - )) + ExternalArgument::Regular(parse_external_string(working_set, span)) } } @@ -274,18 +406,7 @@ pub fn parse_external_call(working_set: &mut StateWorkingSet, spans: &[Span]) -> let arg = parse_expression(working_set, &[head_span]); Box::new(arg) } else { - // Eval stage will unquote the string, so we don't bother with that here - let (contents, err) = unescape_string_preserving_quotes(&head_contents, head_span); - if let Some(err) = err { - working_set.error(err) - } - - Box::new(Expression::new( - working_set, - Expr::String(contents), - head_span, - Type::String, - )) + Box::new(parse_external_string(working_set, head_span)) }; let args = spans[1..] @@ -1857,7 +1978,7 @@ pub fn parse_string_interpolation(working_set: &mut StateWorkingSet, span: Span) Expression::new( working_set, - Expr::StringInterpolation(output), + Expr::StringInterpolation(output, double_quote), span, Type::String, ) @@ -2639,23 +2760,6 @@ pub fn unescape_unquote_string(bytes: &[u8], span: Span) -> (String, Option (String, Option) { - let (bytes, err) = if bytes.starts_with(b"\"") { - let (bytes, err) = unescape_string(bytes, span); - (Cow::Owned(bytes), err) - } else { - (Cow::Borrowed(bytes), None) - }; - - // The original code for args used lossy conversion here, even though that's not what we - // typically use for strings. Revisit whether that's actually desirable later, but don't - // want to introduce a breaking change for this patch. - let token = String::from_utf8_lossy(&bytes).into_owned(); - (token, err) -} - pub fn parse_string(working_set: &mut StateWorkingSet, span: Span) -> Expression { trace!("parsing: string"); @@ -6012,7 +6116,7 @@ pub fn discover_captures_in_expr( } Expr::String(_) => {} Expr::RawString(_) => {} - Expr::StringInterpolation(exprs) => { + Expr::StringInterpolation(exprs, _) => { for expr in exprs { discover_captures_in_expr(working_set, expr, seen, seen_blocks, output)?; } diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index 2646b3cc90..924caad763 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -182,7 +182,7 @@ pub fn multi_test_parse_int() { Test( "ranges or relative paths not confused for int", b"./a/b", - Expr::String("./a/b".into()), + Expr::GlobPattern("./a/b".into(), false), None, ), Test( @@ -713,63 +713,94 @@ pub fn parse_call_missing_req_flag() { r"foo\external-call", "bare word with backslash and caret" )] +pub fn test_external_call_head_glob( + #[case] input: &str, + #[case] expected: &str, + #[case] tag: &str, +) { +} + +#[rstest] #[case( - "^'foo external call'", - "'foo external call'", - "single quote with caret" + r##"^r#"foo-external-call"#"##, + "foo-external-call", + "raw string with caret" )] +#[case( + r##"^r#"foo/external-call"#"##, + "foo/external-call", + "raw string with forward slash and caret" +)] +#[case( + r##"^r#"foo\external-call"#"##, + r"foo\external-call", + "raw string with backslash and caret" +)] +pub fn test_external_call_head_raw_string( + #[case] input: &str, + #[case] expected: &str, + #[case] tag: &str, +) { +} + +#[rstest] +#[case("^'foo external call'", "foo external call", "single quote with caret")] #[case( "^'foo/external call'", - "'foo/external call'", + "foo/external call", "single quote with forward slash and caret" )] #[case( r"^'foo\external call'", - r"'foo\external call'", + r"foo\external call", "single quote with backslash and caret" )] #[case( r#"^"foo external call""#, - r#""foo external call""#, + r#"foo external call"#, "double quote with caret" )] #[case( r#"^"foo/external call""#, - r#""foo/external call""#, + r#"foo/external call"#, "double quote with forward slash and caret" )] #[case( r#"^"foo\\external call""#, - r#""foo\external call""#, + r#"foo\external call"#, "double quote with backslash and caret" )] -#[case("`foo external call`", "`foo external call`", "backtick quote")] +#[case("`foo external call`", "foo external call", "backtick quote")] #[case( "^`foo external call`", - "`foo external call`", + "foo external call", "backtick quote with caret" )] #[case( "`foo/external call`", - "`foo/external call`", + "foo/external call", "backtick quote with forward slash" )] #[case( "^`foo/external call`", - "`foo/external call`", + "foo/external call", "backtick quote with forward slash and caret" )] #[case( - r"^`foo\external call`", r"`foo\external call`", + r"foo\external call", "backtick quote with backslash" )] #[case( r"^`foo\external call`", - r"`foo\external call`", + r"foo\external call", "backtick quote with backslash and caret" )] -fn test_external_call_name(#[case] input: &str, #[case] expected: &str, #[case] tag: &str) { +pub fn test_external_call_head_string( + #[case] input: &str, + #[case] expected: &str, + #[case] tag: &str, +) { let engine_state = EngineState::new(); let mut working_set = StateWorkingSet::new(&engine_state); let block = parse(&mut working_set, None, input.as_bytes(), true); @@ -801,19 +832,109 @@ fn test_external_call_name(#[case] input: &str, #[case] expected: &str, #[case] } #[rstest] -#[case("^foo bar-baz", "bar-baz", "bare word")] -#[case("^foo bar/baz", "bar/baz", "bare word with forward slash")] -#[case(r"^foo bar\baz", r"bar\baz", "bare word with backslash")] -#[case("^foo 'bar baz'", "'bar baz'", "single quote")] -#[case("foo 'bar/baz'", "'bar/baz'", "single quote with forward slash")] -#[case(r"foo 'bar\baz'", r"'bar\baz'", "single quote with backslash")] -#[case(r#"^foo "bar baz""#, r#""bar baz""#, "double quote")] -#[case(r#"^foo "bar/baz""#, r#""bar/baz""#, "double quote with forward slash")] -#[case(r#"^foo "bar\\baz""#, r#""bar\baz""#, "double quote with backslash")] -#[case("^foo `bar baz`", "`bar baz`", "backtick quote")] -#[case("^foo `bar/baz`", "`bar/baz`", "backtick quote with forward slash")] -#[case(r"^foo `bar\baz`", r"`bar\baz`", "backtick quote with backslash")] -fn test_external_call_argument_regular( +#[case(r"~/.foo/(1)", 2, false, "unquoted interpolated string")] +#[case( + r"~\.foo(2)\(1)", + 3, + false, + "unquoted interpolated string with backslash" +)] +#[case(r"^~/.foo/(1)", 2, false, "unquoted interpolated string with caret")] +#[case(r#"^$"~/.foo/(1)""#, 2, true, "quoted interpolated string with caret")] +pub fn test_external_call_head_interpolated_string( + #[case] input: &str, + #[case] subexpr_count: usize, + #[case] quoted: bool, + #[case] tag: &str, +) { +} + +#[rstest] +#[case("^foo foo-external-call", "foo-external-call", "bare word")] +#[case( + "^foo foo/external-call", + "foo/external-call", + "bare word with forward slash" +)] +#[case( + r"^foo foo\external-call", + r"foo\external-call", + "bare word with backslash" +)] +pub fn test_external_call_arg_glob(#[case] input: &str, #[case] expected: &str, #[case] tag: &str) { +} + +#[rstest] +#[case(r##"^foo r#"foo-external-call"#"##, "foo-external-call", "raw string")] +#[case( + r##"^foo r#"foo/external-call"#"##, + "foo/external-call", + "raw string with forward slash" +)] +#[case( + r##"^foo r#"foo\external-call"#"##, + r"foo\external-call", + "raw string with backslash" +)] +pub fn test_external_call_arg_raw_string( + #[case] input: &str, + #[case] expected: &str, + #[case] tag: &str, +) { +} + +#[rstest] +#[case( + "^foo 'foo external call'", + "foo external call", + "single quote with caret" +)] +#[case( + "^foo 'foo/external call'", + "foo/external call", + "single quote with forward slash and caret" +)] +#[case( + r"^foo 'foo\external call'", + r"foo\external call", + "single quote with backslash and caret" +)] +#[case( + r#"^foo "foo external call""#, + r#"foo external call"#, + "double quote with caret" +)] +#[case( + r#"^foo "foo/external call""#, + r#"foo/external call"#, + "double quote with forward slash and caret" +)] +#[case( + r#"^foo "foo\\external call""#, + r#"foo\external call"#, + "double quote with backslash and caret" +)] +#[case( + "^foo `foo external call`", + "foo external call", + "backtick quote with caret" +)] +#[case( + "^foo `foo/external call`", + "foo/external call", + "backtick quote with forward slash" +)] +#[case( + "^foo `foo/external call`", + "foo/external call", + "backtick quote with forward slash and caret" +)] +#[case( + r"^foo `foo\external call`", + r"foo\external call", + "backtick quote with backslash and caret" +)] +pub fn test_external_call_arg_string( #[case] input: &str, #[case] expected: &str, #[case] tag: &str, @@ -833,7 +954,7 @@ fn test_external_call_argument_regular( match &element.expr.expr { Expr::ExternalCall(name, args) => { match &name.expr { - Expr::String(string) => { + Expr::GlobPattern(string, _) => { assert_eq!("foo", string, "{tag}: incorrect name"); } other => { @@ -861,6 +982,17 @@ fn test_external_call_argument_regular( } } +#[rstest] +#[case(r"^foo ~/.foo/(1)", 2, false, "unquoted interpolated string")] +#[case(r#"^foo $"~/.foo/(1)""#, 2, true, "quoted interpolated string")] +pub fn test_external_call_arg_interpolated_string( + #[case] input: &str, + #[case] subexpr_count: usize, + #[case] quoted: bool, + #[case] tag: &str, +) { +} + #[test] fn test_external_call_argument_spread() { let engine_state = EngineState::new(); @@ -878,7 +1010,7 @@ fn test_external_call_argument_spread() { match &element.expr.expr { Expr::ExternalCall(name, args) => { match &name.expr { - Expr::String(string) => { + Expr::GlobPattern(string, _) => { assert_eq!("foo", string, "incorrect name"); } other => { @@ -1037,7 +1169,8 @@ mod string { assert!(element.redirection.is_none()); let subexprs: Vec<&Expr> = match &element.expr.expr { - Expr::StringInterpolation(expressions) => { + Expr::StringInterpolation(expressions, quoted) => { + assert!(quoted); expressions.iter().map(|e| &e.expr).collect() } _ => panic!("Expected an `Expr::StringInterpolation`"), @@ -1066,7 +1199,8 @@ mod string { assert!(element.redirection.is_none()); let subexprs: Vec<&Expr> = match &element.expr.expr { - Expr::StringInterpolation(expressions) => { + Expr::StringInterpolation(expressions, quoted) => { + assert!(quoted); expressions.iter().map(|e| &e.expr).collect() } _ => panic!("Expected an `Expr::StringInterpolation`"), @@ -1093,7 +1227,8 @@ mod string { assert!(element.redirection.is_none()); let subexprs: Vec<&Expr> = match &element.expr.expr { - Expr::StringInterpolation(expressions) => { + Expr::StringInterpolation(expressions, quoted) => { + assert!(quoted); expressions.iter().map(|e| &e.expr).collect() } _ => panic!("Expected an `Expr::StringInterpolation`"), @@ -1122,7 +1257,8 @@ mod string { assert!(element.redirection.is_none()); let subexprs: Vec<&Expr> = match &element.expr.expr { - Expr::StringInterpolation(expressions) => { + Expr::StringInterpolation(expressions, quoted) => { + assert!(quoted); expressions.iter().map(|e| &e.expr).collect() } _ => panic!("Expected an `Expr::StringInterpolation`"), @@ -1132,6 +1268,45 @@ mod string { assert_eq!(subexprs[0], &Expr::String("(1 + 3)(7 - 5)".to_string())); } + #[test] + pub fn parse_string_interpolation_bare() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let block = parse( + &mut working_set, + None, + b"\"\" ++ foo(1 + 3)bar(7 - 5)", + 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, quoted) => { + assert!(!quoted); + expressions.iter().map(|e| &e.expr).collect() + } + _ => panic!("Expected an `Expr::StringInterpolation`"), + }, + _ => panic!("Expected an `Expr::BinaryOp`"), + }; + + assert_eq!(subexprs.len(), 4); + + assert_eq!(subexprs[0], &Expr::String("foo".to_string())); + assert!(matches!(subexprs[1], &Expr::FullCellPath(..))); + assert_eq!(subexprs[2], &Expr::String("bar".to_string())); + assert!(matches!(subexprs[3], &Expr::FullCellPath(..))); + } + #[test] pub fn parse_nested_expressions() { let engine_state = EngineState::new(); diff --git a/crates/nu-protocol/src/ast/expr.rs b/crates/nu-protocol/src/ast/expr.rs index 13d9e42985..caa1fc1cba 100644 --- a/crates/nu-protocol/src/ast/expr.rs +++ b/crates/nu-protocol/src/ast/expr.rs @@ -32,8 +32,11 @@ pub enum Expr { Keyword(Box), ValueWithUnit(Box), DateTime(chrono::DateTime), + /// The boolean is `true` if the string is quoted. Filepath(String, bool), + /// The boolean is `true` if the string is quoted. Directory(String, bool), + /// The boolean is `true` if the string is quoted. GlobPattern(String, bool), String(String), RawString(String), @@ -42,7 +45,8 @@ pub enum Expr { ImportPattern(Box), Overlay(Option), // block ID of the overlay's origin module Signature(Box), - StringInterpolation(Vec), + /// The boolean is `true` if the string is quoted. + StringInterpolation(Vec, bool), Nothing, Garbage, } @@ -83,7 +87,7 @@ impl Expr { | Expr::String(_) | Expr::RawString(_) | Expr::CellPath(_) - | Expr::StringInterpolation(_) + | Expr::StringInterpolation(_, _) | Expr::Nothing => { // These expressions do not use the output of the pipeline in any meaningful way, // so we can discard the previous output by redirecting it to `Null`. diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index 832bc1f438..18be862a3a 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -232,7 +232,7 @@ impl Expression { } false } - Expr::StringInterpolation(items) => { + Expr::StringInterpolation(items, _) => { for i in items { if i.has_in_variable(working_set) { return true; @@ -441,7 +441,7 @@ impl Expression { Expr::Signature(_) => {} Expr::String(_) => {} Expr::RawString(_) => {} - Expr::StringInterpolation(items) => { + Expr::StringInterpolation(items, _) => { for i in items { i.replace_span(working_set, replaced, new_span) } diff --git a/crates/nu-protocol/src/debugger/profiler.rs b/crates/nu-protocol/src/debugger/profiler.rs index 0fa2d4f3aa..79c8ea8720 100644 --- a/crates/nu-protocol/src/debugger/profiler.rs +++ b/crates/nu-protocol/src/debugger/profiler.rs @@ -257,7 +257,7 @@ fn expr_to_string(engine_state: &EngineState, expr: &Expr) -> String { Expr::RowCondition(_) => "row condition".to_string(), Expr::Signature(_) => "signature".to_string(), Expr::String(_) | Expr::RawString(_) => "string".to_string(), - Expr::StringInterpolation(_) => "string interpolation".to_string(), + Expr::StringInterpolation(_, _) => "string interpolation".to_string(), Expr::Subexpression(_) => "subexpression".to_string(), Expr::Table(_) => "table".to_string(), Expr::UnaryNot(_) => "unary not".to_string(), diff --git a/crates/nu-protocol/src/eval_base.rs b/crates/nu-protocol/src/eval_base.rs index 6a1c0d302f..ee75cae751 100644 --- a/crates/nu-protocol/src/eval_base.rs +++ b/crates/nu-protocol/src/eval_base.rs @@ -276,7 +276,7 @@ pub trait Eval { Expr::RowCondition(block_id) | Expr::Closure(block_id) => { Self::eval_row_condition_or_closure(state, mut_state, *block_id, expr.span) } - Expr::StringInterpolation(exprs) => { + Expr::StringInterpolation(exprs, _) => { let config = Self::get_config(state, mut_state); let str = exprs .iter()