sum: fix crash on invalid inputs, move to clap, add tests (#1952)

Cargo.lock

@@ -2250,9 +2250,9 @@ dependencies = [
 name = "uu_sum"
 version = "0.0.4"
 dependencies = [
- "getopts",
+ "clap 2.33.3 (registry+https://github.com/rust-lang/crates.io-index)",
- "uucore",
+ "uucore 0.0.7",
- "uucore_procs",
+ "uucore_procs 0.0.5",
 ]
 
 [[package]]

src/uu/sum/Cargo.toml

@@ -15,7 +15,7 @@ edition = "2018"
 path = "src/sum.rs"
 
 [dependencies]
-getopts = "0.2.18"
+clap = "2.33"
 uucore = { version=">=0.0.7", package="uucore", path="../../uucore" }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
 

src/uu/sum/src/sum.rs

@@ -10,12 +10,16 @@
 #[macro_use]
 extern crate uucore;
 
+use clap::{App, Arg};
 use std::fs::File;
 use std::io::{stdin, Read, Result};
 use std::path::Path;
 
 static NAME: &str = "sum";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
+static USAGE: &str =
+    "[OPTION]... [FILE]...\nWith no FILE, or when  FILE is -, read standard input.";
+static SUMMARY: &str = "Checksum and count the blocks in a file.";
 
 fn bsd_sum(mut reader: Box<dyn Read>) -> (usize, u16) {
     let mut buf = [0; 1024];
@@ -64,55 +68,59 @@ fn open(name: &str) -> Result<Box<dyn Read>> {
     match name {
         "-" => Ok(Box::new(stdin()) as Box<dyn Read>),
         _ => {
-            let f = File::open(&Path::new(name))?;
+            let path = &Path::new(name);
+            if path.is_dir() {
+                return Err(std::io::Error::new(
+                    std::io::ErrorKind::InvalidInput,
+                    "Is a directory",
+                ));
+            };
+            if !path.metadata().is_ok() {
+                return Err(std::io::Error::new(
+                    std::io::ErrorKind::NotFound,
+                    "No such file or directory",
+                ));
+            };
+            let f = File::open(path)?;
             Ok(Box::new(f) as Box<dyn Read>)
         }
     }
 }
 
+mod options {
+    pub static FILE: &str = "file";
+    pub static BSD_COMPATIBLE: &str = "r";
+    pub static SYSTEM_V_COMPATIBLE: &str = "sysv";
+}
+
 pub fn uumain(args: impl uucore::Args) -> i32 {
     let args = args.collect_str();
 
-    let mut opts = getopts::Options::new();
+    let matches = App::new(executable!())
+        .name(NAME)
+        .version(VERSION)
+        .usage(USAGE)
+        .about(SUMMARY)
+        .arg(Arg::with_name(options::FILE).multiple(true).hidden(true))
+        .arg(
+            Arg::with_name(options::BSD_COMPATIBLE)
+                .short(options::BSD_COMPATIBLE)
+                .help("use the BSD compatible algorithm (default)"),
+        )
+        .arg(
+            Arg::with_name(options::SYSTEM_V_COMPATIBLE)
+                .short("s")
+                .long(options::SYSTEM_V_COMPATIBLE)
+                .help("use the BSD compatible algorithm (default)"),
+        )
+        .get_matches_from(args);
 
-    opts.optflag("r", "", "use the BSD compatible algorithm (default)");
+    let files: Vec<String> = match matches.values_of(options::FILE) {
-    opts.optflag("s", "sysv", "use System V compatible algorithm");
+        Some(v) => v.clone().map(|v| v.to_owned()).collect(),
-    opts.optflag("h", "help", "show this help message");
+        None => vec!["-".to_owned()],
-    opts.optflag("v", "version", "print the version and exit");
-
-    let matches = match opts.parse(&args[1..]) {
-        Ok(m) => m,
-        Err(f) => crash!(1, "Invalid options\n{}", f),
     };
 
-    if matches.opt_present("help") {
+    let sysv = matches.is_present(options::SYSTEM_V_COMPATIBLE);
-        let msg = format!(
-            "{0} {1}
-
-Usage:
-  {0} [OPTION]... [FILE]...
-
-Checksum and count the blocks in a file.",
-            NAME, VERSION
-        );
-        println!(
-            "{}\nWith no FILE, or when  FILE is -, read standard input.",
-            opts.usage(&msg)
-        );
-        return 0;
-    }
-    if matches.opt_present("version") {
-        println!("{} {}", NAME, VERSION);
-        return 0;
-    }
-
-    let sysv = matches.opt_present("sysv");
-
-    let files = if matches.free.is_empty() {
-        vec!["-".to_owned()]
-    } else {
-        matches.free
-    };
 
     let print_names = if sysv {
         files.len() > 1 || files[0] != "-"
@@ -120,10 +128,15 @@ Checksum and count the blocks in a file.",
         files.len() > 1
     };
 
+    let mut exit_code = 0;
     for file in &files {
         let reader = match open(file) {
             Ok(f) => f,
-            _ => crash!(1, "unable to open file"),
+            Err(error) => {
+                show_error!("'{}' {}", file, error);
+                exit_code = 2;
+                continue;
+            }
         };
         let (blocks, sum) = if sysv {
             sysv_sum(reader)
@@ -138,5 +151,5 @@ Checksum and count the blocks in a file.",
         }
     }
 
-    0
+    exit_code
 }

tests/by-util/test_sum.rs

@@ -52,3 +52,23 @@ fn test_sysv_stdin() {
         .succeeds()
         .stdout_only_fixture("sysv_stdin.expected");
 }
+
+#[test]
+fn test_invalid_file() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.mkdir("a");
+
+    ucmd.arg("a")
+        .fails()
+        .stderr_is("sum: error: 'a' Is a directory");
+}
+
+#[test]
+fn test_invalid_metadata() {
+    let (_, mut ucmd) = at_and_ucmd!();
+
+    ucmd.arg("b")
+        .fails()
+        .stderr_is("sum: error: 'b' No such file or directory");
+}