Skip to content

Commit

Permalink
numfmt: add --invalid option (uutils#4249)
Browse files Browse the repository at this point in the history
* numfmt: add invalid option

* numfmt: return code 0 if ignore or warn

* numfmt: implement all --invalid modes

* numfmt: validate stdout and stderr

* numfmt: remove unnecessary code

* numfmt: apply formatting

* numfmt: fix clippy issues

* numfmt: fix failing test cases

* numfmt: fix formatting

* numfmt: fix bug when handling broken pipe

* numfmt: fix bug where extra newline was added

* numfmt: add test cases for edge cases

* numfmt: simplify error handling

* numfmt: remove redundant if

* numfmt: add newline between functions

* numfmt: fix failing test cases

* numfmt: add support for arg numbers using --invalid

* numfmt: simplify error handling in value handlers

* numfmt: fix merge conflict and align prints

* numfmt: fix clippy suggestion

* numfmt: replace "valid" with "invalid" in tests

* numfmt: move INVALID to respect alph. order

* numfmt: move printlns outside of match to avoid duplication

Co-authored-by: Sylvestre Ledru <[email protected]>

* numfmt: remove empty line

---------

Co-authored-by: Daniel Hofstetter <[email protected]>
Co-authored-by: Sylvestre Ledru <[email protected]>
  • Loading branch information
3 people committed Jul 5, 2023
1 parent ac45b83 commit 60df688
Show file tree
Hide file tree
Showing 3 changed files with 241 additions and 21 deletions.
146 changes: 127 additions & 19 deletions src/uu/numfmt/src/numfmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ use crate::options::*;
use crate::units::{Result, Unit};
use clap::{crate_version, parser::ValueSource, Arg, ArgAction, ArgMatches, Command};
use std::io::{BufRead, Write};
use std::str::FromStr;

use units::{IEC_BASES, SI_BASES};
use uucore::display::Quotable;
use uucore::error::UResult;
use uucore::error::{UError, UResult};
use uucore::ranges::Range;
use uucore::{format_usage, help_about, help_section, help_usage};
use uucore::{format_usage, help_about, help_section, help_usage, show, show_error};

pub mod errors;
pub mod format;
Expand All @@ -28,36 +30,50 @@ const USAGE: &str = help_usage!("numfmt.md");

fn handle_args<'a>(args: impl Iterator<Item = &'a str>, options: &NumfmtOptions) -> UResult<()> {
for l in args {
match format_and_print(l, options) {
Ok(_) => Ok(()),
Err(e) => Err(NumfmtError::FormattingError(e.to_string())),
}?;
format_and_handle_validation(l, options)?;
}

Ok(())
}

