Merge pull request #5578 from cakebaker/expr_fix

expr: adapt error messages, revert most of #5559

src/uu/expr/src/expr.rs

@@ -5,7 +5,7 @@
 
 use clap::{crate_version, Arg, ArgAction, Command};
 use uucore::{
-    error::{UResult, USimpleError},
+    error::{UResult, USimpleError, UUsageError},
     format_usage, help_about, help_section, help_usage,
 };
 
@@ -58,6 +58,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         .map(|v| v.into_iter().map(|s| s.as_ref()).collect::<Vec<_>>())
         .unwrap_or_default();
 
+    if token_strings.is_empty() {
+        return Err(UUsageError::new(2, "missing operand"));
+    }
+
     match process_expr(&token_strings[..]) {
         Ok(expr_result) => print_expr_ok(&expr_result),
         Err(expr_error) => Err(USimpleError::new(2, &expr_error)),

src/uu/expr/src/syntax_tree.rs

@@ -215,7 +215,7 @@ pub fn tokens_to_ast(
         assert!(op_stack.is_empty());
 
         maybe_dump_rpn(&out_stack);
-        let result = ast_from_rpn(&mut out_stack, None);
+        let result = ast_from_rpn(&mut out_stack);
         if out_stack.is_empty() {
             maybe_dump_ast(&result);
             result
@@ -254,13 +254,9 @@ fn maybe_dump_rpn(rpn: &TokenStack) {
     }
 }
 
-fn ast_from_rpn(rpn: &mut TokenStack, op_type: Option<&str>) -> Result<Box<AstNode>, String> {
+fn ast_from_rpn(rpn: &mut TokenStack) -> Result<Box<AstNode>, String> {
     match rpn.pop() {
-        None => Err(match op_type {
+        None => Err("syntax error (premature end of expression)".to_owned()),
-            Some(value) => format!("syntax error: unexpected argument {}", value.quote()),
-            None => "missing operand".to_owned(),
-        }),
-
         Some((token_idx, Token::Value { value })) => Ok(AstNode::new_leaf(token_idx, &value)),
 
         Some((token_idx, Token::InfixOp { value, .. })) => {
@@ -285,7 +281,7 @@ fn maybe_ast_node(
 ) -> Result<Box<AstNode>, String> {
     let mut operands = Vec::with_capacity(arity);
     for _ in 0..arity {
-        let operand = ast_from_rpn(rpn, Some(op_type))?;
+        let operand = ast_from_rpn(rpn)?;
         operands.push(operand);
     }
     operands.reverse();
@@ -335,12 +331,24 @@ fn push_token_to_either_stack(
             }
         }
 
-        Token::PrefixOp { .. } | Token::ParOpen => {
+        Token::ParOpen => {
             if out_stack.is_empty() {
                 op_stack.push((token_idx, token.clone()));
                 Ok(())
             } else {
-                Err(String::from("syntax error (operation should be prefix)"))
+                Err("syntax error: unexpected argument '('".to_string())
+            }
+        }
+
+        Token::PrefixOp { value, .. } => {
+            if out_stack.is_empty() {
+                op_stack.push((token_idx, token.clone()));
+                Ok(())
+            } else {
+                Err(format!(
+                    "syntax error: unexpected argument {}",
+                    value.quote()
+                ))
             }
         }
 

tests/by-util/test_expr.rs

@@ -11,7 +11,7 @@ fn test_no_arguments() {
     new_ucmd!()
         .fails()
         .code_is(2)
-        .stderr_only("expr: missing operand\n");
+        .usage_error("missing operand");
 }
 
 #[test]
@@ -112,7 +112,8 @@ fn test_parenthesis() {
     new_ucmd!()
         .args(&["1", "(", ")"])
         .fails()
-        .stderr_only("expr: syntax error (operation should be prefix)\n");
+        .code_is(2)
+        .stderr_only("expr: syntax error: unexpected argument '('\n");
 }
 
 #[test]
@@ -238,7 +239,8 @@ fn test_index() {
     new_ucmd!()
         .args(&["αbcdef", "index", "α"])
         .fails()
-        .stderr_only("expr: syntax error (operation should be prefix)\n");
+        .code_is(2)
+        .stderr_only("expr: syntax error: unexpected argument 'index'\n");
 }
 
 #[test]
@@ -256,7 +258,8 @@ fn test_length() {
     new_ucmd!()
         .args(&["abcdef", "length"])
         .fails()
-        .stderr_only("expr: syntax error (operation should be prefix)\n");
+        .code_is(2)
+        .stderr_only("expr: syntax error: unexpected argument 'length'\n");
 }
 
 #[test]
@@ -298,17 +301,12 @@ fn test_substr() {
     new_ucmd!()
         .args(&["abc", "substr", "1", "1"])
         .fails()
-        .stderr_only("expr: syntax error (operation should be prefix)\n");
+        .code_is(2)
+        .stderr_only("expr: syntax error: unexpected argument 'substr'\n");
 }
 
 #[test]
 fn test_invalid_substr() {
-    new_ucmd!()
-        .args(&["56", "substr"])
-        .fails()
-        .code_is(2)
-        .stderr_only("expr: syntax error: unexpected argument 'substr'\n");
-
     new_ucmd!()
         .args(&["substr", "abc", "0", "1"])
         .fails()