holm/nushell
1
1
Fork 0
forked from extern/nushell
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix custom command's default value (#11043)

Browse Source 
# 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
This commit is contained in:
WindSoilder 2023-11-14 20:46:05 +08:00 committed by GitHub
parent 51abe4c262
commit 942ff7df4d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 129 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

166
crates/nu-parser/src/parser.rs
View File

@ -3085,9 +3085,16 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
#[derive(Debug)] #[derive(Debug)]
enum Arg { enum Arg {
Positional(PositionalArg, bool), // bool - required Positional {
arg: PositionalArg,
required: bool,
type_annotated: bool,
},
RestPositional(PositionalArg), RestPositional(PositionalArg),
Flag(Flag), Flag {
flag: Flag,
type_annotated: bool,
},
} }
let source = working_set.get_span_contents(span); 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<Arg> = vec![]; let mut args: Vec<Arg> = vec![];
let mut parse_mode = ParseMode::ArgMode; let mut parse_mode = ParseMode::ArgMode;
let mut arg_explicit_type = false;
for token in &output { for token in &output {
match token { 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 // The = symbol separates a variable from its default value
else if contents == b"=" { else if contents == b"=" {
match parse_mode { match parse_mode {
ParseMode::ArgMode | ParseMode::TypeMode => { ParseMode::TypeMode | ParseMode::ArgMode => {
parse_mode = ParseMode::DefaultValueMode; parse_mode = ParseMode::DefaultValueMode;
} }
ParseMode::AfterCommaArgMode => { 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)); working_set.error(ParseError::Expected("default value", span));
} }
} }
arg_explicit_type = false;
} else { } else {
match parse_mode { match parse_mode {
ParseMode::ArgMode | ParseMode::AfterCommaArgMode => { 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 there's no short flag, exit now. Otherwise, parse it.
if flags.len() == 1 { if flags.len() == 1 {
args.push(Arg::Flag(Flag { args.push(Arg::Flag {
arg: None, flag: Flag {
desc: String::new(), arg: None,
long, desc: String::new(),
short: None, long,
required: false, short: None,
var_id: Some(var_id), required: false,
default_value: None, var_id: Some(var_id),
})); default_value: None,
},
type_annotated: false,
});
} else if flags.len() >= 3 { } else if flags.len() >= 3 {
working_set.error(ParseError::Expected( working_set.error(ParseError::Expected(
"only one short flag alternative", "only one short flag alternative",
@ -3250,15 +3258,18 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
); );
if chars.len() == 1 { if chars.len() == 1 {
args.push(Arg::Flag(Flag { args.push(Arg::Flag {
arg: None, flag: Flag {
desc: String::new(), arg: None,
long, desc: String::new(),
short: Some(chars[0]), long,
required: false, short: Some(chars[0]),
var_id: Some(var_id), required: false,
default_value: None, var_id: Some(var_id),
})); default_value: None,
},
type_annotated: false,
});
} else { } else {
working_set.error(ParseError::Expected("short flag", span)); 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 = let var_id =
working_set.add_variable(variable_name, span, Type::Any, false); working_set.add_variable(variable_name, span, Type::Any, false);
args.push(Arg::Flag(Flag { args.push(Arg::Flag {
arg: None, flag: Flag {
desc: String::new(), arg: None,
long: String::new(), desc: String::new(),
short: Some(chars[0]), long: String::new(),
required: false, short: Some(chars[0]),
var_id: Some(var_id), required: false,
default_value: None, var_id: Some(var_id),
})); default_value: None,
},
type_annotated: false,
});
parse_mode = ParseMode::ArgMode; parse_mode = ParseMode::ArgMode;
} }
// Short flag alias for long flag, e.g. --b (-a) // 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 { if chars.len() == 1 {
match args.last_mut() { match args.last_mut() {
Some(Arg::Flag(flag)) => { Some(Arg::Flag { flag, .. }) => {
if flag.short.is_some() { if flag.short.is_some() {
working_set.error(ParseError::Expected( working_set.error(ParseError::Expected(
"one short flag", "one short flag",
@ -3355,16 +3369,17 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
let var_id = let var_id =
working_set.add_variable(contents, span, Type::Any, false); working_set.add_variable(contents, span, Type::Any, false);
args.push(Arg::Positional( args.push(Arg::Positional {
PositionalArg { arg: PositionalArg {
desc: String::new(), desc: String::new(),
name, name,
shape: SyntaxShape::Any, shape: SyntaxShape::Any,
var_id: Some(var_id), var_id: Some(var_id),
default_value: None, default_value: None,
}, },
false, required: false,
)); type_annotated: false,
});
parse_mode = ParseMode::ArgMode; parse_mode = ParseMode::ArgMode;
} }
// Rest param // 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); working_set.add_variable(contents_vec, span, Type::Any, false);
// Positional arg, required // Positional arg, required
args.push(Arg::Positional( args.push(Arg::Positional {
PositionalArg { arg: PositionalArg {
desc: String::new(), desc: String::new(),
name, name,
shape: SyntaxShape::Any, shape: SyntaxShape::Any,
var_id: Some(var_id), var_id: Some(var_id),
default_value: None, default_value: None,
}, },
true, required: true,
)); type_annotated: false,
});
parse_mode = ParseMode::ArgMode; 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 //TODO check if we're replacing a custom parameter already
match last { 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()); working_set.set_variable_type(var_id.expect("internal error: all custom parameters must have var_ids"), syntax_shape.to_type());
*shape = syntax_shape; *shape = syntax_shape;
*type_annotated = true;
} }
Arg::RestPositional(PositionalArg { Arg::RestPositional(PositionalArg {
shape, var_id, .. 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()))); 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; *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()); 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); *arg = Some(syntax_shape);
*type_annotated = true;
} }
} }
arg_explicit_type = true;
} }
parse_mode = ParseMode::ArgMode; 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 //TODO check if we're replacing a custom parameter already
match last { match last {
Arg::Positional( Arg::Positional {
PositionalArg { arg:
shape, PositionalArg {
var_id, shape,
default_value, var_id,
.. default_value,
}, ..
},
required, required,
) => { type_annotated,
} => {
let var_id = var_id.expect("internal error: all custom parameters must have var_ids"); 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; let var_type = &working_set.get_variable(var_id).ty;
match var_type { match var_type {
Type::Any => { Type::Any => {
if !arg_explicit_type { if !*type_annotated {
working_set.set_variable_type( working_set.set_variable_type(
var_id, var_id,
expression.ty.clone(), expression.ty.clone(),
@ -3501,7 +3527,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
None None
}; };
if !arg_explicit_type { if !*type_annotated {
*shape = expression.ty.to_shape(); *shape = expression.ty.to_shape();
} }
*required = false; *required = false;
@ -3513,12 +3539,16 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
expression.span, expression.span,
)) ))
} }
Arg::Flag(Flag { Arg::Flag {
arg, flag:
var_id, Flag {
default_value, arg,
.. var_id,
}) => { default_value,
..
},
type_annotated,
} => {
let expression_span = expression.span; let expression_span = expression.span;
*default_value = if let Ok(value) = *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. // in the case, `var_type` is any.
match var_type { match var_type {
Type::Any => { Type::Any => {
if !arg_explicit_type { if !*type_annotated {
*arg = Some(expression_ty.to_shape()); *arg = Some(expression_ty.to_shape());
working_set working_set
.set_variable_type(var_id, expression_ty); .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() { if let Some(last) = args.last_mut() {
match last { match last {
Arg::Flag(flag) => { Arg::Flag { flag, .. } => {
if !flag.desc.is_empty() { if !flag.desc.is_empty() {
flag.desc.push('\n'); flag.desc.push('\n');
} }
flag.desc.push_str(&contents); flag.desc.push_str(&contents);
} }
Arg::Positional(positional, ..) => { Arg::Positional {
arg: positional, ..
} => {
if !positional.desc.is_empty() { if !positional.desc.is_empty() {
positional.desc.push('\n'); positional.desc.push('\n');
} }
@ -3609,7 +3641,11 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
for arg in args { for arg in args {
match arg { match arg {
Arg::Positional(positional, required) => { Arg::Positional {
arg: positional,
required,
..
} => {
if required { if required {
if !sig.optional_positional.is_empty() { if !sig.optional_positional.is_empty() {
working_set.error(ParseError::RequiredAfterOptional( 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) sig.optional_positional.push(positional)
} }
} }
Arg::Flag(flag) => sig.named.push(flag), Arg::Flag { flag, .. } => sig.named.push(flag),
Arg::RestPositional(positional) => { Arg::RestPositional(positional) => {
if positional.name.is_empty() { if positional.name.is_empty() {
working_set.error(ParseError::RestNeedsName(span)) working_set.error(ParseError::RestNeedsName(span))

28
src/tests/test_custom_commands.rs
View File

@ -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] #[test]
fn simple_var_closing() -> TestResult { fn simple_var_closing() -> TestResult {
run_test("let $x = 10; def foo [] { $x }; foo", "10") run_test("let $x = 10; def foo [] { $x }; foo", "10")