From 7ce66a9b77a5b4742b5539d59ed0e1190294d1fa Mon Sep 17 00:00:00 2001 From: marienz Date: Thu, 17 Jul 2025 23:38:08 +1000 Subject: [PATCH] Also type-check optional arguments (#16194) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Type-check all closure arguments, not just required arguments. Not doing so looks like an oversight. # User-Facing Changes Previously, passing an argument of the wrong type to a closure would fail if the argument is required, but be accepted (ignoring the type annotation) if the argument is optional: ``` > do {|x: string| $x} 4 Error: nu::shell::cant_convert × Can't convert to string. ╭─[entry #13:1:21] 1 │ do {|x: string| $x} 4 · ┬ · ╰── can't convert int to string ╰──── > do {|x?: string| $x} 4 4 > do {|x?: string| $x} 4 | describe int ``` It now fails the same way in both cases. # Tests + Formatting Added tests, the existing tests still pass. Please let me know if I added the wrong type of test or added them in the wrong place (I didn't spot similar tests in the nu-cmd-lang crate, so I put them next to the most-related existing tests I could find... # After Submitting I think this is minor enough it doesn't need a doc update, but please point me in the right direction if not. --- crates/nu-cmd-lang/src/core_commands/do_.rs | 2 +- crates/nu-command/tests/commands/do_.rs | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/nu-cmd-lang/src/core_commands/do_.rs b/crates/nu-cmd-lang/src/core_commands/do_.rs index 6c94f9eaa1..0360bb35b9 100644 --- a/crates/nu-cmd-lang/src/core_commands/do_.rs +++ b/crates/nu-cmd-lang/src/core_commands/do_.rs @@ -264,7 +264,7 @@ fn bind_args_to( .expect("internal error: all custom parameters must have var_ids"); if let Some(result) = val_iter.next() { let param_type = param.shape.to_type(); - if required && !result.is_subtype_of(¶m_type) { + if !result.is_subtype_of(¶m_type) { return Err(ShellError::CantConvert { to_type: param.shape.to_type().to_string(), from_type: result.get_type().to_string(), diff --git a/crates/nu-command/tests/commands/do_.rs b/crates/nu-command/tests/commands/do_.rs index 606c534073..5cf2279e7b 100644 --- a/crates/nu-command/tests/commands/do_.rs +++ b/crates/nu-command/tests/commands/do_.rs @@ -60,3 +60,17 @@ fn run_closure_with_it_using() { assert!(actual.err.is_empty()); assert_eq!(actual.out, "3"); } + +#[test] +fn required_argument_type_checked() { + let actual = nu!(r#"do {|x: string| $x} 4"#); + assert!(actual.out.is_empty()); + assert!(actual.err.contains("nu::shell::cant_convert")); +} + +#[test] +fn optional_argument_type_checked() { + let actual = nu!(r#"do {|x?: string| $x} 4"#); + assert_eq!(actual.out, ""); + assert!(actual.err.contains("nu::shell::cant_convert")); +}