diff --git a/src/uu/test/src/error.rs b/src/uu/test/src/error.rs new file mode 100644 index 000000000..f237124ab --- /dev/null +++ b/src/uu/test/src/error.rs @@ -0,0 +1,44 @@ +/// Represents an error encountered while parsing a test expression +#[derive(Debug)] +pub enum ParseError { + ExpectedValue, + Expected(String), + ExtraArgument(String), + MissingArgument(String), + UnknownOperator(String), + InvalidInteger(String), +} + +/// A Result type for parsing test expressions +pub type ParseResult = Result; + +/// Implement Display trait for ParseError to make it easier to print useful errors. +impl std::fmt::Display for ParseError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Expected(s) => write!(f, "expected {}", s), + Self::ExpectedValue => write!(f, "expected value"), + Self::MissingArgument(s) => write!(f, "missing argument after {}", s), + Self::ExtraArgument(s) => write!(f, "extra argument {}", s), + Self::UnknownOperator(s) => write!(f, "unknown operator {}", s), + Self::InvalidInteger(s) => write!(f, "invalid integer {}", s), + } + } +} + +/// Implement UError trait for ParseError to make it easier to return useful error codes from main(). +impl uucore::error::UError for ParseError { + fn code(&self) -> i32 { + match self { + Self::Expected(_) => 2, + Self::MissingArgument(_) => 2, + Self::ExtraArgument(_) => 2, + Self::UnknownOperator(_) => 2, + Self::ExpectedValue => 2, + Self::InvalidInteger(_) => 2, + } + } +} + +/// Implement standard Error trait for UError +impl std::error::Error for ParseError {} diff --git a/src/uu/test/src/parser.rs b/src/uu/test/src/parser.rs index 3d221cef5..dc791c934 100644 --- a/src/uu/test/src/parser.rs +++ b/src/uu/test/src/parser.rs @@ -10,7 +10,10 @@ use std::ffi::OsString; use std::iter::Peekable; +use super::error::{ParseError, ParseResult}; + use uucore::display::Quotable; +use uucore::error::UResult; /// Represents one of the binary comparison operators for strings, integers, or files #[derive(Debug, PartialEq, Eq)] @@ -39,25 +42,6 @@ pub enum Symbol { None, } -/// Represents an error encountered while parsing a test expression -pub enum ParseError { - MissingArgument(OsString), - Expected(OsString), -} - -/// A Result type for parsing test expressions -type ParseResult = Result; - -/// Display an error message for a ParseError -impl std::fmt::Display for ParseError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Expected(s) => write!(f, "expected {}", s.to_string_lossy()), - Self::MissingArgument(s) => write!(f, "missing argument for {}", s.to_string_lossy()), - } - } -} - impl Symbol { /// Create a new Symbol from an OsString. /// @@ -108,6 +92,27 @@ impl Symbol { } } +/// Implement Display trait for Symbol to make it easier to print useful errors. +/// We will try to match the format in which the symbol appears in the input. +impl std::fmt::Display for Symbol { + /// Format a Symbol for printing + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let s = match &self { + Self::LParen => OsString::from("("), + Self::Bang => OsString::from("!"), + Self::BoolOp(s) + | Self::Literal(s) + | Self::Op(Operator::String(s)) + | Self::Op(Operator::Int(s)) + | Self::Op(Operator::File(s)) + | Self::UnaryOp(UnaryOperator::StrlenOp(s)) + | Self::UnaryOp(UnaryOperator::FiletestOp(s)) => s.clone(), + Self::None => OsString::from("None"), + }; + write!(f, "{}", s.quote()) + } +} + /// Recursive descent parser for test, which converts a list of OsStrings /// (typically command line arguments) into a stack of Symbols in postfix /// order. @@ -224,7 +229,7 @@ impl Parser { } // case 2: error if end of stream is `( ` - [symbol] => Err(ParseError::MissingArgument(format!("{symbol:#?}").into())), + [symbol] => Err(ParseError::MissingArgument(format!("{symbol}"))), // case 3: `( uop )` → parenthesized unary operation; // this case ensures we don’t get confused by `( -f ) )` @@ -407,7 +412,7 @@ impl Parser { match self.next_token() { Symbol::None => { - return Err(ParseError::MissingArgument(format!("{op:#?}").into())); + return Err(ParseError::MissingArgument(format!("{op}"))); } token => self.stack.push(token.into_literal()), } @@ -419,11 +424,11 @@ impl Parser { /// Parser entry point: parse the token stream `self.tokens`, storing the /// resulting `Symbol` stack in `self.stack`. - fn parse(&mut self) -> Result<(), String> { - self.expr().map_err(|e| e.to_string())?; + fn parse(&mut self) -> UResult<()> { + self.expr()?; match self.tokens.next() { - Some(token) => Err(format!("extra argument {}", token.quote())), + Some(token) => Err(ParseError::ExtraArgument(token.quote().to_string()).into()), None => Ok(()), } } @@ -431,7 +436,7 @@ impl Parser { /// Parse the token stream `args`, returning a `Symbol` stack representing the /// operations to perform in postfix order. -pub fn parse(args: Vec) -> Result, String> { +pub fn parse(args: Vec) -> UResult> { let mut p = Parser::new(args); p.parse()?; Ok(p.stack) diff --git a/src/uu/test/src/test.rs b/src/uu/test/src/test.rs index 8ce4564a3..168ce77a8 100644 --- a/src/uu/test/src/test.rs +++ b/src/uu/test/src/test.rs @@ -8,6 +8,7 @@ // spell-checker:ignore (vars) egid euid FiletestOp StrlenOp +pub(crate) mod error; mod parser; use clap::{crate_version, Command}; @@ -19,6 +20,7 @@ use std::os::unix::fs::MetadataExt; use uucore::display::Quotable; use uucore::error::{UResult, USimpleError}; use uucore::format_usage; +use error::{ParseError, ParseResult}; const USAGE: &str = "\ {} EXPRESSION @@ -128,23 +130,18 @@ pub fn uumain(mut args: impl uucore::Args) -> UResult<()> { } } - let result = parse(args).and_then(|mut stack| eval(&mut stack)); + let result = parse(args).map(|mut stack| eval(&mut stack))??; - match result { - Ok(result) => { - if result { - Ok(()) - } else { - Err(1.into()) - } - } - Err(e) => Err(USimpleError::new(2, e)), + if result { + Ok(()) + } else { + Err(1.into()) } } /// Evaluate a stack of Symbols, returning the result of the evaluation or /// an error message if evaluation failed. -fn eval(stack: &mut Vec) -> Result { +fn eval(stack: &mut Vec) -> ParseResult { macro_rules! pop_literal { () => { match stack.pop() { @@ -186,7 +183,7 @@ fn eval(stack: &mut Vec) -> Result { return Ok(true); } _ => { - return Err(format!("missing argument after '{op:?}'")); + return Err(ParseError::MissingArgument(op.quote().to_string())); } }; @@ -233,7 +230,7 @@ fn eval(stack: &mut Vec) -> Result { Ok(if op == "-a" { a && b } else { a || b }) } - _ => Err("expected value".to_string()), + _ => Err(ParseError::ExpectedValue), } } @@ -241,19 +238,17 @@ fn eval(stack: &mut Vec) -> Result { /// `a` is the left hand side /// `b` is the left hand side /// `op` the operation (ex: -eq, -lt, etc) -fn integers(a: &OsStr, b: &OsStr, op: &OsStr) -> Result { - let format_err = |value: &OsStr| format!("invalid integer {}", value.quote()); - +fn integers(a: &OsStr, b: &OsStr, op: &OsStr) -> ParseResult { // Parse the two inputs let a: i128 = a .to_str() .and_then(|s| s.parse().ok()) - .ok_or_else(|| format_err(a))?; + .ok_or_else(|| ParseError::InvalidInteger(a.quote().to_string()))?; let b: i128 = b .to_str() .and_then(|s| s.parse().ok()) - .ok_or_else(|| format_err(b))?; + .ok_or_else(|| ParseError::InvalidInteger(b.quote().to_string()))?; // Do the maths Ok(match op.to_str() { @@ -263,7 +258,7 @@ fn integers(a: &OsStr, b: &OsStr, op: &OsStr) -> Result { Some("-ge") => a >= b, Some("-lt") => a < b, Some("-le") => a <= b, - _ => return Err(format!("unknown operator {}", op.quote())), + _ => return Err(ParseError::UnknownOperator(op.quote().to_string())), }) } @@ -271,7 +266,7 @@ fn integers(a: &OsStr, b: &OsStr, op: &OsStr) -> Result { /// `a` is the left hand side /// `b` is the left hand side /// `op` the operation (ex: -ef, -nt, etc) -fn files(a: &OsStr, b: &OsStr, op: &OsStr) -> Result { +fn files(a: &OsStr, b: &OsStr, op: &OsStr) -> ParseResult { // Don't manage the error. GNU doesn't show error when doing // test foo -nt bar let f_a = match fs::metadata(a) { @@ -290,14 +285,14 @@ fn files(a: &OsStr, b: &OsStr, op: &OsStr) -> Result { Some("-ef") => unimplemented!(), Some("-nt") => f_a.modified().unwrap() > f_b.modified().unwrap(), Some("-ot") => f_a.created().unwrap() > f_b.created().unwrap(), - _ => return Err(format!("unknown operator {}", op.quote())), + _ => return Err(ParseError::UnknownOperator(op.quote().to_string())), }) } -fn isatty(fd: &OsStr) -> Result { +fn isatty(fd: &OsStr) -> ParseResult { fd.to_str() .and_then(|s| s.parse().ok()) - .ok_or_else(|| format!("invalid integer {}", fd.quote())) + .ok_or_else(|| ParseError::InvalidInteger(fd.quote().to_string())) .map(|i| { #[cfg(not(target_os = "redox"))] unsafe { diff --git a/tests/by-util/test_test.rs b/tests/by-util/test_test.rs index f3ed83348..bf5c50b7f 100644 --- a/tests/by-util/test_test.rs +++ b/tests/by-util/test_test.rs @@ -928,3 +928,16 @@ fn test_long_integer() { ]) .fails(); } + +#[test] +fn test_missing_argument_after() { + let mut ucmd = new_ucmd!(); + + let result = ucmd.args(&["(", "foo"]).fails(); + result.no_stdout(); + assert_eq!(result.exit_status().code().unwrap(), 2); + assert_eq!( + result.stderr_str().trim(), + "test: missing argument after 'foo'" + ); +}