Disallow spreading lists automatically when calling externals (#11857)

<!--
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!
-->

# 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.
-->

Spreading lists automatically when calling externals was deprecated in
0.89 (#11289), and this PR is to remove it in 0.91.

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

The new error message looks like this:

```
>  ^echo [1 2]
Error: nu:🐚:cannot_pass_list_to_external

  × Lists are not automatically spread when calling external commands
   ╭─[entry #13:1:8]
 1 │  ^echo [1 2]
   ·        ──┬──
   ·          ╰── Spread operator (...) is necessary to spread lists
   ╰────
  help: Either convert the list to a string or use the spread operator, like so: ...[1 2]
```

The old error message didn't say exactly where to put the `...` and
seemed to confuse a lot of people, so hopefully this helps.

# 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
> ```
-->

There was one test to check that implicit spread was deprecated before,
updated that to check that it's disallowed now.

# 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.
-->
This commit is contained in:
Yash Thakur 2024-02-14 18:16:19 -05:00 committed by GitHub
parent fd7eef1499
commit cb67de675e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 36 additions and 28 deletions

View File

@ -135,29 +135,23 @@ pub fn create_external_command(
let mut spanned_args = vec![]; let mut spanned_args = vec![];
let mut arg_keep_raw = vec![]; let mut arg_keep_raw = vec![];
for (arg, spread) in call.rest_iter(1) { 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)? { match eval_expression(engine_state, stack, arg)? {
Value::List { vals, .. } => { Value::List { vals, .. } => {
if !spread { if spread {
nu_protocol::report_error_new( // turn all the strings in the array into params.
engine_state, // Example: one_arg may be something like ["ls" "-a"]
&ShellError::GenericError { // convert it to "ls" "-a"
error: "Automatically spreading lists is deprecated".into(), for v in vals {
msg: "Spreading lists automatically when calling external commands is deprecated and will be removed in 0.91.".into(), spanned_args.push(value_as_spanned(v)?);
span: Some(arg.span), // for arguments in list, it's always treated as a whole arguments
help: Some("Use the spread operator (put a '...' before the argument)".into()), arg_keep_raw.push(true);
inner: vec![], }
}, } else {
); return Err(ShellError::CannotPassListToExternal {
} arg: String::from_utf8_lossy(engine_state.get_span_contents(arg.span))
// turn all the strings in the array into params. .into(),
// Example: one_arg may be something like ["ls" "-a"] span: arg.span,
// convert it to "ls" "-a" });
for v in vals {
spanned_args.push(value_as_spanned(v)?);
// for arguments in list, it's always treated as a whole arguments
arg_keep_raw.push(true);
} }
} }
val => { val => {

View File

@ -1308,6 +1308,22 @@ This is an internal Nushell error, please file an issue https://github.com/nushe
span: Span, span: Span,
}, },
/// Lists are not automatically spread when calling external commands
///
/// ## Resolution
///
/// Use the spread operator (put a '...' before the argument)
#[error("Lists are not automatically spread when calling external commands")]
#[diagnostic(
code(nu::shell::cannot_pass_list_to_external),
help("Either convert the list to a string or use the spread operator, like so: ...{arg}")
)]
CannotPassListToExternal {
arg: String,
#[label = "Spread operator (...) is necessary to spread lists"]
span: Span,
},
/// Out of bounds. /// Out of bounds.
/// ///
/// ## Resolution /// ## Resolution

View File

@ -182,13 +182,11 @@ fn explain_spread_args() -> TestResult {
} }
#[test] #[test]
fn deprecate_implicit_spread_for_externals() { fn disallow_implicit_spread_for_externals() -> TestResult {
// TODO: When automatic spreading is removed, test that list literals fail at parse time fail_test(
let result = nu!(r#"nu --testbin cococo [1 2]"#); r#"nu --testbin cococo [1 2]"#,
assert!(result "Lists are not automatically spread",
.err )
.contains("Automatically spreading lists is deprecated"));
assert_eq!(result.out, "1 2");
} }
#[test] #[test]