Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cusom command: default value shouldn't restrict argument's type #10410

Closed
WindSoilder opened this issue Sep 18, 2023 · 2 comments · Fixed by #10424
Closed

Cusom command: default value shouldn't restrict argument's type #10410

WindSoilder opened this issue Sep 18, 2023 · 2 comments · Fixed by #10424
Labels
needs-triage An issue that hasn't had any proper look type-system Problems or features related to nushell's type system
Milestone

Comments

@WindSoilder
Copy link
Collaborator

WindSoilder commented Sep 18, 2023

Describe the bug

Notice that when we define a custom command's argument with default value, the type of argument will be bounded.

How to reproduce

Run the following script:

def a [b: any = null] { print ($b == null) }
a 3

Then nushell throws the following error:

Error: nu::parser::parse_mismatch_with_full_string_msg

  × Parse mismatch during operation.
   ╭─[entry #16:1:1]
 1  def a [b: any = null] { print ($b == null) }
 2  a 3
   ·   
   ·   ╰── expected nothing
   ╰────

Expected behavior

I expected nu to print false

Screenshots

No response

Configuration

key value
version 0.84.1
branch main
commit_hash 715c157
build_os macos-x86_64
build_target x86_64-apple-darwin
rust_version rustc 1.70.0 (90c541806 2023-05-31)
rust_channel stable-x86_64-apple-darwin
cargo_version cargo 1.70.0 (ec8a8a0ca 2023-04-25)
build_time 2023-08-29 16:48:30 +08:00
build_rust_channel release
allocator standard
features default, extra, sqlite, trash, which, zip
installed_plugins

Additional context

No response

@WindSoilder WindSoilder added the needs-triage An issue that hasn't had any proper look label Sep 18, 2023
@WindSoilder
Copy link
Collaborator Author

WindSoilder commented Sep 18, 2023

Should the following two things behave differently?

def a [b: any = 3] { print ($b | describe) }
def a [b = 3] { print ($b | describe) }

Currently, they all bound the type of $b to int.

I think in the first case, the type of $b should be any, because we define it explicitly.
For the second one, it can be int, because we don't define the type, so the type can be inferred by default value

@amtoine amtoine added the type-system Problems or features related to nushell's type system label Sep 18, 2023
@amtoine
Copy link
Member

amtoine commented Sep 18, 2023

@WindSoilder
my two 🪙, i think

def a [b: any = 3] { print ($b | describe) }
a "foo"

should definitely work because both 3: int and "foo": string are of type any 👍

sophiajt pushed a commit that referenced this issue Sep 20, 2023
# 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)
@hustcer hustcer added this to the v0.86.0 milestone Sep 20, 2023
hardfau1t pushed a commit to hardfau1t/nushell that referenced this issue Dec 14, 2023
# Description
Fixes: nushell#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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage An issue that hasn't had any proper look type-system Problems or features related to nushell's type system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants