Merge branch 'master' into sort/input-is-output

src/uu/ln/src/ln.rs

@@ -11,9 +11,12 @@
 extern crate uucore;
 
 use clap::{crate_version, App, Arg};
+use uucore::error::{UCustomError, UResult};
 
 use std::borrow::Cow;
+use std::error::Error;
 use std::ffi::OsStr;
+use std::fmt::Display;
 use std::fs;
 
 use std::io::{stdin, Result};
@@ -44,6 +47,51 @@ pub enum OverwriteMode {
     Force,
 }
 
+#[derive(Debug)]
+enum LnError {
+    TargetIsDirectory(String),
+    SomeLinksFailed,
+    FailedToLink(String),
+    MissingDestination(String),
+    ExtraOperand(String),
+    InvalidBackupMode(String),
+}
+
+impl Display for LnError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::TargetIsDirectory(s) => write!(f, "target '{}' is not a directory", s),
+            Self::FailedToLink(s) => write!(f, "failed to link '{}'", s),
+            Self::SomeLinksFailed => write!(f, "some links failed to create"),
+            Self::MissingDestination(s) => {
+                write!(f, "missing destination file operand after '{}'", s)
+            }
+            Self::ExtraOperand(s) => write!(
+                f,
+                "extra operand '{}'\nTry '{} --help' for more information.",
+                s,
+                executable!()
+            ),
+            Self::InvalidBackupMode(s) => write!(f, "{}", s),
+        }
+    }
+}
+
+impl Error for LnError {}
+
+impl UCustomError for LnError {
+    fn code(&self) -> i32 {
+        match self {
+            Self::TargetIsDirectory(_) => 1,
+            Self::SomeLinksFailed => 1,
+            Self::FailedToLink(_) => 1,
+            Self::MissingDestination(_) => 1,
+            Self::ExtraOperand(_) => 1,
+            Self::InvalidBackupMode(_) => 1,
+        }
+    }
+}
+
 fn get_usage() -> String {
     format!(
         "{0} [OPTION]... [-T] TARGET LINK_NAME   (1st form)
@@ -86,7 +134,8 @@ mod options {
 
 static ARG_FILES: &str = "files";
 
-pub fn uumain(args: impl uucore::Args) -> i32 {
+#[uucore_procs::gen_uumain]
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let usage = get_usage();
     let long_usage = get_long_usage();
 
@@ -122,8 +171,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     );
     let backup_mode = match backup_mode {
         Err(err) => {
-            show_usage_error!("{}", err);
-            return 1;
+            return Err(LnError::InvalidBackupMode(err).into());
         }
         Ok(mode) => mode,
     };
@@ -246,7 +294,7 @@ pub fn uu_app() -> App<'static, 'static> {
         )
 }
 
-fn exec(files: &[PathBuf], settings: &Settings) -> i32 {
+fn exec(files: &[PathBuf], settings: &Settings) -> UResult<()> {
     // Handle cases where we create links in a directory first.
     if let Some(ref name) = settings.target_dir {
         // 4th form: a directory is specified by -t.
@@ -267,35 +315,22 @@ fn exec(files: &[PathBuf], settings: &Settings) -> i32 {
     // 1st form. Now there should be only two operands, but if -T is
     // specified we may have a wrong number of operands.
     if files.len() == 1 {
-        show_error!(
-            "missing destination file operand after '{}'",
-            files[0].to_string_lossy()
-        );
-        return 1;
+        return Err(LnError::MissingDestination(files[0].to_string_lossy().into()).into());
     }
     if files.len() > 2 {
-        show_error!(
-            "extra operand '{}'\nTry '{} --help' for more information.",
-            files[2].display(),
-            executable!()
-        );
-        return 1;
+        return Err(LnError::ExtraOperand(files[2].display().to_string()).into());
     }
     assert!(!files.is_empty());
 
     match link(&files[0], &files[1], settings) {
-        Ok(_) => 0,
-        Err(e) => {
-            show_error!("{}", e);
-            1
-        }
+        Ok(_) => Ok(()),
+        Err(e) => Err(LnError::FailedToLink(e.to_string()).into()),
     }
 }
 
-fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) -> i32 {
+fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) -> UResult<()> {
     if !target_dir.is_dir() {
-        show_error!("target '{}' is not a directory", target_dir.display());
-        return 1;
+        return Err(LnError::TargetIsDirectory(target_dir.display().to_string()).into());
     }
 
     let mut all_successful = true;
@@ -354,9 +389,9 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings)
         }
     }
     if all_successful {
-        0
+        Ok(())
     } else {
-        1
+        Err(LnError::SomeLinksFailed.into())
     }
 }
 

src/uu/sort/src/sort.rs

@@ -969,9 +969,23 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     let usage = get_usage();
     let mut settings: GlobalSettings = Default::default();
 
-    let matches = uu_app().usage(&usage[..]).get_matches_from_safe(args);
-
-    let matches = crash_if_err!(2, matches);
+    let matches = match uu_app().usage(&usage[..]).get_matches_from_safe(args) {
+        Ok(t) => t,
+        Err(e) => {
+            // not all clap "Errors" are because of a failure to parse arguments.
+            // "--version" also causes an Error to be returned, but we should not print to stderr
+            // nor return with a non-zero exit code in this case (we should print to stdout and return 0).
+            // This logic is similar to the code in clap, but we return 2 as the exit code in case of real failure
+            // (clap returns 1).
+            if e.use_stderr() {
+                eprintln!("{}", e.message);
+                return 2;
+            } else {
+                println!("{}", e.message);
+                return 0;
+            }
+        }
+    };
 
     settings.debug = matches.is_present(options::DEBUG);
 

tests/by-util/test_sort.rs

@@ -1048,3 +1048,20 @@ fn test_merge_empty_input() {
         .no_stderr()
         .no_stdout();
 }
+
+#[test]
+fn test_no_error_for_version() {
+    new_ucmd!()
+        .arg("--version")
+        .succeeds()
+        .stdout_contains("sort");
+}
+
+#[test]
+fn test_wrong_args_exit_code() {
+    new_ucmd!()
+        .arg("--misspelled")
+        .fails()
+        .status_code(2)
+        .stderr_contains("--misspelled");
+}