numfmt: add --invalid option (#4249)

* 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 <sledru@mozilla.com>

* numfmt: remove empty line

---------

Co-authored-by: Daniel Hofstetter <daniel.hofstetter@42dh.com>
Co-authored-by: Sylvestre Ledru <sledru@mozilla.com>

Author: 3 people

src/uu/numfmt/src/numfmt.rs

@@ -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;
@@ -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),
@@ -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,
@@ -210,6 +229,7 @@ fn parse_options(args: &ArgMatches) -> Result<NumfmtOptions> {
         round,
         suffix,
         format,
+        invalid,
     })
 }
 
@@ -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)
@@ -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 {}
@@ -394,6 +427,7 @@ mod tests {
             round: RoundMethod::Nearest,
             suffix: None,
             format: FormatOptions::default(),
+            invalid: InvalidModes::Abort,
         }
     }
 
@@ -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";
@@ -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());

src/uu/numfmt/src/options.rs

@@ -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";
@@ -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,
@@ -38,6 +47,7 @@ pub struct NumfmtOptions {
     pub round: RoundMethod,
     pub suffix: Option<String>,
     pub format: FormatOptions,
+    pub invalid: InvalidModes,
 }
 
 #[derive(Clone, Copy)]
@@ -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::*;
@@ -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());
+    }
 }

tests/by-util/test_numfmt.rs

@@ -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]