Allow spreading arguments to commands (#11289)

<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

Finishes implementing https://github.com/nushell/nushell/issues/10598,
which asks for a spread operator in lists, in records, and when calling
commands.

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

This PR will allow spreading arguments to commands (both internal and
external). It will also deprecate spreading arguments automatically when
passing to external commands.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

- Users will be able to use `...` to spread arguments to custom/builtin
commands that have rest parameters or allow unknown arguments, or to any
external command
- If a custom command doesn't have a rest parameter and it doesn't allow
unknown arguments either, the spread operator will not be allowed
- Passing lists to external commands without `...` will work for now but
will cause a deprecation warning saying that it'll stop working in 0.91
(is 2 versions enough time?)

Here's a function to help with demonstrating some behavior:
```nushell
> def foo [ a, b, c?, d?, ...rest ] { [$a $b $c $d $rest] | to nuon }
```

You can pass a list of arguments to fill in the `rest` parameter using
`...`:
```nushell
> foo 1 2 3 4 ...[5 6]
[1, 2, 3, 4, [5, 6]]
```

If you don't use `...`, the list `[5 6]` will be treated as a single
argument:

```nushell
> foo 1 2 3 4 [5 6] # Note the double [[]]
[1, 2, 3, 4, [[5, 6]]]
```

You can omit optional parameters before the spread arguments:
```nushell
> foo 1 2 3 ...[4 5] # d is omitted here
[1, 2, 3, null, [4, 5]]
```

If you have multiple lists, you can spread them all:
```nushell
> foo 1 2 3 ...[4 5] 6 7 ...[8] ...[]
[1, 2, 3, null, [4, 5, 6, 7, 8]]
```

Here's the kind of error you get when you try to spread arguments to a
command with no rest parameter:

![image](https://github.com/nushell/nushell/assets/45539777/93faceae-00eb-4e59-ac3f-17f98436e6e4)

And this is the warning you get when you pass a list to an external now
(without `...`):


![image](https://github.com/nushell/nushell/assets/45539777/d368f590-201e-49fb-8b20-68476ced415e)


# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

Added tests to cover the following cases:
- Spreading arguments to a command that doesn't have a rest parameter
(unexpected spread argument error)
- Spreading arguments to a command that doesn't have a rest parameter
*but* there's also a missing positional argument (missing positional
error)
- Spreading arguments to a command that doesn't have a rest parameter
but does allow unknown arguments, such as `exec` (allowed)
- Spreading a list literal containing arguments of the wrong type (parse
error)
- Spreading a non-list value, both to internal and external commands
- Having named arguments in the middle of rest arguments
- `explain`ing a command call that spreads its arguments

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->

# Examples

Suppose you have multiple tables:
```nushell
let people = [[id name age]; [0 alice 100] [1 bob 200] [2 eve 300]]
let evil_twins = [[id name age]; [0 ecila 100] [-1 bob 200] [-2 eve 300]]
```

Maybe you often find yourself needing to merge multiple tables and want
a utility to do that. You could write a function like this:
```nushell
def merge_all [ ...tables ] { $tables | reduce { |it, acc| $acc | merge $it } }
```

Then you can use it like this:
```nushell
> merge_all ...([$people $evil_twins] | each { |$it| $it | select name age })
╭───┬───────┬─────╮
│ # │ name  │ age │
├───┼───────┼─────┤
│ 0 │ ecila │ 100 │
│ 1 │ bob   │ 200 │
│ 2 │ eve   │ 300 │
╰───┴───────┴─────╯
```

Except they had duplicate columns, so now you first want to suffix every
column with a number to tell you which table the column came from. You
can make a command for that:
```nushell
def select_and_merge [ --cols: list<string>, ...tables ] {
  let renamed_tables = $tables
    | enumerate
    | each { |it|
      $it.item | select $cols | rename ...($cols | each { |col| $col + ($it.index | into string) })
    };
  merge_all ...$renamed_tables
}
```
And call it like this:
```nushell
> select_and_merge --cols [name age] $people $evil_twins
╭───┬───────┬──────┬───────┬──────╮
│ # │ name0 │ age0 │ name1 │ age1 │
├───┼───────┼──────┼───────┼──────┤
│ 0 │ alice │  100 │ ecila │  100 │
│ 1 │ bob   │  200 │ bob   │  200 │
│ 2 │ eve   │  300 │ eve   │  300 │
╰───┴───────┴──────┴───────┴──────╯
```

---

Suppose someone's made a command to search for APT packages:

```nushell
# The main command
def search-pkgs [
    --install                   # Whether to install any packages it finds
    log_level: int              # Pretend it's a good idea to make this a required positional parameter
    exclude?: list<string>      # Packages to exclude
    repositories?: list<string> # Which repositories to look in (searches in all if not given)
    ...pkgs                     # Package names to search for
] {
  { install: $install, log_level: $log_level, exclude: ($exclude | to nuon), repositories: ($repositories | to nuon), pkgs: ($pkgs | to nuon) }
}
```

It has a lot of parameters to configure it, so you might make your own
helper commands to wrap around it for specific cases. Here's one
example:
```nushell
# Only look for packages locally
def search-pkgs-local [
    --install              # Whether to install any packages it finds
    log_level: int
    exclude?: list<string> # Packages to exclude
    ...pkgs                # Package names to search for
] {
  # All required and optional positional parameters are given
  search-pkgs --install=$install $log_level [] ["<local URI or something>"] ...$pkgs
}
```
And you can run it like this:
```nushell
> search-pkgs-local --install=false 5 ...["python2.7" "vim"]
╭──────────────┬──────────────────────────────╮
│ install      │ false                        │
│ log_level    │ 5                            │
│ exclude      │ []                           │
│ repositories │ ["<local URI or something>"] │
│ pkgs         │ ["python2.7", vim]           │
╰──────────────┴──────────────────────────────╯
```

One thing I realized when writing this was that if we decide to not
allow passing optional arguments using the spread operator, then you can
(mis?)use the spread operator to skip optional parameters. Here, I
didn't want to give `exclude` explicitly, so I used a spread operator to
pass the packages to install. Without it, I would've needed to do
`search-pkgs-local --install=false 5 [] "python2.7" "vim"` (explicitly
pass `[]` (or `null`, in the general case) to `exclude`). There are
probably more idiomatic ways to do this, but I just thought it was
something interesting.

If you're a virologist of the [xkcd](https://xkcd.com/350/) kind,
another helper command you might make is this:
```nushell
# Install any packages it finds
def live-dangerously [ ...pkgs ] {
  # One optional argument was given (exclude), while another was not (repositories)
  search-pkgs 0 [] ...$pkgs --install # Flags can go after spread arguments
}
```

Running it:
```nushell
> live-dangerously "git" "*vi*" # *vi* because I don't feel like typing out vim and neovim
╭──────────────┬─────────────╮
│ install      │ true        │
│ log_level    │ 0           │
│ exclude      │ []          │
│ repositories │ null        │
│ pkgs         │ [git, *vi*] │
╰──────────────┴─────────────╯
```

Here's an example that uses the spread operator more than once within
the same command call:
```nushell
let extras = [ chrome firefox python java git ]

def search-pkgs-curated [ ...pkgs ] {
  (search-pkgs
      1
      [emacs]
      ["example.com", "foo.com"]
      vim # A must for everyone!
      ...($pkgs | filter { |p| not ($p | str contains "*") }) # Remove packages with globs
      python # Good tool to have
      ...$extras
      --install=false
      python3) # I forget, did I already put Python in extras?
}
```

Running it:
```nushell
> search-pkgs-curated "git" "*vi*"
╭──────────────┬───────────────────────────────────────────────────────────────────╮
│ install      │ false                                                             │
│ log_level    │ 1                                                                 │
│ exclude      │ [emacs]                                                           │
│ repositories │ [example.com, foo.com]                                            │
│ pkgs         │ [vim, git, python, chrome, firefox, python, java, git, "python3"] │
╰──────────────┴───────────────────────────────────────────────────────────────────╯
```
This commit is contained in:
Yash Thakur 2023-12-28 02:43:20 -05:00 committed by GitHub
parent a86a7e6c29
commit 21b3eeed99
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
24 changed files with 429 additions and 171 deletions

View File

@ -385,6 +385,7 @@ fn find_matching_block_end_in_expr(
Argument::Named((_, _, opt_expr)) => opt_expr.as_ref(),
Argument::Positional(inner_expr) => Some(inner_expr),
Argument::Unknown(inner_expr) => Some(inner_expr),
Argument::Spread(inner_expr) => Some(inner_expr),
};
if let Some(inner_expr) = opt_expr {

View File

@ -1,4 +1,4 @@
use nu_engine::eval_expression;
use nu_engine::{eval_expression, CallExt};
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
@ -48,8 +48,7 @@ impl Command for BytesBuild {
_input: PipelineData,
) -> Result<PipelineData, ShellError> {
let mut output = vec![];
for expr in call.positional_iter() {
let val = eval_expression(engine_state, stack, expr)?;
for val in call.rest_iter_flattened(0, |expr| eval_expression(engine_state, stack, expr))? {
match val {
Value::Binary { mut val, .. } => output.append(&mut val),
// Explicitly propagate errors instead of dropping them.

View File

@ -191,6 +191,24 @@ fn get_arguments(engine_state: &EngineState, stack: &mut Stack, call: Call) -> V
let arg_value_name_span_start = evaled_span.start as i64;
let arg_value_name_span_end = evaled_span.end as i64;
let record = record! {
"arg_type" => Value::string(arg_type, span),
"name" => Value::string(arg_value_name, inner_expr.span),
"type" => Value::string(arg_value_type, span),
"span_start" => Value::int(arg_value_name_span_start, span),
"span_end" => Value::int(arg_value_name_span_end, span),
};
arg_value.push(Value::record(record, inner_expr.span));
}
Argument::Spread(inner_expr) => {
let arg_type = "spread";
let evaluated_expression = get_expression_as_value(engine_state, stack, inner_expr);
let arg_value_name = debug_string_without_formatting(&evaluated_expression);
let arg_value_type = &evaluated_expression.get_type().to_string();
let evaled_span = evaluated_expression.span();
let arg_value_name_span_start = evaled_span.start as i64;
let arg_value_name_span_end = evaled_span.end as i64;
let record = record! {
"arg_type" => Value::string(arg_type, span),
"name" => Value::string(arg_value_name, inner_expr.span),

View File

@ -1,8 +1,9 @@
use nu_cmd_base::hook::eval_hook;
use nu_engine::env_to_strings;
use nu_engine::eval_expression;
use nu_engine::CallExt;
use nu_protocol::{
ast::{Call, Expr, Expression},
ast::{Call, Expr},
did_you_mean,
engine::{Command, EngineState, Stack},
Category, Example, ListStream, PipelineData, RawStream, ShellError, Signature, Span, Spanned,
@ -113,7 +114,6 @@ pub fn create_external_command(
trim_end_newline: bool,
) -> Result<ExternalCommand, ShellError> {
let name: Spanned<String> = call.req(engine_state, stack, 0)?;
let args: Vec<Value> = call.rest(engine_state, stack, 1)?;
// Translate environment variables from Values to Strings
let env_vars_str = env_to_strings(engine_state, stack)?;
@ -132,11 +132,24 @@ pub fn create_external_command(
}
let mut spanned_args = vec![];
let args_expr: Vec<Expression> = call.positional_iter().skip(1).cloned().collect();
let mut arg_keep_raw = vec![];
for (one_arg, one_arg_expr) in args.into_iter().zip(args_expr) {
match one_arg {
for (arg, spread) in call.rest_iter(1) {
// TODO: Disallow automatic spreading entirely later. This match block will
// have to be refactored, and lists will have to be disallowed in the parser too
match eval_expression(engine_state, stack, arg)? {
Value::List { vals, .. } => {
if !spread {
nu_protocol::report_error_new(
engine_state,
&ShellError::GenericError {
error: "Automatically spreading lists is deprecated".into(),
msg: "Spreading lists automatically when calling external commands is deprecated and will be removed in 0.91.".into(),
span: Some(arg.span),
help: Some("Use the spread operator (put a '...' before the argument)".into()),
inner: vec![],
},
);
}
// turn all the strings in the array into params.
// Example: one_arg may be something like ["ls" "-a"]
// convert it to "ls" "-a"
@ -147,15 +160,20 @@ pub fn create_external_command(
}
}
val => {
spanned_args.push(value_as_spanned(val)?);
match one_arg_expr.expr {
// refer to `parse_dollar_expr` function
// the expression type of $variable_name, $"($variable_name)"
// will be Expr::StringInterpolation, Expr::FullCellPath
Expr::StringInterpolation(_) | Expr::FullCellPath(_) => arg_keep_raw.push(true),
_ => arg_keep_raw.push(false),
if spread {
return Err(ShellError::CannotSpreadAsList { span: arg.span });
} else {
spanned_args.push(value_as_spanned(val)?);
match arg.expr {
// refer to `parse_dollar_expr` function
// the expression type of $variable_name, $"($variable_name)"
// will be Expr::StringInterpolation, Expr::FullCellPath
Expr::StringInterpolation(_) | Expr::FullCellPath(_) => {
arg_keep_raw.push(true)
}
_ => arg_keep_raw.push(false),
}
}
{}
}
}
}

View File

@ -197,7 +197,7 @@ fn def_wrapped_with_block() {
#[test]
fn def_wrapped_from_module() {
let actual = nu!(r#"module spam {
export def --wrapped my-echo [...rest] { ^echo $rest }
export def --wrapped my-echo [...rest] { ^echo ...$rest }
}
use spam

View File

@ -49,7 +49,7 @@ fn exec_misc_values() {
let actual = nu!(
cwd: dirs.test(), pipeline(
r#"
nu -c 'let x = "abc"; exec nu --testbin cococo $x [ a b c ]'
nu -c 'let x = "abc"; exec nu --testbin cococo $x ...[ a b c ]'
"#
));

View File

@ -328,7 +328,7 @@ fn redirect_combine() {
let actual = nu!(
cwd: dirs.test(), pipeline(
r#"
run-external --redirect-combine sh [-c 'echo Foo; echo >&2 Bar']
run-external --redirect-combine sh ...[-c 'echo Foo; echo >&2 Bar']
"#
));

View File

@ -1,8 +1,8 @@
use nu_protocol::{
ast::Call,
ast::{Call, Expression},
engine::{EngineState, Stack, StateWorkingSet},
eval_const::eval_constant,
FromValue, ShellError,
FromValue, ShellError, Value,
};
use crate::eval_expression;
@ -34,6 +34,10 @@ pub trait CallExt {
starting_pos: usize,
) -> Result<Vec<T>, ShellError>;
fn rest_iter_flattened<F>(&self, start: usize, eval: F) -> Result<Vec<Value>, ShellError>
where
F: FnMut(&Expression) -> Result<Value, ShellError>;
fn opt<T: FromValue>(
&self,
engine_state: &EngineState,
@ -98,8 +102,9 @@ impl CallExt for Call {
) -> Result<Vec<T>, ShellError> {
let mut output = vec![];
for expr in self.positional_iter().skip(starting_pos) {
let result = eval_expression(engine_state, stack, expr)?;
for result in self.rest_iter_flattened(starting_pos, |expr| {
eval_expression(engine_state, stack, expr)
})? {
output.push(FromValue::from_value(result)?);
}
@ -113,14 +118,36 @@ impl CallExt for Call {
) -> Result<Vec<T>, ShellError> {
let mut output = vec![];
for expr in self.positional_iter().skip(starting_pos) {
let result = eval_constant(working_set, expr)?;
for result in
self.rest_iter_flattened(starting_pos, |expr| eval_constant(working_set, expr))?
{
output.push(FromValue::from_value(result)?);
}
Ok(output)
}
fn rest_iter_flattened<F>(&self, start: usize, mut eval: F) -> Result<Vec<Value>, ShellError>
where
F: FnMut(&Expression) -> Result<Value, ShellError>,
{
let mut output = Vec::new();
for (expr, spread) in self.rest_iter(start) {
let result = eval(expr)?;
if spread {
match result {
Value::List { mut vals, .. } => output.append(&mut vals),
_ => return Err(ShellError::CannotSpreadAsList { span: expr.span }),
}
} else {
output.push(result);
}
}
Ok(output)
}
fn opt<T: FromValue>(
&self,
engine_state: &EngineState,

View File

@ -1,9 +1,9 @@
use crate::{current_dir_str, get_full_help};
use crate::{call_ext::CallExt, current_dir_str, get_full_help};
use nu_path::expand_path_with;
use nu_protocol::{
ast::{
Argument, Assignment, Block, Call, Expr, Expression, PathMember, PipelineElement,
Redirection,
Argument, Assignment, Block, Call, Expr, Expression, ExternalArgument, PathMember,
PipelineElement, Redirection,
},
engine::{Closure, EngineState, Stack},
eval_base::Eval,
@ -66,11 +66,11 @@ pub fn eval_call(
if let Some(rest_positional) = decl.signature().rest_positional {
let mut rest_items = vec![];
for arg in call.positional_iter().skip(
for result in call.rest_iter_flattened(
decl.signature().required_positional.len()
+ decl.signature().optional_positional.len(),
) {
let result = eval_expression(engine_state, caller_stack, arg)?;
|expr| eval_expression(engine_state, caller_stack, expr),
)? {
rest_items.push(result);
}
@ -182,7 +182,7 @@ fn eval_external(
engine_state: &EngineState,
stack: &mut Stack,
head: &Expression,
args: &[Expression],
args: &[ExternalArgument],
input: PipelineData,
redirect_target: RedirectTarget,
is_subexpression: bool,
@ -198,7 +198,10 @@ fn eval_external(
call.add_positional(head.clone());
for arg in args {
call.add_positional(arg.clone())
match arg {
ExternalArgument::Regular(expr) => call.add_positional(expr.clone()),
ExternalArgument::Spread(expr) => call.add_spread(expr.clone()),
}
}
match redirect_target {
@ -947,7 +950,7 @@ impl Eval for EvalRuntime {
engine_state: &EngineState,
stack: &mut Stack,
head: &Expression,
args: &[Expression],
args: &[ExternalArgument],
is_subexpression: bool,
_: Span,
) -> Result<Value, ShellError> {

View File

@ -1,6 +1,6 @@
use nu_protocol::ast::{
Block, Expr, Expression, ImportPatternMember, MatchPattern, PathMember, Pattern, Pipeline,
PipelineElement, RecordItem,
Argument, Block, Expr, Expression, ExternalArgument, ImportPatternMember, MatchPattern,
PathMember, Pattern, Pipeline, PipelineElement, RecordItem,
};
use nu_protocol::{engine::StateWorkingSet, Span};
use nu_protocol::{DeclId, VarId};
@ -193,17 +193,28 @@ pub fn flatten_expression(
}
let mut args = vec![];
for positional in call.positional_iter() {
let flattened = flatten_expression(working_set, positional);
args.extend(flattened);
}
for named in call.named_iter() {
if named.0.span.end != 0 {
// Ignore synthetic flags
args.push((named.0.span, FlatShape::Flag));
}
if let Some(expr) = &named.2 {
args.extend(flatten_expression(working_set, expr));
for arg in &call.arguments {
match arg {
Argument::Positional(positional) | Argument::Unknown(positional) => {
let flattened = flatten_expression(working_set, positional);
args.extend(flattened);
}
Argument::Named(named) => {
if named.0.span.end != 0 {
// Ignore synthetic flags
args.push((named.0.span, FlatShape::Flag));
}
if let Some(expr) = &named.2 {
args.extend(flatten_expression(working_set, expr));
}
}
Argument::Spread(expr) => {
args.push((
Span::new(expr.span.start - 3, expr.span.start),
FlatShape::Operator,
));
args.extend(flatten_expression(working_set, expr));
}
}
}
// sort these since flags and positional args can be intermixed
@ -231,15 +242,24 @@ pub fn flatten_expression(
for arg in args {
//output.push((*arg, FlatShape::ExternalArg));
match arg {
Expression {
expr: Expr::String(..),
span,
..
} => {
output.push((*span, FlatShape::ExternalArg));
}
_ => {
output.extend(flatten_expression(working_set, arg));
ExternalArgument::Regular(expr) => match expr {
Expression {
expr: Expr::String(..),
span,
..
} => {
output.push((*span, FlatShape::ExternalArg));
}
_ => {
output.extend(flatten_expression(working_set, expr));
}
},
ExternalArgument::Spread(expr) => {
output.push((
Span::new(expr.span.start - 3, expr.span.start),
FlatShape::Operator,
));
output.extend(flatten_expression(working_set, expr));
}
}
}

View File

@ -106,6 +106,7 @@ impl Command for KnownExternal {
}
}
Argument::Unknown(unknown) => extern_call.add_unknown(unknown.clone()),
Argument::Spread(args) => extern_call.add_spread(args.clone()),
}
}

View File

@ -12,9 +12,9 @@ use nu_engine::DIR_VAR_PARSER_INFO;
use nu_protocol::{
ast::{
Argument, Assignment, Bits, Block, Boolean, Call, CellPath, Comparison, Expr, Expression,
FullCellPath, ImportPattern, ImportPatternHead, ImportPatternMember, MatchPattern, Math,
Operator, PathMember, Pattern, Pipeline, PipelineElement, RangeInclusion, RangeOperator,
RecordItem,
ExternalArgument, FullCellPath, ImportPattern, ImportPatternHead, ImportPatternMember,
MatchPattern, Math, Operator, PathMember, Pattern, Pipeline, PipelineElement,
RangeInclusion, RangeOperator, RecordItem,
},
engine::StateWorkingSet,
eval_const::eval_constant,
@ -266,13 +266,22 @@ pub fn check_name<'a>(working_set: &mut StateWorkingSet, spans: &'a [Span]) -> O
}
}
fn parse_external_arg(working_set: &mut StateWorkingSet, span: Span) -> Expression {
fn parse_external_arg(working_set: &mut StateWorkingSet, span: Span) -> ExternalArgument {
let contents = working_set.get_span_contents(span);
if contents.starts_with(b"$") || contents.starts_with(b"(") {
parse_dollar_expr(working_set, span)
ExternalArgument::Regular(parse_dollar_expr(working_set, span))
} else if contents.starts_with(b"[") {
parse_list_expression(working_set, span, &SyntaxShape::Any)
ExternalArgument::Regular(parse_list_expression(working_set, span, &SyntaxShape::Any))
} else if contents.len() > 3
&& contents.starts_with(b"...")
&& (contents[3] == b'$' || contents[3] == b'[' || contents[3] == b'(')
{
ExternalArgument::Spread(parse_value(
working_set,
Span::new(span.start + 3, span.end),
&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 = if contents.starts_with(b"\"") {
@ -285,12 +294,12 @@ fn parse_external_arg(working_set: &mut StateWorkingSet, span: Span) -> Expressi
String::from_utf8_lossy(contents).to_string()
};
Expression {
ExternalArgument::Regular(Expression {
expr: Expr::String(contents),
span,
ty: Type::String,
custom_completion: None,
}
})
}
}
@ -978,6 +987,49 @@ pub fn parse_internal_call(
continue;
}
{
let contents = working_set.get_span_contents(spans[spans_idx]);
if contents.len() > 3
&& contents.starts_with(b"...")
&& (contents[3] == b'$' || contents[3] == b'[' || contents[3] == b'(')
{
if signature.rest_positional.is_none() && !signature.allows_unknown_args {
working_set.error(ParseError::UnexpectedSpreadArg(
signature.call_signature(),
arg_span,
));
call.add_positional(Expression::garbage(arg_span));
} else if positional_idx < signature.required_positional.len() {
working_set.error(ParseError::MissingPositional(
signature.required_positional[positional_idx].name.clone(),
Span::new(spans[spans_idx].start, spans[spans_idx].start),
signature.call_signature(),
));
call.add_positional(Expression::garbage(arg_span));
} else {
let rest_shape = match &signature.rest_positional {
Some(arg) => arg.shape.clone(),
None => SyntaxShape::Any,
};
// Parse list of arguments to be spread
let args = parse_value(
working_set,
Span::new(arg_span.start + 3, arg_span.end),
&SyntaxShape::List(Box::new(rest_shape)),
);
call.add_spread(args);
// Let the parser know that it's parsing rest arguments now
positional_idx =
signature.required_positional.len() + signature.optional_positional.len();
}
spans_idx += 1;
continue;
}
}
// Parse a positional arg if there is one
if let Some(positional) = signature.get_positional(positional_idx) {
let end = calculate_end_span(working_set, &signature, spans, spans_idx, positional_idx);
@ -5885,22 +5937,27 @@ pub fn discover_captures_in_expr(
}
}
for named in call.named_iter() {
if let Some(arg) = &named.2 {
discover_captures_in_expr(working_set, arg, seen, seen_blocks, output)?;
for arg in &call.arguments {
match arg {
Argument::Named(named) => {
if let Some(arg) = &named.2 {
discover_captures_in_expr(working_set, arg, seen, seen_blocks, output)?;
}
}
Argument::Positional(expr)
| Argument::Unknown(expr)
| Argument::Spread(expr) => {
discover_captures_in_expr(working_set, expr, seen, seen_blocks, output)?;
}
}
}
for positional in call.positional_iter() {
discover_captures_in_expr(working_set, positional, seen, seen_blocks, output)?;
}
}
Expr::CellPath(_) => {}
Expr::DateTime(_) => {}
Expr::ExternalCall(head, exprs, _) => {
Expr::ExternalCall(head, args, _) => {
discover_captures_in_expr(working_set, head, seen, seen_blocks, output)?;
for expr in exprs {
for ExternalArgument::Regular(expr) | ExternalArgument::Spread(expr) in args {
discover_captures_in_expr(working_set, expr, seen, seen_blocks, output)?;
}
}

View File

@ -1,4 +1,4 @@
use nu_engine::eval_expression;
use nu_engine::{eval_expression, CallExt};
use nu_protocol::{
ast::Call,
engine::{EngineState, Stack},
@ -33,10 +33,8 @@ impl EvaluatedCall {
engine_state: &EngineState,
stack: &mut Stack,
) -> Result<Self, ShellError> {
let positional = call
.positional_iter()
.map(|expr| eval_expression(engine_state, stack, expr))
.collect::<Result<Vec<Value>, ShellError>>()?;
let positional =
call.rest_iter_flattened(0, |expr| eval_expression(engine_state, stack, expr))?;
let mut named = Vec::with_capacity(call.named_len());
for (string, _, expr) in call.named_iter() {

View File

@ -10,6 +10,7 @@ pub enum Argument {
Positional(Expression),
Named((Spanned<String>, Option<Spanned<String>>, Option<Expression>)),
Unknown(Expression), // unknown argument used in "fall-through" signatures
Spread(Expression), // a list spread to fill in rest arguments
}
impl Argument {
@ -30,10 +31,17 @@ impl Argument {
Span::new(start, end)
}
Argument::Unknown(e) => e.span,
Argument::Spread(e) => e.span,
}
}
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub enum ExternalArgument {
Regular(Expression),
Spread(Expression),
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Call {
/// identifier of the declaration to call
@ -85,6 +93,7 @@ impl Call {
Argument::Named(named) => Some(named),
Argument::Positional(_) => None,
Argument::Unknown(_) => None,
Argument::Spread(_) => None,
})
}
@ -96,6 +105,7 @@ impl Call {
Argument::Named(named) => Some(named),
Argument::Positional(_) => None,
Argument::Unknown(_) => None,
Argument::Spread(_) => None,
})
}
@ -118,26 +128,45 @@ impl Call {
self.arguments.push(Argument::Unknown(unknown));
}
pub fn add_spread(&mut self, args: Expression) {
self.arguments.push(Argument::Spread(args));
}
pub fn positional_iter(&self) -> impl Iterator<Item = &Expression> {
self.arguments.iter().filter_map(|arg| match arg {
Argument::Named(_) => None,
Argument::Positional(positional) => Some(positional),
Argument::Unknown(unknown) => Some(unknown),
})
self.arguments
.iter()
.take_while(|arg| match arg {
Argument::Spread(_) => false, // Don't include positional arguments given to rest parameter
_ => true,
})
.filter_map(|arg| match arg {
Argument::Named(_) => None,
Argument::Positional(positional) => Some(positional),
Argument::Unknown(unknown) => Some(unknown),
Argument::Spread(_) => None,
})
}
pub fn positional_iter_mut(&mut self) -> impl Iterator<Item = &mut Expression> {
self.arguments.iter_mut().filter_map(|arg| match arg {
Argument::Named(_) => None,
Argument::Positional(positional) => Some(positional),
Argument::Unknown(unknown) => Some(unknown),
})
self.arguments
.iter_mut()
.take_while(|arg| match arg {
Argument::Spread(_) => false, // Don't include positional arguments given to rest parameter
_ => true,
})
.filter_map(|arg| match arg {
Argument::Named(_) => None,
Argument::Positional(positional) => Some(positional),
Argument::Unknown(unknown) => Some(unknown),
Argument::Spread(_) => None,
})
}
pub fn positional_nth(&self, i: usize) -> Option<&Expression> {
self.positional_iter().nth(i)
}
// TODO this method is never used. Delete?
pub fn positional_nth_mut(&mut self, i: usize) -> Option<&mut Expression> {
self.positional_iter_mut().nth(i)
}
@ -146,6 +175,24 @@ impl Call {
self.positional_iter().count()
}
/// Returns every argument to the rest parameter, as well as whether each argument
/// is spread or a normal positional argument (true for spread, false for normal)
pub fn rest_iter(&self, start: usize) -> impl Iterator<Item = (&Expression, bool)> {
// todo maybe rewrite to be more elegant or something
let args = self
.arguments
.iter()
.filter_map(|arg| match arg {
Argument::Named(_) => None,
Argument::Positional(positional) => Some((positional, false)),
Argument::Unknown(unknown) => Some((unknown, false)),
Argument::Spread(args) => Some((args, true)),
})
.collect::<Vec<_>>();
let spread_start = args.iter().position(|(_, spread)| *spread).unwrap_or(start);
args.into_iter().skip(start.min(spread_start))
}
pub fn get_parser_info(&self, name: &str) -> Option<&Expression> {
self.parser_info.get(name)
}

View File

@ -1,7 +1,10 @@
use chrono::FixedOffset;
use serde::{Deserialize, Serialize};
use super::{Call, CellPath, Expression, FullCellPath, MatchPattern, Operator, RangeOperator};
use super::{
Call, CellPath, Expression, ExternalArgument, FullCellPath, MatchPattern, Operator,
RangeOperator,
};
use crate::{ast::ImportPattern, BlockId, Signature, Span, Spanned, Unit, VarId};
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@ -19,7 +22,7 @@ pub enum Expr {
Var(VarId),
VarDecl(VarId),
Call(Box<Call>),
ExternalCall(Box<Expression>, Vec<Expression>, bool), // head, args, is_subexpression
ExternalCall(Box<Expression>, Vec<ExternalArgument>, bool), // head, args, is_subexpression
Operator(Operator),
RowCondition(BlockId),
UnaryNot(Box<Expression>),

View File

@ -1,6 +1,6 @@
use serde::{Deserialize, Serialize};
use super::{Expr, RecordItem};
use super::{Argument, Expr, ExternalArgument, RecordItem};
use crate::ast::ImportPattern;
use crate::DeclId;
use crate::{engine::StateWorkingSet, BlockId, Signature, Span, Type, VarId, IN_VARIABLE_ID};
@ -162,15 +162,21 @@ impl Expression {
Expr::Binary(_) => false,
Expr::Bool(_) => false,
Expr::Call(call) => {
for positional in call.positional_iter() {
if positional.has_in_variable(working_set) {
return true;
}
}
for named in call.named_iter() {
if let Some(expr) = &named.2 {
if expr.has_in_variable(working_set) {
return true;
for arg in &call.arguments {
match arg {
Argument::Positional(expr)
| Argument::Unknown(expr)
| Argument::Spread(expr) => {
if expr.has_in_variable(working_set) {
return true;
}
}
Argument::Named(named) => {
if let Some(expr) = &named.2 {
if expr.has_in_variable(working_set) {
return true;
}
}
}
}
}
@ -182,8 +188,8 @@ impl Expression {
if head.has_in_variable(working_set) {
return true;
}
for arg in args {
if arg.has_in_variable(working_set) {
for ExternalArgument::Regular(expr) | ExternalArgument::Spread(expr) in args {
if expr.has_in_variable(working_set) {
return true;
}
}
@ -346,12 +352,18 @@ impl Expression {
if replaced.contains_span(call.head) {
call.head = new_span;
}
for positional in call.positional_iter_mut() {
positional.replace_span(working_set, replaced, new_span);
}
for named in call.named_iter_mut() {
if let Some(expr) = &mut named.2 {
expr.replace_span(working_set, replaced, new_span)
for arg in call.arguments.iter_mut() {
match arg {
Argument::Positional(expr)
| Argument::Unknown(expr)
| Argument::Spread(expr) => {
expr.replace_span(working_set, replaced, new_span);
}
Argument::Named(named) => {
if let Some(expr) = &mut named.2 {
expr.replace_span(working_set, replaced, new_span);
}
}
}
}
}
@ -359,8 +371,8 @@ impl Expression {
Expr::DateTime(_) => {}
Expr::ExternalCall(head, args, _) => {
head.replace_span(working_set, replaced, new_span);
for arg in args {
arg.replace_span(working_set, replaced, new_span)
for ExternalArgument::Regular(expr) | ExternalArgument::Spread(expr) in args {
expr.replace_span(working_set, replaced, new_span);
}
}
Expr::Filepath(_) => {}

View File

@ -1,7 +1,7 @@
use crate::{
ast::{
eval_operator, Assignment, Bits, Boolean, Call, Comparison, Expr, Expression, Math,
Operator, RecordItem,
eval_operator, Assignment, Bits, Boolean, Call, Comparison, Expr, Expression,
ExternalArgument, Math, Operator, RecordItem,
},
Range, Record, ShellError, Span, Value, VarId,
};
@ -319,7 +319,7 @@ pub trait Eval {
state: Self::State<'_>,
mut_state: &mut Self::MutState,
head: &Expression,
args: &[Expression],
args: &[ExternalArgument],
is_subexpression: bool,
span: Span,
) -> Result<Value, ShellError>;

View File

@ -1,5 +1,5 @@
use crate::{
ast::{Assignment, Block, Call, Expr, Expression, PipelineElement},
ast::{Assignment, Block, Call, Expr, Expression, ExternalArgument, PipelineElement},
engine::{EngineState, StateWorkingSet},
eval_base::Eval,
record, HistoryFileFormat, PipelineData, Record, ShellError, Span, Value, VarId,
@ -317,7 +317,7 @@ impl Eval for EvalConst {
_: &StateWorkingSet,
_: &mut (),
_: &Expression,
_: &[Expression],
_: &[ExternalArgument],
_: bool,
span: Span,
) -> Result<Value, ShellError> {

View File

@ -484,9 +484,12 @@ pub enum ParseError {
#[label("...and here")] Option<Span>,
),
#[error("Unexpected spread operator outside list")]
#[diagnostic(code(nu::parser::unexpected_spread_operator))]
UnexpectedSpread(#[label("Spread operator not allowed here")] Span),
#[error("This command does not have a ...rest parameter")]
#[diagnostic(
code(nu::parser::unexpected_spread_arg),
help("To spread arguments, the command needs to define a multi-positional parameter in its signature, such as ...rest")
)]
UnexpectedSpreadArg(String, #[label = "unexpected spread argument"] Span),
}
impl ParseError {
@ -573,7 +576,7 @@ impl ParseError {
ParseError::InvalidLiteral(_, _, s) => *s,
ParseError::LabeledErrorWithHelp { span: s, .. } => *s,
ParseError::RedirectionInLetMut(s, _) => *s,
ParseError::UnexpectedSpread(s) => *s,
ParseError::UnexpectedSpreadArg(_, s) => *s,
}
}
}

View File

@ -1281,15 +1281,15 @@ This is an internal Nushell error, please file an issue https://github.com/nushe
span: Span,
},
/// Tried spreading a non-list inside a list.
/// Tried spreading a non-list inside a list or command call.
///
/// ## Resolution
///
/// Only lists can be spread inside lists. Try converting the value to a list before spreading.
/// Only lists can be spread inside lists and command calls. Try converting the value to a list before spreading.
#[error("Not a list")]
#[diagnostic(
code(nu::shell::cannot_spread_as_list),
help("Only lists can be spread inside lists. Try converting the value to a list before spreading")
help("Only lists can be spread inside lists and command calls. Try converting the value to a list before spreading.")
)]
CannotSpreadAsList {
#[label = "cannot spread value"]

View File

@ -45,7 +45,7 @@ pub fn pipeline(commands: &str) -> String {
}
pub fn nu_repl_code(source_lines: &[&str]) -> String {
let mut out = String::from("nu --testbin=nu_repl [ ");
let mut out = String::from("nu --testbin=nu_repl ...[ ");
for line in source_lines.iter() {
// convert each "line" to really be a single line to prevent nu! macro joining the newlines

View File

@ -99,8 +99,8 @@ fn known_external_misc_values() -> TestResult {
run_test(
r#"
let x = 'abc'
extern echo []
echo $x [ a b c ]
extern echo [...args]
echo $x ...[ a b c ]
"#,
"abc a b c",
)

View File

@ -1,4 +1,5 @@
use crate::tests::{fail_test, run_test, TestResult};
use nu_test_support::nu;
#[test]
fn spread_in_list() -> TestResult {
@ -24,30 +25,6 @@ fn spread_in_list() -> TestResult {
)
}
#[test]
fn const_spread_in_list() -> TestResult {
run_test(r#"const x = [...[]]; $x | to nuon"#, "[]").unwrap();
run_test(
r#"const x = [1 2 ...[[3] {x: 1}] 5]; $x | to nuon"#,
"[1, 2, [3], {x: 1}, 5]",
)
.unwrap();
run_test(
r#"const x = [...([f o o]) 10]; $x | to nuon"#,
"[f, o, o, 10]",
)
.unwrap();
run_test(
r#"const l = [1, 2, [3]]; const x = [...$l $l]; $x | to nuon"#,
"[1, 2, [3], [1, 2, [3]]]",
)
.unwrap();
run_test(
r#"[ ...[ ...[ ...[ a ] b ] c ] d ] | to nuon"#,
"[a, b, c, d]",
)
}
#[test]
fn not_spread() -> TestResult {
run_test(r#"def ... [x] { $x }; ... ..."#, "...").unwrap();
@ -95,15 +72,6 @@ fn spread_in_record() -> TestResult {
)
}
#[test]
fn const_spread_in_record() -> TestResult {
run_test(r#"const x = {...{...{...{}}}}; $x | to nuon"#, "{}").unwrap();
run_test(
r#"const x = {foo: bar ...{a: {x: 1}} b: 3}; $x | to nuon"#,
"{foo: bar, a: {x: 1}, b: 3}",
)
}
#[test]
fn duplicate_cols() -> TestResult {
fail_test(r#"{a: 1, ...{a: 3}}"#, "column used twice").unwrap();
@ -111,16 +79,6 @@ fn duplicate_cols() -> TestResult {
fail_test(r#"{...{a: 0, x: 2}, ...{x: 5}}"#, "column used twice")
}
#[test]
fn const_duplicate_cols() -> TestResult {
fail_test(r#"const _ = {a: 1, ...{a: 3}}"#, "column used twice").unwrap();
fail_test(r#"const _ = {...{a: 4, x: 3}, x: 1}"#, "column used twice").unwrap();
fail_test(
r#"const _ = {...{a: 0, x: 2}, ...{x: 5}}"#,
"column used twice",
)
}
#[test]
fn bad_spread_on_non_record() -> TestResult {
fail_test(r#"let x = 5; { ...$x }"#, "cannot spread").unwrap();
@ -139,3 +97,96 @@ fn spread_type_record() -> TestResult {
"type_mismatch",
)
}
#[test]
fn spread_external_args() {
assert_eq!(
nu!(r#"nu --testbin cococo ...[1 "foo"] 2 ...[3 "bar"]"#).out,
"1 foo 2 3 bar",
);
// exec doesn't have rest parameters but allows unknown arguments
assert_eq!(
nu!(r#"exec nu --testbin cococo "foo" ...[5 6]"#).out,
"foo 5 6"
);
}
#[test]
fn spread_internal_args() -> TestResult {
run_test(
r#"
let list = ["foo" 4]
def f [a b c? d? ...x] { [$a $b $c $d $x] | to nuon }
f 1 2 ...[5 6] 7 ...$list"#,
"[1, 2, null, null, [5, 6, 7, foo, 4]]",
)
.unwrap();
run_test(
r#"
def f [a b c? d? ...x] { [$a $b $c $d $x] | to nuon }
f 1 2 3 ...[5 6]"#,
"[1, 2, 3, null, [5, 6]]",
)
.unwrap();
run_test(
r#"
def f [--flag: int ...x] { [$flag $x] | to nuon }
f 2 ...[foo] 4 --flag 5 6 ...[7 8]"#,
"[5, [2, foo, 4, 6, 7, 8]]",
)
.unwrap();
run_test(
r#"
def f [a b? --flag: int ...x] { [$a $b $flag $x] | to nuon }
f 1 ...[foo] 4 --flag 5 6 ...[7 8]"#,
"[1, null, 5, [foo, 4, 6, 7, 8]]",
)
}
#[test]
fn bad_spread_internal_args() -> TestResult {
fail_test(
r#"
def f [a b c? d? ...x] { echo $a $b $c $d $x }
f 1 ...[5 6]"#,
"Missing required positional argument",
)
.unwrap();
fail_test(
r#"
def f [a b?] { echo a b c d }
f ...[5 6]"#,
"unexpected spread argument",
)
}
#[test]
fn spread_non_list_args() {
fail_test(r#"echo ...(1)"#, "cannot spread value").unwrap();
assert!(nu!(r#"nu --testbin cococo ...(1)"#)
.err
.contains("cannot spread value"));
}
#[test]
fn spread_args_type() -> TestResult {
fail_test(r#"def f [...x: int] {}; f ...["abc"]"#, "expected int")
}
#[test]
fn explain_spread_args() -> TestResult {
run_test(
r#"(explain { || echo ...[1 2] }).cmd_args.0 | select arg_type name type | to nuon"#,
r#"[[arg_type, name, type]; [spread, "[1 2]", list<int>]]"#,
)
}
#[test]
fn deprecate_implicit_spread_for_externals() {
// TODO: When automatic spreading is removed, test that list literals fail at parse time
let result = nu!(r#"nu --testbin cococo [1 2]"#);
assert!(result
.err
.contains("Automatically spreading lists is deprecated"));
assert_eq!(result.out, "1 2");
}

View File

@ -354,7 +354,7 @@ mod nu_commands {
#[test]
fn command_list_arg_test() {
let actual = nu!("
nu ['-c' 'version']
nu ...['-c' 'version']
");
assert!(actual.out.contains("version"));
@ -365,7 +365,7 @@ mod nu_commands {
#[test]
fn command_cell_path_arg_test() {
let actual = nu!("
nu ([ '-c' 'version' ])
nu ...([ '-c' 'version' ])
");
assert!(actual.out.contains("version"));
@ -436,7 +436,7 @@ mod external_command_arguments {
let actual = nu!(
cwd: dirs.test(), pipeline(
"
nu --testbin cococo (ls | get name)
nu --testbin cococo ...(ls | get name)
"
));