From aa010b71b8665cc7a91b80fc398c68852215d552 Mon Sep 17 00:00:00 2001
From: Joseph Jon Booker <joe@neoturbine.net>
Date: Mon, 25 Mar 2024 22:02:04 -0500
Subject: [PATCH] expr: Change error messages when missing closing parenthesis

This situation now has two errors - one if we parse a character that
isn't a closing parenthesis, and the other if we don't parse anything.
---
 src/uu/expr/src/expr.rs        |  6 +++++-
 src/uu/expr/src/syntax_tree.rs | 32 +++++++++++++++++++++++++++-----
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/uu/expr/src/expr.rs b/src/uu/expr/src/expr.rs
index 4e41a6929..371f4377e 100644
--- a/src/uu/expr/src/expr.rs
+++ b/src/uu/expr/src/expr.rs
@@ -34,6 +34,7 @@ pub enum ExprError {
     DivisionByZero,
     InvalidRegexExpression,
     ExpectedClosingBraceAfter(String),
+    ExpectedClosingBraceInsteadOf(String),
 }
 
 impl Display for ExprError {
@@ -50,7 +51,10 @@ impl Display for ExprError {
             Self::DivisionByZero => write!(f, "division by zero"),
             Self::InvalidRegexExpression => write!(f, "Invalid regex expression"),
             Self::ExpectedClosingBraceAfter(s) => {
-                write!(f, "expected ')' after {}", s.quote())
+                write!(f, "syntax error: expecting ')' after {}", s.quote())
+            }
+            Self::ExpectedClosingBraceInsteadOf(s) => {
+                write!(f, "syntax error: expecting ')' instead of {}", s.quote())
             }
         }
     }
diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs
index 0a947a158..30d50fb26 100644
--- a/src/uu/expr/src/syntax_tree.rs
+++ b/src/uu/expr/src/syntax_tree.rs
@@ -442,13 +442,21 @@ impl<'a> Parser<'a> {
             },
             "(" => {
                 let s = self.parse_expression()?;
-                let close_paren = self.next()?;
-                if close_paren != ")" {
+                match self.next() {
+                    Ok(")") => {}
                     // Since we have parsed at least a '(', there will be a token
                     // at `self.index - 1`. So this indexing won't panic.
-                    return Err(ExprError::ExpectedClosingBraceAfter(
-                        self.input[self.index - 1].into(),
-                    ));
+                    Ok(_) => {
+                        return Err(ExprError::ExpectedClosingBraceInsteadOf(
+                            self.input[self.index - 1].into(),
+                        ));
+                    }
+                    Err(ExprError::MissingArgument(_)) => {
+                        return Err(ExprError::ExpectedClosingBraceAfter(
+                            self.input[self.index - 1].into(),
+                        ));
+                    }
+                    Err(e) => return Err(e),
                 }
                 s
             }
@@ -484,6 +492,8 @@ pub fn is_truthy(s: &NumOrStr) -> bool {
 
 #[cfg(test)]
 mod test {
+    use crate::ExprError;
+
     use super::{AstNode, BinOp, NumericOp, RelationOp, StringOp};
 
     impl From<&str> for AstNode {
@@ -587,4 +597,16 @@ mod test {
             )),
         );
     }
+
+    #[test]
+    fn missing_closing_parenthesis() {
+        assert_eq!(
+            AstNode::parse(&["(", "42"]),
+            Err(ExprError::ExpectedClosingBraceAfter("42".to_string()))
+        );
+        assert_eq!(
+            AstNode::parse(&["(", "42", "a"]),
+            Err(ExprError::ExpectedClosingBraceInsteadOf("a".to_string()))
+        );
+    }
 }