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 {
- Some(value) => format!("syntax error: unexpected argument {}", value.quote()),
- None => "missing operand".to_owned(),
- }),
-
+ None => Err("syntax error (premature end of expression)".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()