From bdc32345bd6767071c169ffbcd9ffc30ea935791 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Wed, 19 Jun 2024 21:00:03 -0700 Subject: [PATCH] Move most of the peculiar argument handling for external calls into the parser (#13089) # Description We've had a lot of different issues and PRs related to arg handling with externals since the rewrite of `run-external` in #12921: - #12950 - #12955 - #13000 - #13001 - #13021 - #13027 - #13028 - #13073 Many of these are caused by the argument handling of external calls and `run-external` being very special and involving the parser handing quoted strings over to `run-external` so that it knows whether to expand tildes and globs and so on. This is really unusual and also makes it harder to use `run-external`, and also harder to understand it (and probably is part of the reason why it was rewritten in the first place). This PR moves a lot more of that work over to the parser, so that by the time `run-external` gets it, it's dealing with much more normal Nushell values. In particular: - Unquoted strings are handled as globs with no expand - The unescaped-but-quoted handling of strings was removed, and the parser constructs normal looking strings instead, removing internal quotes so that `run-external` doesn't have to do it - Bare word interpolation is now supported and expansion is done in this case - Expressions typed as `Glob` containing `Expr::StringInterpolation` now produce `Value::Glob` instead, with the quoted status from the expr passed through so we know if it was a bare word - Bare word interpolation for values typed as `glob` now possible, but not implemented - Because expansion is now triggered by `Value::Glob(_, false)` instead of looking at the expr, externals now support glob types # User-Facing Changes - Bare word interpolation works for external command options, and otherwise embedded in other strings: ```nushell ^echo --foo=(2 + 2) # prints --foo=4 ^echo -foo=$"(2 + 2)" # prints -foo=4 ^echo foo="(2 + 2)" # prints (no interpolation!) foo=(2 + 2) ^echo foo,(2 + 2),bar # prints foo,4,bar ``` - Bare word interpolation expands for external command head/args: ```nushell let name = "exa" ~/.cargo/bin/($name) # this works, and expands the tilde ^$"~/.cargo/bin/($name)" # this doesn't expand the tilde ^echo ~/($name)/* # this glob is expanded ^echo $"~/($name)/*" # this isn't expanded ``` - Ndots are now supported for the head of an external command (`^.../foo` works) - Glob values are now supported for head/args of an external command, and expanded appropriately: ```nushell ^("~/.cargo/bin/exa" | into glob) # the tilde is expanded ^echo ("*.txt" | into glob) # this glob is expanded ``` - `run-external` now works more like any other command, without expecting a special call convention for its args: ```nushell run-external echo "'foo'" # before PR: 'foo' # after PR: foo run-external echo "*.txt" # before PR: (glob is expanded) # after PR: *.txt ``` # Tests + Formatting Lots of tests added and cleaned up. Some tests that weren't active on Windows changed to use `nu --testbin cococo` so that they can work. Added a test for Linux only to make sure tilde expansion of commands works, because changing `HOME` there causes `~` to reliably change. - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting - [ ] release notes: make sure to mention the new syntaxes that are supported --- crates/nu-cli/src/syntax_highlight.rs | 21 +- crates/nu-color-config/src/shape_color.rs | 1 + crates/nu-command/src/system/run_external.rs | 317 +++------- .../nu-command/tests/commands/run_external.rs | 145 +++-- crates/nu-parser/src/flatten.rs | 25 +- crates/nu-parser/src/parser.rs | 251 ++++++-- crates/nu-parser/tests/test_parser.rs | 564 +++++++++++++----- crates/nu-protocol/src/ast/expr.rs | 6 + crates/nu-protocol/src/ast/expression.rs | 4 +- crates/nu-protocol/src/debugger/profiler.rs | 1 + crates/nu-protocol/src/eval_base.rs | 9 + crates/nu-test-support/src/macros.rs | 6 + crates/nuon/src/from.rs | 6 + 13 files changed, 880 insertions(+), 476 deletions(-) diff --git a/crates/nu-cli/src/syntax_highlight.rs b/crates/nu-cli/src/syntax_highlight.rs index e296943af6..41ef168390 100644 --- a/crates/nu-cli/src/syntax_highlight.rs +++ b/crates/nu-cli/src/syntax_highlight.rs @@ -138,6 +138,7 @@ impl Highlighter for NuHighlighter { FlatShape::Filepath => add_colored_token(&shape.1, next_token), FlatShape::Directory => add_colored_token(&shape.1, next_token), + FlatShape::GlobInterpolation => add_colored_token(&shape.1, next_token), FlatShape::GlobPattern => add_colored_token(&shape.1, next_token), FlatShape::Variable(_) | FlatShape::VarDecl(_) => { add_colored_token(&shape.1, next_token) @@ -452,15 +453,17 @@ fn find_matching_block_end_in_expr( } } - Expr::StringInterpolation(exprs) => exprs.iter().find_map(|expr| { - find_matching_block_end_in_expr( - line, - working_set, - expr, - global_span_offset, - global_cursor_offset, - ) - }), + Expr::StringInterpolation(exprs) | Expr::GlobInterpolation(exprs, _) => { + exprs.iter().find_map(|expr| { + find_matching_block_end_in_expr( + line, + working_set, + expr, + global_span_offset, + global_cursor_offset, + ) + }) + } Expr::List(list) => { if expr_last == global_cursor_offset { diff --git a/crates/nu-color-config/src/shape_color.rs b/crates/nu-color-config/src/shape_color.rs index 72cc955e0b..8da397e914 100644 --- a/crates/nu-color-config/src/shape_color.rs +++ b/crates/nu-color-config/src/shape_color.rs @@ -20,6 +20,7 @@ pub fn default_shape_color(shape: &str) -> Style { "shape_flag" => Style::new().fg(Color::Blue).bold(), "shape_float" => Style::new().fg(Color::Purple).bold(), "shape_garbage" => Style::new().fg(Color::White).on(Color::Red).bold(), + "shape_glob_interpolation" => Style::new().fg(Color::Cyan).bold(), "shape_globpattern" => Style::new().fg(Color::Cyan).bold(), "shape_int" => Style::new().fg(Color::Purple).bold(), "shape_internalcall" => Style::new().fg(Color::Cyan).bold(), diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index bc6152e7c8..104729a786 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -1,16 +1,15 @@ use nu_cmd_base::hook::eval_hook; use nu_engine::{command_prelude::*, env_to_strings, get_eval_expression}; +use nu_path::{dots::expand_ndots, expand_tilde}; use nu_protocol::{ - ast::{Expr, Expression}, - did_you_mean, - process::ChildProcess, - ByteStream, NuGlob, OutDest, + ast::Expression, did_you_mean, process::ChildProcess, ByteStream, NuGlob, OutDest, }; use nu_system::ForegroundChild; use nu_utils::IgnoreCaseExt; use pathdiff::diff_paths; use std::{ borrow::Cow, + ffi::{OsStr, OsString}, io::Write, path::{Path, PathBuf}, process::Stdio, @@ -33,8 +32,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) } @@ -47,42 +54,31 @@ impl Command for External { ) -> Result { let cwd = engine_state.cwd(Some(stack))?; - // Evaluate the command name in the same way the arguments are evaluated. Since this isn't - // a spread, it should return a one-element vec. - let name_expr = call - .positional_nth(0) - .ok_or_else(|| ShellError::MissingParameter { - param_name: "command".into(), - span: call.head, - })?; - let name = eval_argument(engine_state, stack, name_expr, false)? - .pop() - .expect("eval_argument returned zero-element vec") - .into_spanned(name_expr.span); + let name: Value = call.req(engine_state, stack, 0)?; + + let name_str: Cow = match &name { + Value::Glob { val, .. } => Cow::Borrowed(val), + Value::String { val, .. } => Cow::Borrowed(val), + _ => Cow::Owned(name.clone().coerce_into_string()?), + }; + + let expanded_name = match &name { + // Expand tilde and ndots on the name if it's a bare string / glob (#13000) + Value::Glob { no_expand, .. } if !*no_expand => expand_ndots(expand_tilde(&*name_str)), + _ => Path::new(&*name_str).to_owned(), + }; // Find the absolute path to the executable. On Windows, set the // executable to "cmd.exe" if it's is a CMD internal command. If the // command is not found, display a helpful error message. - let executable = if cfg!(windows) && is_cmd_internal_command(&name.item) { + let executable = if cfg!(windows) && is_cmd_internal_command(&name_str) { PathBuf::from("cmd.exe") } else { - // Expand tilde on the name if it's a bare string (#13000) - let expanded_name = if is_bare_string(name_expr) { - expand_tilde(&name.item) - } else { - name.item.clone() - }; - // Determine the PATH to be used and then use `which` to find it - though this has no // effect if it's an absolute path already let paths = nu_engine::env::path_str(engine_state, stack, call.head)?; - let Some(executable) = which(&expanded_name, &paths, &cwd) else { - return Err(command_not_found( - &name.item, - call.head, - engine_state, - stack, - )); + let Some(executable) = which(expanded_name, &paths, &cwd) else { + return Err(command_not_found(&name_str, call.head, engine_state, stack)); }; executable }; @@ -101,15 +97,15 @@ impl Command for External { // Configure args. let args = eval_arguments_from_call(engine_state, stack, call)?; #[cfg(windows)] - if is_cmd_internal_command(&name.item) { + if is_cmd_internal_command(&name_str) { use std::os::windows::process::CommandExt; // The /D flag disables execution of AutoRun commands from registry. // The /C flag followed by a command name instructs CMD to execute // that command and quit. - command.args(["/D", "/C", &name.item]); + command.args(["/D", "/C", &name_str]); for arg in &args { - command.raw_arg(escape_cmd_argument(arg)?.as_ref()); + command.raw_arg(escape_cmd_argument(arg)?); } } else { command.args(args.into_iter().map(|s| s.item)); @@ -217,76 +213,54 @@ 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) -> Cow<'_, 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 { - Cow::Owned(s[1..s.len() - 1].to_string().replace(r#"\""#, "\"")) - } else if quoted_by_single_quotes || quoted_by_backticks { - Cow::Borrowed(&s[1..s.len() - 1]) - } else { - Cow::Borrowed(s) - } -} - /// Evaluate all arguments from a call, performing expansions when necessary. pub fn eval_arguments_from_call( engine_state: &EngineState, stack: &mut Stack, call: &Call, -) -> Result>, ShellError> { +) -> Result>, ShellError> { let ctrlc = &engine_state.ctrlc; let cwd = engine_state.cwd(Some(stack))?; - let mut args: Vec> = vec![]; + 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. - 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)); + for arg in eval_argument(engine_state, stack, expr, spread)? { + match arg { + // Expand globs passed to run-external + Value::Glob { val, no_expand, .. } if !no_expand => args.extend( + expand_glob(&val, &cwd, expr.span, ctrlc)? + .into_iter() + .map(|s| s.into_spanned(expr.span)), + ), + other => { + args.push(OsString::from(coerce_into_string(other)?).into_spanned(expr.span)) } } - } else { - for arg in eval_argument(engine_state, stack, expr, spread)? { - args.push(arg.into_spanned(expr.span)); - } } } Ok(args) } -/// Evaluates an expression, coercing the values to strings. -/// -/// Note: The parser currently has a special hack that retains surrounding -/// quotes for string literals in `Expression`, so that we can decide whether -/// the expression is considered a bare string. The hack doesn't affect string -/// literals within lists or records. This function will remove the quotes -/// before evaluating the expression. +/// Custom `coerce_into_string()`, including globs, since those are often args to `run-external` +/// as well +fn coerce_into_string(val: Value) -> Result { + match val { + Value::Glob { val, .. } => Ok(val), + _ => val.coerce_into_string(), + } +} + +/// Evaluate an argument, returning more than one value if it was a list to be spread. fn eval_argument( engine_state: &EngineState, stack: &mut Stack, 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()); - } - +) -> Result, ShellError> { let eval = get_eval_expression(engine_state); - match eval(engine_state, stack, &expr)? { + match eval(engine_state, stack, expr)? { Value::List { vals, .. } => { if spread { - vals.into_iter() - .map(|val| val.coerce_into_string()) - .collect() + Ok(vals) } else { Err(ShellError::CannotPassListToExternal { arg: String::from_utf8_lossy(engine_state.get_span_contents(expr.span)).into(), @@ -298,31 +272,12 @@ fn eval_argument( if spread { Err(ShellError::CannotSpreadAsList { span: expr.span }) } else { - Ok(vec![value.coerce_into_string()?]) + Ok(vec![value]) } } } } -/// 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. -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 -} - -/// Performs tilde expansion on `arg`. Returns the original string if `arg` -/// doesn't start with tilde. -fn expand_tilde(arg: &str) -> String { - nu_path::expand_tilde(arg).to_string_lossy().to_string() -} - /// Performs glob expansion on `arg`. If the expansion found no matches or the pattern /// is not a valid glob, then this returns the original string as the expansion result. /// @@ -333,19 +288,21 @@ fn expand_glob( cwd: &Path, span: Span, interrupt: &Option>, -) -> Result, ShellError> { +) -> Result, ShellError> { const GLOB_CHARS: &[char] = &['*', '?', '[']; - // Don't expand something that doesn't include the GLOB_CHARS + // For an argument that doesn't include the GLOB_CHARS, just do the `expand_tilde` + // and `expand_ndots` expansion if !arg.contains(GLOB_CHARS) { - return Ok(vec![arg.into()]); + let path = expand_ndots(expand_tilde(arg)); + return Ok(vec![path.into()]); } // We must use `nu_engine::glob_from` here, in order to ensure we get paths from the correct // dir let glob = NuGlob::Expand(arg.to_owned()).into_spanned(span); if let Ok((prefix, matches)) = nu_engine::glob_from(&glob, cwd, span, None) { - let mut result = vec![]; + let mut result: Vec = vec![]; for m in matches { if nu_utils::ctrl_c::was_pressed(interrupt) { @@ -353,7 +310,7 @@ fn expand_glob( } if let Ok(arg) = m { let arg = resolve_globbed_path_to_cwd_relative(arg, prefix.as_ref(), cwd); - result.push(arg.to_string_lossy().to_string()); + result.push(arg.into()); } else { result.push(arg.into()); } @@ -392,23 +349,6 @@ fn resolve_globbed_path_to_cwd_relative( } } -/// 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> { - // 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. /// @@ -577,7 +517,7 @@ pub fn command_not_found( /// Note: the `which.rs` crate always uses PATHEXT from the environment. As /// such, changing PATHEXT within Nushell doesn't work without updating the /// actual environment of the Nushell process. -pub fn which(name: &str, paths: &str, cwd: &Path) -> Option { +pub fn which(name: impl AsRef, paths: &str, cwd: &Path) -> Option { #[cfg(windows)] let paths = format!("{};{}", cwd.display(), paths); which::which_in(name, Some(paths), cwd).ok() @@ -593,17 +533,18 @@ fn is_cmd_internal_command(name: &str) -> bool { } /// Returns true if a string contains CMD special characters. -#[cfg(windows)] -fn has_cmd_special_character(s: &str) -> bool { - const SPECIAL_CHARS: &[char] = &['<', '>', '&', '|', '^']; - SPECIAL_CHARS.iter().any(|c| s.contains(*c)) +fn has_cmd_special_character(s: impl AsRef<[u8]>) -> bool { + s.as_ref() + .iter() + .any(|b| matches!(b, b'<' | b'>' | b'&' | b'|' | b'^')) } /// Escape an argument for CMD internal commands. The result can be safely passed to `raw_arg()`. -#[cfg(windows)] -fn escape_cmd_argument(arg: &Spanned) -> Result, ShellError> { +#[cfg_attr(not(windows), allow(dead_code))] +fn escape_cmd_argument(arg: &Spanned) -> Result, ShellError> { let Spanned { item: arg, span } = arg; - if arg.contains(['\r', '\n', '%']) { + let bytes = arg.as_encoded_bytes(); + if bytes.iter().any(|b| matches!(b, b'\r' | b'\n' | b'%')) { // \r and \n trunacte the rest of the arguments and % can expand environment variables Err(ShellError::ExternalCommand { label: @@ -612,12 +553,12 @@ fn escape_cmd_argument(arg: &Spanned) -> Result, ShellError help: "some characters currently cannot be securely escaped".into(), span: *span, }) - } else if arg.contains('"') { + } else if bytes.contains(&b'"') { // If `arg` is already quoted by double quotes, confirm there's no // embedded double quotes, then leave it as is. - if arg.chars().filter(|c| *c == '"').count() == 2 - && arg.starts_with('"') - && arg.ends_with('"') + if bytes.iter().filter(|b| **b == b'"').count() == 2 + && bytes.starts_with(b"\"") + && bytes.ends_with(b"\"") { Ok(Cow::Borrowed(arg)) } else { @@ -628,9 +569,13 @@ fn escape_cmd_argument(arg: &Spanned) -> Result, ShellError span: *span, }) } - } else if arg.contains(' ') || has_cmd_special_character(arg) { + } else if bytes.contains(&b' ') || has_cmd_special_character(bytes) { // If `arg` contains space or special characters, quote the entire argument by double quotes. - Ok(Cow::Owned(format!("\"{arg}\""))) + let mut new_str = OsString::new(); + new_str.push("\""); + new_str.push(arg); + new_str.push("\""); + Ok(Cow::Owned(new_str)) } else { // FIXME?: what if `arg.is_empty()`? Ok(Cow::Borrowed(arg)) @@ -640,64 +585,8 @@ fn escape_cmd_argument(arg: &Spanned) -> Result, ShellError #[cfg(test)] mod test { use super::*; - 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'#"#); - assert_eq!(remove_quotes(r#""foo\" bar""#), r#"foo" bar"#); - } - - #[test] - fn test_eval_argument() { - fn expression(expr: Expr) -> Expression { - Expression::new_unknown(expr, Span::unknown(), Type::Any) - } - - fn eval(expr: Expr, spread: bool) -> Result, ShellError> { - let engine_state = EngineState::new(); - let mut stack = Stack::new(); - eval_argument(&engine_state, &mut stack, &expression(expr), spread) - } - - let actual = eval(Expr::String("".into()), false).unwrap(); - let expected = &[""]; - assert_eq!(actual, expected); - - let actual = eval(Expr::String("'foo'".into()), false).unwrap(); - let expected = &["foo"]; - assert_eq!(actual, expected); - - let actual = eval(Expr::RawString("'foo'".into()), false).unwrap(); - let expected = &["'foo'"]; - assert_eq!(actual, expected); - - let actual = eval(Expr::List(vec![]), true).unwrap(); - let expected: &[&str] = &[]; - assert_eq!(actual, expected); - - let actual = eval( - Expr::List(vec![ - ListItem::Item(expression(Expr::String("'foo'".into()))), - ListItem::Item(expression(Expr::String("bar".into()))), - ]), - true, - ) - .unwrap(); - let expected = &["'foo'", "bar"]; - assert_eq!(actual, expected); - - eval(Expr::String("".into()), true).unwrap_err(); - eval(Expr::List(vec![]), false).unwrap_err(); - } - #[test] fn test_expand_glob() { Playground::setup("test_expand_glob", |dirs, play| { @@ -721,46 +610,20 @@ mod test { assert_eq!(actual, expected); let actual = expand_glob("./a.txt", cwd, Span::unknown(), &None).unwrap(); - let expected = &["./a.txt"]; + let expected: Vec = vec![Path::new(".").join("a.txt").into()]; assert_eq!(actual, expected); let actual = expand_glob("[*.txt", cwd, Span::unknown(), &None).unwrap(); let expected = &["[*.txt"]; assert_eq!(actual, expected); + + let actual = expand_glob("~/foo.txt", cwd, Span::unknown(), &None).unwrap(); + let home = dirs_next::home_dir().expect("failed to get home dir"); + let expected: Vec = vec![home.join("foo.txt").into()]; + assert_eq!(actual, expected); }) } - #[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); - - 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="v\"value""#); - let expected = r#"option=v"value"#; - assert_eq!(actual, expected); - } - #[test] fn test_write_pipeline_data() { let engine_state = EngineState::new(); diff --git a/crates/nu-command/tests/commands/run_external.rs b/crates/nu-command/tests/commands/run_external.rs index 14428edcbe..daa5abb25b 100644 --- a/crates/nu-command/tests/commands/run_external.rs +++ b/crates/nu-command/tests/commands/run_external.rs @@ -1,4 +1,3 @@ -#[cfg(not(windows))] use nu_test_support::fs::Stub::EmptyFile; use nu_test_support::playground::Playground; use nu_test_support::{nu, pipeline}; @@ -17,7 +16,6 @@ fn better_empty_redirection() { assert!(!actual.out.contains('2')); } -#[cfg(not(windows))] #[test] fn explicit_glob() { Playground::setup("external with explicit glob", |dirs, sandbox| { @@ -30,15 +28,15 @@ fn explicit_glob() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - ^ls | glob '*.txt' | length + ^nu --testbin cococo ('*.txt' | into glob) "# )); - assert_eq!(actual.out, "2"); + assert!(actual.out.contains("D&D_volume_1.txt")); + assert!(actual.out.contains("D&D_volume_2.txt")); }) } -#[cfg(not(windows))] #[test] fn bare_word_expand_path_glob() { Playground::setup("bare word should do the expansion", |dirs, sandbox| { @@ -51,7 +49,7 @@ fn bare_word_expand_path_glob() { let actual = nu!( cwd: dirs.test(), pipeline( " - ^ls *.txt + ^nu --testbin cococo *.txt " )); @@ -60,7 +58,6 @@ fn bare_word_expand_path_glob() { }) } -#[cfg(not(windows))] #[test] fn backtick_expand_path_glob() { Playground::setup("backtick should do the expansion", |dirs, sandbox| { @@ -73,7 +70,7 @@ fn backtick_expand_path_glob() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - ^ls `*.txt` + ^nu --testbin cococo `*.txt` "# )); @@ -82,7 +79,6 @@ fn backtick_expand_path_glob() { }) } -#[cfg(not(windows))] #[test] fn single_quote_does_not_expand_path_glob() { Playground::setup("single quote do not run the expansion", |dirs, sandbox| { @@ -95,15 +91,14 @@ fn single_quote_does_not_expand_path_glob() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - ^ls '*.txt' + ^nu --testbin cococo '*.txt' "# )); - assert!(actual.err.contains("No such file or directory")); + assert_eq!(actual.out, "*.txt"); }) } -#[cfg(not(windows))] #[test] fn double_quote_does_not_expand_path_glob() { Playground::setup("double quote do not run the expansion", |dirs, sandbox| { @@ -116,22 +111,21 @@ fn double_quote_does_not_expand_path_glob() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - ^ls "*.txt" + ^nu --testbin cococo "*.txt" "# )); - assert!(actual.err.contains("No such file or directory")); + assert_eq!(actual.out, "*.txt"); }) } -#[cfg(not(windows))] #[test] fn failed_command_with_semicolon_will_not_execute_following_cmds() { Playground::setup("external failed command with semicolon", |dirs, _| { let actual = nu!( cwd: dirs.test(), pipeline( " - ^ls *.abc; echo done + nu --testbin fail; echo done " )); @@ -155,16 +149,51 @@ fn external_args_with_quoted() { #[cfg(not(windows))] #[test] -fn external_arg_with_long_flag_value_quoted() { - Playground::setup("external failed command with semicolon", |dirs, _| { +fn external_arg_with_option_like_embedded_quotes() { + // TODO: would be nice to make this work with cococo, but arg parsing interferes + Playground::setup( + "external arg with option like embedded quotes", + |dirs, _| { + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + ^echo --foo='bar' -foo='bar' + "# + )); + + assert_eq!(actual.out, "--foo=bar -foo=bar"); + }, + ) +} + +#[test] +fn external_arg_with_non_option_like_embedded_quotes() { + Playground::setup( + "external arg with non option like embedded quotes", + |dirs, _| { + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + ^nu --testbin cococo foo='bar' 'foo'=bar + "# + )); + + assert_eq!(actual.out, "foo=bar foo=bar"); + }, + ) +} + +#[test] +fn external_arg_with_string_interpolation() { + Playground::setup("external arg with string interpolation", |dirs, _| { let actual = nu!( cwd: dirs.test(), pipeline( r#" - ^echo --foo='bar' + ^nu --testbin cococo foo=(2 + 2) $"foo=(2 + 2)" foo=$"(2 + 2)" "# )); - assert_eq!(actual.out, "--foo=bar"); + assert_eq!(actual.out, "foo=4 foo=4 foo=4"); }) } @@ -200,6 +229,67 @@ fn external_command_escape_args() { }) } +#[test] +#[cfg_attr( + not(target_os = "linux"), + ignore = "only runs on Linux, where controlling the HOME var is reliable" +)] +fn external_command_expand_tilde() { + Playground::setup("external command expand tilde", |dirs, _| { + // Make a copy of the nu executable that we can use + let mut src = std::fs::File::open(nu_test_support::fs::binaries().join("nu")) + .expect("failed to open nu"); + let mut dst = std::fs::File::create_new(dirs.test().join("test_nu")) + .expect("failed to create test_nu file"); + std::io::copy(&mut src, &mut dst).expect("failed to copy data for nu binary"); + + // Make test_nu have the same permissions so that it's executable + dst.set_permissions( + src.metadata() + .expect("failed to get nu metadata") + .permissions(), + ) + .expect("failed to set permissions on test_nu"); + + // Close the files + drop(dst); + drop(src); + + let actual = nu!( + envs: vec![ + ("HOME".to_string(), dirs.test().to_string_lossy().into_owned()), + ], + r#" + ^~/test_nu --testbin cococo hello + "# + ); + assert_eq!(actual.out, "hello"); + }) +} + +#[test] +fn external_arg_expand_tilde() { + Playground::setup("external arg expand tilde", |dirs, _| { + let actual = nu!( + cwd: dirs.test(), pipeline( + r#" + ^nu --testbin cococo ~/foo ~/(2 + 2) + "# + )); + + let home = dirs_next::home_dir().expect("failed to find home dir"); + + assert_eq!( + actual.out, + format!( + "{} {}", + home.join("foo").display(), + home.join("4").display() + ) + ); + }) +} + #[test] fn external_command_not_expand_tilde_with_quotes() { Playground::setup( @@ -231,21 +321,6 @@ fn external_command_receives_raw_binary_data() { }) } -#[cfg(windows)] -#[test] -fn failed_command_with_semicolon_will_not_execute_following_cmds_windows() { - Playground::setup("external failed command with semicolon", |dirs, _| { - let actual = nu!( - cwd: dirs.test(), pipeline( - " - ^cargo asdf; echo done - " - )); - - assert!(!actual.out.contains("done")); - }) -} - #[cfg(windows)] #[test] fn can_run_batch_files() { diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index 92b424783a..88638b2475 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -26,6 +26,7 @@ pub enum FlatShape { Flag, Float, Garbage, + GlobInterpolation, GlobPattern, Int, InternalCall(DeclId), @@ -67,6 +68,7 @@ impl FlatShape { FlatShape::Flag => "shape_flag", FlatShape::Float => "shape_float", FlatShape::Garbage => "shape_garbage", + FlatShape::GlobInterpolation => "shape_glob_interpolation", FlatShape::GlobPattern => "shape_globpattern", FlatShape::Int => "shape_int", FlatShape::InternalCall(_) => "shape_internalcall", @@ -277,7 +279,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); @@ -286,7 +288,7 @@ fn flatten_expression_into( for arg in args.as_ref() { match arg { ExternalArgument::Regular(expr) => { - if let Expr::String(..) = &expr.expr { + if let Expr::String(..) | Expr::GlobPattern(..) = &expr.expr { output.push((expr.span, FlatShape::ExternalArg)); } else { flatten_expression_into(working_set, expr, output); @@ -431,6 +433,25 @@ fn flatten_expression_into( } output.extend(flattened); } + Expr::GlobInterpolation(exprs, quoted) => { + let mut flattened = vec![]; + for expr in exprs { + flatten_expression_into(working_set, expr, &mut flattened); + } + + 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::GlobInterpolation, + )); + flattened.push(( + Span::new(expr.span.end - 1, expr.span.end), + FlatShape::GlobInterpolation, + )); + } + output.extend(flattened); + } Expr::Record(list) => { let outer_span = expr.span; let mut last_end = outer_span.start; diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 149bce8958..ba718cc771 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,209 @@ 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')')) + { + enum State { + Bare { + from: usize, + }, + Quote { + from: usize, + quote_char: u8, + escaped: bool, + depth: i32, + }, + } + // Find the spans of parts of the string that can be parsed as their own strings for + // concatenation. + // + // By passing each of these parts to `parse_string()`, we can eliminate the quotes and also + // handle string interpolation. + 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 }; + let mut index = 0; + while index < contents.len() { + let ch = contents[index]; + match &mut state { + State::Bare { from } => match ch { + 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, + }; + } + b'$' => { + if let Some("e_char @ (b'"' | b'\'')) = contents.get(index + 1) { + // Start a dollar quote (interpolated string) + if index != *from { + spans.push(make_span(*from, index)); + } + state = State::Quote { + from: index, + quote_char, + escaped: false, + depth: 1, + }; + // Skip over two chars (the dollar sign and the quote) + index += 2; + continue; + } + } + // Continue to consume + _ => (), + }, + State::Quote { + from, + quote_char, + escaped, + depth, + } => match ch { + ch if ch == *quote_char && !*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; + } + }, + } + index += 1; + } + + // 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:?}") + } + + // Check if the whole thing is quoted. If not, it should be a glob + let quoted = + (contents.len() >= 3 && contents.starts_with(b"$\"") && contents.ends_with(b"\"")) + || is_quoted(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::(); + if quoted { + Expression::new(working_set, Expr::String(string), span, Type::String) + } else { + Expression::new( + working_set, + Expr::GlobPattern(string, false), + span, + Type::Glob, + ) + } + } else { + // Flatten any string interpolations contained with the exprs. + let exprs = exprs + .into_iter() + .flat_map(|expr| match expr.expr { + Expr::StringInterpolation(subexprs) => subexprs, + _ => vec![expr], + }) + .collect(); + // Make an interpolation out of the expressions. Use `GlobInterpolation` if it's a bare + // word, so that the unquoted state can get passed through to `run-external`. + if quoted { + Expression::new( + working_set, + Expr::StringInterpolation(exprs), + span, + Type::String, + ) + } else { + Expression::new( + working_set, + Expr::GlobInterpolation(exprs, false), + span, + Type::Glob, + ) + } + } + } 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 +431,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 +441,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 +463,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..] @@ -2639,23 +2817,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 +6173,7 @@ pub fn discover_captures_in_expr( } Expr::String(_) => {} Expr::RawString(_) => {} - Expr::StringInterpolation(exprs) => { + Expr::StringInterpolation(exprs) | Expr::GlobInterpolation(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..0784fe69d4 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -1,8 +1,8 @@ use nu_parser::*; use nu_protocol::{ - ast::{Argument, Call, Expr, ExternalArgument, PathMember, Range}, + ast::{Argument, Call, Expr, Expression, ExternalArgument, PathMember, Range}, engine::{Command, EngineState, Stack, StateWorkingSet}, - ParseError, PipelineData, ShellError, Signature, Span, SyntaxShape, + ParseError, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, }; use rstest::rstest; @@ -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( @@ -694,6 +694,50 @@ pub fn parse_call_missing_req_flag() { )); } +fn test_external_call(input: &str, tag: &str, f: impl FnOnce(&Expression, &[ExternalArgument])) { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + let block = parse(&mut working_set, None, input.as_bytes(), true); + assert!( + working_set.parse_errors.is_empty(), + "{tag}: errors: {:?}", + working_set.parse_errors + ); + + let pipeline = &block.pipelines[0]; + assert_eq!(1, pipeline.len()); + let element = &pipeline.elements[0]; + match &element.expr.expr { + Expr::ExternalCall(name, args) => f(name, args), + other => { + panic!("{tag}: Unexpected expression in pipeline: {other:?}"); + } + } +} + +fn check_external_call_interpolation( + tag: &str, + subexpr_count: usize, + quoted: bool, + expr: &Expression, +) -> bool { + match &expr.expr { + Expr::StringInterpolation(exprs) => { + assert!(quoted, "{tag}: quoted"); + assert_eq!(expr.ty, Type::String, "{tag}: expr.ty"); + assert_eq!(subexpr_count, exprs.len(), "{tag}: subexpr_count"); + true + } + Expr::GlobInterpolation(exprs, is_quoted) => { + assert_eq!(quoted, *is_quoted, "{tag}: quoted"); + assert_eq!(expr.ty, Type::Glob, "{tag}: expr.ty"); + assert_eq!(subexpr_count, exprs.len(), "{tag}: subexpr_count"); + true + } + _ => false, + } +} + #[rstest] #[case("foo-external-call", "foo-external-call", "bare word")] #[case("^foo-external-call", "foo-external-call", "bare word with caret")] @@ -713,200 +757,370 @@ pub fn parse_call_missing_req_flag() { r"foo\external-call", "bare word with backslash and caret" )] -#[case( - "^'foo external call'", - "'foo external call'", - "single quote with caret" -)] -#[case( - "^'foo/external call'", - "'foo/external call'", - "single quote with forward slash and caret" -)] -#[case( - r"^'foo\external call'", - r"'foo\external call'", - "single quote with backslash and caret" -)] -#[case( - r#"^"foo external call""#, - r#""foo external call""#, - "double quote with caret" -)] -#[case( - r#"^"foo/external call""#, - r#""foo/external call""#, - "double quote with forward slash and caret" -)] -#[case( - 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) { - let engine_state = EngineState::new(); - let mut working_set = StateWorkingSet::new(&engine_state); - let block = parse(&mut working_set, None, input.as_bytes(), true); - assert!( - working_set.parse_errors.is_empty(), - "{tag}: errors: {:?}", - working_set.parse_errors - ); - - let pipeline = &block.pipelines[0]; - assert_eq!(1, pipeline.len()); - let element = &pipeline.elements[0]; - match &element.expr.expr { - Expr::ExternalCall(name, args) => { - match &name.expr { - Expr::String(string) => { - assert_eq!(expected, string); - } - other => { - panic!("{tag}: Unexpected expression in command name position: {other:?}"); - } - } - assert_eq!(0, args.len()); - } - other => { - panic!("{tag}: Unexpected expression in pipeline: {other:?}"); - } - } -} - -#[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( +pub fn test_external_call_head_glob( #[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); - assert!( - working_set.parse_errors.is_empty(), - "{tag}: errors: {:?}", - working_set.parse_errors - ); + test_external_call(input, tag, |name, args| { + match &name.expr { + Expr::GlobPattern(string, is_quoted) => { + assert_eq!(expected, string, "{tag}: incorrect name"); + assert!(!*is_quoted); + } + other => { + panic!("{tag}: Unexpected expression in command name position: {other:?}"); + } + } + assert_eq!(0, args.len()); + }) +} - let pipeline = &block.pipelines[0]; - assert_eq!(1, pipeline.len()); - let element = &pipeline.elements[0]; - match &element.expr.expr { - Expr::ExternalCall(name, args) => { - match &name.expr { - Expr::String(string) => { - assert_eq!("foo", string, "{tag}: incorrect name"); +#[rstest] +#[case( + 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, +) { + test_external_call(input, tag, |name, args| { + match &name.expr { + Expr::RawString(string) => { + assert_eq!(expected, string, "{tag}: incorrect name"); + } + other => { + panic!("{tag}: Unexpected expression in command name position: {other:?}"); + } + } + assert_eq!(0, args.len()); + }) +} + +#[rstest] +#[case("^'foo external call'", "foo external call", "single quote with caret")] +#[case( + "^'foo/external call'", + "foo/external call", + "single quote with forward slash and caret" +)] +#[case( + r"^'foo\external call'", + r"foo\external call", + "single quote with backslash and caret" +)] +#[case( + r#"^"foo external call""#, + r#"foo external call"#, + "double quote with caret" +)] +#[case( + r#"^"foo/external call""#, + r#"foo/external call"#, + "double quote with forward slash and caret" +)] +#[case( + r#"^"foo\\external call""#, + r#"foo\external call"#, + "double quote with backslash and caret" +)] +pub fn test_external_call_head_string( + #[case] input: &str, + #[case] expected: &str, + #[case] tag: &str, +) { + test_external_call(input, tag, |name, args| { + match &name.expr { + Expr::String(string) => { + assert_eq!(expected, string); + } + other => { + panic!("{tag}: Unexpected expression in command name position: {other:?}"); + } + } + assert_eq!(0, args.len()); + }) +} + +#[rstest] +#[case(r"~/.foo/(1)", 2, false, "unquoted interpolated string")] +#[case( + r"~\.foo(2)\(1)", + 4, + 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, +) { + test_external_call(input, tag, |name, args| { + if !check_external_call_interpolation(tag, subexpr_count, quoted, name) { + panic!("{tag}: Unexpected expression in command name position: {name:?}"); + } + assert_eq!(0, args.len()); + }) +} + +#[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" +)] +#[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( + r"^foo `foo\external call`", + r"foo\external call", + "backtick quote with backslash" +)] +pub fn test_external_call_arg_glob(#[case] input: &str, #[case] expected: &str, #[case] tag: &str) { + test_external_call(input, tag, |name, args| { + match &name.expr { + Expr::GlobPattern(string, _) => { + assert_eq!("foo", string, "{tag}: incorrect name"); + } + other => { + panic!("{tag}: Unexpected expression in command name position: {other:?}"); + } + } + assert_eq!(1, args.len()); + match &args[0] { + ExternalArgument::Regular(expr) => match &expr.expr { + Expr::GlobPattern(string, is_quoted) => { + assert_eq!(expected, string, "{tag}: incorrect arg"); + assert!(!*is_quoted); } other => { - panic!("{tag}: Unexpected expression in command name position: {other:?}"); - } - } - assert_eq!(1, args.len()); - match &args[0] { - ExternalArgument::Regular(expr) => match &expr.expr { - Expr::String(string) => { - assert_eq!(expected, string, "{tag}: incorrect arg"); - } - other => { - panic!("Unexpected expression in command arg position: {other:?}") - } - }, - other @ ExternalArgument::Spread(..) => { - panic!("Unexpected external spread argument in command arg position: {other:?}") + panic!("Unexpected expression in command arg position: {other:?}") } + }, + other @ ExternalArgument::Spread(..) => { + panic!("Unexpected external spread argument in command arg position: {other:?}") } } - other => { - panic!("{tag}: Unexpected expression in pipeline: {other:?}"); + }) +} + +#[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, +) { + test_external_call(input, tag, |name, args| { + match &name.expr { + Expr::GlobPattern(string, _) => { + assert_eq!("foo", string, "{tag}: incorrect name"); + } + other => { + panic!("{tag}: Unexpected expression in command name position: {other:?}"); + } } - } + assert_eq!(1, args.len()); + match &args[0] { + ExternalArgument::Regular(expr) => match &expr.expr { + Expr::RawString(string) => { + assert_eq!(expected, string, "{tag}: incorrect arg"); + } + other => { + panic!("Unexpected expression in command arg position: {other:?}") + } + }, + other @ ExternalArgument::Spread(..) => { + panic!("Unexpected external spread argument in command arg position: {other:?}") + } + } + }) +} + +#[rstest] +#[case("^foo 'foo external call'", "foo external call", "single quote")] +#[case( + "^foo 'foo/external call'", + "foo/external call", + "single quote with forward slash" +)] +#[case( + r"^foo 'foo\external call'", + r"foo\external call", + "single quote with backslash" +)] +#[case(r#"^foo "foo external call""#, r#"foo external call"#, "double quote")] +#[case( + r#"^foo "foo/external call""#, + r#"foo/external call"#, + "double quote with forward slash" +)] +#[case( + r#"^foo "foo\\external call""#, + r#"foo\external call"#, + "double quote with backslash" +)] +pub fn test_external_call_arg_string( + #[case] input: &str, + #[case] expected: &str, + #[case] tag: &str, +) { + test_external_call(input, tag, |name, args| { + match &name.expr { + Expr::GlobPattern(string, _) => { + assert_eq!("foo", string, "{tag}: incorrect name"); + } + other => { + panic!("{tag}: Unexpected expression in command name position: {other:?}"); + } + } + assert_eq!(1, args.len()); + match &args[0] { + ExternalArgument::Regular(expr) => match &expr.expr { + Expr::String(string) => { + assert_eq!(expected, string, "{tag}: incorrect arg"); + } + other => { + panic!("{tag}: Unexpected expression in command arg position: {other:?}") + } + }, + other @ ExternalArgument::Spread(..) => { + panic!( + "{tag}: Unexpected external spread argument in command arg position: {other:?}" + ) + } + } + }) +} + +#[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_external_call(input, tag, |name, args| { + match &name.expr { + Expr::GlobPattern(string, _) => { + assert_eq!("foo", string, "{tag}: incorrect name"); + } + other => { + panic!("{tag}: Unexpected expression in command name position: {other:?}"); + } + } + assert_eq!(1, args.len()); + match &args[0] { + ExternalArgument::Regular(expr) => { + if !check_external_call_interpolation(tag, subexpr_count, quoted, expr) { + panic!("Unexpected expression in command arg position: {expr:?}") + } + } + other @ ExternalArgument::Spread(..) => { + panic!("Unexpected external spread argument in command arg position: {other:?}") + } + } + }) } #[test] fn test_external_call_argument_spread() { - let engine_state = EngineState::new(); - let mut working_set = StateWorkingSet::new(&engine_state); - let block = parse(&mut working_set, None, b"^foo ...[a b c]", true); - assert!( - working_set.parse_errors.is_empty(), - "errors: {:?}", - working_set.parse_errors - ); + let input = r"^foo ...[a b c]"; + let tag = "spread"; - let pipeline = &block.pipelines[0]; - assert_eq!(1, pipeline.len()); - let element = &pipeline.elements[0]; - match &element.expr.expr { - Expr::ExternalCall(name, args) => { - match &name.expr { - Expr::String(string) => { - assert_eq!("foo", string, "incorrect name"); + test_external_call(input, tag, |name, args| { + match &name.expr { + Expr::GlobPattern(string, _) => { + assert_eq!("foo", string, "incorrect name"); + } + other => { + panic!("Unexpected expression in command name position: {other:?}"); + } + } + assert_eq!(1, args.len()); + match &args[0] { + ExternalArgument::Spread(expr) => match &expr.expr { + Expr::List(items) => { + assert_eq!(3, items.len()); + // that's good enough, don't really need to go so deep into it... } other => { - panic!("Unexpected expression in command name position: {other:?}"); - } - } - assert_eq!(1, args.len()); - match &args[0] { - ExternalArgument::Spread(expr) => match &expr.expr { - Expr::List(items) => { - assert_eq!(3, items.len()); - // that's good enough, don't really need to go so deep into it... - } - other => { - panic!("Unexpected expression in command arg position: {other:?}") - } - }, - other @ ExternalArgument::Regular(..) => { - panic!( - "Unexpected external regular argument in command arg position: {other:?}" - ) + panic!("Unexpected expression in command arg position: {other:?}") } + }, + other @ ExternalArgument::Regular(..) => { + panic!("Unexpected external regular argument in command arg position: {other:?}") } } - other => { - panic!("Unexpected expression in pipeline: {other:?}"); - } - } + }) } #[test] @@ -1132,6 +1346,44 @@ 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) => { + 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..0e561e5c8f 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), @@ -43,6 +46,8 @@ pub enum Expr { Overlay(Option), // block ID of the overlay's origin module Signature(Box), StringInterpolation(Vec), + /// The boolean is `true` if the string is quoted. + GlobInterpolation(Vec, bool), Nothing, Garbage, } @@ -84,6 +89,7 @@ impl Expr { | Expr::RawString(_) | Expr::CellPath(_) | Expr::StringInterpolation(_) + | Expr::GlobInterpolation(_, _) | 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 08ecc59aaf..040e140cab 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) | Expr::GlobInterpolation(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) | Expr::GlobInterpolation(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..c3066c0a14 100644 --- a/crates/nu-protocol/src/debugger/profiler.rs +++ b/crates/nu-protocol/src/debugger/profiler.rs @@ -258,6 +258,7 @@ fn expr_to_string(engine_state: &EngineState, expr: &Expr) -> String { Expr::Signature(_) => "signature".to_string(), Expr::String(_) | Expr::RawString(_) => "string".to_string(), Expr::StringInterpolation(_) => "string interpolation".to_string(), + Expr::GlobInterpolation(_, _) => "glob 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 2317149460..7e1e2b0a7c 100644 --- a/crates/nu-protocol/src/eval_base.rs +++ b/crates/nu-protocol/src/eval_base.rs @@ -290,6 +290,15 @@ pub trait Eval { Ok(Value::string(str, expr_span)) } + Expr::GlobInterpolation(exprs, quoted) => { + let config = Self::get_config(state, mut_state); + let str = exprs + .iter() + .map(|expr| Self::eval::(state, mut_state, expr).map(|v| v.to_expanded_string(", ", &config))) + .collect::>()?; + + Ok(Value::glob(str, *quoted, expr_span)) + } Expr::Overlay(_) => Self::eval_overlay(state, expr_span), Expr::GlobPattern(pattern, quoted) => { // GlobPattern is similar to Filepath diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs index 51d743e1a4..e83b4354da 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/crates/nu-test-support/src/macros.rs @@ -245,6 +245,7 @@ use tempfile::tempdir; pub struct NuOpts { pub cwd: Option, pub locale: Option, + pub envs: Option>, pub collapse_output: Option, } @@ -278,6 +279,11 @@ pub fn nu_run_test(opts: NuOpts, commands: impl AsRef, with_std: bool) -> O command .env(nu_utils::locale::LOCALE_OVERRIDE_ENV_VAR, locale) .env(NATIVE_PATH_ENV_VAR, paths_joined); + + if let Some(envs) = opts.envs { + command.envs(envs); + } + // Ensure that the user's config doesn't interfere with the tests command.arg("--no-config-file"); if !with_std { diff --git a/crates/nuon/src/from.rs b/crates/nuon/src/from.rs index f9230d6140..2cfeaebc61 100644 --- a/crates/nuon/src/from.rs +++ b/crates/nuon/src/from.rs @@ -323,6 +323,12 @@ fn convert_to_value( msg: "string interpolation not supported in nuon".into(), span: expr.span, }), + Expr::GlobInterpolation(..) => Err(ShellError::OutsideSpannedLabeledError { + src: original_text.to_string(), + error: "Error when loading".into(), + msg: "glob interpolation not supported in nuon".into(), + span: expr.span, + }), Expr::Subexpression(..) => Err(ShellError::OutsideSpannedLabeledError { src: original_text.to_string(), error: "Error when loading".into(),