factor: deduplicate code, refactor & optimize parsing

src/uu/factor/Cargo.toml

@@ -15,13 +15,9 @@ readme.workspace = true
 [lints]
 workspace = true
 
-[build-dependencies]
-num-traits = { workspace = true } # used in src/numerics.rs, which is included by build.rs
-
 [dependencies]
 clap = { workspace = true }
-num-traits = { workspace = true }
-uucore = { workspace = true, features = ["quoting-style"] }
+uucore = { workspace = true }
 memchr = { workspace = true }
 num-bigint = { workspace = true }
 num-prime = { workspace = true }

src/uu/factor/src/factor.rs

@@ -10,12 +10,12 @@ use std::fmt::Display;
 use std::io::BufRead;
 use std::io::{self, Write, stdin, stdout};
 use std::iter::once;
-use std::str::FromStr;
+use std::num::IntErrorKind;
 
 use clap::{Arg, ArgAction, Command};
 use memchr::memchr3_iter;
 use num_bigint::BigUint;
-use num_traits::FromPrimitive;
+use num_prime::nt_funcs::{factorize64, factorize128, factors};
 use uucore::display::Quotable;
 use uucore::error::{FromIo, UResult, USimpleError, set_exit_code};
 use uucore::translate;
@@ -33,72 +33,77 @@ const DELIM_SPACE: u8 = b' ';
 const DELIM_TAB: u8 = b'\t';
 const DELIM_NULL: u8 = b'\0';
 
