Merge pull request #3457 from tertsdiepraam/clap-exit-code

`clap` exit code

src/uu/base32/src/base_common.rs

@@ -90,7 +90,7 @@ pub fn parse_base_cmd_args(args: impl uucore::Args, about: &str, usage: &str) ->
     let arg_list = args
         .collect_str(InvalidEncodingHandling::ConvertLossy)
         .accept_any();
-    Config::from(&command.get_matches_from(arg_list))
+    Config::from(&command.try_get_matches_from(arg_list)?)
 }
 
 pub fn base_app<'a>(about: &'a str, usage: &'a str) -> Command<'a> {

src/uu/cat/src/cat.rs

@@ -188,7 +188,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         .collect_str(InvalidEncodingHandling::Ignore)
         .accept_any();
 
-    let matches = uu_app().get_matches_from(args);
+    let matches = uu_app().try_get_matches_from(args)?;
 
     let number_mode = if matches.is_present(options::NUMBER_NONBLANK) {
         NumberingMode::NonEmpty

src/uu/cp/src/cp.rs

@@ -57,7 +57,7 @@ use std::path::{Path, PathBuf, StripPrefixError};
 use std::str::FromStr;
 use std::string::ToString;
 use uucore::backup_control::{self, BackupMode};
-use uucore::error::{set_exit_code, ExitCode, UError, UResult};
+use uucore::error::{set_exit_code, ExitCode, UClapError, UError, UResult};
 use uucore::fs::{canonicalize, MissingHandling, ResolveMode};
 use walkdir::WalkDir;
 
@@ -485,7 +485,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
                 app.print_help()?;
             }
             clap::ErrorKind::DisplayVersion => println!("{}", app.render_version()),
-            _ => return Err(Box::new(e)),
+            _ => return Err(Box::new(e.with_exit_code(1))),
         };
     } else if let Ok(matches) = matches {
         let options = Options::from_matches(&matches)?;

src/uu/nice/src/nice.rs

@@ -17,7 +17,7 @@ use std::ptr;
 
 use clap::{crate_version, Arg, Command};
 use uucore::{
-    error::{set_exit_code, UResult, USimpleError, UUsageError},
+    error::{set_exit_code, UClapError, UResult, USimpleError, UUsageError},
     format_usage,
 };
 
@@ -35,7 +35,7 @@ const USAGE: &str = "{} [OPTIONS] [COMMAND [ARGS]]";
 
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
-    let matches = uu_app().get_matches_from(args);
+    let matches = uu_app().try_get_matches_from(args).with_exit_code(125)?;
 
     let mut niceness = unsafe {
         nix::errno::Errno::clear();

src/uucore/src/lib/mods/error.rs

@@ -617,12 +617,75 @@ impl From<i32> for Box<dyn UError> {
     }
 }
 
-/// Implementations for clap::Error
-impl UError for clap::Error {
+/// A wrapper for `clap::Error` that implements [`UError`]
+///
+/// Contains a custom error code. When `Display::fmt` is called on this struct
+/// the [`clap::Error`] will be printed _directly to `stdout` or `stderr`_.
+/// This is because `clap` only supports colored output when it prints directly.
+///
+/// [`ClapErrorWrapper`] is generally created by calling the
+/// [`UClapError::with_exit_code`] method on [`clap::Error`] or using the [`From`]
+/// implementation from [`clap::Error`] to `Box<dyn UError>`, which constructs
+/// a [`ClapErrorWrapper`] with an exit code of `1`.
+///
+/// ```rust
+/// use uucore::error::{ClapErrorWrapper, UError, UClapError};
+/// let command = clap::Command::new("test");
+/// let result: Result<_, ClapErrorWrapper> = command.try_get_matches().with_exit_code(125);
+///
+/// let command = clap::Command::new("test");
+/// let result: Result<_, Box<dyn UError>> = command.try_get_matches().map_err(Into::into);
+/// ```
+#[derive(Debug)]
+pub struct ClapErrorWrapper {
+    code: i32,
+    error: clap::Error,
+}
+
+/// Extension trait for `clap::Error` to adjust the exit code.
+pub trait UClapError<T> {
+    fn with_exit_code(self, code: i32) -> T;
+}
+
+impl From<clap::Error> for Box<dyn UError> {
+    fn from(e: clap::Error) -> Self {
+        Box::new(ClapErrorWrapper { code: 1, error: e })
+    }
+}
+
+impl UClapError<ClapErrorWrapper> for clap::Error {
+    fn with_exit_code(self, code: i32) -> ClapErrorWrapper {
+        ClapErrorWrapper { code, error: self }
+    }
+}
+
+impl UClapError<Result<clap::ArgMatches, ClapErrorWrapper>>
+    for Result<clap::ArgMatches, clap::Error>
+{
+    fn with_exit_code(self, code: i32) -> Result<clap::ArgMatches, ClapErrorWrapper> {
+        self.map_err(|e| e.with_exit_code(code))
+    }
+}
+
+impl UError for ClapErrorWrapper {
     fn code(&self) -> i32 {
-        match self.kind() {
-            clap::ErrorKind::DisplayHelp | clap::ErrorKind::DisplayVersion => 0,
-            _ => 1,
+        // If the error is a DisplayHelp or DisplayVersion variant,
+        // we don't want to apply the custom error code, but leave
+        // it 0.
+        if let clap::ErrorKind::DisplayHelp | clap::ErrorKind::DisplayVersion = self.error.kind() {
+            0
+        } else {
+            self.code
         }
     }
 }
+
+impl Error for ClapErrorWrapper {}
+
+// This is abuse of the Display trait
+impl Display for ClapErrorWrapper {
+    fn fmt(&self, _f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> {
+        self.error.print().unwrap();
+        Ok(())
+    }
+}

tests/by-util/test_nice.rs

@@ -58,3 +58,8 @@ fn test_command_where_command_takes_n_flag() {
         .run()
         .stdout_is("a");
 }
+
+#[test]
+fn test_invalid_argument() {
+    new_ucmd!().arg("--invalid").fails().code_is(125);
+}