Merge pull request #7332 from RenjiSann/expr-other-approach

expr: Evaluate parenthesis content before checking for closing parenthesis

Cargo.lock

@@ -2781,6 +2781,7 @@ dependencies = [
  "num-bigint",
  "num-traits",
  "onig",
+ "thiserror 2.0.11",
  "uucore",
 ]
 

src/uu/expr/Cargo.toml

@@ -22,6 +22,7 @@ num-bigint = { workspace = true }
 num-traits = { workspace = true }
 onig = { workspace = true }
 uucore = { workspace = true }
+thiserror = { workspace = true }
 
 [[bin]]
 name = "expr"

src/uu/expr/src/expr.rs

@@ -3,18 +3,15 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-use std::fmt::Display;
-
 use clap::{crate_version, Arg, ArgAction, Command};
-use syntax_tree::AstNode;
+use syntax_tree::{is_truthy, AstNode};
+use thiserror::Error;
 use uucore::{
     display::Quotable,
     error::{UError, UResult},
     format_usage, help_about, help_section, help_usage,
 };
 
-use crate::syntax_tree::is_truthy;
-
 mod syntax_tree;
 
 mod options {
@@ -25,63 +22,36 @@ mod options {
 
 pub type ExprResult<T> = Result<T, ExprError>;
 
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Error, Clone, Debug, PartialEq, Eq)]
 pub enum ExprError {
+    #[error("syntax error: unexpected argument {}", .0.quote())]
     UnexpectedArgument(String),
+    #[error("syntax error: missing argument after {}", .0.quote())]
     MissingArgument(String),
+    #[error("non-integer argument")]
     NonIntegerArgument,
+    #[error("missing operand")]
     MissingOperand,
+    #[error("division by zero")]
     DivisionByZero,
+    #[error("Invalid regex expression")]
     InvalidRegexExpression,
+    #[error("syntax error: expecting ')' after {}", .0.quote())]
     ExpectedClosingBraceAfter(String),
+    #[error("syntax error: expecting ')' instead of {}", .0.quote())]
     ExpectedClosingBraceInsteadOf(String),
+    #[error("Unmatched ( or \\(")]
     UnmatchedOpeningParenthesis,
+    #[error("Unmatched ) or \\)")]
     UnmatchedClosingParenthesis,
+    #[error("Unmatched \\{{")]
     UnmatchedOpeningBrace,
+    #[error("Unmatched ) or \\}}")]
     UnmatchedClosingBrace,
+    #[error("Invalid content of {0}")]
     InvalidContent(String),
 }
 
-impl Display for ExprError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Self::UnexpectedArgument(s) => {
-                write!(f, "syntax error: unexpected argument {}", s.quote())
-            }
-            Self::MissingArgument(s) => {
-                write!(f, "syntax error: missing argument after {}", s.quote())
-            }
-            Self::NonIntegerArgument => write!(f, "non-integer argument"),
-            Self::MissingOperand => write!(f, "missing operand"),
-            Self::DivisionByZero => write!(f, "division by zero"),
-            Self::InvalidRegexExpression => write!(f, "Invalid regex expression"),
-            Self::ExpectedClosingBraceAfter(s) => {
-                write!(f, "syntax error: expecting ')' after {}", s.quote())
-            }
-            Self::ExpectedClosingBraceInsteadOf(s) => {
-                write!(f, "syntax error: expecting ')' instead of {}", s.quote())
-            }
-            Self::UnmatchedOpeningParenthesis => {
-                write!(f, "Unmatched ( or \\(")
-            }
-            Self::UnmatchedClosingParenthesis => {
-                write!(f, "Unmatched ) or \\)")
-            }
-            Self::UnmatchedOpeningBrace => {
-                write!(f, "Unmatched \\{{")
-            }
-            Self::UnmatchedClosingBrace => {
-                write!(f, "Unmatched ) or \\}}")
-            }
-            Self::InvalidContent(s) => {
-                write!(f, "Invalid content of {}", s)
-            }
-        }
-    }
-}
-
-impl std::error::Error for ExprError {}
-
 impl UError for ExprError {
     fn code(&self) -> i32 {
         2

src/uu/expr/src/syntax_tree.rs

@@ -292,7 +292,7 @@ const PRECEDENCE: &[&[(&str, BinOp)]] = &[
     &[(":", BinOp::String(StringOp::Match))],
 ];
 
-#[derive(Debug)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub enum NumOrStr {
     Num(BigInt),
     Str(String),
@@ -343,6 +343,9 @@ impl NumOrStr {
 
 #[derive(Debug, PartialEq, Eq)]
 pub enum AstNode {
+    Evaluated {
+        value: NumOrStr,
+    },
     Leaf {
         value: String,
     },
@@ -366,8 +369,15 @@ impl AstNode {
         Parser::new(input).parse()
     }
 
+    pub fn evaluated(self) -> ExprResult<Self> {
+        Ok(Self::Evaluated {
+            value: self.eval()?,
+        })
+    }
+
     pub fn eval(&self) -> ExprResult<NumOrStr> {
         match self {
+            Self::Evaluated { value } => Ok(value.clone()),
             Self::Leaf { value } => Ok(value.to_string().into()),
             Self::BinOp {
                 op_type,
@@ -536,7 +546,10 @@ impl<'a> Parser<'a> {
                 value: self.next()?.into(),
             },
             "(" => {
-                let s = self.parse_expression()?;
+                // Evaluate the node just after parsing to we detect arithmetic
+                // errors before checking for the closing parenthesis.
+                let s = self.parse_expression()?.evaluated()?;
+
                 match self.next() {
                     Ok(")") => {}
                     // Since we have parsed at least a '(', there will be a token
@@ -680,7 +693,9 @@ mod test {
             AstNode::parse(&["(", "1", "+", "2", ")", "*", "3"]),
             Ok(op(
                 BinOp::Numeric(NumericOp::Mul),
-                op(BinOp::Numeric(NumericOp::Add), "1", "2"),
+                op(BinOp::Numeric(NumericOp::Add), "1", "2")
+                    .evaluated()
+                    .unwrap(),
                 "3"
             ))
         );

tests/by-util/test_expr.rs

@@ -370,3 +370,11 @@ fn test_num_str_comparison() {
         .succeeds()
         .stdout_is("1\n");
 }
+
+#[test]
+fn test_eager_evaluation() {
+    new_ucmd!()
+        .args(&["(", "1", "/", "0"])
+        .fails()
+        .stderr_contains("division by zero");
+}