diff --git a/src/lib.rs b/src/lib.rs index ba28fa2c..a670f43b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -178,8 +178,7 @@ impl Options { hasarg: HasArg, occur: Occur, ) -> &mut Options { - validate_names(short_name, long_name); - self.grps.push(OptGroup { + self.push_group(OptGroup { short_name: short_name.to_string(), long_name: long_name.to_string(), hint: hint.to_string(), @@ -207,8 +206,7 @@ impl Options { /// assert!(matches.opt_present("h")); /// ``` pub fn optflag(&mut self, short_name: &str, long_name: &str, desc: &str) -> &mut Options { - validate_names(short_name, long_name); - self.grps.push(OptGroup { + self.push_group(OptGroup { short_name: short_name.to_string(), long_name: long_name.to_string(), hint: "".to_string(), @@ -237,8 +235,7 @@ impl Options { /// assert_eq!(2, matches.opt_count("v")); /// ``` pub fn optflagmulti(&mut self, short_name: &str, long_name: &str, desc: &str) -> &mut Options { - validate_names(short_name, long_name); - self.grps.push(OptGroup { + self.push_group(OptGroup { short_name: short_name.to_string(), long_name: long_name.to_string(), hint: "".to_string(), @@ -277,8 +274,7 @@ impl Options { desc: &str, hint: &str, ) -> &mut Options { - validate_names(short_name, long_name); - self.grps.push(OptGroup { + self.push_group(OptGroup { short_name: short_name.to_string(), long_name: long_name.to_string(), hint: hint.to_string(), @@ -319,8 +315,7 @@ impl Options { desc: &str, hint: &str, ) -> &mut Options { - validate_names(short_name, long_name); - self.grps.push(OptGroup { + self.push_group(OptGroup { short_name: short_name.to_string(), long_name: long_name.to_string(), hint: hint.to_string(), @@ -360,8 +355,7 @@ impl Options { desc: &str, hint: &str, ) -> &mut Options { - validate_names(short_name, long_name); - self.grps.push(OptGroup { + self.push_group(OptGroup { short_name: short_name.to_string(), long_name: long_name.to_string(), hint: hint.to_string(), @@ -403,8 +397,7 @@ impl Options { desc: &str, hint: &str, ) -> &mut Options { - validate_names(short_name, long_name); - self.grps.push(OptGroup { + self.push_group(OptGroup { short_name: short_name.to_string(), long_name: long_name.to_string(), hint: hint.to_string(), @@ -680,21 +673,39 @@ impl Options { Box::new(rows) } -} -fn validate_names(short_name: &str, long_name: &str) { - let len = short_name.len(); - assert!( - len == 1 || len == 0, - "the short_name (first argument) should be a single character, \ - or an empty string for none" - ); - let len = long_name.len(); - assert!( - len == 0 || len > 1, - "the long_name (second argument) should be longer than a single \ - character, or an empty string for none" - ); + fn push_group(&mut self, group: OptGroup) { + self.validate_names(&group); + self.grps.push(group); + } + + fn validate_names(&self, group: &OptGroup) { + let len = group.short_name.len(); + assert!( + len == 1 || len == 0, + "the short_name (first argument) should be a single character, \ + or an empty string for none" + ); + let len = group.long_name.len(); + assert!( + len == 0 || len > 1, + "the long_name (second argument) should be longer than a single \ + character, or an empty string for none" + ); + + debug_assert!( + group.short_name.is_empty() + || self.grps.iter().all(|g| g.short_name != group.short_name), + "the short option name -{} caused conflict among multiple options", + group.short_name, + ); + debug_assert!( + group.long_name.is_empty() + || self.grps.iter().all(|g| g.long_name != group.long_name), + "the long option name --{} caused conflict among multiple options", + group.long_name, + ); + } } /// What parsing style to use when parsing arguments. diff --git a/src/tests/mod.rs b/src/tests/mod.rs index f1eb9410..6d3abf77 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -1319,3 +1319,31 @@ fn test_opt_strs_pos() { ] ); } + +#[cfg(debug_assertions)] +#[test] +#[should_panic(expected = "the short option name -s caused conflict among multiple options")] +fn test_check_conflicts_in_short_name() { + let mut opts = Options::new(); + opts.optflag("s", "long", ""); + opts.optflag("s", "other", ""); +} + +#[cfg(debug_assertions)] +#[test] +#[should_panic(expected = "the long option name --long caused conflict among multiple options")] +fn test_check_conflicts_in_long_name() { + let mut opts = Options::new(); + opts.optflag("a", "long", ""); + opts.optflag("b", "long", ""); +} + +#[cfg(debug_assertions)] +#[test] +fn test_empty_names_should_not_cause_conflict() { + let mut opts = Options::new(); + opts.optflag("", "long1", ""); + opts.optflag("", "long2", ""); + opts.optflag("a", "", ""); + opts.optflag("b", "", ""); +}