mirror of
https://github.com/nushell/nushell.git
synced 2024-11-25 09:53:43 +01:00
Avoid panic when pipe a variable to a custom command which have recursive call (#12491)
# Description Fixes: #11351 And comment here is also fixed: https://github.com/nushell/nushell/issues/11351#issuecomment-1996191537 The panic can happened if we pipe a variable to a custom command which recursively called itself inside another block. TBH, I think I figure out how it works to panic, but I'm not sure if there is a potention issue if nushell don't mutate a block in such case. # User-Facing Changes Nan # Tests + Formatting Done # After Submitting Done --------- Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com>
This commit is contained in:
parent
bed236362a
commit
b0acc1d890
@ -6289,7 +6289,19 @@ pub fn parse(
|
|||||||
// panic (again, in theory, this shouldn't be possible)
|
// panic (again, in theory, this shouldn't be possible)
|
||||||
let block = working_set.get_block(block_id);
|
let block = working_set.get_block(block_id);
|
||||||
let block_captures_empty = block.captures.is_empty();
|
let block_captures_empty = block.captures.is_empty();
|
||||||
if !captures.is_empty() && block_captures_empty {
|
// need to check block_id >= working_set.permanent_state.num_blocks()
|
||||||
|
// to avoid mutate a block that is in the permanent state.
|
||||||
|
// this can happened if user defines a function with recursive call
|
||||||
|
// and pipe a variable to the command, e.g:
|
||||||
|
// def px [] { if true { 42 } else { px } }; # the block px is saved in permanent state.
|
||||||
|
// let x = 3
|
||||||
|
// $x | px
|
||||||
|
// If we don't guard for `block_id`, it will change captures of `px`, which is
|
||||||
|
// already saved in permanent state
|
||||||
|
if !captures.is_empty()
|
||||||
|
&& block_captures_empty
|
||||||
|
&& block_id >= working_set.permanent_state.num_blocks()
|
||||||
|
{
|
||||||
let block = working_set.get_block_mut(block_id);
|
let block = working_set.get_block_mut(block_id);
|
||||||
block.captures = captures.into_iter().map(|(var_id, _)| var_id).collect();
|
block.captures = captures.into_iter().map(|(var_id, _)| var_id).collect();
|
||||||
}
|
}
|
||||||
|
@ -1,4 +1,5 @@
|
|||||||
use crate::tests::{fail_test, run_test, run_test_with_env, TestResult};
|
use crate::tests::{fail_test, run_test, run_test_with_env, TestResult};
|
||||||
|
use nu_test_support::{nu, nu_repl_code};
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
|
|
||||||
use super::run_test_contains;
|
use super::run_test_contains;
|
||||||
@ -818,3 +819,41 @@ fn record_missing_value() -> TestResult {
|
|||||||
fn def_requires_body_closure() -> TestResult {
|
fn def_requires_body_closure() -> TestResult {
|
||||||
fail_test("def a [] (echo 4)", "expected definition body closure")
|
fail_test("def a [] (echo 4)", "expected definition body closure")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn not_panic_with_recursive_call() {
|
||||||
|
let result = nu!(nu_repl_code(&[
|
||||||
|
"def px [] { if true { 3 } else { px } }",
|
||||||
|
"let x = 1",
|
||||||
|
"$x | px",
|
||||||
|
]));
|
||||||
|
assert_eq!(result.out, "3");
|
||||||
|
|
||||||
|
let result = nu!(nu_repl_code(&[
|
||||||
|
"def px [n=0] { let l = $in; if $n == 0 { return false } else { $l | px ($n - 1) } }",
|
||||||
|
"let x = 1",
|
||||||
|
"$x | px"
|
||||||
|
]));
|
||||||
|
assert_eq!(result.out, "false");
|
||||||
|
|
||||||
|
let result = nu!(nu_repl_code(&[
|
||||||
|
"def px [n=0] { let l = $in; if $n == 0 { return false } else { $l | px ($n - 1) } }",
|
||||||
|
"let x = 1",
|
||||||
|
"def foo [] { $x }",
|
||||||
|
"foo | px"
|
||||||
|
]));
|
||||||
|
assert_eq!(result.out, "false");
|
||||||
|
|
||||||
|
let result = nu!(nu_repl_code(&[
|
||||||
|
"def px [n=0] { let l = $in; if $n == 0 { return false } else { $l | px ($n - 1) } }",
|
||||||
|
"let x = 1",
|
||||||
|
"do {|| $x } | px"
|
||||||
|
]));
|
||||||
|
assert_eq!(result.out, "false");
|
||||||
|
|
||||||
|
let result = nu!(
|
||||||
|
cwd: "tests/parsing/samples",
|
||||||
|
"nu recursive_func_with_alias.nu"
|
||||||
|
);
|
||||||
|
assert!(result.status.success());
|
||||||
|
}
|
||||||
|
22
tests/parsing/samples/recursive_func_with_alias.nu
Normal file
22
tests/parsing/samples/recursive_func_with_alias.nu
Normal file
@ -0,0 +1,22 @@
|
|||||||
|
alias "orig update" = update
|
||||||
|
|
||||||
|
# Update a column to have a new value if it exists.
|
||||||
|
#
|
||||||
|
# If the column exists with the value `null` it will be skipped.
|
||||||
|
export def "update" [
|
||||||
|
field: cell-path # The name of the column to maybe update.
|
||||||
|
value: any # The new value to give the cell(s), or a closure to create the value.
|
||||||
|
]: [record -> record, table -> table, list<any> -> list<any>] {
|
||||||
|
let input = $in
|
||||||
|
match ($input | describe | str replace --regex '<.*' '') {
|
||||||
|
record => {
|
||||||
|
if ($input | get -i $field) != null {
|
||||||
|
$input | orig update $field $value
|
||||||
|
} else { $input }
|
||||||
|
}
|
||||||
|
table|list => {
|
||||||
|
$input | each {|| update $field $value }
|
||||||
|
}
|
||||||
|
_ => { $input | orig update $field $value }
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue
Block a user