Skip to content

Commit

Permalink
don't overrite arg's type if it's annotated explicitly (#10424)
Browse files Browse the repository at this point in the history
# 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)
  • Loading branch information
WindSoilder committed Sep 20, 2023
1 parent 989a147 commit bf40f03
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 8 deletions.
16 changes: 16 additions & 0 deletions crates/nu-command/tests/commands/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
27 changes: 19 additions & 8 deletions crates/nu-parser/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3374,6 +3374,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->

let mut args: Vec<Arg> = vec![];
let mut parse_mode = ParseMode::ArgMode;
let mut arg_explicit_type = false;

for token in &output {
match token {
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -3710,6 +3712,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
}
}
}
arg_explicit_type = true;
}
parse_mode = ParseMode::ArgMode;
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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(..) => {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit bf40f03

Please sign in to comment.