shuf: move from getopts to clap (#1950)

src/uu/shuf/Cargo.toml

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

src/uu/shuf/src/shuf.rs

@@ -10,132 +10,162 @@
 #[macro_use]
 extern crate uucore;
 
+use clap::{App, Arg};
 use rand::Rng;
 use std::fs::File;
 use std::io::{stdin, stdout, BufReader, BufWriter, Read, Write};
-use std::usize::MAX as MAX_USIZE;
-
-enum Mode {
-    Default,
-    Echo,
-    InputRange((usize, usize)),
-}
 
 static NAME: &str = "shuf";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
+static USAGE: &str = r#"shuf [OPTION]... [FILE]
+  or:  shuf -e [OPTION]... [ARG]...
+  or:  shuf -i LO-HI [OPTION]...
+Write a random permutation of the input lines to standard output.
+
+With no FILE, or when FILE is -, read standard input.
+"#;
+static TEMPLATE: &str = "Usage: {usage}\nMandatory arguments to long options are mandatory for short options too.\n{unified}";
+
+struct Options {
+    head_count: usize,
+    output: Option<String>,
+    random_source: Option<String>,
+    repeat: bool,
+    sep: u8,
+}
+
+enum Mode {
+    Default(String),
+    Echo(Vec<String>),
+    InputRange((usize, usize)),
+}
+
+mod options {
+    pub static ECHO: &str = "echo";
+    pub static INPUT_RANGE: &str = "input-range";
+    pub static HEAD_COUNT: &str = "head-count";
+    pub static OUTPUT: &str = "output";
+    pub static RANDOM_SOURCE: &str = "random-source";
+    pub static REPEAT: &str = "repeat";
+    pub static ZERO_TERMINATED: &str = "zero-terminated";
+    pub static FILE: &str = "file";
+}
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
-    let args = args.collect_str();
+    let matches = App::new(executable!())
+        .name(NAME)
+        .version(VERSION)
+        .template(TEMPLATE)
+        .usage(USAGE)
+        .arg(
+            Arg::with_name(options::ECHO)
+                .short("e")
+                .long(options::ECHO)
+                .takes_value(true)
+                .value_name("ARG")
+                .help("treat each ARG as an input line")
+                .multiple(true)
+                .use_delimiter(false)
+                .min_values(0)
+                .conflicts_with(options::INPUT_RANGE),
+        )
+        .arg(
+            Arg::with_name(options::INPUT_RANGE)
+                .short("i")
+                .long(options::INPUT_RANGE)
+                .takes_value(true)
+                .value_name("LO-HI")
+                .help("treat each number LO through HI as an input line")
+                .conflicts_with(options::FILE),
+        )
+        .arg(
+            Arg::with_name(options::HEAD_COUNT)
+                .short("n")
+                .long(options::HEAD_COUNT)
+                .takes_value(true)
+                .value_name("COUNT")
+                .help("output at most COUNT lines"),
+        )
+        .arg(
+            Arg::with_name(options::OUTPUT)
+                .short("o")
+                .long(options::OUTPUT)
+                .takes_value(true)
+                .value_name("FILE")
+                .help("write result to FILE instead of standard output"),
+        )
+        .arg(
+            Arg::with_name(options::RANDOM_SOURCE)
+                .long(options::RANDOM_SOURCE)
+                .takes_value(true)
+                .value_name("FILE")
+                .help("get random bytes from FILE"),
+        )
+        .arg(
+            Arg::with_name(options::REPEAT)
+                .short("r")
+                .long(options::REPEAT)
+                .help("output lines can be repeated"),
+        )
+        .arg(
+            Arg::with_name(options::ZERO_TERMINATED)
+                .short("z")
+                .long(options::ZERO_TERMINATED)
+                .help("line delimiter is NUL, not newline"),
+        )
+        .arg(Arg::with_name(options::FILE).takes_value(true))
+        .get_matches_from(args);
 
-    let mut opts = getopts::Options::new();
+    let mode = if let Some(args) = matches.values_of(options::ECHO) {
-    opts.optflag("e", "echo", "treat each ARG as an input line");
+        Mode::Echo(args.map(String::from).collect())
-    opts.optopt(
+    } else if let Some(range) = matches.value_of(options::INPUT_RANGE) {
-        "i",
-        "input-range",
-        "treat each number LO through HI as an input line",
-        "LO-HI",
-    );
-    opts.optopt("n", "head-count", "output at most COUNT lines", "COUNT");
-    opts.optopt(
-        "o",
-        "output",
-        "write result to FILE instead of standard output",
-        "FILE",
-    );
-    opts.optopt("", "random-source", "get random bytes from FILE", "FILE");
-    opts.optflag("r", "repeat", "output lines can be repeated");
-    opts.optflag("z", "zero-terminated", "end lines with 0 byte, not newline");
-    opts.optflag("h", "help", "display this help and exit");
-    opts.optflag("V", "version", "output version information and exit");
-    let mut matches = match opts.parse(&args[1..]) {
-        Ok(m) => m,
-        Err(f) => crash!(1, "{}", f),
-    };
-    if matches.opt_present("help") {
-        let msg = format!(
-            "{0} {1}
-
-Usage:
-  {0} [OPTION]... [FILE]
-  {0} -e [OPTION]... [ARG]...
-  {0} -i LO-HI [OPTION]...
-
-Write a random permutation of the input lines to standard output.
-With no FILE, or when FILE is -, read standard input.",
-            NAME, VERSION
-        );
-        print!("{}", opts.usage(&msg));
-    } else if matches.opt_present("version") {
-        println!("{} {}", NAME, VERSION);
-    } else {
-        let echo = matches.opt_present("echo");
-        let mode = match matches.opt_str("input-range") {
-            Some(range) => {
-                if echo {
-                    show_error!("cannot specify more than one mode");
-                    return 1;
-                }
         match parse_range(range) {
             Ok(m) => Mode::InputRange(m),
             Err(msg) => {
                 crash!(1, "{}", msg);
             }
         }
-            }
-            None => {
-                if echo {
-                    Mode::Echo
     } else {
-                    if matches.free.is_empty() {
+        Mode::Default(matches.value_of(options::FILE).unwrap_or("-").to_string())
-                        matches.free.push("-".to_owned());
-                    } else if matches.free.len() > 1 {
-                        show_error!("extra operand '{}'", &matches.free[1][..]);
-                    }
-                    Mode::Default
-                }
-            }
     };
-        let repeat = matches.opt_present("repeat");
+
-        let sep = if matches.opt_present("zero-terminated") {
+    let options = Options {
-            0x00_u8
+        head_count: match matches.value_of(options::HEAD_COUNT) {
-        } else {
+            Some(count) => match count.parse::<usize>() {
-            0x0a_u8
-        };
-        let count = match matches.opt_str("head-count") {
-            Some(cnt) => match cnt.parse::<usize>() {
                 Ok(val) => val,
-                Err(e) => {
+                Err(_) => {
-                    show_error!("'{}' is not a valid count: {}", cnt, e);
+                    show_error!("invalid line count: '{}'", count);
                     return 1;
                 }
             },
-            None => MAX_USIZE,
+            None => std::usize::MAX,
+        },
+        output: matches.value_of(options::OUTPUT).map(String::from),
+        random_source: matches.value_of(options::RANDOM_SOURCE).map(String::from),
+        repeat: matches.is_present(options::REPEAT),
+        sep: if matches.is_present(options::ZERO_TERMINATED) {
+            0x00_u8
+        } else {
+            0x0a_u8
+        },
     };
-        let output = matches.opt_str("output");
-        let random = matches.opt_str("random-source");
 
     match mode {
-            Mode::Echo => {
+        Mode::Echo(args) => {
-                // XXX: this doesn't correctly handle non-UTF-8 cmdline args
+            let mut evec = args.iter().map(String::as_bytes).collect::<Vec<_>>();
-                let mut evec = matches
+            find_seps(&mut evec, options.sep);
-                    .free
+            shuf_bytes(&mut evec, options);
-                    .iter()
-                    .map(String::as_bytes)
-                    .collect::<Vec<&[u8]>>();
-                find_seps(&mut evec, sep);
-                shuf_bytes(&mut evec, repeat, count, sep, output, random);
         }
         Mode::InputRange((b, e)) => {
             let rvec = (b..e).map(|x| format!("{}", x)).collect::<Vec<String>>();
             let mut rvec = rvec.iter().map(String::as_bytes).collect::<Vec<&[u8]>>();
-                shuf_bytes(&mut rvec, repeat, count, sep, output, random);
+            shuf_bytes(&mut rvec, options);
         }
-            Mode::Default => {
+        Mode::Default(filename) => {
-                let fdata = read_input_file(&matches.free[0][..]);
+            let fdata = read_input_file(&filename);
             let mut fdata = vec![&fdata[..]];
-                find_seps(&mut fdata, sep);
+            find_seps(&mut fdata, options.sep);
-                shuf_bytes(&mut fdata, repeat, count, sep, output, random);
+            shuf_bytes(&mut fdata, options);
-            }
         }
     }
 
@@ -193,15 +223,8 @@ fn find_seps(data: &mut Vec<&[u8]>, sep: u8) {
     }
 }
 
-fn shuf_bytes(
+fn shuf_bytes(input: &mut Vec<&[u8]>, opts: Options) {
-    input: &mut Vec<&[u8]>,
+    let mut output = BufWriter::new(match opts.output {
-    repeat: bool,
-    count: usize,
-    sep: u8,
-    output: Option<String>,
-    random: Option<String>,
-) {
-    let mut output = BufWriter::new(match output {
         None => Box::new(stdout()) as Box<dyn Write>,
         Some(s) => match File::create(&s[..]) {
             Ok(f) => Box::new(f) as Box<dyn Write>,
@@ -209,7 +232,7 @@ fn shuf_bytes(
         },
     });
 
-    let mut rng = match random {
+    let mut rng = match opts.random_source {
         Some(r) => WrappedRng::RngFile(rand::read::ReadRng::new(match File::open(&r[..]) {
             Ok(f) => f,
             Err(e) => crash!(1, "failed to open random source '{}': {}", &r[..], e),
@@ -225,7 +248,7 @@ fn shuf_bytes(
         len_mod <<= 1;
     }
 
-    let mut count = count;
+    let mut count = opts.head_count;
     while count > 0 && !input.is_empty() {
         let mut r = input.len();
         while r >= input.len() {
@@ -237,11 +260,11 @@ fn shuf_bytes(
             .write_all(input[r])
             .unwrap_or_else(|e| crash!(1, "write failed: {}", e));
         output
-            .write_all(&[sep])
+            .write_all(&[opts.sep])
             .unwrap_or_else(|e| crash!(1, "write failed: {}", e));
 
         // if we do not allow repeats, remove the chosen value from the input vector
-        if !repeat {
+        if !opts.repeat {
             // shrink the mask if we will drop below a power of 2
             if input.len() % 2 == 0 && len_mod > 2 {
                 len_mod >>= 1;
@@ -253,18 +276,18 @@ fn shuf_bytes(
     }
 }
 
-fn parse_range(input_range: String) -> Result<(usize, usize), String> {
+fn parse_range(input_range: &str) -> Result<(usize, usize), String> {
     let split: Vec<&str> = input_range.split('-').collect();
     if split.len() != 2 {
-        Err("invalid range format".to_owned())
+        Err(format!("invalid input range: '{}'", input_range))
     } else {
         let begin = match split[0].parse::<usize>() {
             Ok(m) => m,
-            Err(e) => return Err(format!("{} is not a valid number: {}", split[0], e)),
+            Err(_) => return Err(format!("invalid input range: '{}'", split[0])),
         };
         let end = match split[1].parse::<usize>() {
             Ok(m) => m,
-            Err(e) => return Err(format!("{} is not a valid number: {}", split[1], e)),
+            Err(_) => return Err(format!("invalid input range: '{}'", split[1])),
         };
         Ok((begin, end + 1))
     }

tests/by-util/test_shuf.rs

@@ -1 +1,55 @@
-// ToDO: add tests
+// ToDO: add more tests
+
+use crate::common::util::*;
+
+#[test]
+fn test_shuf_echo_and_input_range_not_allowed() {
+    let result = new_ucmd!().args(&["-e", "0", "-i", "0-2"]).run();
+
+    assert!(!result.success);
+    assert!(result
+        .stderr
+        .contains("The argument '--input-range <LO-HI>' cannot be used with '--echo <ARG>...'"));
+}
+
+#[test]
+fn test_shuf_input_range_and_file_not_allowed() {
+    let result = new_ucmd!().args(&["-i", "0-9", "file"]).run();
+
+    assert!(!result.success);
+    assert!(result
+        .stderr
+        .contains("The argument '<file>' cannot be used with '--input-range <LO-HI>'"));
+}
+
+#[test]
+fn test_shuf_invalid_input_range_one() {
+    let result = new_ucmd!().args(&["-i", "0"]).run();
+
+    assert!(!result.success);
+    assert!(result.stderr.contains("invalid input range"));
+}
+
+#[test]
+fn test_shuf_invalid_input_range_two() {
+    let result = new_ucmd!().args(&["-i", "a-9"]).run();
+
+    assert!(!result.success);
+    assert!(result.stderr.contains("invalid input range: 'a'"));
+}
+
+#[test]
+fn test_shuf_invalid_input_range_three() {
+    let result = new_ucmd!().args(&["-i", "0-b"]).run();
+
+    assert!(!result.success);
+    assert!(result.stderr.contains("invalid input range: 'b'"));
+}
+
+#[test]
+fn test_shuf_invalid_input_line_count() {
+    let result = new_ucmd!().args(&["-n", "a"]).run();
+
+    assert!(!result.success);
+    assert!(result.stderr.contains("invalid line count: 'a'"));
+}