From 0c0e4dbda4f8594e2fe4dc32db66bebf24381a9d Mon Sep 17 00:00:00 2001
From: Daniel Hofstetter <daniel.hofstetter@42dh.com>
Date: Thu, 9 Jun 2022 08:18:50 +0200
Subject: [PATCH] unexpand: return Result instead of calling crash!

---
 src/uu/unexpand/src/unexpand.rs | 63 ++++++++++++++++++++++++---------
 tests/by-util/test_unexpand.rs  | 28 +++++++++++++++
 2 files changed, 74 insertions(+), 17 deletions(-)

diff --git a/src/uu/unexpand/src/unexpand.rs b/src/uu/unexpand/src/unexpand.rs
index 0ebfea613..0a143a808 100644
--- a/src/uu/unexpand/src/unexpand.rs
+++ b/src/uu/unexpand/src/unexpand.rs
@@ -12,12 +12,14 @@
 #[macro_use]
 extern crate uucore;
 use clap::{crate_version, Arg, Command};
+use std::error::Error;
+use std::fmt;
 use std::fs::File;
 use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Stdout, Write};
 use std::str::from_utf8;
 use unicode_width::UnicodeWidthChar;
 use uucore::display::Quotable;
-use uucore::error::{FromIo, UResult};
+use uucore::error::{FromIo, UError, UResult};
 use uucore::{format_usage, InvalidEncodingHandling};
 
 static NAME: &str = "unexpand";
@@ -27,28 +29,55 @@ static SUMMARY: &str = "Convert blanks in each FILE to tabs, writing to standard
 
 const DEFAULT_TABSTOP: usize = 8;
 
-fn tabstops_parse(s: &str) -> Vec<usize> {
+#[derive(Debug)]
+enum ParseError {
+    InvalidCharacter(String),
+    TabSizeCannotBeZero,
+    TabSizesMustBeAscending,
+}
+
+impl Error for ParseError {}
+impl UError for ParseError {}
+
+impl fmt::Display for ParseError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Self::InvalidCharacter(s) => {
+                write!(f, "tab size contains invalid character(s): {}", s.quote())
+            }
+            Self::TabSizeCannotBeZero => write!(f, "tab size cannot be 0"),
+            Self::TabSizesMustBeAscending => write!(f, "tab sizes must be ascending"),
+        }
+    }
+}
+
+fn tabstops_parse(s: &str) -> Result<Vec<usize>, ParseError> {
     let words = s.split(',');
 
-    let nums = words
-        .map(|sn| {
-            sn.parse()
-                .unwrap_or_else(|_| crash!(1, "{}\n", "tab size contains invalid character(s)"))
-        })
-        .collect::<Vec<usize>>();
+    let mut nums = Vec::new();
+
+    for word in words {
+        if let Ok(num) = word.parse() {
+            nums.push(num);
+        } else {
+            return Err(ParseError::InvalidCharacter(
+                word.trim_start_matches(char::is_numeric).to_string(),
+            ));
+        }
+    }
 
     if nums.iter().any(|&n| n == 0) {
-        crash!(1, "{}\n", "tab size cannot be 0");
+        return Err(ParseError::TabSizeCannotBeZero);
     }
 
     if let (false, _) = nums
         .iter()
-        .fold((true, 0), |(acc, last), &n| (acc && last <= n, n))
+        .fold((true, 0), |(acc, last), &n| (acc && last < n, n))
     {
-        crash!(1, "{}\n", "tab sizes must be ascending");
+        return Err(ParseError::TabSizesMustBeAscending);
     }
 
-    nums
+    Ok(nums)
 }
 
 mod options {
@@ -67,10 +96,10 @@ struct Options {
 }
 
 impl Options {
-    fn new(matches: &clap::ArgMatches) -> Self {
+    fn new(matches: &clap::ArgMatches) -> Result<Self, ParseError> {
         let tabstops = match matches.value_of(options::TABS) {
             None => vec![DEFAULT_TABSTOP],
-            Some(s) => tabstops_parse(s),
+            Some(s) => tabstops_parse(s)?,
         };
 
         let aflag = (matches.is_present(options::ALL) || matches.is_present(options::TABS))
@@ -82,12 +111,12 @@ impl Options {
             None => vec!["-".to_owned()],
         };
 
-        Self {
+        Ok(Self {
             files,
             tabstops,
             aflag,
             uflag,
-        }
+        })
     }
 }
 
@@ -99,7 +128,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
 
     let matches = uu_app().get_matches_from(args);
 
-    unexpand(&Options::new(&matches)).map_err_context(String::new)
+    unexpand(&Options::new(&matches)?).map_err_context(String::new)
 }
 
 pub fn uu_app<'a>() -> Command<'a> {
diff --git a/tests/by-util/test_unexpand.rs b/tests/by-util/test_unexpand.rs
index 692599361..ccb217393 100644
--- a/tests/by-util/test_unexpand.rs
+++ b/tests/by-util/test_unexpand.rs
@@ -155,3 +155,31 @@ fn unexpand_read_from_two_file() {
         .run()
         .success();
 }
+
+#[test]
+fn test_tabs_cannot_be_zero() {
+    new_ucmd!()
+        .arg("--tabs=0")
+        .fails()
+        .stderr_contains("tab size cannot be 0");
+}
+
+#[test]
+fn test_tabs_must_be_ascending() {
+    new_ucmd!()
+        .arg("--tabs=1,1")
+        .fails()
+        .stderr_contains("tab sizes must be ascending");
+}
+
+#[test]
+fn test_tabs_with_invalid_chars() {
+    new_ucmd!()
+        .arg("--tabs=x")
+        .fails()
+        .stderr_contains("tab size contains invalid character(s): 'x'");
+    new_ucmd!()
+        .arg("--tabs=1x2")
+        .fails()
+        .stderr_contains("tab size contains invalid character(s): 'x2'");
+}