From bf40f035f6a9a5245c82112cf60072764cc85867 Mon Sep 17 00:00:00 2001 From: WindSoilder Date: Wed, 20 Sep 2023 23:58:29 +0800 Subject: [PATCH] don't overrite arg's type if it's annotated explicitly (#10424) # Description Fixes: #10410 So the following script is possible: ```nushell def a [b: any = null] { let b = ($b | default "default_b"); } a "given_b" ``` ## About the change When parsing signature, and nushell meets something like `a: any`, it force the parser to treat `a` as `any` type. This is what `arg_explicit_type` means, it's only set when we goes into `ParseMode::TypeMode`, and it will be reset to `false` if the token goes to next argument. so, when we have something like this: `def a [b: any = null] { $b }`, the type of `$b` won't be overwritten. But if we have something like this: `def a [b = null] { $b }`, the type of `$b` is not annotated, so we make it to be `nothing`(which is the type of null) --- crates/nu-command/tests/commands/def.rs | 16 +++++++++++++++ crates/nu-parser/src/parser.rs | 27 +++++++++++++++++-------- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/crates/nu-command/tests/commands/def.rs b/crates/nu-command/tests/commands/def.rs index 65d5d58db3..e149c159fc 100644 --- a/crates/nu-command/tests/commands/def.rs +++ b/crates/nu-command/tests/commands/def.rs @@ -164,3 +164,19 @@ fn extern_with_block() { assert_eq!(actual.out, "--bar,baz,--,-q,-u,-x"); } + +#[test] +fn def_default_value_shouldnt_restrict_explicit_type() { + let actual = nu!("def foo [x: any = null] { $x }; foo 1"); + assert_eq!(actual.out, "1"); + let actual2 = nu!("def foo [--x: any = null] { $x }; foo --x 1"); + assert_eq!(actual2.out, "1"); +} + +#[test] +fn def_default_value_should_restrict_implicit_type() { + let actual = nu!("def foo [x = 3] { $x }; foo 3.0"); + assert!(actual.err.contains("expected int")); + let actual2 = nu!("def foo2 [--x = 3] { $x }; foo2 --x 3.0"); + assert!(actual2.err.contains("expected int")); +} diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 89598a209d..d12670cb21 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -3374,6 +3374,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> let mut args: Vec = vec![]; let mut parse_mode = ParseMode::ArgMode; + let mut arg_explicit_type = false; for token in &output { match token { @@ -3428,6 +3429,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> working_set.error(ParseError::Expected("default value", span)); } } + arg_explicit_type = false; } else { match parse_mode { ParseMode::ArgMode | ParseMode::AfterCommaArgMode => { @@ -3710,6 +3712,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> } } } + arg_explicit_type = true; } parse_mode = ParseMode::ArgMode; } @@ -3732,10 +3735,12 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> let var_type = &working_set.get_variable(var_id).ty; match var_type { Type::Any => { - working_set.set_variable_type( - var_id, - expression.ty.clone(), - ); + if !arg_explicit_type { + working_set.set_variable_type( + var_id, + expression.ty.clone(), + ); + } } _ => { if !type_compatible(var_type, &expression.ty) { @@ -3763,7 +3768,9 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> None }; - *shape = expression.ty.to_shape(); + if !arg_explicit_type { + *shape = expression.ty.to_shape(); + } *required = false; } Arg::RestPositional(..) => { @@ -3800,9 +3807,13 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> if var_type != &Type::Bool { match var_type { Type::Any => { - *arg = Some(expression_ty.to_shape()); - working_set - .set_variable_type(var_id, expression_ty); + if !arg_explicit_type { + *arg = Some(expression_ty.to_shape()); + working_set.set_variable_type( + var_id, + expression_ty, + ); + } } t => { if t != &expression_ty {