mirror of
https://github.com/nushell/nushell.git
synced 2025-01-11 08:48:23 +01:00
Respect SyntaxShape when parsing spread operator (#11674)
<!-- 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! --> This fixes an issue brought up by nihilander in [Discord](https://discord.com/channels/601130461678272522/614593951969574961/1201594105986285649). # 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. --> Nushell panics when the spread operator is used like this (the `...$rest` shouldn't actually be parsed as a spread operator at all): ```nu $ def foo [...rest: string] {...$rest} $ foo bar baz thread 'main' panicked at /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nu-protocol-0.89.0/src/signature.rs:650:9: Internal error: can't run a predeclaration without a body stack backtrace: 0: rust_begin_unwind 1: core::panicking::panic_fmt 2: <nu_protocol::signature::Predeclaration as nu_protocol::engine::command::Command>::run 3: nu_engine::eval::eval_call 4: nu_engine::eval::eval_expression_with_input 5: nu_engine::eval::eval_element_with_input 6: nu_engine::eval::eval_block 7: nu_cli::util::eval_source 8: nu_cli::repl::evaluate_repl 9: nu::run::run_repl 10: nu::main note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. ``` The problem was that whenever the parser saw something like `{...$`, `{...(`, or `{...[`, it would treat that as a record with a spread expression, ignoring the syntax shape of the block it was parsing. This should now be fixed, and the snippet above instead gives the following error: ```nu Error: nu:🐚:external_command × External command failed ╭─[entry #1:1:1] 1 │ def foo [...rest] {...$rest} · ────┬─── · ╰── executable was not found ╰──── help: No such file or directory (os error 2) ``` # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> Stuff like `do { ...$rest }` will now try to run a command `...$rest` rather than complaining that variable `$rest` doesn't exist. # 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 > ``` --> # 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. --> Sorry about the issue, I am not touching the parser again for a long time :)
This commit is contained in:
parent
175dab4898
commit
c08f46f836
@ -1700,16 +1700,18 @@ pub fn parse_brace_expr(
|
||||
parse_closure_expression(working_set, shape, span)
|
||||
} else if matches!(third_token, Some(b":")) {
|
||||
parse_full_cell_path(working_set, None, span)
|
||||
} else if second_token.is_some_and(|c| {
|
||||
c.len() > 3 && c.starts_with(b"...") && (c[3] == b'$' || c[3] == b'{' || c[3] == b'(')
|
||||
}) {
|
||||
parse_record(working_set, span)
|
||||
} else if matches!(shape, SyntaxShape::Closure(_)) || matches!(shape, SyntaxShape::Any) {
|
||||
} else if matches!(shape, SyntaxShape::Closure(_)) {
|
||||
parse_closure_expression(working_set, shape, span)
|
||||
} else if matches!(shape, SyntaxShape::Block) {
|
||||
parse_block_expression(working_set, span)
|
||||
} else if matches!(shape, SyntaxShape::MatchBlock) {
|
||||
parse_match_block_expression(working_set, span)
|
||||
} else if second_token.is_some_and(|c| {
|
||||
c.len() > 3 && c.starts_with(b"...") && (c[3] == b'$' || c[3] == b'{' || c[3] == b'(')
|
||||
}) {
|
||||
parse_record(working_set, span)
|
||||
} else if matches!(shape, SyntaxShape::Any) {
|
||||
parse_closure_expression(working_set, shape, span)
|
||||
} else {
|
||||
working_set.error(ParseError::ExpectedWithStringMsg(
|
||||
format!("non-block value: {shape}"),
|
||||
|
@ -190,3 +190,15 @@ fn deprecate_implicit_spread_for_externals() {
|
||||
.contains("Automatically spreading lists is deprecated"));
|
||||
assert_eq!(result.out, "1 2");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn respect_shape() -> TestResult {
|
||||
fail_test(
|
||||
"def foo [...rest] { ...$rest }; foo bar baz",
|
||||
"executable was not found",
|
||||
)
|
||||
.unwrap();
|
||||
fail_test("module foo { ...$bar }", "expected_keyword").unwrap();
|
||||
run_test(r#"def "...$foo" [] {2}; do { ...$foo }"#, "2").unwrap();
|
||||
run_test(r#"match "...$foo" { ...$foo => 5 }"#, "5")
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user