Merge pull request #3112 from ndd7xv/err_conv-to-result

uucore(memo): replace err_conv with result

src/uu/printf/src/printf.rs

@@ -284,7 +284,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         None => vec![],
     };
 
-    memo::Memo::run_all(format_string, &values[..]);
+    memo::Memo::run_all(format_string, &values[..])?;
     Ok(())
 }
 

src/uu/seq/src/seq.rs

@@ -5,6 +5,7 @@
 // TODO: Support -f flag
 // spell-checker:ignore (ToDO) istr chiter argptr ilen extendedbigdecimal extendedbigint numberparse
 use std::io::{stdout, ErrorKind, Write};
+use std::process::exit;
 
 use clap::{crate_version, App, AppSettings, Arg};
 use num_traits::Zero;
@@ -12,6 +13,7 @@ use num_traits::Zero;
 use uucore::error::FromIo;
 use uucore::error::UResult;
 use uucore::memo::Memo;
+use uucore::show;
 
 mod error;
 mod extendedbigdecimal;
@@ -287,7 +289,10 @@ fn print_seq(
         match format {
             Some(f) => {
                 let s = format!("{}", value);
-                Memo::run_all(f, &[s]);
+                if let Err(x) = Memo::run_all(f, &[s]) {
+                    show!(x);
+                    exit(1);
+                }
             }
             None => write_value_float(
                 &mut stdout,
@@ -349,7 +354,10 @@ fn print_seq_integers(
         match format {
             Some(f) => {
                 let s = format!("{}", value);
-                Memo::run_all(f, &[s]);
+                if let Err(x) = Memo::run_all(f, &[s]) {
+                    show!(x);
+                    exit(1);
+                }
             }
             None => write_value_int(&mut stdout, &value, padding, pad, is_first_iteration)?,
         }

src/uucore/src/lib/features/memo.rs

@@ -6,6 +6,7 @@
 //! that prints tokens.
 
 use crate::display::Quotable;
+use crate::error::UResult;
 use crate::features::tokenize::sub::Sub;
 use crate::features::tokenize::token::{Token, Tokenizer};
 use crate::features::tokenize::unescaped_text::UnescapedText;
@@ -26,17 +27,17 @@ fn warn_excess_args(first_arg: &str) {
 }
 
 impl Memo {
-    pub fn new(pf_string: &str, pf_args_it: &mut Peekable<Iter<String>>) -> Self {
+    pub fn new(pf_string: &str, pf_args_it: &mut Peekable<Iter<String>>) -> UResult<Self> {
         let mut pm = Self { tokens: Vec::new() };
         let mut tmp_token: Option<Box<dyn Token>>;
         let mut it = put_back_n(pf_string.chars());
         let mut has_sub = false;
         loop {
-            tmp_token = UnescapedText::from_it(&mut it, pf_args_it);
+            tmp_token = UnescapedText::from_it(&mut it, pf_args_it)?;
             if let Some(x) = tmp_token {
                 pm.tokens.push(x);
             }
-            tmp_token = Sub::from_it(&mut it, pf_args_it);
+            tmp_token = Sub::from_it(&mut it, pf_args_it)?;
             if let Some(x) = tmp_token {
                 if !has_sub {
                     has_sub = true;
@@ -64,19 +65,19 @@ impl Memo {
                 }
             }
         }
-        pm
+        Ok(pm)
     }
     pub fn apply(&self, pf_args_it: &mut Peekable<Iter<String>>) {
         for tkn in &self.tokens {
             tkn.print(pf_args_it);
         }
     }
-    pub fn run_all(pf_string: &str, pf_args: &[String]) {
+    pub fn run_all(pf_string: &str, pf_args: &[String]) -> UResult<()> {
         let mut arg_it = pf_args.iter().peekable();
-        let pm = Self::new(pf_string, &mut arg_it);
+        let pm = Self::new(pf_string, &mut arg_it)?;
         loop {
             if arg_it.peek().is_none() {
-                break;
+                return Ok(());
             }
             pm.apply(&mut arg_it);
         }

src/uucore/src/lib/features/tokenize/sub.rs

@@ -5,8 +5,10 @@
 //! it is created by Sub's implementation of the Tokenizer trait
 //! Subs which have numeric field chars make use of the num_format
 //! submodule
-use crate::show_error;
+use crate::error::{UError, UResult};
 use itertools::{put_back_n, PutBackN};
+use std::error::Error;
+use std::fmt::Display;
 use std::iter::Peekable;
 use std::process::exit;
 use std::slice::Iter;
@@ -20,11 +22,23 @@ use super::unescaped_text::UnescapedText;
 
 const EXIT_ERR: i32 = 1;
 
-fn err_conv(sofar: &str) {
-    show_error!("%{}: invalid conversion specification", sofar);
-    exit(EXIT_ERR);
+#[derive(Debug)]
+pub enum SubError {
+    InvalidSpec(String),
 }
 
+impl Display for SubError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
+        match self {
+            Self::InvalidSpec(s) => write!(f, "%{}: invalid conversion specification", s),
+        }
+    }
+}
+
+impl Error for SubError {}
+
+impl UError for SubError {}
+
 fn convert_asterisk_arg_int(asterisk_arg: &str) -> isize {
     // this is a costly way to parse the
     // args used for asterisk values into integers
@@ -116,14 +130,14 @@ impl SubParser {
     fn from_it(
         it: &mut PutBackN<Chars>,
         args: &mut Peekable<Iter<String>>,
-    ) -> Option<Box<dyn token::Token>> {
+    ) -> UResult<Option<Box<dyn token::Token>>> {
         let mut parser = Self::new();
-        if parser.sub_vals_retrieved(it) {
+        if parser.sub_vals_retrieved(it)? {
             let t: Box<dyn token::Token> = Self::build_token(parser);
             t.print(args);
-            Some(t)
+            Ok(Some(t))
         } else {
-            None
+            Ok(None)
         }
     }
     fn build_token(parser: Self) -> Box<dyn token::Token> {
@@ -151,9 +165,9 @@ impl SubParser {
         ));
         t
     }
-    fn sub_vals_retrieved(&mut self, it: &mut PutBackN<Chars>) -> bool {
-        if !Self::successfully_eat_prefix(it, &mut self.text_so_far) {
-            return false;
+    fn sub_vals_retrieved(&mut self, it: &mut PutBackN<Chars>) -> UResult<bool> {
+        if !Self::successfully_eat_prefix(it, &mut self.text_so_far)? {
+            return Ok(false);
         }
         // this fn in particular is much longer than it needs to be
         // .could get a lot
@@ -177,7 +191,7 @@ impl SubParser {
                 '-' | '*' | '0'..='9' => {
                     if !self.past_decimal {
                         if self.min_width_is_asterisk || self.specifiers_found {
-                            err_conv(&self.text_so_far);
+                            return Err(SubError::InvalidSpec(self.text_so_far.clone()).into());
                         }
                         if self.min_width_tmp.is_none() {
                             self.min_width_tmp = Some(String::new());
@@ -185,7 +199,9 @@ impl SubParser {
                         match self.min_width_tmp.as_mut() {
                             Some(x) => {
                                 if (ch == '-' || ch == '*') && !x.is_empty() {
-                                    err_conv(&self.text_so_far);
+                                    return Err(
+                                        SubError::InvalidSpec(self.text_so_far.clone()).into()
+                                    );
                                 }
                                 if ch == '*' {
                                     self.min_width_is_asterisk = true;
@@ -200,7 +216,7 @@ impl SubParser {
                         // second field should never have a
                         // negative value
                         if self.second_field_is_asterisk || ch == '-' || self.specifiers_found {
-                            err_conv(&self.text_so_far);
+                            return Err(SubError::InvalidSpec(self.text_so_far.clone()).into());
                         }
                         if self.second_field_tmp.is_none() {
                             self.second_field_tmp = Some(String::new());
@@ -208,7 +224,9 @@ impl SubParser {
                         match self.second_field_tmp.as_mut() {
                             Some(x) => {
                                 if ch == '*' && !x.is_empty() {
-                                    err_conv(&self.text_so_far);
+                                    return Err(
+                                        SubError::InvalidSpec(self.text_so_far.clone()).into()
+                                    );
                                 }
                                 if ch == '*' {
                                     self.second_field_is_asterisk = true;
@@ -225,7 +243,7 @@ impl SubParser {
                     if !self.past_decimal {
                         self.past_decimal = true;
                     } else {
-                        err_conv(&self.text_so_far);
+                        return Err(SubError::InvalidSpec(self.text_so_far.clone()).into());
                     }
                 }
                 x if legal_fields.binary_search(&x).is_ok() => {
@@ -242,18 +260,18 @@ impl SubParser {
                     }
                 }
                 _ => {
-                    err_conv(&self.text_so_far);
+                    return Err(SubError::InvalidSpec(self.text_so_far.clone()).into());
                 }
             }
         }
         if self.field_char.is_none() {
-            err_conv(&self.text_so_far);
+            return Err(SubError::InvalidSpec(self.text_so_far.clone()).into());
         }
         let field_char_retrieved = self.field_char.unwrap();
         if self.past_decimal && self.second_field_tmp.is_none() {
             self.second_field_tmp = Some(String::from("0"));
         }
-        self.validate_field_params(field_char_retrieved);
+        self.validate_field_params(field_char_retrieved)?;
         // if the dot is provided without a second field
         // printf interprets it as 0.
         if let Some(x) = self.second_field_tmp.as_mut() {
@@ -262,9 +280,12 @@ impl SubParser {
             }
         }
 
-        true
+        Ok(true)
     }
-    fn successfully_eat_prefix(it: &mut PutBackN<Chars>, text_so_far: &mut String) -> bool {
+    fn successfully_eat_prefix(
+        it: &mut PutBackN<Chars>,
+        text_so_far: &mut String,
+    ) -> UResult<bool> {
         // get next two chars,
         // if they're '%%' we're not tokenizing it
         // else put chars back
@@ -274,12 +295,11 @@ impl SubParser {
             match n_ch {
                 Some(x) => {
                     it.put_back(x);
-                    true
+                    Ok(true)
                 }
                 None => {
                     text_so_far.push('%');
-                    err_conv(text_so_far);
-                    false
+                    Err(SubError::InvalidSpec(text_so_far.clone()).into())
                 }
             }
         } else {
@@ -289,10 +309,10 @@ impl SubParser {
             if let Some(x) = preface {
                 it.put_back(x);
             };
-            false
+            Ok(false)
         }
     }
-    fn validate_field_params(&self, field_char: char) {
+    fn validate_field_params(&self, field_char: char) -> UResult<()> {
         // check for illegal combinations here when possible vs
         // on each application so we check less per application
         // to do: move these checks to Sub::new
@@ -304,8 +324,12 @@ impl SubParser {
                     || self.past_decimal
                     || self.second_field_tmp.is_some()))
         {
-            err_conv(&self.text_so_far);
+            // invalid string substitution
+            // to do: include information about an invalid
+            // string substitution
+            return Err(SubError::InvalidSpec(self.text_so_far.clone()).into());
         }
+        Ok(())
     }
 }
 
@@ -313,7 +337,7 @@ impl token::Tokenizer for Sub {
     fn from_it(
         it: &mut PutBackN<Chars>,
         args: &mut Peekable<Iter<String>>,
-    ) -> Option<Box<dyn token::Token>> {
+    ) -> UResult<Option<Box<dyn token::Token>>> {
         SubParser::from_it(it, args)
     }
 }

src/uucore/src/lib/features/tokenize/token.rs

@@ -4,6 +4,8 @@ use std::iter::Peekable;
 use std::slice::Iter;
 use std::str::Chars;
 
+use crate::error::UResult;
+
 // A token object is an object that can print the expected output
 // of a contiguous segment of the format string, and
 // requires at most 1 argument
@@ -27,5 +29,5 @@ pub trait Tokenizer {
     fn from_it(
         it: &mut PutBackN<Chars>,
         args: &mut Peekable<Iter<String>>,
-    ) -> Option<Box<dyn Token>>;
+    ) -> UResult<Option<Box<dyn Token>>>;
 }

src/uucore/src/lib/features/tokenize/unescaped_text.rs

@@ -13,6 +13,8 @@ use std::process::exit;
 use std::slice::Iter;
 use std::str::Chars;
 
+use crate::error::UResult;
+
 use super::token;
 
 const EXIT_OK: i32 = 0;
@@ -266,8 +268,8 @@ impl token::Tokenizer for UnescapedText {
     fn from_it(
         it: &mut PutBackN<Chars>,
         _: &mut Peekable<Iter<String>>,
-    ) -> Option<Box<dyn token::Token>> {
-        Self::from_it_core(it, false)
+    ) -> UResult<Option<Box<dyn token::Token>>> {
+        Ok(Self::from_it_core(it, false))
     }
 }
 impl token::Token for UnescapedText {