fn handle_buffer<R>(input: R, options: &NumfmtOptions) -> UResult<()>
where
R: BufRead,
{
let mut lines = input.lines();
for (idx, line) in lines.by_ref().enumerate() {
match line {
Ok(l) if idx < options.header => {
println!("{l}");
for (idx, line_result) in input.lines().by_ref().enumerate() {
match line_result {
Ok(line) if idx < options.header => {
println!("{line}");
Ok(())
}
Ok(l) => match format_and_print(&l, options) {
Ok(_) => Ok(()),
Err(e) => Err(NumfmtError::FormattingError(e.to_string())),
},
Err(e) => Err(NumfmtError::IoError(e.to_string())),
Ok(line) => format_and_handle_validation(line.as_ref(), options),
Err(err) => return Err(Box::new(NumfmtError::IoError(err.to_string()))),
}?;
}
Ok(())
}

fn format_and_handle_validation(input_line: &str, options: &NumfmtOptions) -> UResult<()> {
let handled_line = format_and_print(input_line, options);

if let Err(error_message) = handled_line {
match options.invalid {
InvalidModes::Abort => {
return Err(Box::new(NumfmtError::FormattingError(error_message)));
}
InvalidModes::Fail => {
show!(NumfmtError::FormattingError(error_message));
}
InvalidModes::Warn => {
show_error!("{}", error_message);
}
InvalidModes::Ignore => {}
};
println!("{}", input_line);
}

Ok(())
}

fn parse_unit(s: &str) -> Result<Unit> {
match s {
"auto" => Ok(Unit::Auto),
Expand Down Expand Up @@ -201,6 +217,9 @@ fn parse_options(args: &ArgMatches) -> Result<NumfmtOptions> {
.get_one::<String>(options::SUFFIX)
.map(|s| s.to_owned());

let invalid =
InvalidModes::from_str(args.get_one::<String>(options::INVALID).unwrap()).unwrap();

Ok(NumfmtOptions {
transform,
padding,
Expand All @@ -210,6 +229,7 @@ fn parse_options(args: &ArgMatches) -> Result<NumfmtOptions> {
round,
suffix,
format,
invalid,
})
}

Expand Down Expand Up @@ -357,6 +377,17 @@ pub fn uu_app() -> Command {
)
.value_name("SUFFIX"),
)
.arg(
Arg::new(options::INVALID)
.long(options::INVALID)
.help(
"set the failure mode for invalid input; \
valid options are abort, fail, warn or ignore",
)
.default_value("abort")
.value_parser(["abort", "fail", "warn", "ignore"])
.value_name("INVALID"),
)
.arg(
Arg::new(options::NUMBER)
.hide(true)
Expand All @@ -366,9 +397,11 @@ pub fn uu_app() -> Command {

#[cfg(test)]
mod tests {
use uucore::error::get_exit_code;

use super::{
handle_buffer, parse_unit_size, parse_unit_size_suffix, FormatOptions, NumfmtOptions,
Range, RoundMethod, TransformOptions, Unit,
handle_args, handle_buffer, parse_unit_size, parse_unit_size_suffix, FormatOptions,
InvalidModes, NumfmtOptions, Range, RoundMethod, TransformOptions, Unit,
};
use std::io::{BufReader, Error, ErrorKind, Read};
struct MockBuffer {}
Expand All @@ -394,6 +427,7 @@ mod tests {
round: RoundMethod::Nearest,
suffix: None,
format: FormatOptions::default(),
invalid: InvalidModes::Abort,
}
}

Expand All @@ -409,6 +443,20 @@ mod tests {
assert_eq!(result.code(), 1);
}

#[test]
fn broken_buffer_returns_io_error_after_header() {
let mock_buffer = MockBuffer {};
let mut options = get_valid_options();
options.header = 0;
let result = handle_buffer(BufReader::new(mock_buffer), &options)
.expect_err("returned Ok after receiving IO error");
let result_debug = format!("{:?}", result);
let result_display = format!("{}", result);
assert_eq!(result_debug, "IoError(\"broken pipe\")");
assert_eq!(result_display, "broken pipe");
assert_eq!(result.code(), 1);
}

#[test]
fn non_numeric_returns_formatting_error() {
let input_value = b"135\nhello";
Expand All @@ -431,6 +479,66 @@ mod tests {
assert!(result.is_ok(), "did not return Ok for valid input");
}

#[test]
fn warn_returns_ok_for_invalid_input() {
let input_value = b"5\n4Q\n";
let mut options = get_valid_options();
options.invalid = InvalidModes::Warn;
let result = handle_buffer(BufReader::new(&input_value[..]), &options);
assert!(result.is_ok(), "did not return Ok for invalid input");
}

#[test]
fn ignore_returns_ok_for_invalid_input() {
let input_value = b"5\n4Q\n";
let mut options = get_valid_options();
options.invalid = InvalidModes::Ignore;
let result = handle_buffer(BufReader::new(&input_value[..]), &options);
assert!(result.is_ok(), "did not return Ok for invalid input");
}

#[test]
fn buffer_fail_returns_status_2_for_invalid_input() {
let input_value = b"5\n4Q\n";
let mut options = get_valid_options();
options.invalid = InvalidModes::Fail;
handle_buffer(BufReader::new(&input_value[..]), &options).unwrap();
assert!(
get_exit_code() == 2,
"should set exit code 2 for formatting errors"
);
}

#[test]
fn abort_returns_status_2_for_invalid_input() {
let input_value = b"5\n4Q\n";
let mut options = get_valid_options();
options.invalid = InvalidModes::Abort;
let result = handle_buffer(BufReader::new(&input_value[..]), &options);
assert!(result.is_err(), "did not return err for invalid input");
}

#[test]
fn args_fail_returns_status_2_for_invalid_input() {
let input_value = ["5", "4Q"].into_iter();
let mut options = get_valid_options();
options.invalid = InvalidModes::Fail;
handle_args(input_value, &options).unwrap();
assert!(
get_exit_code() == 2,
"should set exit code 2 for formatting errors"
);
}

#[test]
fn args_warn_returns_status_0_for_invalid_input() {
let input_value = ["5", "4Q"].into_iter();
let mut options = get_valid_options();
options.invalid = InvalidModes::Warn;
let result = handle_args(input_value, &options);
assert!(result.is_ok(), "did not return ok for invalid input");
}

#[test]
fn test_parse_unit_size() {
assert_eq!(1, parse_unit_size("1").unwrap());
Expand Down
41 changes: 41 additions & 0 deletions src/uu/numfmt/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub const FROM_UNIT: &str = "from-unit";
pub const FROM_UNIT_DEFAULT: &str = "1";
pub const HEADER: &str = "header";
pub const HEADER_DEFAULT: &str = "1";
pub const INVALID: &str = "invalid";
pub const NUMBER: &str = "NUMBER";
pub const PADDING: &str = "padding";
pub const ROUND: &str = "round";
Expand All @@ -29,6 +30,14 @@ pub struct TransformOptions {
pub to_unit: usize,
}

#[derive(Debug, PartialEq, Eq)]
pub enum InvalidModes {
Abort,
Fail,
Warn,
Ignore,
}

pub struct NumfmtOptions {
pub transform: TransformOptions,
pub padding: isize,
Expand All @@ -38,6 +47,7 @@ pub struct NumfmtOptions {
pub round: RoundMethod,
pub suffix: Option<String>,
pub format: FormatOptions,
pub invalid: InvalidModes,
}

#[derive(Clone, Copy)]
Expand Down Expand Up @@ -227,6 +237,20 @@ impl FromStr for FormatOptions {
}
}

impl FromStr for InvalidModes {
type Err = String;

fn from_str(s: &str) -> Result<Self, Self::Err> {
match s.to_lowercase().as_str() {
"abort" => Ok(Self::Abort),
"fail" => Ok(Self::Fail),
"warn" => Ok(Self::Warn),
"ignore" => Ok(Self::Ignore),
unknown => Err(format!("Unknown invalid mode: {unknown}")),
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -336,4 +360,21 @@ mod tests {
assert_eq!(expected_options, "%0'0'0'f".parse().unwrap());
assert_eq!(expected_options, "%'0'0'0f".parse().unwrap());
}

#[test]
fn test_set_invalid_mode() {
assert_eq!(Ok(InvalidModes::Abort), InvalidModes::from_str("abort"));
assert_eq!(Ok(InvalidModes::Abort), InvalidModes::from_str("ABORT"));

assert_eq!(Ok(InvalidModes::Fail), InvalidModes::from_str("fail"));
assert_eq!(Ok(InvalidModes::Fail), InvalidModes::from_str("FAIL"));

assert_eq!(Ok(InvalidModes::Ignore), InvalidModes::from_str("ignore"));
assert_eq!(Ok(InvalidModes::Ignore), InvalidModes::from_str("IGNORE"));

assert_eq!(Ok(InvalidModes::Warn), InvalidModes::from_str("warn"));
assert_eq!(Ok(InvalidModes::Warn), InvalidModes::from_str("WARN"));

assert!(InvalidModes::from_str("something unknown").is_err());
}
}
75 changes: 73 additions & 2 deletions tests/by-util/test_numfmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,8 +666,79 @@ fn test_invalid_stdin_number_in_middle_of_input() {
}

#[test]
fn test_invalid_argument_number_returns_status_2() {
new_ucmd!().args(&["hello"]).fails().code_is(2);
fn test_invalid_stdin_number_with_warn_returns_status_0() {
new_ucmd!()
.args(&["--invalid=warn"])
.pipe_in("4Q")
.succeeds()
.stdout_is("4Q\n")
.stderr_is("numfmt: invalid suffix in input: '4Q'\n");
}

#[test]
fn test_invalid_stdin_number_with_ignore_returns_status_0() {
new_ucmd!()
.args(&["--invalid=ignore"])
.pipe_in("4Q")
.succeeds()
.stdout_only("4Q\n");
}

#[test]
fn test_invalid_stdin_number_with_abort_returns_status_2() {
new_ucmd!()
.args(&["--invalid=abort"])
.pipe_in("4Q")
.fails()
.code_is(2)
.stderr_only("numfmt: invalid suffix in input: '4Q'\n");
}

#[test]
fn test_invalid_stdin_number_with_fail_returns_status_2() {
new_ucmd!()
.args(&["--invalid=fail"])
.pipe_in("4Q")
.fails()
.code_is(2)
.stdout_is("4Q\n")
.stderr_is("numfmt: invalid suffix in input: '4Q'\n");
}

#[test]
fn test_invalid_arg_number_with_warn_returns_status_0() {
new_ucmd!()
.args(&["--invalid=warn", "4Q"])
.succeeds()
.stdout_is("4Q\n")
.stderr_is("numfmt: invalid suffix in input: '4Q'\n");
}

#[test]
fn test_invalid_arg_number_with_ignore_returns_status_0() {
new_ucmd!()
.args(&["--invalid=ignore", "4Q"])
.succeeds()
.stdout_only("4Q\n");
}

#[test]
fn test_invalid_arg_number_with_abort_returns_status_2() {
new_ucmd!()
.args(&["--invalid=abort", "4Q"])
.fails()
.code_is(2)
.stderr_only("numfmt: invalid suffix in input: '4Q'\n");
}

#[test]
fn test_invalid_arg_number_with_fail_returns_status_2() {
new_ucmd!()
.args(&["--invalid=fail", "4Q"])
.fails()
.code_is(2)
.stdout_is("4Q\n")
.stderr_is("numfmt: invalid suffix in input: '4Q'\n");
}

#[test]
Expand Down

0 comments on commit 60df688

Please sign in to comment.