From e0a91679fc22b3bf25fda8c6a661e15ff841f954 Mon Sep 17 00:00:00 2001 From: sigoden Date: Sat, 17 Aug 2024 06:42:43 +0800 Subject: [PATCH] fix: incorrectly handle args that startswith `-` but are not flags/options (#344) --- src/build.rs | 77 +++++++++++++------ src/command/mod.rs | 4 +- src/matcher.rs | 36 ++++++--- src/parser.rs | 11 ++- .../integration__spec__option_value_dash.snap | 38 +++++++++ tests/spec.rs | 15 ++++ 6 files changed, 145 insertions(+), 36 deletions(-) create mode 100644 tests/snapshots/integration__spec__option_value_dash.snap diff --git a/src/build.rs b/src/build.rs index 1eab594..7f99b94 100644 --- a/src/build.rs +++ b/src/build.rs @@ -5,8 +5,9 @@ use crate::{ ChoiceValue, DefaultValue, }; use anyhow::Result; +use indexmap::IndexSet; -const UTIL_FNS: [(&str, &str); 6] = [ +const UTIL_FNS: [(&str, &str); 7] = [ ( "_argc_take_args", r#" @@ -23,7 +24,7 @@ _argc_take_args() { else while [[ $_argc_take_index -lt $_argc_len ]]; do _argc_take_value="${argc__args[_argc_take_index]}" - if [[ -n "$signs" ]] && [[ "$_argc_take_value" =~ ^["$signs"] ]]; then + if _argc_maybe_flag_option "$signs" "$_argc_take_value"; then if [[ "${#_argc_take_value}" -gt 1 ]]; then break fi @@ -180,6 +181,35 @@ _argc_check_bool() { _argc_die "error: environment variable '$env_name' has invalid value for param '$param_name'" fi } +"#, + ), + ( + "_argc_maybe_flag_option", + r#" +_argc_maybe_flag_option() { + local signs="$1" arg="$2" + if [[ -z "$signs" ]]; then + return 1 + fi + local cond=false + if [[ "$signs" == *"+"* ]]; then + if [[ "$arg" =~ ^\+[^+].* ]]; then + cond=true + fi + elif [[ "$arg" == -* ]]; then + if (( ${#arg} < 3 )) || [[ ! "$arg" =~ ^---.* ]]; then + cond=true + fi + fi + if [[ "$cond" == "false" ]]; then + return 1 + fi + local value="${arg%%=*}" + if [[ "$value" =~ [[:space:]] ]]; then + return 1 + fi + return 0 +} "#, ), ]; @@ -232,7 +262,7 @@ fn build_root(cmd: &Command, wrap_width: Option) -> String { let after_hook = if after_hook { "\n _argc_after" } else { "" }; let mut util_fns = String::new(); for (fn_name, util_fn) in UTIL_FNS { - if command.contains(fn_name) { + if command.contains(fn_name) || util_fns.contains(fn_name) { util_fns.push_str(util_fn); } } @@ -369,21 +399,6 @@ fn build_parse(cmd: &Command, suffix: &str) -> String { } else { String::new() }; - let parse_unknown_flag_options = if !cmd.flag_option_params.is_empty() { - let unknown_flags = flag_option_signs - .chars() - .map(|v| format!("{v}?*")) - .collect::>() - .join(" | "); - format!( - r#" - {unknown_flags}) - _argc_die "error: unexpected argument \`$_argc_key\` found" - ;;"# - ) - } else { - String::new() - }; let parse_subcommands = if !cmd.subcommands.is_empty() { let mut parses: Vec = cmd .subcommands @@ -437,13 +452,24 @@ fn build_parse(cmd: &Command, suffix: &str) -> String { String::new() }; + let handle_unknown_flag_options = if !cmd.flag_option_params.is_empty() { + let signs = flag_option_signs.iter().collect::(); + format!( + r#" + if _argc_maybe_flag_option "{signs}" "$_argc_item"; then + _argc_die "error: unexpected argument \`$_argc_key\` found" + fi"#, + ) + } else { + String::new() + }; let parse_fallback = if !cmd.subcommands.is_empty() && cmd.positional_params.is_empty() { let name = cmd.full_name(); if let Some(subcmd) = cmd.find_default_subcommand() { let paths = subcmd.paths.join("_"); format!( r#" - *) + *){handle_unknown_flag_options} if [[ "${{#argc__positionals[@]}}" -eq 0 ]]; then _argc_action=_argc_parse_{paths} break @@ -453,7 +479,7 @@ fn build_parse(cmd: &Command, suffix: &str) -> String { } else { format!( r#" - *) + *){handle_unknown_flag_options} _argc_die "error: \`{name}\` requires a subcommand but one was not provided"$'\n'" [subcommands: $_argc_subcmds]" ;;"# ) @@ -473,7 +499,7 @@ fn build_parse(cmd: &Command, suffix: &str) -> String { }; format!( r#" - *) + *){handle_unknown_flag_options} argc__positionals+=("$_argc_item") _argc_index=$((_argc_index + 1)){terminated} ;;"# @@ -499,7 +525,6 @@ fn build_parse(cmd: &Command, suffix: &str) -> String { parse_dash, parse_flag_options, parse_subcommands, - parse_unknown_flag_options, parse_fallback, ] .join(""); @@ -524,7 +549,7 @@ _argc_parse{suffix}() {{ ) } -fn build_parse_flag_option(param: &FlagOptionParam, signs: &str) -> String { +fn build_parse_flag_option(param: &FlagOptionParam, signs: &IndexSet) -> String { let names = param.list_names().join(" | "); let long_name = param.long_name(); let var_name = param.var_name(); @@ -555,7 +580,11 @@ fn build_parse_flag_option(param: &FlagOptionParam, signs: &str) -> String { ;;"# ) } else { - let signs = if param.terminated() { "" } else { signs }; + let signs: String = if param.terminated() { + "".into() + } else { + signs.iter().collect::() + }; let delimiter = match param.delimiter() { Some(v) => v.to_string(), None => String::new(), diff --git a/src/command/mod.rs b/src/command/mod.rs index e751417..7313c83 100644 --- a/src/command/mod.rs +++ b/src/command/mod.rs @@ -370,7 +370,7 @@ impl Command { } } - pub(crate) fn flag_option_signs(&self) -> String { + pub(crate) fn flag_option_signs(&self) -> IndexSet { let mut signs: IndexSet = ['-'].into(); for param in &self.flag_option_params { if let Some(short) = param.short() { @@ -378,7 +378,7 @@ impl Command { } signs.extend(param.long_prefix().chars().take(1)) } - signs.into_iter().collect() + signs } pub(crate) fn cmd_name(&self) -> String { diff --git a/src/matcher.rs b/src/matcher.rs index f30c72a..cce8e90 100644 --- a/src/matcher.rs +++ b/src/matcher.rs @@ -99,7 +99,7 @@ impl<'a, 'b, T: Runtime> Matcher<'a, 'b, T> { } else { let mut is_rest_args_positional = false; // option(e.g. -f --foo) will be treated as positional arg if let Some(arg) = args.last() { - if arg.starts_with(|c| root_cmd.flag_option_signs().contains(c)) { + if maybe_flag_option(arg, &root_cmd.flag_option_signs()) { arg_comp = ArgComp::FlagOrOption; } else if !arg.is_empty() { arg_comp = ArgComp::CommandOrPositional; @@ -137,7 +137,7 @@ impl<'a, 'b, T: Runtime> Matcher<'a, 'b, T> { &mut is_rest_args_positional, cmd, ); - } else if arg.len() > 1 && arg.starts_with(|c| signs.contains(c)) { + } else if arg.len() > 1 && maybe_flag_option(arg, &signs) { if let Some((k, v)) = arg.split_once('=') { if let Some(param) = cmd.find_flag_option(k) { add_param_choice_fn(&mut choice_fns, param); @@ -1181,7 +1181,7 @@ fn take_value_args<'a>( args: &'a [String], start: usize, len: usize, - signs: &str, + signs: &IndexSet, assigned: bool, compgen: bool, ) -> Vec<&'a str> { @@ -1192,9 +1192,7 @@ fn take_value_args<'a>( let args_len = args.len(); let end = (start + len).min(args_len); for (i, arg) in args.iter().enumerate().take(end).skip(start) { - if arg.starts_with(|c| signs.contains(c)) - && (arg.len() > 1 || (compgen && i == args_len - 1)) - { + if maybe_flag_option(arg, signs) && (arg.len() > 1 || (compgen && i == args_len - 1)) { break; } output.push(arg.as_str()); @@ -1243,7 +1241,7 @@ fn match_flag_option<'a, 'b>( arg_comp: &mut ArgComp, split_last_arg_at: &mut Option, combine_shorts: bool, - signs: &str, + signs: &IndexSet, compgen: bool, ) { let arg = &args[*arg_index]; @@ -1361,8 +1359,7 @@ fn comp_subcomands(cmd: &Command, flag: bool) -> Vec { if i > 0 && v.len() < 2 { continue; } - if (flag && v.starts_with(|c| signs.contains(c))) - || (!flag && !v.starts_with(|c| signs.contains(c))) + if (flag && maybe_flag_option(&v, &signs)) || (!flag && !maybe_flag_option(&v, &signs)) { if !flag { has_help_subcmd = true; @@ -1499,3 +1496,24 @@ fn delimit_arg_values<'x, T: Param>(param: &T, values: &[&'x str]) -> Vec<&'x st fn is_bool_value(value: &str) -> bool { matches!(value, "true" | "false" | "0" | "1") } + +fn maybe_flag_option(arg: &str, signs: &IndexSet) -> bool { + let cond = if signs.contains(&'+') && arg.starts_with('+') { + !arg.starts_with("++") + } else if arg.starts_with('-') { + arg.len() < 3 || !arg.starts_with("---") + } else { + false + }; + if !cond { + return false; + } + let value = match arg.split_once('=') { + Some((v, _)) => v, + _ => arg, + }; + if value.contains(|c: char| c.is_whitespace()) { + return false; + } + true +} diff --git a/src/parser.rs b/src/parser.rs index b97b74c..fba7e12 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -398,7 +398,14 @@ fn parse_with_long_head(input: &str) -> nom::IResult<&str, (Option<&str>, &str)> ), peek(space1), )), - preceded(space0, alt((tag("--"), tag("-"), tag("+")))), + preceded( + space0, + alt(( + terminated(tag("--"), peek(not(char('-')))), + terminated(tag("-"), peek(not(char('-')))), + terminated(tag("+"), peek(not(char('+')))), + )), + ), ),)), |(short, long_prefix)| (short.map(|_| &input[0..2]), long_prefix), )(input) @@ -1047,6 +1054,8 @@ mod tests { Ok(("foo", (Some("+f"), "+"))) ); assert_eq!(parse_with_long_head("+foo"), Ok(("foo", (None, "+")))); + assert!(parse_with_long_head("-f ---foo").is_err()); + assert!(parse_with_long_head("+f ++foo").is_err()); } #[test] diff --git a/tests/snapshots/integration__spec__option_value_dash.snap b/tests/snapshots/integration__spec__option_value_dash.snap new file mode 100644 index 0000000..87d31f8 --- /dev/null +++ b/tests/snapshots/integration__spec__option_value_dash.snap @@ -0,0 +1,38 @@ +--- +source: tests/spec.rs +expression: data +--- +************ RUN ************ +prog --oa --- + +# OUTPUT +argc_oa=--- +argc__args=( prog --oa --- ) +argc__positionals=( ) + +# RUN_OUTPUT +argc__args=([0]="prog" [1]="--oa" [2]="---") +argc__positionals=() +argc_oa=--- + +************ RUN ************ +prog --- + +# OUTPUT +argc__args=( prog --- ) +argc__positionals=( --- ) + +# RUN_OUTPUT +argc__args=([0]="prog" [1]="---") +argc__positionals=([0]="---") + +************ RUN ************ +prog --a b + +# OUTPUT +argc__args=( prog '--a b' ) +argc__positionals=( '--a b' ) + +# RUN_OUTPUT +argc__args=([0]="prog" [1]="--a b") +argc__positionals=([0]="--a b") diff --git a/tests/spec.rs b/tests/spec.rs index 12fd336..bf3b66b 100644 --- a/tests/spec.rs +++ b/tests/spec.rs @@ -417,3 +417,18 @@ fn option_single_dash() { [vec!["prog", "--oa", "-"], vec!["prog", "--oa", "-", ""],] ); } + +#[test] +fn option_value_dash() { + let script = r###" +# @option --oa +"###; + snapshot_multi!( + script, + [ + vec!["prog", "--oa", "---"], + vec!["prog", "---"], + vec!["prog", "--a b"] + ] + ); +}