From 0f3bc237393adef485182656b0edc986575c592d Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Thu, 29 Apr 2021 23:13:20 -0400 Subject: [PATCH 01/17] tr: implement translate and squeeze (-s) mode Add translate and squeeze mode to the `tr` program. For example: $ printf xx | tr -s x y y Fixes #2141. --- src/uu/tr/src/tr.rs | 54 ++++++++++++++++++++++++++++++++++++++-- tests/by-util/test_tr.rs | 18 ++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/uu/tr/src/tr.rs b/src/uu/tr/src/tr.rs index 6c1c0746a..09c4304a5 100644 --- a/src/uu/tr/src/tr.rs +++ b/src/uu/tr/src/tr.rs @@ -278,8 +278,58 @@ pub fn uumain(args: impl uucore::Args) -> i32 { translate_input(&mut locked_stdin, &mut buffered_stdout, op); } } else if squeeze_flag { - let op = SqueezeOperation::new(set1, complement_flag); - translate_input(&mut locked_stdin, &mut buffered_stdout, op); + if sets.len() < 2 { + let op = SqueezeOperation::new(set1, complement_flag); + translate_input(&mut locked_stdin, &mut buffered_stdout, op); + } else { + // Define a closure that computes the translation using a hash map. + // + // The `unwrap()` should never panic because the + // `TranslateOperation.translate()` method always returns + // `Some`. + let mut set2 = ExpandSet::new(sets[1].as_ref()); + let translator = TranslateOperation::new(set1, &mut set2, truncate_flag); + let translate = |c| translator.translate(c, 0 as char).unwrap(); + + // Prepare some variables to be used for the closure that + // computes the squeeze operation. + // + // The `squeeze()` closure needs to be defined anew for + // each line of input, but these variables do not change + // while reading the input so they can be defined before + // the `while` loop. + let set2 = ExpandSet::new(sets[1].as_ref()); + let squeezer = SqueezeOperation::new(set2, complement_flag); + + // Prepare some memory to read each line of the input (`buf`) and to write + let mut buf = String::with_capacity(BUFFER_LEN + 4); + + // Loop over each line of stdin. + while let Ok(length) = locked_stdin.read_line(&mut buf) { + if length == 0 { + break; + } + + // Define a closure that computes the squeeze operation. + // + // We keep track of the previously seen character on + // each call to `squeeze()`, but we need to reset the + // `prev_c` variable at the beginning of each line of + // the input. That's why we define the closure inside + // the `while` loop. + let mut prev_c = 0 as char; + let squeeze = |c| { + let result = squeezer.translate(c, prev_c); + prev_c = c; + result + }; + + // First translate, then squeeze each character of the input line. + let filtered: String = buf.chars().map(translate).filter_map(squeeze).collect(); + buf.clear(); + buffered_stdout.write_all(filtered.as_bytes()).unwrap(); + } + } } else { let mut set2 = ExpandSet::new(sets[1].as_ref()); let op = TranslateOperation::new(set1, &mut set2, truncate_flag); diff --git a/tests/by-util/test_tr.rs b/tests/by-util/test_tr.rs index 630c305c6..5d044a187 100644 --- a/tests/by-util/test_tr.rs +++ b/tests/by-util/test_tr.rs @@ -63,6 +63,24 @@ fn test_squeeze_complement() { .stdout_is("aaBcDcc"); } +#[test] +fn test_translate_and_squeeze() { + new_ucmd!() + .args(&["-s", "x", "y"]) + .pipe_in("xx") + .run() + .stdout_is("y"); +} + +#[test] +fn test_translate_and_squeeze_multiple_lines() { + new_ucmd!() + .args(&["-s", "x", "y"]) + .pipe_in("xxaax\nxaaxx") + .run() + .stdout_is("yaay\nyaay"); +} + #[test] fn test_delete_and_squeeze() { new_ucmd!() From 11f387dc93f4d0f218870cc63ad1f1d5d4a80532 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sat, 1 May 2021 17:29:00 +0200 Subject: [PATCH 02/17] ls: fix style test --- tests/by-util/test_ls.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 2b8f311d1..79e26f200 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -584,7 +584,6 @@ fn test_ls_order_birthtime() { } #[test] -#[ignore] fn test_ls_styles() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; @@ -598,7 +597,7 @@ fn test_ls_styles() { Regex::new(r"[a-z-]* \d* \w* \w* \d* \d{4}-\d{2}-\d{2} \d{2}:\d{2} test\n").unwrap(); let re_iso = Regex::new(r"[a-z-]* \d* \w* \w* \d* \d{2}-\d{2} \d{2}:\d{2} test\n").unwrap(); let re_locale = - Regex::new(r"[a-z-]* \d* \w* \w* \d* [A-Z][a-z]{2} \d{2} \d{2}:\d{2} test\n").unwrap(); + Regex::new(r"[a-z-]* \d* \w* \w* \d* [A-Z][a-z]{2} ( |\d)\d \d{2}:\d{2} test\n").unwrap(); //full-iso let result = scene From 5674d093275ea9ddc3be9ffbb3627eb5d6cae3a1 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 1 May 2021 13:01:55 -0400 Subject: [PATCH 03/17] fixup! tr: implement translate and squeeze (-s) mode --- src/uu/tr/src/tr.rs | 89 +++++++++++++++++++++------------------------ 1 file changed, 42 insertions(+), 47 deletions(-) diff --git a/src/uu/tr/src/tr.rs b/src/uu/tr/src/tr.rs index 09c4304a5..e04737a45 100644 --- a/src/uu/tr/src/tr.rs +++ b/src/uu/tr/src/tr.rs @@ -151,6 +151,35 @@ impl SymbolTranslator for TranslateOperation { } } +struct TranslateAndSqueezeOperation { + translate: TranslateOperation, + squeeze: SqueezeOperation, +} + +impl TranslateAndSqueezeOperation { + fn new( + set1: ExpandSet, + set2: &mut ExpandSet, + set2_: ExpandSet, + truncate: bool, + complement: bool, + ) -> TranslateAndSqueezeOperation { + TranslateAndSqueezeOperation { + translate: TranslateOperation::new(set1, set2, truncate), + squeeze: SqueezeOperation::new(set2_, complement), + } + } +} + +impl SymbolTranslator for TranslateAndSqueezeOperation { + fn translate(&self, c: char, prev_c: char) -> Option { + // `unwrap()` will never panic because `Translate.translate()` + // always returns `Some`. + self.squeeze + .translate(self.translate.translate(c, 0 as char).unwrap(), prev_c) + } +} + fn translate_input( input: &mut dyn BufRead, output: &mut dyn Write, @@ -168,8 +197,11 @@ fn translate_input( // isolation to make borrow checker happy let filtered = buf.chars().filter_map(|c| { let res = translator.translate(c, prev_c); + // Set `prev_c` to the post-translate character. This + // allows the squeeze operation to correctly function + // after the translate operation. if res.is_some() { - prev_c = c; + prev_c = res.unwrap(); } res }); @@ -282,53 +314,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let op = SqueezeOperation::new(set1, complement_flag); translate_input(&mut locked_stdin, &mut buffered_stdout, op); } else { - // Define a closure that computes the translation using a hash map. - // - // The `unwrap()` should never panic because the - // `TranslateOperation.translate()` method always returns - // `Some`. let mut set2 = ExpandSet::new(sets[1].as_ref()); - let translator = TranslateOperation::new(set1, &mut set2, truncate_flag); - let translate = |c| translator.translate(c, 0 as char).unwrap(); - - // Prepare some variables to be used for the closure that - // computes the squeeze operation. - // - // The `squeeze()` closure needs to be defined anew for - // each line of input, but these variables do not change - // while reading the input so they can be defined before - // the `while` loop. - let set2 = ExpandSet::new(sets[1].as_ref()); - let squeezer = SqueezeOperation::new(set2, complement_flag); - - // Prepare some memory to read each line of the input (`buf`) and to write - let mut buf = String::with_capacity(BUFFER_LEN + 4); - - // Loop over each line of stdin. - while let Ok(length) = locked_stdin.read_line(&mut buf) { - if length == 0 { - break; - } - - // Define a closure that computes the squeeze operation. - // - // We keep track of the previously seen character on - // each call to `squeeze()`, but we need to reset the - // `prev_c` variable at the beginning of each line of - // the input. That's why we define the closure inside - // the `while` loop. - let mut prev_c = 0 as char; - let squeeze = |c| { - let result = squeezer.translate(c, prev_c); - prev_c = c; - result - }; - - // First translate, then squeeze each character of the input line. - let filtered: String = buf.chars().map(translate).filter_map(squeeze).collect(); - buf.clear(); - buffered_stdout.write_all(filtered.as_bytes()).unwrap(); - } + let set2_ = ExpandSet::new(sets[1].as_ref()); + let op = TranslateAndSqueezeOperation::new( + set1, + &mut set2, + set2_, + complement_flag, + truncate_flag, + ); + translate_input(&mut locked_stdin, &mut buffered_stdout, op); } } else { let mut set2 = ExpandSet::new(sets[1].as_ref()); From 05b20c32a996e56c9ced87e520e7a23cd19358af Mon Sep 17 00:00:00 2001 From: Ricardo Iglesias Date: Wed, 28 Apr 2021 00:36:27 -0700 Subject: [PATCH 04/17] base64: Moved argument parsing to clap. Moved argument parsing to clap and added tests to cover using "-" as stdin, passing in too many file arguments, and updated the "wrap" error message in the tests. --- Cargo.lock | 1 + src/uu/base64/Cargo.toml | 1 + src/uu/base64/src/base64.rs | 139 +++++++++++++++++++++++-- src/uu/base64/src/base_common.rs | 61 +---------- tests/by-util/test_base64.rs | 40 ++++++- tests/fixtures/base64/input-simple.txt | 1 + 6 files changed, 171 insertions(+), 72 deletions(-) create mode 100644 tests/fixtures/base64/input-simple.txt diff --git a/Cargo.lock b/Cargo.lock index c988d6d12..254ae8fda 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1688,6 +1688,7 @@ dependencies = [ name = "uu_base64" version = "0.0.6" dependencies = [ + "clap", "uucore", "uucore_procs", ] diff --git a/src/uu/base64/Cargo.toml b/src/uu/base64/Cargo.toml index 841ab140c..97241a571 100644 --- a/src/uu/base64/Cargo.toml +++ b/src/uu/base64/Cargo.toml @@ -15,6 +15,7 @@ edition = "2018" path = "src/base64.rs" [dependencies] +clap = "2.33" uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features = ["encoding"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/base64/src/base64.rs b/src/uu/base64/src/base64.rs index 61a8dc5cb..62d36cc5f 100644 --- a/src/uu/base64/src/base64.rs +++ b/src/uu/base64/src/base64.rs @@ -8,12 +8,19 @@ #[macro_use] extern crate uucore; + use uucore::encoding::Format; use uucore::InvalidEncodingHandling; +use std::fs::File; +use std::io::{stdin, BufReader}; +use std::path::Path; + +use clap::{App, Arg}; + mod base_common; -static SYNTAX: &str = "[OPTION]... [FILE]"; +static BASE64_ARG_ERROR: i32 = 1; static SUMMARY: &str = "Base64 encode or decode FILE, or standard input, to standard output."; static LONG_HELP: &str = " With no FILE, or when FILE is -, read standard input. @@ -24,14 +31,128 @@ static LONG_HELP: &str = " to attempt to recover from any other non-alphabet bytes in the encoded stream. "; +static VERSION: &str = env!("CARGO_PKG_VERSION"); + +fn get_usage() -> String { + format!("{0} [OPTION]... [FILE]", executable!()) +} + +pub mod options { + pub static DECODE: &str = "decode"; + pub static WRAP: &str = "wrap"; + pub static IGNORE_GARBAGE: &str = "ignore-garbage"; + pub static FILE: &str = "file"; +} + +struct Config { + decode: bool, + ignore_garbage: bool, + wrap_cols: Option, + to_read: Option, +} + +impl Config { + fn from(options: clap::ArgMatches) -> Config { + let file: Option = match options.values_of(options::FILE) { + Some(mut values) => { + let name = values.next().unwrap(); + if values.len() != 0 { + crash!(BASE64_ARG_ERROR, "extra operand ‘{}’", name); + } + + if name == "-" { + None + } else { + if !Path::exists(Path::new(name)) { + crash!(BASE64_ARG_ERROR, "{}: No such file or directory", name); + } + Some(name.to_owned()) + } + } + None => None, + }; + + let cols = match options.value_of(options::WRAP) { + Some(num) => match num.parse::() { + Ok(n) => Some(n), + Err(e) => { + crash!(BASE64_ARG_ERROR, "invalid wrap size: ‘{}’: {}", num, e); + } + }, + None => None, + }; + + Config { + decode: options.is_present(options::DECODE), + ignore_garbage: options.is_present(options::IGNORE_GARBAGE), + wrap_cols: cols, + to_read: file, + } + } +} pub fn uumain(args: impl uucore::Args) -> i32 { - base_common::execute( - args.collect_str(InvalidEncodingHandling::Ignore) - .accept_any(), - SYNTAX, - SUMMARY, - LONG_HELP, - Format::Base64, - ) + let format = Format::Base64; + let usage = get_usage(); + let app = App::new(executable!()) + .version(VERSION) + .about(SUMMARY) + .usage(&usage[..]) + .about(LONG_HELP) + // Format arguments. + .arg( + Arg::with_name(options::DECODE) + .short("d") + .long(options::DECODE) + .help("decode data"), + ) + .arg( + Arg::with_name(options::IGNORE_GARBAGE) + .short("i") + .long(options::IGNORE_GARBAGE) + .help("when decoding, ignore non-alphabetic characters"), + ) + .arg( + Arg::with_name(options::WRAP) + .short("w") + .long(options::WRAP) + .takes_value(true) + .help( + "wrap encoded lines after COLS character (default 76, 0 to disable wrapping)", + ), + ) + // "multiple" arguments are used to check whether there is more than one + // file passed in. + .arg(Arg::with_name(options::FILE).index(1).multiple(true)); + + let arg_list = args + .collect_str(InvalidEncodingHandling::ConvertLossy) + .accept_any(); + let config: Config = Config::from(app.get_matches_from(arg_list)); + match config.to_read { + // Read from file. + Some(name) => { + let file_buf = safe_unwrap!(File::open(Path::new(&name))); + let mut input = BufReader::new(file_buf); + base_common::handle_input( + &mut input, + format, + config.wrap_cols, + config.ignore_garbage, + config.decode, + ); + } + // stdin + None => { + base_common::handle_input( + &mut stdin().lock(), + format, + config.wrap_cols, + config.ignore_garbage, + config.decode, + ); + } + }; + + 0 } diff --git a/src/uu/base64/src/base_common.rs b/src/uu/base64/src/base_common.rs index 3f1436fb2..a4b49e499 100644 --- a/src/uu/base64/src/base_common.rs +++ b/src/uu/base64/src/base_common.rs @@ -7,68 +7,11 @@ // For the full copyright and license information, please view the LICENSE file // that was distributed with this source code. -use std::fs::File; -use std::io::{stdin, stdout, BufReader, Read, Write}; -use std::path::Path; +use std::io::{stdout, Read, Write}; use uucore::encoding::{wrap_print, Data, Format}; -pub fn execute( - args: Vec, - syntax: &str, - summary: &str, - long_help: &str, - format: Format, -) -> i32 { - let matches = app!(syntax, summary, long_help) - .optflag("d", "decode", "decode data") - .optflag( - "i", - "ignore-garbage", - "when decoding, ignore non-alphabetic characters", - ) - .optopt( - "w", - "wrap", - "wrap encoded lines after COLS character (default 76, 0 to disable wrapping)", - "COLS", - ) - .parse(args); - - let line_wrap = matches.opt_str("wrap").map(|s| match s.parse() { - Ok(n) => n, - Err(e) => { - crash!(1, "invalid wrap size: ‘{}’: {}", s, e); - } - }); - let ignore_garbage = matches.opt_present("ignore-garbage"); - let decode = matches.opt_present("decode"); - - if matches.free.len() > 1 { - show_usage_error!("extra operand ‘{}’", matches.free[0]); - return 1; - } - - if matches.free.is_empty() || &matches.free[0][..] == "-" { - let stdin_raw = stdin(); - handle_input( - &mut stdin_raw.lock(), - format, - line_wrap, - ignore_garbage, - decode, - ); - } else { - let path = Path::new(matches.free[0].as_str()); - let file_buf = safe_unwrap!(File::open(&path)); - let mut input = BufReader::new(file_buf); - handle_input(&mut input, format, line_wrap, ignore_garbage, decode); - }; - - 0 -} - -fn handle_input( +pub fn handle_input( input: &mut R, format: Format, line_wrap: Option, diff --git a/tests/by-util/test_base64.rs b/tests/by-util/test_base64.rs index 6bc0436c5..c9adc5c0f 100644 --- a/tests/by-util/test_base64.rs +++ b/tests/by-util/test_base64.rs @@ -7,6 +7,21 @@ fn test_encode() { .pipe_in(input) .succeeds() .stdout_only("aGVsbG8sIHdvcmxkIQ==\n"); + + // Using '-' as our file + new_ucmd!() + .arg("-") + .pipe_in(input) + .succeeds() + .stdout_only("aGVsbG8sIHdvcmxkIQ==\n"); +} + +#[test] +fn test_base64_encode_file() { + new_ucmd!() + .arg("input-simple.txt") + .succeeds() + .stdout_only("SGVsbG8sIFdvcmxkIQo=\n"); } #[test] @@ -60,10 +75,9 @@ fn test_wrap() { #[test] fn test_wrap_no_arg() { for wrap_param in vec!["-w", "--wrap"] { - new_ucmd!().arg(wrap_param).fails().stderr_only(format!( - "base64: error: Argument to option '{}' missing\n", - if wrap_param == "-w" { "w" } else { "wrap" } - )); + new_ucmd!().arg(wrap_param).fails().stderr_contains( + &"The argument '--wrap ' requires a value but none was supplied", + ); } } @@ -77,3 +91,21 @@ fn test_wrap_bad_arg() { .stderr_only("base64: error: invalid wrap size: ‘b’: invalid digit found in string\n"); } } + +#[test] +fn test_base64_extra_operand() { + // Expect a failure when multiple files are specified. + new_ucmd!() + .arg("a.txt") + .arg("a.txt") + .fails() + .stderr_only("base64: error: extra operand ‘a.txt’"); +} + +#[test] +fn test_base64_file_not_found() { + new_ucmd!() + .arg("a.txt") + .fails() + .stderr_only("base64: error: a.txt: No such file or directory"); +} diff --git a/tests/fixtures/base64/input-simple.txt b/tests/fixtures/base64/input-simple.txt new file mode 100644 index 000000000..8ab686eaf --- /dev/null +++ b/tests/fixtures/base64/input-simple.txt @@ -0,0 +1 @@ +Hello, World! From f307de22d0636247c7cfbb4b6db88ea29971a7d6 Mon Sep 17 00:00:00 2001 From: Ricardo Iglesias Date: Thu, 29 Apr 2021 01:59:43 -0700 Subject: [PATCH 05/17] base64: Refactor argument parsing Moved most of the argument parsing logic to `base32/base_common.rs` to allow for significant code reuse. --- Cargo.lock | 1 + src/uu/base32/src/base32.rs | 152 +++++++------------------------ src/uu/base32/src/base_common.rs | 125 ++++++++++++++++++++++++- src/uu/base64/Cargo.toml | 1 + src/uu/base64/src/base64.rs | 151 ++++++------------------------ src/uu/base64/src/base_common.rs | 40 -------- tests/by-util/test_base32.rs | 2 +- tests/by-util/test_base64.rs | 2 +- 8 files changed, 187 insertions(+), 287 deletions(-) delete mode 100644 src/uu/base64/src/base_common.rs diff --git a/Cargo.lock b/Cargo.lock index 254ae8fda..54ddbd88e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1689,6 +1689,7 @@ name = "uu_base64" version = "0.0.6" dependencies = [ "clap", + "uu_base32", "uucore", "uucore_procs", ] diff --git a/src/uu/base32/src/base32.rs b/src/uu/base32/src/base32.rs index 6d9f7f08f..f0e187c31 100644 --- a/src/uu/base32/src/base32.rs +++ b/src/uu/base32/src/base32.rs @@ -7,19 +7,14 @@ #[macro_use] extern crate uucore; + +use std::io::{stdin, Read}; + use uucore::encoding::Format; -use uucore::InvalidEncodingHandling; -use std::fs::File; -use std::io::{stdin, BufReader}; -use std::path::Path; +pub mod base_common; -use clap::{App, Arg}; - -mod base_common; - -static SUMMARY: &str = "Base32 encode or decode FILE, or standard input, to standard output."; -static LONG_HELP: &str = " +static ABOUT: &str = " With no FILE, or when FILE is -, read standard input. The data are encoded as described for the base32 alphabet in RFC @@ -30,126 +25,41 @@ static LONG_HELP: &str = " "; static VERSION: &str = env!("CARGO_PKG_VERSION"); +static BASE_CMD_PARSE_ERROR: i32 = 1; + fn get_usage() -> String { format!("{0} [OPTION]... [FILE]", executable!()) } -pub mod options { - pub static DECODE: &str = "decode"; - pub static WRAP: &str = "wrap"; - pub static IGNORE_GARBAGE: &str = "ignore-garbage"; - pub static FILE: &str = "file"; -} - -struct Config { - decode: bool, - ignore_garbage: bool, - wrap_cols: Option, - to_read: Option, -} - -impl Config { - fn from(options: clap::ArgMatches) -> Config { - let file: Option = match options.values_of(options::FILE) { - Some(mut values) => { - let name = values.next().unwrap(); - if values.len() != 0 { - crash!(3, "extra operand ‘{}’", name); - } - - if name == "-" { - None - } else { - if !Path::exists(Path::new(name)) { - crash!(2, "{}: No such file or directory", name); - } - Some(name.to_owned()) - } - } - None => None, - }; - - let cols = match options.value_of(options::WRAP) { - Some(num) => match num.parse::() { - Ok(n) => Some(n), - Err(e) => { - crash!(1, "invalid wrap size: ‘{}’: {}", num, e); - } - }, - None => None, - }; - - Config { - decode: options.is_present(options::DECODE), - ignore_garbage: options.is_present(options::IGNORE_GARBAGE), - wrap_cols: cols, - to_read: file, - } - } -} - pub fn uumain(args: impl uucore::Args) -> i32 { let format = Format::Base32; let usage = get_usage(); - let app = App::new(executable!()) - .version(VERSION) - .about(SUMMARY) - .usage(&usage[..]) - .about(LONG_HELP) - // Format arguments. - .arg( - Arg::with_name(options::DECODE) - .short("d") - .long(options::DECODE) - .help("decode data"), - ) - .arg( - Arg::with_name(options::IGNORE_GARBAGE) - .short("i") - .long(options::IGNORE_GARBAGE) - .help("when decoding, ignore non-alphabetic characters"), - ) - .arg( - Arg::with_name(options::WRAP) - .short("w") - .long(options::WRAP) - .takes_value(true) - .help( - "wrap encoded lines after COLS character (default 76, 0 to disable wrapping)", - ), - ) - // "multiple" arguments are used to check whether there is more than one - // file passed in. - .arg(Arg::with_name(options::FILE).index(1).multiple(true)); + let name = executable!(); - let arg_list = args - .collect_str(InvalidEncodingHandling::ConvertLossy) - .accept_any(); - let config: Config = Config::from(app.get_matches_from(arg_list)); - match config.to_read { - // Read from file. - Some(name) => { - let file_buf = safe_unwrap!(File::open(Path::new(&name))); - let mut input = BufReader::new(file_buf); - base_common::handle_input( - &mut input, - format, - config.wrap_cols, - config.ignore_garbage, - config.decode, - ); + let config_result: Result = + base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); + + if config_result.is_err() { + match config_result { + Ok(_) => panic!(), + Err(s) => crash!(BASE_CMD_PARSE_ERROR, "{}", s), } - // stdin - None => { - base_common::handle_input( - &mut stdin().lock(), - format, - config.wrap_cols, - config.ignore_garbage, - config.decode, - ); - } - }; + } + + // Create a reference to stdin so we can return a locked stdin from + // parse_base_cmd_args + let stdin_raw = stdin(); + let config = config_result.unwrap(); + let mut input: Box = base_common::get_input(&config, &stdin_raw); + + base_common::handle_input( + &mut input, + format, + config.wrap_cols, + config.ignore_garbage, + config.decode, + name, + ); 0 } diff --git a/src/uu/base32/src/base_common.rs b/src/uu/base32/src/base_common.rs index a4b49e499..d0f47f500 100644 --- a/src/uu/base32/src/base_common.rs +++ b/src/uu/base32/src/base_common.rs @@ -10,6 +10,122 @@ use std::io::{stdout, Read, Write}; use uucore::encoding::{wrap_print, Data, Format}; +use uucore::InvalidEncodingHandling; + +use std::fs::File; +use std::io::{BufReader, Stdin}; +use std::path::Path; + +use clap::{App, Arg}; + +// Config. +pub struct Config { + pub decode: bool, + pub ignore_garbage: bool, + pub wrap_cols: Option, + pub to_read: Option, +} + +pub mod options { + pub static DECODE: &str = "decode"; + pub static WRAP: &str = "wrap"; + pub static IGNORE_GARBAGE: &str = "ignore-garbage"; + pub static FILE: &str = "file"; +} + +impl Config { + fn from(options: clap::ArgMatches) -> Result { + let file: Option = match options.values_of(options::FILE) { + Some(mut values) => { + let name = values.next().unwrap(); + if values.len() != 0 { + return Err(format!("extra operand ‘{}’", name)); + } + + if name == "-" { + None + } else { + if !Path::exists(Path::new(name)) { + return Err(format!("{}: No such file or directory", name)); + } + Some(name.to_owned()) + } + } + None => None, + }; + + let cols = match options.value_of(options::WRAP) { + Some(num) => match num.parse::() { + Ok(n) => Some(n), + Err(e) => { + return Err(format!("Invalid wrap size: ‘{}’: {}", num, e)); + } + }, + None => None, + }; + + Ok(Config { + decode: options.is_present(options::DECODE), + ignore_garbage: options.is_present(options::IGNORE_GARBAGE), + wrap_cols: cols, + to_read: file, + }) + } +} + +pub fn parse_base_cmd_args( + args: impl uucore::Args, + name: &str, + version: &str, + about: &str, + usage: &str, +) -> Result { + let app = App::new(name) + .version(version) + .about(about) + .usage(&usage[..]) + // Format arguments. + .arg( + Arg::with_name(options::DECODE) + .short("d") + .long(options::DECODE) + .help("decode data"), + ) + .arg( + Arg::with_name(options::IGNORE_GARBAGE) + .short("i") + .long(options::IGNORE_GARBAGE) + .help("when decoding, ignore non-alphabetic characters"), + ) + .arg( + Arg::with_name(options::WRAP) + .short("w") + .long(options::WRAP) + .takes_value(true) + .help( + "wrap encoded lines after COLS character (default 76, 0 to disable wrapping)", + ), + ) + // "multiple" arguments are used to check whether there is more than one + // file passed in. + .arg(Arg::with_name(options::FILE).index(1).multiple(true)); + let arg_list = args + .collect_str(InvalidEncodingHandling::ConvertLossy) + .accept_any(); + Config::from(app.get_matches_from(arg_list)) +} + +pub fn get_input<'a>(config: &Config, stdin_ref: &'a Stdin) -> Box { + match &config.to_read { + Some(name) => { + let file_buf = safe_unwrap!(File::open(Path::new(name))); + Box::new(BufReader::new(file_buf)) // as Box + } + None => { + Box::new(stdin_ref.lock()) // as Box + } + } +} pub fn handle_input( input: &mut R, @@ -17,6 +133,7 @@ pub fn handle_input( line_wrap: Option, ignore_garbage: bool, decode: bool, + name: &str, ) { let mut data = Data::new(input, format).ignore_garbage(ignore_garbage); if let Some(wrap) = line_wrap { @@ -31,10 +148,14 @@ pub fn handle_input( Ok(s) => { if stdout().write_all(&s).is_err() { // on windows console, writing invalid utf8 returns an error - crash!(1, "Cannot write non-utf8 data"); + eprintln!("{}: error: Cannot write non-utf8 data", name); + exit!(1) } } - Err(_) => crash!(1, "invalid input"), + Err(_) => { + eprintln!("{}: error: invalid input", name); + exit!(1) + } } } } diff --git a/src/uu/base64/Cargo.toml b/src/uu/base64/Cargo.toml index 97241a571..d4ee69f03 100644 --- a/src/uu/base64/Cargo.toml +++ b/src/uu/base64/Cargo.toml @@ -18,6 +18,7 @@ path = "src/base64.rs" clap = "2.33" uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features = ["encoding"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } +uu_base32 = { version=">=0.0.6", package="uu_base32", path="../base32"} [[bin]] name = "base64" diff --git a/src/uu/base64/src/base64.rs b/src/uu/base64/src/base64.rs index 62d36cc5f..810df4fe8 100644 --- a/src/uu/base64/src/base64.rs +++ b/src/uu/base64/src/base64.rs @@ -9,20 +9,13 @@ #[macro_use] extern crate uucore; +use uu_base32::base_common; + use uucore::encoding::Format; -use uucore::InvalidEncodingHandling; -use std::fs::File; -use std::io::{stdin, BufReader}; -use std::path::Path; +use std::io::{stdin, Read}; -use clap::{App, Arg}; - -mod base_common; - -static BASE64_ARG_ERROR: i32 = 1; -static SUMMARY: &str = "Base64 encode or decode FILE, or standard input, to standard output."; -static LONG_HELP: &str = " +static ABOUT: &str = " With no FILE, or when FILE is -, read standard input. The data are encoded as described for the base64 alphabet in RFC @@ -33,126 +26,40 @@ static LONG_HELP: &str = " "; static VERSION: &str = env!("CARGO_PKG_VERSION"); +static BASE_CMD_PARSE_ERROR: i32 = 1; + fn get_usage() -> String { format!("{0} [OPTION]... [FILE]", executable!()) } -pub mod options { - pub static DECODE: &str = "decode"; - pub static WRAP: &str = "wrap"; - pub static IGNORE_GARBAGE: &str = "ignore-garbage"; - pub static FILE: &str = "file"; -} - -struct Config { - decode: bool, - ignore_garbage: bool, - wrap_cols: Option, - to_read: Option, -} - -impl Config { - fn from(options: clap::ArgMatches) -> Config { - let file: Option = match options.values_of(options::FILE) { - Some(mut values) => { - let name = values.next().unwrap(); - if values.len() != 0 { - crash!(BASE64_ARG_ERROR, "extra operand ‘{}’", name); - } - - if name == "-" { - None - } else { - if !Path::exists(Path::new(name)) { - crash!(BASE64_ARG_ERROR, "{}: No such file or directory", name); - } - Some(name.to_owned()) - } - } - None => None, - }; - - let cols = match options.value_of(options::WRAP) { - Some(num) => match num.parse::() { - Ok(n) => Some(n), - Err(e) => { - crash!(BASE64_ARG_ERROR, "invalid wrap size: ‘{}’: {}", num, e); - } - }, - None => None, - }; - - Config { - decode: options.is_present(options::DECODE), - ignore_garbage: options.is_present(options::IGNORE_GARBAGE), - wrap_cols: cols, - to_read: file, - } - } -} - pub fn uumain(args: impl uucore::Args) -> i32 { let format = Format::Base64; let usage = get_usage(); - let app = App::new(executable!()) - .version(VERSION) - .about(SUMMARY) - .usage(&usage[..]) - .about(LONG_HELP) - // Format arguments. - .arg( - Arg::with_name(options::DECODE) - .short("d") - .long(options::DECODE) - .help("decode data"), - ) - .arg( - Arg::with_name(options::IGNORE_GARBAGE) - .short("i") - .long(options::IGNORE_GARBAGE) - .help("when decoding, ignore non-alphabetic characters"), - ) - .arg( - Arg::with_name(options::WRAP) - .short("w") - .long(options::WRAP) - .takes_value(true) - .help( - "wrap encoded lines after COLS character (default 76, 0 to disable wrapping)", - ), - ) - // "multiple" arguments are used to check whether there is more than one - // file passed in. - .arg(Arg::with_name(options::FILE).index(1).multiple(true)); + let name = executable!(); + let config_result: Result = + base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); - let arg_list = args - .collect_str(InvalidEncodingHandling::ConvertLossy) - .accept_any(); - let config: Config = Config::from(app.get_matches_from(arg_list)); - match config.to_read { - // Read from file. - Some(name) => { - let file_buf = safe_unwrap!(File::open(Path::new(&name))); - let mut input = BufReader::new(file_buf); - base_common::handle_input( - &mut input, - format, - config.wrap_cols, - config.ignore_garbage, - config.decode, - ); + if config_result.is_err() { + match config_result { + Ok(_) => panic!(), + Err(s) => crash!(BASE_CMD_PARSE_ERROR, "{}", s), } - // stdin - None => { - base_common::handle_input( - &mut stdin().lock(), - format, - config.wrap_cols, - config.ignore_garbage, - config.decode, - ); - } - }; + } + + // Create a reference to stdin so we can return a locked stdin from + // parse_base_cmd_args + let stdin_raw = stdin(); + let config = config_result.unwrap(); + let mut input: Box = base_common::get_input(&config, &stdin_raw); + + base_common::handle_input( + &mut input, + format, + config.wrap_cols, + config.ignore_garbage, + config.decode, + name, + ); 0 } diff --git a/src/uu/base64/src/base_common.rs b/src/uu/base64/src/base_common.rs deleted file mode 100644 index a4b49e499..000000000 --- a/src/uu/base64/src/base_common.rs +++ /dev/null @@ -1,40 +0,0 @@ -// This file is part of the uutils coreutils package. -// -// (c) Jordy Dickinson -// (c) Jian Zeng -// (c) Alex Lyon -// -// For the full copyright and license information, please view the LICENSE file -// that was distributed with this source code. - -use std::io::{stdout, Read, Write}; - -use uucore::encoding::{wrap_print, Data, Format}; - -pub fn handle_input( - input: &mut R, - format: Format, - line_wrap: Option, - ignore_garbage: bool, - decode: bool, -) { - let mut data = Data::new(input, format).ignore_garbage(ignore_garbage); - if let Some(wrap) = line_wrap { - data = data.line_wrap(wrap); - } - - if !decode { - let encoded = data.encode(); - wrap_print(&data, encoded); - } else { - match data.decode() { - Ok(s) => { - if stdout().write_all(&s).is_err() { - // on windows console, writing invalid utf8 returns an error - crash!(1, "Cannot write non-utf8 data"); - } - } - Err(_) => crash!(1, "invalid input"), - } - } -} diff --git a/tests/by-util/test_base32.rs b/tests/by-util/test_base32.rs index 157503d83..fd49aa951 100644 --- a/tests/by-util/test_base32.rs +++ b/tests/by-util/test_base32.rs @@ -98,7 +98,7 @@ fn test_wrap_bad_arg() { .arg(wrap_param) .arg("b") .fails() - .stderr_only("base32: error: invalid wrap size: ‘b’: invalid digit found in string\n"); + .stderr_only("base32: error: Invalid wrap size: ‘b’: invalid digit found in string\n"); } } diff --git a/tests/by-util/test_base64.rs b/tests/by-util/test_base64.rs index c9adc5c0f..8d9dc5639 100644 --- a/tests/by-util/test_base64.rs +++ b/tests/by-util/test_base64.rs @@ -88,7 +88,7 @@ fn test_wrap_bad_arg() { .arg(wrap_param) .arg("b") .fails() - .stderr_only("base64: error: invalid wrap size: ‘b’: invalid digit found in string\n"); + .stderr_only("base64: error: Invalid wrap size: ‘b’: invalid digit found in string\n"); } } From 193ad56c2a5e9a615618de8e74368c8cc2c1baad Mon Sep 17 00:00:00 2001 From: Ricardo Iglesias Date: Thu, 29 Apr 2021 02:07:59 -0700 Subject: [PATCH 06/17] Removed clippy warnings. --- 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 d0f47f500..ee5fe8675 100644 --- a/src/uu/base32/src/base_common.rs +++ b/src/uu/base32/src/base_common.rs @@ -83,7 +83,7 @@ pub fn parse_base_cmd_args( let app = App::new(name) .version(version) .about(about) - .usage(&usage[..]) + .usage(usage) // Format arguments. .arg( Arg::with_name(options::DECODE) From c3912d53ac1430b718cebbcf724de3af59a06e2e Mon Sep 17 00:00:00 2001 From: Daniel Rocco Date: Thu, 22 Apr 2021 20:29:04 -0400 Subject: [PATCH 07/17] test: add tests for basic tests & edge cases Some edge cases covered: - no args - operator by itself (!, -a, etc.) - string, file tests of nothing - compound negations --- tests/by-util/test_test.rs | 442 ++++++++++++++++++++++++++++- tests/fixtures/test/non_empty_file | 1 + tests/fixtures/test/regular_file | 0 3 files changed, 442 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/test/non_empty_file create mode 100644 tests/fixtures/test/regular_file diff --git a/tests/by-util/test_test.rs b/tests/by-util/test_test.rs index 4b9a9ff55..2173371fc 100644 --- a/tests/by-util/test_test.rs +++ b/tests/by-util/test_test.rs @@ -2,6 +2,7 @@ // This file is part of the uutils coreutils package. // // (c) mahkoh (ju.orth [at] gmail [dot] com) +// (c) Daniel Rocco // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. @@ -9,6 +10,436 @@ use crate::common::util::*; +#[test] +fn test_empty_test_equivalent_to_false() { + new_ucmd!().run().status_code(1); +} + +#[test] +fn test_empty_string_is_false() { + new_ucmd!().arg("").run().status_code(1); +} + +#[test] +fn test_solo_not() { + new_ucmd!().arg("!").succeeds(); +} + +#[test] +fn test_solo_and_or_or_is_a_literal() { + // /bin/test '' -a '' => 1; so test(1) must interpret `-a` by itself as + // a literal string + new_ucmd!().arg("-a").succeeds(); + new_ucmd!().arg("-o").succeeds(); +} + +#[test] +fn test_double_not_is_false() { + new_ucmd!().args(&["!", "!"]).run().status_code(1); +} + +#[test] +#[ignore = "fixme: parse/evaluation error (code 2); GNU returns 1"] +fn test_and_not_is_false() { + new_ucmd!().args(&["-a", "!"]).run().status_code(1); +} + +#[test] +fn test_not_and_is_false() { + // `-a` is a literal here & has nonzero length + new_ucmd!().args(&["!", "-a"]).run().status_code(1); +} + +#[test] +#[ignore = "fixme: parse/evaluation error (code 2); GNU returns 0"] +fn test_not_and_not_succeeds() { + new_ucmd!().args(&["!", "-a", "!"]).succeeds(); +} + +#[test] +fn test_simple_or() { + new_ucmd!().args(&["foo", "-o", ""]).succeeds(); +} + +#[test] +#[ignore = "fixme: parse/evaluation error (code 0); GNU returns 1"] +fn test_negated_or() { + new_ucmd!() + .args(&["!", "foo", "-o", "bar"]) + .run() + .status_code(1); + new_ucmd!().args(&["foo", "-o", "!", "bar"]).succeeds(); + new_ucmd!() + .args(&["!", "foo", "-o", "!", "bar"]) + .run() + .status_code(1); +} + +#[test] +fn test_strlen_of_nothing() { + // odd but matches GNU, which must interpret -n as a literal here + new_ucmd!().arg("-n").succeeds(); +} + +#[test] +fn test_strlen_of_empty() { + new_ucmd!().args(&["-n", ""]).run().status_code(1); + + // STRING equivalent to -n STRING + new_ucmd!().arg("").run().status_code(1); +} + +#[test] +fn test_nothing_is_empty() { + // -z is a literal here and has nonzero length + new_ucmd!().arg("-z").succeeds(); +} + +#[test] +fn test_zero_len_of_empty() { + new_ucmd!().args(&["-z", ""]).succeeds(); +} + +#[test] +fn test_solo_paren_is_literal() { + let scenario = TestScenario::new(util_name!()); + let tests = [["("], [")"]]; + + for test in &tests { + scenario.ucmd().args(&test[..]).succeeds(); + } +} + +#[test] +#[ignore = "GNU considers this an error"] +fn test_solo_empty_parenthetical_is_error() { + new_ucmd!() + .args(&["(", ")"]) + .run() + .status_code(2) + .stderr_is("test: missing argument after ‘)’"); +} + +#[test] +fn test_zero_len_equals_zero_len() { + new_ucmd!().args(&["", "=", ""]).succeeds(); +} + +#[test] +fn test_zero_len_not_equals_zero_len_is_false() { + new_ucmd!().args(&["", "!=", ""]).run().status_code(1); +} + +#[test] +fn test_string_comparison() { + let scenario = TestScenario::new(util_name!()); + let tests = [ + ["foo", "!=", "bar"], + ["contained\nnewline", "=", "contained\nnewline"], + ["(", "=", "("], + ["(", "!=", ")"], + ["!", "=", "!"], + ]; + + for test in &tests { + scenario.ucmd().args(&test[..]).succeeds(); + } +} + +#[test] +#[ignore = "fixme: error reporting"] +fn test_dangling_string_comparison_is_error() { + new_ucmd!() + .args(&["missing_something", "="]) + .run() + .status_code(2) + .stderr_is("test: missing argument after ‘=’"); +} + +#[test] +fn test_stringop_is_literal_after_bang() { + let scenario = TestScenario::new(util_name!()); + let tests = [ + ["!", "="], + ["!", "!="], + ["!", "-eq"], + ["!", "-ne"], + ["!", "-lt"], + ["!", "-le"], + ["!", "-gt"], + ["!", "-ge"], + ["!", "-ef"], + ["!", "-nt"], + ["!", "-ot"], + ]; + + for test in &tests { + scenario.ucmd().args(&test[..]).run().status_code(1); + } +} + +#[test] +fn test_a_bunch_of_not() { + new_ucmd!() + .args(&["!", "", "!=", "", "-a", "!", "", "!=", ""]) + .succeeds(); +} + +#[test] +fn test_pseudofloat_equal() { + new_ucmd!().args(&["123.45", "=", "123.45"]).succeeds(); +} + +#[test] +fn test_pseudofloat_not_equal() { + new_ucmd!().args(&["123.45", "!=", "123.450"]).succeeds(); +} + +#[test] +fn test_negative_arg_is_a_string() { + new_ucmd!().arg("-12345").succeeds(); + new_ucmd!().arg("--qwert").succeeds(); +} + +#[test] +fn test_some_int_compares() { + let scenario = TestScenario::new(util_name!()); + + let tests = [ + ["0", "-eq", "0"], + ["0", "-ne", "1"], + ["421", "-lt", "3720"], + ["0", "-le", "0"], + ["11", "-gt", "10"], + ["1024", "-ge", "512"], + ["9223372036854775806", "-le", "9223372036854775807"], + ]; + + for test in &tests { + scenario.ucmd().args(&test[..]).succeeds(); + } +} + +#[test] +#[ignore = "fixme: evaluation error (code 1); GNU returns 0"] +fn test_values_greater_than_i64_allowed() { + new_ucmd!() + .args(&["9223372036854775808", "-gt", "0"]) + .succeeds(); +} + +#[test] +fn test_negative_int_compare() { + let scenario = TestScenario::new(util_name!()); + + let tests = [ + ["-1", "-eq", "-1"], + ["-1", "-ne", "-2"], + ["-3720", "-lt", "-421"], + ["-10", "-le", "-10"], + ["-21", "-gt", "-22"], + ["-128", "-ge", "-256"], + ["-9223372036854775808", "-le", "-9223372036854775807"], + ]; + + for test in &tests { + scenario.ucmd().args(&test[..]).succeeds(); + } +} + +#[test] +#[ignore = "fixme: error reporting"] +fn test_float_inequality_is_error() { + new_ucmd!() + .args(&["123.45", "-ge", "6"]) + .run() + .status_code(2) + .stderr_is("test: invalid integer ‘123.45’"); +} + +#[test] +#[ignore = "fixme: parse/evaluation error (code 2); GNU returns 1"] +fn test_file_is_itself() { + new_ucmd!() + .args(&["regular_file", "-ef", "regular_file"]) + .succeeds(); +} + +#[test] +#[ignore = "fixme: parse/evaluation error (code 2); GNU returns 1"] +fn test_file_is_newer_than_and_older_than_itself() { + // odd but matches GNU + new_ucmd!() + .args(&["regular_file", "-nt", "regular_file"]) + .run() + .status_code(1); + new_ucmd!() + .args(&["regular_file", "-ot", "regular_file"]) + .run() + .status_code(1); +} + +#[test] +#[ignore = "todo: implement these"] +fn test_newer_file() { + let scenario = TestScenario::new(util_name!()); + + scenario.cmd("touch").arg("newer_file").succeeds(); + scenario + .cmd("touch") + .args(&["-m", "-d", "last Thursday", "regular_file"]) + .succeeds(); + + scenario + .ucmd() + .args(&["newer_file", "-nt", "regular_file"]) + .succeeds(); + scenario + .ucmd() + .args(&["regular_file", "-ot", "newer_file"]) + .succeeds(); +} + +#[test] +fn test_file_exists() { + new_ucmd!().args(&["-e", "regular_file"]).succeeds(); +} + +#[test] +fn test_nonexistent_file_does_not_exist() { + new_ucmd!() + .args(&["-e", "nonexistent_file"]) + .run() + .status_code(1); +} + +#[test] +fn test_nonexistent_file_is_not_regular() { + new_ucmd!() + .args(&["-f", "nonexistent_file"]) + .run() + .status_code(1); +} + +#[test] +fn test_file_exists_and_is_regular() { + new_ucmd!().args(&["-f", "regular_file"]).succeeds(); +} + +#[test] +#[cfg(not(windows))] // FIXME: implement on Windows +fn test_file_is_readable() { + new_ucmd!().args(&["-r", "regular_file"]).succeeds(); +} + +#[test] +#[cfg(not(windows))] // FIXME: implement on Windows +fn test_file_is_not_readable() { + let scenario = TestScenario::new(util_name!()); + let mut ucmd = scenario.ucmd(); + let mut chmod = scenario.cmd("chmod"); + + scenario.fixtures.touch("crypto_file"); + chmod.args(&["u-r", "crypto_file"]).succeeds(); + + ucmd.args(&["!", "-r", "crypto_file"]).succeeds(); +} + +#[test] +#[cfg(not(windows))] // FIXME: implement on Windows +fn test_file_is_writable() { + new_ucmd!().args(&["-w", "regular_file"]).succeeds(); +} + +#[test] +#[cfg(not(windows))] // FIXME: implement on Windows +fn test_file_is_not_writable() { + let scenario = TestScenario::new(util_name!()); + let mut ucmd = scenario.ucmd(); + let mut chmod = scenario.cmd("chmod"); + + scenario.fixtures.touch("immutable_file"); + chmod.args(&["u-w", "immutable_file"]).succeeds(); + + ucmd.args(&["!", "-w", "immutable_file"]).succeeds(); +} + +#[test] +fn test_file_is_not_executable() { + new_ucmd!().args(&["!", "-x", "regular_file"]).succeeds(); +} + +#[test] +#[cfg(not(windows))] // FIXME: implement on Windows +fn test_file_is_executable() { + let scenario = TestScenario::new(util_name!()); + let mut chmod = scenario.cmd("chmod"); + + chmod.args(&["u+x", "regular_file"]).succeeds(); + + scenario.ucmd().args(&["-x", "regular_file"]).succeeds(); +} + +#[test] +fn test_is_not_empty() { + new_ucmd!().args(&["-s", "non_empty_file"]).succeeds(); +} + +#[test] +fn test_nonexistent_file_size_test_is_false() { + new_ucmd!() + .args(&["-s", "nonexistent_file"]) + .run() + .status_code(1); +} + +#[test] +fn test_not_is_not_empty() { + new_ucmd!().args(&["!", "-s", "regular_file"]).succeeds(); +} + +#[test] +#[cfg(not(windows))] +fn test_symlink_is_symlink() { + let scenario = TestScenario::new(util_name!()); + let mut ln = scenario.cmd("ln"); + + // creating symlinks requires admin on Windows + ln.args(&["-s", "regular_file", "symlink"]).succeeds(); + + // FIXME: implement on Windows + scenario.ucmd().args(&["-h", "symlink"]).succeeds(); + scenario.ucmd().args(&["-L", "symlink"]).succeeds(); +} + +#[test] +fn test_file_is_not_symlink() { + let scenario = TestScenario::new(util_name!()); + + scenario + .ucmd() + .args(&["!", "-h", "regular_file"]) + .succeeds(); + scenario + .ucmd() + .args(&["!", "-L", "regular_file"]) + .succeeds(); +} + +#[test] +fn test_nonexistent_file_is_not_symlink() { + let scenario = TestScenario::new(util_name!()); + + scenario + .ucmd() + .args(&["!", "-h", "nonexistent_file"]) + .succeeds(); + scenario + .ucmd() + .args(&["!", "-L", "nonexistent_file"]) + .succeeds(); +} + #[test] fn test_op_prec_and_or_1() { new_ucmd!().args(&[" ", "-o", "", "-a", ""]).succeeds(); @@ -23,5 +454,14 @@ fn test_op_prec_and_or_2() { #[test] fn test_or_as_filename() { - new_ucmd!().args(&["x", "-a", "-z", "-o"]).fails(); + new_ucmd!() + .args(&["x", "-a", "-z", "-o"]) + .run() + .status_code(1); +} + +#[test] +#[ignore = "GNU considers this an error"] +fn test_strlen_and_nothing() { + new_ucmd!().args(&["-n", "a", "-a"]).run().status_code(2); } diff --git a/tests/fixtures/test/non_empty_file b/tests/fixtures/test/non_empty_file new file mode 100644 index 000000000..34425caf6 --- /dev/null +++ b/tests/fixtures/test/non_empty_file @@ -0,0 +1 @@ +Not empty! diff --git a/tests/fixtures/test/regular_file b/tests/fixtures/test/regular_file new file mode 100644 index 000000000..e69de29bb From 3c126bad72f9f26245b4aad02b1267c38b248032 Mon Sep 17 00:00:00 2001 From: Daniel Rocco Date: Mon, 26 Apr 2021 21:27:29 -0400 Subject: [PATCH 08/17] test: implement parenthesized expressions, additional tests - Replace the parser with a recursive descent implementation that handles parentheses and produces a stack of operations in postfix order. Parsing now operates directly on OsStrings passed by the uucore framework. - Replace the dispatch mechanism with a stack machine operating on the symbol stack produced by the parser. - Add tests for parenthesized expressions. - Begin testing character encoding handling. --- src/uu/test/src/parser.rs | 292 +++++++++++++++++++++++++ src/uu/test/src/test.rs | 436 ++++++++++++------------------------- tests/by-util/test_test.rs | 84 ++++++- 3 files changed, 504 insertions(+), 308 deletions(-) create mode 100644 src/uu/test/src/parser.rs diff --git a/src/uu/test/src/parser.rs b/src/uu/test/src/parser.rs new file mode 100644 index 000000000..f1ca9dad6 --- /dev/null +++ b/src/uu/test/src/parser.rs @@ -0,0 +1,292 @@ +// This file is part of the uutils coreutils package. +// +// (c) Daniel Rocco +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +use std::ffi::OsString; +use std::iter::Peekable; + +/// Represents a parsed token from a test expression +#[derive(Debug, PartialEq)] +pub enum Symbol { + LParen, + Bang, + BoolOp(OsString), + Literal(OsString), + StringOp(OsString), + IntOp(OsString), + FileOp(OsString), + StrlenOp(OsString), + FiletestOp(OsString), + None, +} + +impl Symbol { + /// Create a new Symbol from an OsString. + /// + /// Returns Symbol::None in place of None + fn new(token: Option) -> Symbol { + match token { + Some(s) => match s.to_string_lossy().as_ref() { + "(" => Symbol::LParen, + "!" => Symbol::Bang, + "-a" | "-o" => Symbol::BoolOp(s), + "=" | "!=" => Symbol::StringOp(s), + "-eq" | "-ge" | "-gt" | "-le" | "-lt" | "-ne" => Symbol::IntOp(s), + "-ef" | "-nt" | "-ot" => Symbol::FileOp(s), + "-n" | "-z" => Symbol::StrlenOp(s), + "-b" | "-c" | "-d" | "-e" | "-f" | "-g" | "-G" | "-h" | "-k" | "-L" | "-O" + | "-p" | "-r" | "-s" | "-S" | "-t" | "-u" | "-w" | "-x" => Symbol::FiletestOp(s), + _ => Symbol::Literal(s), + }, + None => Symbol::None, + } + } + + /// Convert this Symbol into a Symbol::Literal, useful for cases where + /// test treats an operator as a string operand (test has no reserved + /// words). + /// + /// # Panics + /// + /// Panics if `self` is Symbol::None + fn into_literal(self) -> Symbol { + Symbol::Literal(match self { + Symbol::LParen => OsString::from("("), + Symbol::Bang => OsString::from("!"), + Symbol::BoolOp(s) + | Symbol::Literal(s) + | Symbol::StringOp(s) + | Symbol::IntOp(s) + | Symbol::FileOp(s) + | Symbol::StrlenOp(s) + | Symbol::FiletestOp(s) => s, + Symbol::None => panic!(), + }) + } +} + +/// Recursive descent parser for test, which converts a list of OsStrings +/// (typically command line arguments) into a stack of Symbols in postfix +/// order. +/// +/// Grammar: +/// +/// EXPR → TERM | EXPR BOOLOP EXPR +/// TERM → ( EXPR ) +/// TERM → ( ) +/// TERM → ! EXPR +/// TERM → UOP str +/// UOP → STRLEN | FILETEST +/// TERM → str OP str +/// TERM → str | 𝜖 +/// OP → STRINGOP | INTOP | FILEOP +/// STRINGOP → = | != +/// INTOP → -eq | -ge | -gt | -le | -lt | -ne +/// FILEOP → -ef | -nt | -ot +/// STRLEN → -n | -z +/// FILETEST → -b | -c | -d | -e | -f | -g | -G | -h | -k | -L | -O | -p | +/// -r | -s | -S | -t | -u | -w | -x +/// BOOLOP → -a | -o +/// +#[derive(Debug)] +struct Parser { + tokens: Peekable>, + pub stack: Vec, +} + +impl Parser { + /// Construct a new Parser from a `Vec` of tokens. + fn new(tokens: Vec) -> Parser { + Parser { + tokens: tokens.into_iter().peekable(), + stack: vec![], + } + } + + /// Fetch the next token from the input stream as a Symbol. + fn next_token(&mut self) -> Symbol { + Symbol::new(self.tokens.next()) + } + + /// Peek at the next token from the input stream, returning it as a Symbol. + /// The stream is unchanged and will return the same Symbol on subsequent + /// calls to `next()` or `peek()`. + fn peek(&mut self) -> Symbol { + Symbol::new(self.tokens.peek().map(|s| s.to_os_string())) + } + + /// Test if the next token in the stream is a BOOLOP (-a or -o), without + /// removing the token from the stream. + fn peek_is_boolop(&mut self) -> bool { + if let Symbol::BoolOp(_) = self.peek() { + true + } else { + false + } + } + + /// Parse an expression. + /// + /// EXPR → TERM | EXPR BOOLOP EXPR + fn expr(&mut self) { + if !self.peek_is_boolop() { + self.term(); + } + self.maybe_boolop(); + } + + /// Parse a term token and possible subsequent symbols: "(", "!", UOP, + /// literal, or None. + fn term(&mut self) { + let symbol = self.next_token(); + + match symbol { + Symbol::LParen => self.lparen(), + Symbol::Bang => self.bang(), + Symbol::StrlenOp(_) => self.uop(symbol), + Symbol::FiletestOp(_) => self.uop(symbol), + Symbol::None => self.stack.push(symbol), + literal => self.literal(literal), + } + } + + /// Parse a (possibly) parenthesized expression. + /// + /// test has no reserved keywords, so "(" will be interpreted as a literal + /// if it is followed by nothing or a comparison operator OP. + fn lparen(&mut self) { + match self.peek() { + // lparen is a literal when followed by nothing or comparison + Symbol::None | Symbol::StringOp(_) | Symbol::IntOp(_) | Symbol::FileOp(_) => { + self.literal(Symbol::Literal(OsString::from("("))); + } + // empty parenthetical + Symbol::Literal(s) if s == ")" => {} + _ => { + self.expr(); + match self.next_token() { + Symbol::Literal(s) if s == ")" => (), + _ => panic!("expected ‘)’"), + } + } + } + } + + /// Parse a (possibly) negated expression. + /// + /// Example cases: + /// + /// * `! =`: negate the result of the implicit string length test of `=` + /// * `! = foo`: compare the literal strings `!` and `foo` + /// * `! `: negate the result of the expression + /// + fn bang(&mut self) { + if let Symbol::StringOp(_) | Symbol::IntOp(_) | Symbol::FileOp(_) = self.peek() { + // we need to peek ahead one more token to disambiguate the first + // two cases listed above: case 1 — `! ` — and + // case 2: ` OP str`. + let peek2 = self.tokens.clone().nth(1); + + if peek2.is_none() { + // op is literal + let op = self.next_token().into_literal(); + self.stack.push(op); + self.stack.push(Symbol::Bang); + } else { + // bang is literal; parsing continues with op + self.literal(Symbol::Literal(OsString::from("!"))); + } + } else { + self.expr(); + self.stack.push(Symbol::Bang); + } + } + + /// Peek at the next token and parse it as a BOOLOP or string literal, + /// as appropriate. + fn maybe_boolop(&mut self) { + if self.peek_is_boolop() { + let token = self.tokens.next().unwrap(); // safe because we peeked + + // BoolOp by itself interpreted as Literal + if let Symbol::None = self.peek() { + self.literal(Symbol::Literal(token)) + } else { + self.boolop(Symbol::BoolOp(token)) + } + } + } + + /// Parse a Boolean expression. + /// + /// Logical and (-a) has higher precedence than or (-o), so in an + /// expression like `foo -o '' -a ''`, the and subexpression is evaluated + /// first. + fn boolop(&mut self, op: Symbol) { + if op == Symbol::BoolOp(OsString::from("-a")) { + self.term(); + self.stack.push(op); + self.maybe_boolop(); + } else { + self.expr(); + self.stack.push(op); + } + } + + /// Parse a (possible) unary argument test (string length or file + /// attribute check). + /// + /// If a UOP is followed by nothing it is interpreted as a literal string. + fn uop(&mut self, op: Symbol) { + match self.next_token() { + Symbol::None => self.stack.push(op.into_literal()), + symbol => { + self.stack.push(symbol.into_literal()); + self.stack.push(op); + } + } + } + + /// Parse a string literal, optionally followed by a comparison operator + /// and a second string literal. + fn literal(&mut self, token: Symbol) { + self.stack.push(token.into_literal()); + + // EXPR → str OP str + match self.peek() { + Symbol::StringOp(_) | Symbol::IntOp(_) | Symbol::FileOp(_) => { + let op = self.next_token(); + + match self.next_token() { + Symbol::None => panic!("missing argument after {:?}", op), + token => self.stack.push(token.into_literal()), + } + + self.stack.push(op); + } + _ => {} + } + } + + /// Parser entry point: parse the token stream `self.tokens`, storing the + /// resulting `Symbol` stack in `self.stack`. + fn parse(&mut self) -> Result<(), String> { + self.expr(); + + match self.tokens.next() { + Some(token) => Err(format!("extra argument ‘{}’", token.to_string_lossy())), + None => Ok(()), + } + } +} + +/// Parse the token stream `args`, returning a `Symbol` stack representing the +/// operations to perform in postfix order. +pub fn parse(args: Vec) -> Result, String> { + let mut p = Parser::new(args); + p.parse()?; + Ok(p.stack) +} diff --git a/src/uu/test/src/test.rs b/src/uu/test/src/test.rs index f882ff5ae..3e97af0a6 100644 --- a/src/uu/test/src/test.rs +++ b/src/uu/test/src/test.rs @@ -1,144 +1,154 @@ -// * This file is part of the uutils coreutils package. -// * -// * (c) mahkoh (ju.orth [at] gmail [dot] com) -// * -// * For the full copyright and license information, please view the LICENSE -// * file that was distributed with this source code. +// This file is part of the uutils coreutils package. +// +// (c) mahkoh (ju.orth [at] gmail [dot] com) +// (c) Daniel Rocco +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. // spell-checker:ignore (ToDO) retval paren prec subprec cond -use std::collections::HashMap; -use std::str::from_utf8; +mod parser; -static NAME: &str = "test"; +use parser::{parse, Symbol}; +use std::ffi::{OsStr, OsString}; pub fn uumain(args: impl uucore::Args) -> i32 { - let args: Vec<_> = args.collect(); - // This is completely disregarding valid windows paths that aren't valid unicode - let args = args - .iter() - .map(|a| a.to_str().unwrap().as_bytes()) - .collect::>(); - if args.is_empty() { - return 2; - } - let args = if !args[0].ends_with(NAME.as_bytes()) { - &args[1..] - } else { - &args[..] - }; - let args = match args[0] { - b"[" => match args[args.len() - 1] { - b"]" => &args[1..args.len() - 1], - _ => return 2, - }, - _ => &args[1..args.len()], - }; - let mut error = false; - let retval = 1 - parse_expr(args, &mut error) as i32; - if error { - 2 - } else { - retval - } -} + // TODO: handle being called as `[` + let args: Vec<_> = args.skip(1).collect(); -fn one(args: &[&[u8]]) -> bool { - !args[0].is_empty() -} + let result = parse(args).and_then(|mut stack| eval(&mut stack)); -fn two(args: &[&[u8]], error: &mut bool) -> bool { - match args[0] { - b"!" => !one(&args[1..]), - b"-b" => path(args[1], PathCondition::BlockSpecial), - b"-c" => path(args[1], PathCondition::CharacterSpecial), - b"-d" => path(args[1], PathCondition::Directory), - b"-e" => path(args[1], PathCondition::Exists), - b"-f" => path(args[1], PathCondition::Regular), - b"-g" => path(args[1], PathCondition::GroupIdFlag), - b"-h" => path(args[1], PathCondition::SymLink), - b"-L" => path(args[1], PathCondition::SymLink), - b"-n" => one(&args[1..]), - b"-p" => path(args[1], PathCondition::Fifo), - b"-r" => path(args[1], PathCondition::Readable), - b"-S" => path(args[1], PathCondition::Socket), - b"-s" => path(args[1], PathCondition::NonEmpty), - b"-t" => isatty(args[1]), - b"-u" => path(args[1], PathCondition::UserIdFlag), - b"-w" => path(args[1], PathCondition::Writable), - b"-x" => path(args[1], PathCondition::Executable), - b"-z" => !one(&args[1..]), - _ => { - *error = true; - false - } - } -} - -fn three(args: &[&[u8]], error: &mut bool) -> bool { - match args[1] { - b"=" => args[0] == args[2], - b"==" => args[0] == args[2], - b"!=" => args[0] != args[2], - b"-eq" => integers(args[0], args[2], IntegerCondition::Equal), - b"-ne" => integers(args[0], args[2], IntegerCondition::Unequal), - b"-gt" => integers(args[0], args[2], IntegerCondition::Greater), - b"-ge" => integers(args[0], args[2], IntegerCondition::GreaterEqual), - b"-lt" => integers(args[0], args[2], IntegerCondition::Less), - b"-le" => integers(args[0], args[2], IntegerCondition::LessEqual), - _ => match args[0] { - b"!" => !two(&args[1..], error), - _ => { - *error = true; - false + match result { + Ok(result) => { + if result { + 0 + } else { + 1 } - }, - } -} - -fn four(args: &[&[u8]], error: &mut bool) -> bool { - match args[0] { - b"!" => !three(&args[1..], error), - _ => { - *error = true; - false + } + Err(e) => { + eprintln!("test: {}", e); + 2 } } } -enum IntegerCondition { - Equal, - Unequal, - Greater, - GreaterEqual, - Less, - LessEqual, -} +/// Evaluate a stack of Symbols, returning the result of the evaluation or +/// an error message if evaluation failed. +fn eval(stack: &mut Vec) -> Result { + macro_rules! pop_literal { + () => { + match stack.pop() { + Some(Symbol::Literal(s)) => s, + _ => panic!(), + } + }; + } -fn integers(a: &[u8], b: &[u8], cond: IntegerCondition) -> bool { - let (a, b): (&str, &str) = match (from_utf8(a), from_utf8(b)) { - (Ok(a), Ok(b)) => (a, b), - _ => return false, - }; - let (a, b): (i64, i64) = match (a.parse(), b.parse()) { - (Ok(a), Ok(b)) => (a, b), - _ => return false, - }; - match cond { - IntegerCondition::Equal => a == b, - IntegerCondition::Unequal => a != b, - IntegerCondition::Greater => a > b, - IntegerCondition::GreaterEqual => a >= b, - IntegerCondition::Less => a < b, - IntegerCondition::LessEqual => a <= b, + let s = stack.pop(); + + match s { + Some(Symbol::Bang) => { + let result = eval(stack)?; + + Ok(!result) + } + Some(Symbol::StringOp(op)) => { + let b = stack.pop(); + let a = stack.pop(); + Ok(if op == "=" { a == b } else { a != b }) + } + Some(Symbol::IntOp(op)) => { + let b = pop_literal!(); + let a = pop_literal!(); + + Ok(integers(&a, &b, &op)?) + } + Some(Symbol::FileOp(_op)) => unimplemented!(), + Some(Symbol::StrlenOp(op)) => { + let s = match stack.pop() { + Some(Symbol::Literal(s)) => s, + Some(Symbol::None) => OsString::from(""), + None => { + return Ok(true); + } + _ => { + return Err(format!("missing argument after ‘{:?}’", op)); + } + }; + + Ok(if op == "-z" { + s.is_empty() + } else { + !s.is_empty() + }) + } + Some(Symbol::FiletestOp(op)) => { + let op = op.to_string_lossy(); + + let f = pop_literal!(); + + Ok(match op.as_ref() { + "-b" => path(&f, PathCondition::BlockSpecial), + "-c" => path(&f, PathCondition::CharacterSpecial), + "-d" => path(&f, PathCondition::Directory), + "-e" => path(&f, PathCondition::Exists), + "-f" => path(&f, PathCondition::Regular), + "-g" => path(&f, PathCondition::GroupIdFlag), + "-h" => path(&f, PathCondition::SymLink), + "-L" => path(&f, PathCondition::SymLink), + "-p" => path(&f, PathCondition::Fifo), + "-r" => path(&f, PathCondition::Readable), + "-S" => path(&f, PathCondition::Socket), + "-s" => path(&f, PathCondition::NonEmpty), + "-t" => isatty(&f)?, + "-u" => path(&f, PathCondition::UserIdFlag), + "-w" => path(&f, PathCondition::Writable), + "-x" => path(&f, PathCondition::Executable), + _ => panic!(), + }) + } + Some(Symbol::Literal(s)) => Ok(!s.is_empty()), + Some(Symbol::None) => Ok(false), + Some(Symbol::BoolOp(op)) => { + let b = eval(stack)?; + let a = eval(stack)?; + + Ok(if op == "-a" { a && b } else { a || b }) + } + None => Ok(false), + _ => Err("expected value".to_string()), } } -fn isatty(fd: &[u8]) -> bool { - from_utf8(fd) - .ok() - .and_then(|s| s.parse().ok()) - .map_or(false, |i| { +fn integers(a: &OsStr, b: &OsStr, cond: &OsStr) -> Result { + let format_err = |value| format!("invalid integer ‘{}’", value); + + let a = a.to_string_lossy(); + let a: i64 = a.parse().map_err(|_| format_err(a))?; + + let b = b.to_string_lossy(); + let b: i64 = b.parse().map_err(|_| format_err(b))?; + + let cond = cond.to_string_lossy(); + Ok(match cond.as_ref() { + "-eq" => a == b, + "-ne" => a != b, + "-gt" => a > b, + "-ge" => a >= b, + "-lt" => a < b, + "-le" => a <= b, + _ => return Err(format!("unknown operator ‘{}’", cond)), + }) +} + +fn isatty(fd: &OsStr) -> Result { + let fd = fd.to_string_lossy(); + + fd.parse() + .map_err(|_| format!("invalid integer ‘{}’", fd)) + .map(|i| { #[cfg(not(target_os = "redox"))] unsafe { libc::isatty(i) == 1 @@ -148,173 +158,6 @@ fn isatty(fd: &[u8]) -> bool { }) } -fn dispatch(args: &mut &[&[u8]], error: &mut bool) -> bool { - let (val, idx) = match args.len() { - 0 => { - *error = true; - (false, 0) - } - 1 => (one(*args), 1), - 2 => dispatch_two(args, error), - 3 => dispatch_three(args, error), - _ => dispatch_four(args, error), - }; - *args = &(*args)[idx..]; - val -} - -fn dispatch_two(args: &mut &[&[u8]], error: &mut bool) -> (bool, usize) { - let val = two(*args, error); - if *error { - *error = false; - (one(*args), 1) - } else { - (val, 2) - } -} - -fn dispatch_three(args: &mut &[&[u8]], error: &mut bool) -> (bool, usize) { - let val = three(*args, error); - if *error { - *error = false; - dispatch_two(args, error) - } else { - (val, 3) - } -} - -fn dispatch_four(args: &mut &[&[u8]], error: &mut bool) -> (bool, usize) { - let val = four(*args, error); - if *error { - *error = false; - dispatch_three(args, error) - } else { - (val, 4) - } -} - -#[derive(Clone, Copy)] -enum Precedence { - Unknown = 0, - Paren, // FIXME: this is useless (parentheses have not been implemented) - Or, - And, - BUnOp, - BinOp, - UnOp, -} - -fn parse_expr(mut args: &[&[u8]], error: &mut bool) -> bool { - if args.is_empty() { - false - } else { - let hashmap = setup_hashmap(); - let lhs = dispatch(&mut args, error); - - if !args.is_empty() { - parse_expr_helper(&hashmap, &mut args, lhs, Precedence::Unknown, error) - } else { - lhs - } - } -} - -fn parse_expr_helper<'a>( - hashmap: &HashMap<&'a [u8], Precedence>, - args: &mut &[&'a [u8]], - mut lhs: bool, - min_prec: Precedence, - error: &mut bool, -) -> bool { - let mut prec = *hashmap.get(&args[0]).unwrap_or_else(|| { - *error = true; - &min_prec - }); - while !*error && !args.is_empty() && prec as usize >= min_prec as usize { - let op = args[0]; - *args = &(*args)[1..]; - let mut rhs = dispatch(args, error); - while !args.is_empty() { - let subprec = *hashmap.get(&args[0]).unwrap_or_else(|| { - *error = true; - &min_prec - }); - if subprec as usize <= prec as usize || *error { - break; - } - rhs = parse_expr_helper(hashmap, args, rhs, subprec, error); - } - lhs = match prec { - Precedence::UnOp | Precedence::BUnOp => { - *error = true; - false - } - Precedence::And => lhs && rhs, - Precedence::Or => lhs || rhs, - Precedence::BinOp => three( - &[ - if lhs { b" " } else { b"" }, - op, - if rhs { b" " } else { b"" }, - ], - error, - ), - Precedence::Paren => unimplemented!(), // TODO: implement parentheses - _ => unreachable!(), - }; - if !args.is_empty() { - prec = *hashmap.get(&args[0]).unwrap_or_else(|| { - *error = true; - &min_prec - }); - } - } - lhs -} - -#[inline] -fn setup_hashmap<'a>() -> HashMap<&'a [u8], Precedence> { - let mut hashmap = HashMap::<&'a [u8], Precedence>::new(); - - hashmap.insert(b"-b", Precedence::UnOp); - hashmap.insert(b"-c", Precedence::UnOp); - hashmap.insert(b"-d", Precedence::UnOp); - hashmap.insert(b"-e", Precedence::UnOp); - hashmap.insert(b"-f", Precedence::UnOp); - hashmap.insert(b"-g", Precedence::UnOp); - hashmap.insert(b"-h", Precedence::UnOp); - hashmap.insert(b"-L", Precedence::UnOp); - hashmap.insert(b"-n", Precedence::UnOp); - hashmap.insert(b"-p", Precedence::UnOp); - hashmap.insert(b"-r", Precedence::UnOp); - hashmap.insert(b"-S", Precedence::UnOp); - hashmap.insert(b"-s", Precedence::UnOp); - hashmap.insert(b"-t", Precedence::UnOp); - hashmap.insert(b"-u", Precedence::UnOp); - hashmap.insert(b"-w", Precedence::UnOp); - hashmap.insert(b"-x", Precedence::UnOp); - hashmap.insert(b"-z", Precedence::UnOp); - - hashmap.insert(b"=", Precedence::BinOp); - hashmap.insert(b"!=", Precedence::BinOp); - hashmap.insert(b"-eq", Precedence::BinOp); - hashmap.insert(b"-ne", Precedence::BinOp); - hashmap.insert(b"-gt", Precedence::BinOp); - hashmap.insert(b"-ge", Precedence::BinOp); - hashmap.insert(b"-lt", Precedence::BinOp); - hashmap.insert(b"-le", Precedence::BinOp); - - hashmap.insert(b"!", Precedence::BUnOp); - - hashmap.insert(b"-a", Precedence::And); - hashmap.insert(b"-o", Precedence::Or); - - hashmap.insert(b"(", Precedence::Paren); - hashmap.insert(b")", Precedence::Paren); - - hashmap -} - #[derive(Eq, PartialEq)] enum PathCondition { BlockSpecial, @@ -334,14 +177,10 @@ enum PathCondition { } #[cfg(not(windows))] -fn path(path: &[u8], cond: PathCondition) -> bool { - use std::ffi::OsStr; +fn path(path: &OsStr, cond: PathCondition) -> bool { use std::fs::{self, Metadata}; - use std::os::unix::ffi::OsStrExt; use std::os::unix::fs::{FileTypeExt, MetadataExt}; - let path = OsStr::from_bytes(path); - const S_ISUID: u32 = 0o4000; const S_ISGID: u32 = 0o2000; @@ -403,13 +242,14 @@ fn path(path: &[u8], cond: PathCondition) -> bool { } #[cfg(windows)] -fn path(path: &[u8], cond: PathCondition) -> bool { +fn path(path: &OsStr, cond: PathCondition) -> bool { use std::fs::metadata; - let path = from_utf8(path).unwrap(); + let stat = match metadata(path) { Ok(s) => s, _ => return false, }; + match cond { PathCondition::BlockSpecial => false, PathCondition::CharacterSpecial => false, diff --git a/tests/by-util/test_test.rs b/tests/by-util/test_test.rs index 2173371fc..000013d9c 100644 --- a/tests/by-util/test_test.rs +++ b/tests/by-util/test_test.rs @@ -39,7 +39,6 @@ fn test_double_not_is_false() { } #[test] -#[ignore = "fixme: parse/evaluation error (code 2); GNU returns 1"] fn test_and_not_is_false() { new_ucmd!().args(&["-a", "!"]).run().status_code(1); } @@ -51,7 +50,6 @@ fn test_not_and_is_false() { } #[test] -#[ignore = "fixme: parse/evaluation error (code 2); GNU returns 0"] fn test_not_and_not_succeeds() { new_ucmd!().args(&["!", "-a", "!"]).succeeds(); } @@ -62,7 +60,6 @@ fn test_simple_or() { } #[test] -#[ignore = "fixme: parse/evaluation error (code 0); GNU returns 1"] fn test_negated_or() { new_ucmd!() .args(&["!", "foo", "-o", "bar"]) @@ -111,13 +108,8 @@ fn test_solo_paren_is_literal() { } #[test] -#[ignore = "GNU considers this an error"] fn test_solo_empty_parenthetical_is_error() { - new_ucmd!() - .args(&["(", ")"]) - .run() - .status_code(2) - .stderr_is("test: missing argument after ‘)’"); + new_ucmd!().args(&["(", ")"]).run().status_code(2); } #[test] @@ -248,7 +240,6 @@ fn test_negative_int_compare() { } #[test] -#[ignore = "fixme: error reporting"] fn test_float_inequality_is_error() { new_ucmd!() .args(&["123.45", "-ge", "6"]) @@ -257,6 +248,32 @@ fn test_float_inequality_is_error() { .stderr_is("test: invalid integer ‘123.45’"); } +#[test] +#[cfg(not(windows))] +fn test_invalid_utf8_integer_compare() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let source = [0x66, 0x6f, 0x80, 0x6f]; + let arg = OsStr::from_bytes(&source[..]); + + let mut cmd = new_ucmd!(); + cmd.arg("123").arg("-ne"); + cmd.raw.arg(arg); + + cmd.run() + .status_code(2) + .stderr_is("test: invalid integer ‘fo�o’"); + + let mut cmd = new_ucmd!(); + cmd.raw.arg(arg); + cmd.arg("-eq").arg("456"); + + cmd.run() + .status_code(2) + .stderr_is("test: invalid integer ‘fo�o’"); +} + #[test] #[ignore = "fixme: parse/evaluation error (code 2); GNU returns 1"] fn test_file_is_itself() { @@ -445,6 +462,14 @@ fn test_op_prec_and_or_1() { new_ucmd!().args(&[" ", "-o", "", "-a", ""]).succeeds(); } +#[test] +fn test_op_prec_and_or_1_overridden_by_parentheses() { + new_ucmd!() + .args(&["(", " ", "-o", "", ")", "-a", ""]) + .run() + .status_code(1); +} + #[test] fn test_op_prec_and_or_2() { new_ucmd!() @@ -452,6 +477,45 @@ fn test_op_prec_and_or_2() { .succeeds(); } +#[test] +fn test_op_prec_and_or_2_overridden_by_parentheses() { + new_ucmd!() + .args(&["", "-a", "(", "", "-o", " ", ")", "-a", " "]) + .run() + .status_code(1); +} + +#[test] +#[ignore = "fixme: error reporting"] +fn test_dangling_parenthesis() { + new_ucmd!() + .args(&["(", "(", "a", "!=", "b", ")", "-o", "-n", "c"]) + .run() + .status_code(2); + new_ucmd!() + .args(&["(", "(", "a", "!=", "b", ")", "-o", "-n", "c", ")"]) + .succeeds(); +} + +#[test] +fn test_complicated_parenthesized_expression() { + new_ucmd!() + .args(&[ + "(", "(", "!", "(", "a", "=", "b", ")", "-o", "c", "=", "d", ")", "-a", "(", "q", "!=", + "r", ")", ")", + ]) + .succeeds(); +} + +#[test] +fn test_erroneous_parenthesized_expression() { + new_ucmd!() + .args(&["a", "!=", "(", "b", "-a", "b", ")", "!=", "c"]) + .run() + .status_code(2) + .stderr_is("test: extra argument ‘b’"); +} + #[test] fn test_or_as_filename() { new_ucmd!() From f5c7d9bd80c27341efad1af981e69d9e47d47234 Mon Sep 17 00:00:00 2001 From: Dean Li Date: Sat, 1 May 2021 09:51:31 +0800 Subject: [PATCH 09/17] link: replace getopts with clap --- Cargo.lock | 1 + src/uu/link/Cargo.toml | 1 + src/uu/link/src/link.rs | 44 +++++++++++++++++++++++++++++------------ 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 31787e626..217533df6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2050,6 +2050,7 @@ dependencies = [ name = "uu_link" version = "0.0.6" dependencies = [ + "clap", "libc", "uucore", "uucore_procs", diff --git a/src/uu/link/Cargo.toml b/src/uu/link/Cargo.toml index 13c3453cf..14a6ac7c9 100644 --- a/src/uu/link/Cargo.toml +++ b/src/uu/link/Cargo.toml @@ -18,6 +18,7 @@ path = "src/link.rs" libc = "0.2.42" uucore = { version=">=0.0.8", package="uucore", path="../../uucore" } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } +clap = "2.33" [[bin]] name = "link" diff --git a/src/uu/link/src/link.rs b/src/uu/link/src/link.rs index b82d7cfac..bd8b33355 100644 --- a/src/uu/link/src/link.rs +++ b/src/uu/link/src/link.rs @@ -8,14 +8,21 @@ #[macro_use] extern crate uucore; +use clap::{App, Arg}; use std::fs::hard_link; use std::io::Error; use std::path::Path; -use uucore::InvalidEncodingHandling; -static SYNTAX: &str = "[OPTIONS] FILE1 FILE2"; -static SUMMARY: &str = "Create a link named FILE2 to FILE1"; -static LONG_HELP: &str = ""; +static VERSION: &str = env!("CARGO_PKG_VERSION"); +static ABOUT: &str = "Call the link function to create a link named FILE2 to an existing FILE1."; + +pub mod options { + pub static FILES: &str = "FILES"; +} + +fn get_usage() -> String { + format!("{0} FILE1 FILE2", executable!()) +} pub fn normalize_error_message(e: Error) -> String { match e.raw_os_error() { @@ -25,16 +32,27 @@ pub fn normalize_error_message(e: Error) -> String { } pub fn uumain(args: impl uucore::Args) -> i32 { - let matches = app!(SYNTAX, SUMMARY, LONG_HELP).parse( - args.collect_str(InvalidEncodingHandling::Ignore) - .accept_any(), - ); - if matches.free.len() != 2 { - crash!(1, "{}", msg_wrong_number_of_arguments!(2)); - } + let usage = get_usage(); + let matches = App::new(executable!()) + .version(VERSION) + .about(ABOUT) + .usage(&usage[..]) + .arg( + Arg::with_name(options::FILES) + .hidden(true) + .required(true) + .min_values(2) + .max_values(2) + .takes_value(true), + ) + .get_matches_from(args); - let old = Path::new(&matches.free[0]); - let new = Path::new(&matches.free[1]); + let files: Vec<_> = matches + .values_of_os(options::FILES) + .unwrap_or_default() + .collect(); + let old = Path::new(files[0]); + let new = Path::new(files[1]); match hard_link(old, new) { Ok(_) => 0, From 5e82b195bd3f8da8e34e4ccf061227498af3abf1 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 2 May 2021 09:35:00 +0200 Subject: [PATCH 10/17] ls: remove redundant import --- src/uu/ls/src/ls.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index dc7ea4c08..381612189 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -39,8 +39,6 @@ use std::{ time::Duration, }; -use chrono; - use term_grid::{Cell, Direction, Filling, Grid, GridOptions}; use unicode_width::UnicodeWidthStr; From e723b8db4321a56ddfe1fec5c538ac88370aff40 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 2 May 2021 09:35:59 +0200 Subject: [PATCH 11/17] factor: unneeded statement --- src/uu/factor/src/factor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index 5a85194c4..138254b51 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -172,7 +172,7 @@ pub fn factor(mut n: u64) -> Factors { #[cfg(feature = "coz")] coz::end!("factorization"); - return r; + r } #[cfg(test)] From 108f9928ef77a6d1b811140a87a0ad053e400ef5 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 2 May 2021 09:39:09 +0200 Subject: [PATCH 12/17] cp: fix 'variable does not need to be mutable' --- src/uu/cp/src/cp.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index ca564e37c..3d6faf66a 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -551,6 +551,10 @@ impl FromStr for Attribute { fn add_all_attributes() -> Vec { use Attribute::*; + #[cfg(target_os = "windows")] + let attr = vec![Ownership, Timestamps, Context, Xattr, Links]; + + #[cfg(not(target_os = "windows"))] let mut attr = vec![Ownership, Timestamps, Context, Xattr, Links]; #[cfg(unix)] From c03a7d88561106f1fef24c1ea805513bd0283ff2 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 2 May 2021 09:41:04 +0200 Subject: [PATCH 13/17] uname(test): fix 'unused variable: result' --- tests/by-util/test_uname.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_uname.rs b/tests/by-util/test_uname.rs index 6b8d2d59d..da901d985 100644 --- a/tests/by-util/test_uname.rs +++ b/tests/by-util/test_uname.rs @@ -36,7 +36,12 @@ fn test_uname_kernel_version() { fn test_uname_kernel() { let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("-o").succeeds(); #[cfg(target_os = "linux")] - assert!(result.stdout_str().to_lowercase().contains("linux")); + { + let result = ucmd.arg("-o").succeeds(); + assert!(result.stdout_str().to_lowercase().contains("linux")); + } + + #[cfg(not(target_os = "linux"))] + let result = ucmd.arg("-o").succeeds(); } From a34b49ad6053e29ba457f2a43deab21f48ccdf68 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 2 May 2021 09:42:30 +0200 Subject: [PATCH 14/17] relpath(test) - fix: 'value assigned to 'result_stdout' is never read' --- tests/by-util/test_relpath.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/by-util/test_relpath.rs b/tests/by-util/test_relpath.rs index cc17b45c3..5094d25a8 100644 --- a/tests/by-util/test_relpath.rs +++ b/tests/by-util/test_relpath.rs @@ -103,7 +103,7 @@ fn test_relpath_with_from_with_d() { at.mkdir_all(from); // d is part of subpath -> expect relative path - let mut result_stdout = scene + let mut _result_stdout = scene .ucmd() .arg(to) .arg(from) @@ -112,17 +112,17 @@ fn test_relpath_with_from_with_d() { .stdout_move_str(); // relax rules for windows test environment #[cfg(not(windows))] - assert!(Path::new(&result_stdout).is_relative()); + assert!(Path::new(&_result_stdout).is_relative()); // d is not part of subpath -> expect absolut path - result_stdout = scene + _result_stdout = scene .ucmd() .arg(to) .arg(from) .arg("-dnon_existing") .succeeds() .stdout_move_str(); - assert!(Path::new(&result_stdout).is_absolute()); + assert!(Path::new(&_result_stdout).is_absolute()); } } @@ -135,12 +135,12 @@ fn test_relpath_no_from_no_d() { let to: &str = &convert_path(test.to); at.mkdir_all(to); - let result_stdout = scene.ucmd().arg(to).succeeds().stdout_move_str(); + let _result_stdout = scene.ucmd().arg(to).succeeds().stdout_move_str(); #[cfg(not(windows))] - assert_eq!(result_stdout, format!("{}\n", to)); + assert_eq!(_result_stdout, format!("{}\n", to)); // relax rules for windows test environment #[cfg(windows)] - assert!(result_stdout.ends_with(&format!("{}\n", to))); + assert!(_result_stdout.ends_with(&format!("{}\n", to))); } } From d0512618d5212d8991556a98c44bb688144c4d45 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 2 May 2021 09:43:32 +0200 Subject: [PATCH 15/17] refresh cargo.lock with recent updates --- Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 091758278..c74f4e2be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1277,9 +1277,9 @@ dependencies = [ [[package]] name = "regex" -version = "1.5.2" +version = "1.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1efb2352a0f4d4b128f734b5c44c79ff80117351138733f12f982fe3e2b13343" +checksum = "ce5f1ceb7f74abbce32601642fcf8e8508a8a8991e0621c7d750295b9095702b" dependencies = [ "aho-corasick", "memchr 2.4.0", @@ -1297,9 +1297,9 @@ dependencies = [ [[package]] name = "regex-syntax" -version = "0.6.24" +version = "0.6.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "00efb87459ba4f6fb2169d20f68565555688e1250ee6825cdf6254f8b48fafb2" +checksum = "f497285884f3fcff424ffc933e56d7cbca511def0c9831a7f9b5f6153e3cc89b" [[package]] name = "remove_dir_all" From 09178360d8286a83e683c16ac18e7e622c1d92fb Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 2 May 2021 10:30:28 +0200 Subject: [PATCH 16/17] date: unneeded 'return' statement --- src/uu/date/src/date.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index 43573437d..317fd72d4 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -348,7 +348,7 @@ fn set_system_datetime(_date: DateTime) -> i32 { #[cfg(target_os = "macos")] fn set_system_datetime(_date: DateTime) -> i32 { eprintln!("date: setting the date is not supported by macOS"); - return 1; + 1 } #[cfg(all(unix, not(target_os = "macos")))] From 9554710ab5d71ce9d42dd86d41e752eea607f4ec Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 2 May 2021 10:31:28 +0200 Subject: [PATCH 17/17] cat: the function 'unistd::write' doesn't need a mutable reference --- src/uu/cat/src/splice.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cat/src/splice.rs b/src/uu/cat/src/splice.rs index ccc625467..bd6be60f1 100644 --- a/src/uu/cat/src/splice.rs +++ b/src/uu/cat/src/splice.rs @@ -81,7 +81,7 @@ fn copy_exact(read_fd: RawFd, write_fd: RawFd, num_bytes: usize) -> nix::Result< let mut buf = [0; BUF_SIZE]; loop { let read = unistd::read(read_fd, &mut buf[..left])?; - let written = unistd::write(write_fd, &mut buf[..read])?; + let written = unistd::write(write_fd, &buf[..read])?; left -= written; if left == 0 { break;