Repository: clap-rs/clap
Pull request #3965 (2022-07-22 14:06:25)

fix(error):! Merge EmptyValue into InvalidValue
There isn't a reason to programmatically differentiate them so this
merges them simplify programamtic cases and to hopefully reduce binary
size.

<!--
Thanks for helping out!

Please link the appropriate issue from your PR.

If you don't have an issue, we'd recommend starting with one first so the PR can focus on the
implementation (unless its an obvious bug or documentation fix that will have
little conversation).
-->



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 08c91d69..2df5baf8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 _gated behind `unstable-v4`_
 
+### Breaking Changes
+
+- `Error::EmptyValue` replaced with `Error::InvalidValue`
+
 ### Features
 
 - *(help)* Show `PossibleValue::help` in long help (`--help`) (#3312)
diff --git a/src/error/kind.rs b/src/error/kind.rs
index cd71e104..32639103 100644
--- a/src/error/kind.rs
+++ b/src/error/kind.rs
@@ -83,24 +83,6 @@ pub enum ErrorKind {
     /// [`UnknownArgument`]: ErrorKind::UnknownArgument
     UnrecognizedSubcommand,
 
-    /// Occurs when the user provides an empty value for an option that does not allow empty
-    /// values.
-    ///
-    /// # Examples
-    ///
-    /// ```rust
-    /// # use clap::{Command, Arg, ErrorKind};
-    /// let res = Command::new("prog")
-    ///     .arg(Arg::new("color")
-    ///          .takes_value(true)
-    ///          .value_parser(clap::builder::NonEmptyStringValueParser::new())
-    ///          .long("color"))
-    ///     .try_get_matches_from(vec!["prog", "--color="]);
-    /// assert!(res.is_err());
-    /// assert_eq!(res.unwrap_err().kind(), ErrorKind::EmptyValue);
-    /// ```
-    EmptyValue,
-
     /// Occurs when the user doesn't use equals for an option that requires equal
     /// sign to provide values.
     ///
@@ -380,7 +362,6 @@ impl ErrorKind {
             }
             Self::InvalidSubcommand => Some("A subcommand wasn't recognized"),
             Self::UnrecognizedSubcommand => Some("A subcommand wasn't recognized"),
-            Self::EmptyValue => Some("An argument requires a value but none was supplied"),
             Self::NoEquals => Some("Equal is needed when assigning values to one of the arguments"),
             Self::ValueValidation => Some("Invalid value for one of the arguments"),
             Self::TooManyValues => Some("An argument received an unexpected value"),
diff --git a/src/error/mod.rs b/src/error/mod.rs
index aa817bc5..59ca224d 100644
--- a/src/error/mod.rs
+++ b/src/error/mod.rs
@@ -244,16 +244,7 @@ impl Error {
     }
 
     pub(crate) fn empty_value(cmd: &Command, good_vals: &[&str], arg: String) -> Self {
-        let mut err = Self::new(ErrorKind::EmptyValue)
-            .with_cmd(cmd)
-            .extend_context_unchecked([(ContextKind::InvalidArg, ContextValue::String(arg))]);
-        if !good_vals.is_empty() {
-            err = err.insert_context_unchecked(
-                ContextKind::ValidValue,
-                ContextValue::Strings(good_vals.iter().map(|s| (*s).to_owned()).collect()),
-            );
-        }
-        err
+        Self::invalid_value(cmd, "".to_owned(), good_vals, arg)
     }
 
     pub(crate) fn no_equals(cmd: &Command, arg: String, usage: String) -> Self {
@@ -531,30 +522,6 @@ impl Error {
                     false
                 }
             }
-            ErrorKind::EmptyValue => {
-                let invalid_arg = self.get_context(ContextKind::InvalidArg);
-                if let Some(ContextValue::String(invalid_arg)) = invalid_arg {
-                    c.none("The argument '");
-                    c.warning(invalid_arg);
-                    c.none("' requires a value but none was supplied");
-
-                    let possible_values = self.get_context(ContextKind::ValidValue);
-                    if let Some(ContextValue::Strings(possible_values)) = possible_values {
-                        c.none("\n\t[possible values: ");
-                        if let Some((last, elements)) = possible_values.split_last() {
-                            for v in elements {
-                                c.good(escape(v));
-                                c.none(", ");
-                            }
-                            c.good(escape(last));
-                        }
-                        c.none("]");
-                    }
-                    true
-                } else {
-                    false
-                }
-            }
             ErrorKind::NoEquals => {
                 let invalid_arg = self.get_context(ContextKind::InvalidArg);
                 if let Some(ContextValue::String(invalid_arg)) = invalid_arg {
@@ -574,10 +541,16 @@ impl Error {
                     Some(ContextValue::String(invalid_value)),
                 ) = (invalid_arg, invalid_value)
                 {
-                    c.none(quote(invalid_value));
-                    c.none(" isn't a valid value for '");
-                    c.warning(invalid_arg);
-                    c.none("'");
+                    if invalid_value.is_empty() {
+                        c.none("The argument '");
+                        c.warning(invalid_arg);
+                        c.none("' requires a value but none was supplied");
+                    } else {
+                        c.none(quote(invalid_value));
+                        c.none(" isn't a valid value for '");
+                        c.warning(invalid_arg);
+                        c.none("'");
+                    }
 
                     let possible_values = self.get_context(ContextKind::ValidValue);
                     if let Some(ContextValue::Strings(possible_values)) = possible_values {


Test patch:
diff --git a/tests/builder/default_vals.rs b/tests/builder/default_vals.rs
index dbe18a1d..9709a061 100644
--- a/tests/builder/default_vals.rs
+++ b/tests/builder/default_vals.rs
@@ -48,7 +48,7 @@ fn opt_without_value_fail() {
         .try_get_matches_from(vec!["", "-o"]);
     assert!(r.is_err());
     let err = r.unwrap_err();
-    assert_eq!(err.kind(), ErrorKind::EmptyValue);
+    assert_eq!(err.kind(), ErrorKind::InvalidValue);
     assert!(err
         .to_string()
         .contains("The argument '-o <opt>' requires a value but none was supplied"));
diff --git a/tests/builder/empty_values.rs b/tests/builder/empty_values.rs
index 9313435c..652177c8 100644
--- a/tests/builder/empty_values.rs
+++ b/tests/builder/empty_values.rs
@@ -37,7 +37,7 @@ fn no_empty_values() {
         )
         .try_get_matches_from(&["config", "--config", ""]);
     assert!(m.is_err());
-    assert_eq!(m.unwrap_err().kind(), ErrorKind::EmptyValue);
+    assert_eq!(m.unwrap_err().kind(), ErrorKind::InvalidValue);
 
     let m = Command::new("config")
         .arg(
@@ -48,7 +48,7 @@ fn no_empty_values() {
         )
         .try_get_matches_from(&["config", "-c", ""]);
     assert!(m.is_err());
-    assert_eq!(m.unwrap_err().kind(), ErrorKind::EmptyValue)
+    assert_eq!(m.unwrap_err().kind(), ErrorKind::InvalidValue)
 }
 
 #[test]
@@ -62,7 +62,7 @@ fn no_empty_values_with_equals() {
         )
         .try_get_matches_from(&["config", "--config="]);
     assert!(m.is_err());
-    assert_eq!(m.unwrap_err().kind(), ErrorKind::EmptyValue);
+    assert_eq!(m.unwrap_err().kind(), ErrorKind::InvalidValue);
 
     let m = Command::new("config")
         .arg(
@@ -73,7 +73,7 @@ fn no_empty_values_with_equals() {
         )
         .try_get_matches_from(&["config", "-c="]);
     assert!(m.is_err());
-    assert_eq!(m.unwrap_err().kind(), ErrorKind::EmptyValue);
+    assert_eq!(m.unwrap_err().kind(), ErrorKind::InvalidValue);
 }
 
 #[test]
@@ -87,7 +87,7 @@ fn no_empty_values_without_equals() {
         )
         .try_get_matches_from(&["config", "--config"]);
     assert!(m.is_err());
-    assert_eq!(m.unwrap_err().kind(), ErrorKind::EmptyValue);
+    assert_eq!(m.unwrap_err().kind(), ErrorKind::InvalidValue);
 
     let m = Command::new("config")
         .arg(
@@ -98,7 +98,7 @@ fn no_empty_values_without_equals() {
         )
         .try_get_matches_from(&["config", "-c"]);
     assert!(m.is_err());
-    assert_eq!(m.unwrap_err().kind(), ErrorKind::EmptyValue)
+    assert_eq!(m.unwrap_err().kind(), ErrorKind::InvalidValue)
 }
 
 #[test]
diff --git a/tests/builder/opts.rs b/tests/builder/opts.rs
index d4209738..1dbe6c39 100644
--- a/tests/builder/opts.rs
+++ b/tests/builder/opts.rs
@@ -112,7 +112,7 @@ fn require_equals_no_empty_values_fail() {
         .arg(Arg::new("some"))
         .try_get_matches_from(vec!["prog", "--config=", "file.conf"]);
     assert!(res.is_err());
-    assert_eq!(res.unwrap_err().kind(), ErrorKind::EmptyValue);
+    assert_eq!(res.unwrap_err().kind(), ErrorKind::InvalidValue);
 }
 
 #[test]
@@ -541,7 +541,7 @@ fn issue_1105_setup(argv: Vec<&'static str>) -> Result<ArgMatches, clap::Error>
 fn issue_1105_empty_value_long_fail() {
     let r = issue_1105_setup(vec!["cmd", "--option", "--flag"]);
     assert!(r.is_err());
-    assert_eq!(r.unwrap_err().kind(), ErrorKind::EmptyValue);
+    assert_eq!(r.unwrap_err().kind(), ErrorKind::InvalidValue);
 }
 
 #[test]
@@ -564,7 +564,7 @@ fn issue_1105_empty_value_long_equals() {
 fn issue_1105_empty_value_short_fail() {
     let r = issue_1105_setup(vec!["cmd", "-o", "--flag"]);
     assert!(r.is_err());
-    assert_eq!(r.unwrap_err().kind(), ErrorKind::EmptyValue);
+    assert_eq!(r.unwrap_err().kind(), ErrorKind::InvalidValue);
 }
 
 #[test]