From 7b84261df42278a23bed2239cc8cb93126503ec0 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sat, 30 Apr 2022 11:25:53 +0200 Subject: [PATCH 1/5] uucore/error: add custom exit codes for clap errors This allows us to use clap errors as UResult and specify the exit code for invalid arguments per util. --- src/uucore/src/lib/mods/error.rs | 73 +++++++++++++++++++++++++++++--- 1 file changed, 68 insertions(+), 5 deletions(-) diff --git a/src/uucore/src/lib/mods/error.rs b/src/uucore/src/lib/mods/error.rs index dbe4d5bc19d..1af6ef7815b 100644 --- a/src/uucore/src/lib/mods/error.rs +++ b/src/uucore/src/lib/mods/error.rs @@ -617,12 +617,75 @@ impl From for Box { } } -/// 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`, 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> = 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 { + fn with_exit_code(self, code: i32) -> T; +} + +impl From for Box { + fn from(e: clap::Error) -> Self { + Box::new(ClapErrorWrapper { code: 1, error: e }) + } +} + +impl UClapError for clap::Error { + fn with_exit_code(self, code: i32) -> ClapErrorWrapper { + ClapErrorWrapper { code, error: self } + } +} + +impl UClapError> + for Result +{ + fn with_exit_code(self, code: i32) -> Result { + 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(()) + } +} From 8df253da69a9826056a8f556e772a549a8900c3f Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sat, 30 Apr 2022 11:28:22 +0200 Subject: [PATCH 2/5] cat: set exit code for invalid arguments to 1 instead of 2 --- src/uu/cat/src/cat.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index edba1b8d070..67de917f7b5 100644 --- a/src/uu/cat/src/cat.rs +++ b/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 From 1bb85acc71b8276c910d850ea79d98f5009c0691 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sat, 30 Apr 2022 22:45:17 +0200 Subject: [PATCH 3/5] nice: set exit code for clap errors to 125 --- src/uu/nice/src/nice.rs | 4 ++-- tests/by-util/test_nice.rs | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/uu/nice/src/nice.rs b/src/uu/nice/src/nice.rs index b75dd979e18..e78de828bc9 100644 --- a/src/uu/nice/src/nice.rs +++ b/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(); diff --git a/tests/by-util/test_nice.rs b/tests/by-util/test_nice.rs index 2b53ed4377e..036723cdefa 100644 --- a/tests/by-util/test_nice.rs +++ b/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); +} From 24097262589b7cdb48ab166407b1a35e11618337 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sun, 1 May 2022 20:51:25 +0200 Subject: [PATCH 4/5] base: set exit code to 1 for clap errors --- src/uu/base32/src/base_common.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/base32/src/base_common.rs b/src/uu/base32/src/base_common.rs index 100c85b1f13..148bad2f833 100644 --- a/src/uu/base32/src/base_common.rs +++ b/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> { From c7b7f19559c84feafa9959495d4f1fb0e44d9935 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Wed, 4 May 2022 19:10:28 +0200 Subject: [PATCH 5/5] cp: use new clap error mechanism --- src/uu/cp/src/cp.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index ce85b58bc4e..1b47b782879 100644 --- a/src/uu/cp/src/cp.rs +++ b/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)?;