+#[derive(Debug, PartialEq, Eq)]
+enum Number {
+    U64(u64),
+    U128(u128),
+    BigUint(BigUint),
+}
+
 fn write_factors_str(
     num_str: &[u8],
     w: &mut io::BufWriter<impl Write>,
     print_exponents: bool,
 ) -> UResult<()> {
-    let parsed = parse_num::<BigUint>(num_str);
+    let parsed = parse_num(num_str);
     show_if_err!(&parsed);
     let Ok(x) = parsed else {
+        // return Ok(). it's non-fatal and we should try the next number.
         return Ok(());
     };
 
-    if x > BigUint::from_u32(1).unwrap() {
+    match x {
         // use num_prime's factorize64 algorithm for u64 integers
-        if x <= BigUint::from_u64(u64::MAX).unwrap() {
-            let prime_factors = num_prime::nt_funcs::factorize64(x.clone().to_u64_digits()[0]);
-            write_result_u64(w, &x, prime_factors, print_exponents)
-                .map_err_context(|| translate!("factor-error-write-error"))?;
-        }
+        Number::U64(x) if x > 1 => write_result(w, &x, factorize64(x), print_exponents),
+        Number::U64(x) => write_result(w, &x, BTreeMap::<u64, usize>::new(), print_exponents),
         // use num_prime's factorize128 algorithm for u128 integers
-        else if x <= BigUint::from_u128(u128::MAX).unwrap() {
-            let parsed = parse_num::<u128>(num_str);
-            show_if_err!(&parsed);
-            let Ok(x) = parsed else {
-                return Ok(());
-            };
-            let prime_factors = num_prime::nt_funcs::factorize128(x);
-            write_result_u128(w, &x, prime_factors, print_exponents)
-                .map_err_context(|| translate!("factor-error-write-error"))?;
-        }
+        Number::U128(x) => write_result(w, &x, factorize128(x), print_exponents),
         // use num_prime's fallible factorization for anything greater than u128::MAX
-        else {
-            let (prime_factors, remaining) = num_prime::nt_funcs::factors(x.clone(), None);
-            if let Some(_remaining) = remaining {
+        Number::BigUint(x) => {
+            let (prime_factors, remaining) = factors(x.clone(), None);
+            if remaining.is_some() {
                 return Err(USimpleError::new(
                     1,
                     translate!("factor-error-factorization-incomplete"),
                 ));
             }
-            write_result_big_uint(w, &x, prime_factors, print_exponents)
-                .map_err_context(|| translate!("factor-error-write-error"))?;
+            write_result(w, &x, prime_factors, print_exponents)
         }
-    } else {
-        let empty_primes: BTreeMap<BigUint, usize> = BTreeMap::new();
-        write_result_big_uint(w, &x, empty_primes, print_exponents)
-            .map_err_context(|| translate!("factor-error-write-error"))?;
     }
-
-    Ok(())
+    .map_err_context(|| translate!("factor-error-write-error"))
 }
 
-fn parse_num<T: FromStr>(slice: &[u8]) -> UResult<T> {
-    str::from_utf8(slice)
-        .ok()
-        .and_then(|str| str.trim_matches(DELIM_SPACE as char).parse().ok())
-        .ok_or_else(|| {
-            let num = NumError(slice).to_string();
-            let num = if num.len() > slice.len() {
-                num.quote() // Force quoting if there are invalid characters.
-            } else {
-                num.maybe_quote()
-            };
-            USimpleError::new(
-                1,
-                format!("warning: {num}: {}", translate!("factor-error-invalid-int")),
-            )
-        })
+fn parse_num(slice: &[u8]) -> UResult<Number> {
+    let err_invalid = |s: &str, force_quoting| {
+        let num = if force_quoting {
+            s.quote() // Force quoting if there are invalid characters.
+        } else {
+            s.maybe_quote()
+        };
+        USimpleError::new(
+            1,
+            format!("warning: {num}: {}", translate!("factor-error-invalid-int")),
+        )
+    };
+    let num = str::from_utf8(slice).map_err(|_| err_invalid(&NumError(slice).to_string(), true))?;
+
+    match num.parse::<u64>() {
+        Ok(x) => return Ok(Number::U64(x)),
+        // If overflown, attempt a greater width
+        Err(e) if matches!(e.kind(), IntErrorKind::PosOverflow) => {}
+        Err(_) => return Err(err_invalid(num, false)),
+    }
+
+    match num.parse::<u128>() {
+        Ok(x) => return Ok(Number::U128(x)),
+        // If overflown, attempt a greater width
+        Err(e) if matches!(e.kind(), IntErrorKind::PosOverflow) => {}
+        Err(_) => return Err(err_invalid(num, false)),
+    }
+
+    num.parse::<BigUint>()
+        .map(Number::BigUint)
+        .map_err(|_| err_invalid(num, false))
 }
 
 /// This is a newtype wrapper over a potentially malformed UTF-8
@@ -132,57 +137,11 @@ impl Display for NumError<'_> {
     }
 }
 
-/// Writing out the prime factors for u64 integers
-fn write_result_u64(
-    w: &mut io::BufWriter<impl Write>,
-    x: &BigUint,
-    factorization: BTreeMap<u64, usize>,
-    print_exponents: bool,
-) -> io::Result<()> {
-    write!(w, "{x}:")?;
-    for (factor, n) in factorization {
-        if print_exponents {
-            if n > 1 {
-                write!(w, " {factor}^{n}")?;
-            } else {
-                write!(w, " {factor}")?;
-            }
-        } else {
-            w.write_all(format!(" {factor}").repeat(n).as_bytes())?;
-        }
-    }
-    writeln!(w)?;
-    w.flush()
-}
-
-/// Writing out the prime factors for u128 integers
-fn write_result_u128(
-    w: &mut io::BufWriter<impl Write>,
-    x: &u128,
-    factorization: BTreeMap<u128, usize>,
-    print_exponents: bool,
-) -> io::Result<()> {
-    write!(w, "{x}:")?;
-    for (factor, n) in factorization {
-        if print_exponents {
-            if n > 1 {
-                write!(w, " {factor}^{n}")?;
-            } else {
-                write!(w, " {factor}")?;
-            }
-        } else {
-            w.write_all(format!(" {factor}").repeat(n).as_bytes())?;
-        }
-    }
-    writeln!(w)?;
-    w.flush()
-}
-
-/// Writing out the prime factors for BigUint integers
-fn write_result_big_uint(
+/// Writing out the prime factors for integers
+fn write_result(
     w: &mut io::BufWriter<impl Write>,
-    x: &BigUint,
-    factorization: BTreeMap<BigUint, usize>,
+    x: &impl Display,
+    factorization: BTreeMap<impl Display, usize>,
     print_exponents: bool,
 ) -> io::Result<()> {
     write!(w, "{x}:")?;
@@ -214,7 +173,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
 
     if let Some(values) = matches.get_many::<String>(options::NUMBER) {
         for number in values {
-            write_factors_str(number.as_bytes(), &mut w, print_exponents)?;
+            write_factors_str(number.trim().as_bytes(), &mut w, print_exponents)?;
         }
     } else {
         let stdin = stdin();
@@ -285,23 +244,24 @@ pub fn uu_app() -> Command {
 
 #[cfg(test)]
 mod tests {
-    use crate::BigUint;
+    use crate::Number;
     use crate::parse_num;
 
     #[test]
     fn test_correct_parsing() {
-        let zero = parse_num::<u64>(b"0").unwrap();
-        let u64_max = parse_num::<u64>(u64::MAX.to_string().as_bytes()).unwrap();
-        let u128_one = parse_num::<u128>((u64::MAX as u128 + 1).to_string().as_bytes()).unwrap();
-        let u128_max = parse_num::<u128>(u128::MAX.to_string().as_bytes()).unwrap();
-        let bigint = parse_num::<BigUint>(u128::MAX.to_string().as_bytes()).unwrap() + 1u64;
+        const U128_MAX_ONE: &str = "340282366920938463463374607431768211456";
+        let zero = parse_num(b"0").unwrap();
+        let u64_max = parse_num(u64::MAX.to_string().as_bytes()).unwrap();
+        let u128_one = parse_num((u64::MAX as u128 + 1).to_string().as_bytes()).unwrap();
+        let u128_max = parse_num(u128::MAX.to_string().as_bytes()).unwrap();
+        let bigint = parse_num(U128_MAX_ONE.as_bytes()).unwrap();
         assert_eq!(
             (
-                0,
-                u64::MAX,
-                (u64::MAX as u128 + 1),
-                u128::MAX,
-                BigUint::from(u128::MAX) + 1u64
+                Number::U64(0),
+                Number::U64(u64::MAX),
+                Number::U128(u64::MAX as u128 + 1),
+                Number::U128(u128::MAX),
+                Number::BigUint(U128_MAX_ONE.parse().unwrap())
             ),
             (zero, u64_max, u128_one, u128_max, bigint)
         );
@@ -310,11 +270,7 @@ mod tests {
     #[test]
     #[should_panic]
     fn test_incorrect_parsing() {
-        parse_num::<u64>(b"abcd").unwrap();
-        parse_num::<u64>(b"12\x00\xFF\x1D").unwrap();
-        parse_num::<u128>(b"abcd").unwrap();
-        parse_num::<u128>(b"12\x00\xFF\x1D").unwrap();
-        parse_num::<BigUint>(b"abcd").unwrap();
-        parse_num::<BigUint>(b"12\x00\xFF\x1D").unwrap();
+        parse_num(b"abcd").unwrap();
+        parse_num(b"12\x00\xFF\x1D").unwrap();
     }
 }