From 942ff7df4d2bef7a23efa185eb4c3901f49bf880 Mon Sep 17 00:00:00 2001 From: WindSoilder Date: Tue, 14 Nov 2023 20:46:05 +0800 Subject: [PATCH] fix custom command's default value (#11043) # Description Fixes: #11033 Sorry for the issue, it's a regression which introduce by this pr: #10456. And this pr is going to fix it. About the change: create a new field named `type_annotated` for `Arg::Flag` and `Arg::Signature` instead of `arg_explicit_type` variable. When we meet a type in `TypeMode`, we set `type_annotated` field of the argument to be true, then we know that if the arg have a annotated type easily --- crates/nu-parser/src/parser.rs | 166 ++++++++++++++++++------------ src/tests/test_custom_commands.rs | 28 +++++ 2 files changed, 129 insertions(+), 65 deletions(-) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 84e1a1197..439f2dc5c 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -3085,9 +3085,16 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> #[derive(Debug)] enum Arg { - Positional(PositionalArg, bool), // bool - required + Positional { + arg: PositionalArg, + required: bool, + type_annotated: bool, + }, RestPositional(PositionalArg), - Flag(Flag), + Flag { + flag: Flag, + type_annotated: bool, + }, } let source = working_set.get_span_contents(span); @@ -3105,7 +3112,6 @@ 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 { @@ -3134,7 +3140,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> // The = symbol separates a variable from its default value else if contents == b"=" { match parse_mode { - ParseMode::ArgMode | ParseMode::TypeMode => { + ParseMode::TypeMode | ParseMode::ArgMode => { parse_mode = ParseMode::DefaultValueMode; } ParseMode::AfterCommaArgMode => { @@ -3160,7 +3166,6 @@ 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 => { @@ -3192,15 +3197,18 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> // If there's no short flag, exit now. Otherwise, parse it. if flags.len() == 1 { - args.push(Arg::Flag(Flag { - arg: None, - desc: String::new(), - long, - short: None, - required: false, - var_id: Some(var_id), - default_value: None, - })); + args.push(Arg::Flag { + flag: Flag { + arg: None, + desc: String::new(), + long, + short: None, + required: false, + var_id: Some(var_id), + default_value: None, + }, + type_annotated: false, + }); } else if flags.len() >= 3 { working_set.error(ParseError::Expected( "only one short flag alternative", @@ -3250,15 +3258,18 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> ); if chars.len() == 1 { - args.push(Arg::Flag(Flag { - arg: None, - desc: String::new(), - long, - short: Some(chars[0]), - required: false, - var_id: Some(var_id), - default_value: None, - })); + args.push(Arg::Flag { + flag: Flag { + arg: None, + desc: String::new(), + long, + short: Some(chars[0]), + required: false, + var_id: Some(var_id), + default_value: None, + }, + type_annotated: false, + }); } else { working_set.error(ParseError::Expected("short flag", span)); } @@ -3289,15 +3300,18 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> let var_id = working_set.add_variable(variable_name, span, Type::Any, false); - args.push(Arg::Flag(Flag { - arg: None, - desc: String::new(), - long: String::new(), - short: Some(chars[0]), - required: false, - var_id: Some(var_id), - default_value: None, - })); + args.push(Arg::Flag { + flag: Flag { + arg: None, + desc: String::new(), + long: String::new(), + short: Some(chars[0]), + required: false, + var_id: Some(var_id), + default_value: None, + }, + type_annotated: false, + }); parse_mode = ParseMode::ArgMode; } // Short flag alias for long flag, e.g. --b (-a) @@ -3321,7 +3335,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> if chars.len() == 1 { match args.last_mut() { - Some(Arg::Flag(flag)) => { + Some(Arg::Flag { flag, .. }) => { if flag.short.is_some() { working_set.error(ParseError::Expected( "one short flag", @@ -3355,16 +3369,17 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> let var_id = working_set.add_variable(contents, span, Type::Any, false); - args.push(Arg::Positional( - PositionalArg { + args.push(Arg::Positional { + arg: PositionalArg { desc: String::new(), name, shape: SyntaxShape::Any, var_id: Some(var_id), default_value: None, }, - false, - )); + required: false, + type_annotated: false, + }); parse_mode = ParseMode::ArgMode; } // Rest param @@ -3407,16 +3422,17 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> working_set.add_variable(contents_vec, span, Type::Any, false); // Positional arg, required - args.push(Arg::Positional( - PositionalArg { + args.push(Arg::Positional { + arg: PositionalArg { desc: String::new(), name, shape: SyntaxShape::Any, var_id: Some(var_id), default_value: None, }, - true, - )); + required: true, + type_annotated: false, + }); parse_mode = ParseMode::ArgMode; } } @@ -3430,9 +3446,14 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> ); //TODO check if we're replacing a custom parameter already match last { - Arg::Positional(PositionalArg { shape, var_id, .. }, ..) => { + Arg::Positional { + arg: PositionalArg { shape, var_id, .. }, + required: _, + type_annotated, + } => { working_set.set_variable_type(var_id.expect("internal error: all custom parameters must have var_ids"), syntax_shape.to_type()); *shape = syntax_shape; + *type_annotated = true; } Arg::RestPositional(PositionalArg { shape, var_id, .. @@ -3440,12 +3461,15 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> working_set.set_variable_type(var_id.expect("internal error: all custom parameters must have var_ids"), Type::List(Box::new(syntax_shape.to_type()))); *shape = syntax_shape; } - Arg::Flag(Flag { arg, var_id, .. }) => { + Arg::Flag { + flag: Flag { arg, var_id, .. }, + type_annotated, + } => { working_set.set_variable_type(var_id.expect("internal error: all custom parameters must have var_ids"), syntax_shape.to_type()); *arg = Some(syntax_shape); + *type_annotated = true; } } - arg_explicit_type = true; } parse_mode = ParseMode::ArgMode; } @@ -3455,20 +3479,22 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> //TODO check if we're replacing a custom parameter already match last { - Arg::Positional( - PositionalArg { - shape, - var_id, - default_value, - .. - }, + Arg::Positional { + arg: + PositionalArg { + shape, + var_id, + default_value, + .. + }, required, - ) => { + type_annotated, + } => { let var_id = var_id.expect("internal error: all custom parameters must have var_ids"); let var_type = &working_set.get_variable(var_id).ty; match var_type { Type::Any => { - if !arg_explicit_type { + if !*type_annotated { working_set.set_variable_type( var_id, expression.ty.clone(), @@ -3501,7 +3527,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> None }; - if !arg_explicit_type { + if !*type_annotated { *shape = expression.ty.to_shape(); } *required = false; @@ -3513,12 +3539,16 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> expression.span, )) } - Arg::Flag(Flag { - arg, - var_id, - default_value, - .. - }) => { + Arg::Flag { + flag: + Flag { + arg, + var_id, + default_value, + .. + }, + type_annotated, + } => { let expression_span = expression.span; *default_value = if let Ok(value) = @@ -3540,7 +3570,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> // in the case, `var_type` is any. match var_type { Type::Any => { - if !arg_explicit_type { + if !*type_annotated { *arg = Some(expression_ty.to_shape()); working_set .set_variable_type(var_id, expression_ty); @@ -3580,13 +3610,15 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> if let Some(last) = args.last_mut() { match last { - Arg::Flag(flag) => { + Arg::Flag { flag, .. } => { if !flag.desc.is_empty() { flag.desc.push('\n'); } flag.desc.push_str(&contents); } - Arg::Positional(positional, ..) => { + Arg::Positional { + arg: positional, .. + } => { if !positional.desc.is_empty() { positional.desc.push('\n'); } @@ -3609,7 +3641,11 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> for arg in args { match arg { - Arg::Positional(positional, required) => { + Arg::Positional { + arg: positional, + required, + .. + } => { if required { if !sig.optional_positional.is_empty() { working_set.error(ParseError::RequiredAfterOptional( @@ -3622,7 +3658,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> sig.optional_positional.push(positional) } } - Arg::Flag(flag) => sig.named.push(flag), + Arg::Flag { flag, .. } => sig.named.push(flag), Arg::RestPositional(positional) => { if positional.name.is_empty() { working_set.error(ParseError::RestNeedsName(span)) diff --git a/src/tests/test_custom_commands.rs b/src/tests/test_custom_commands.rs index ed65ddf1f..f9daa5c54 100644 --- a/src/tests/test_custom_commands.rs +++ b/src/tests/test_custom_commands.rs @@ -81,6 +81,34 @@ fn custom_switch2() -> TestResult { ) } +#[test] +fn custom_switch3() -> TestResult { + run_test( + r#"def florb [ + --age: int = 0 + --name = "foobar" + ] { + ($age | into string) + $name + } + florb"#, + "0foobar", + ) +} + +#[test] +fn custom_switch4() -> TestResult { + run_test( + r#"def florb [ + --age: int + --name = "foobar" + ] { + ($age | into string) + $name + } + florb --age 3"#, + "3foobar", + ) +} + #[test] fn simple_var_closing() -> TestResult { run_test("let $x = 10; def foo [] { $x }; foo", "10")