fix nested formulas

Author: optimistex

Expression.lib.php

@@ -211,7 +211,7 @@ function nfx($expr)
             return $this->trigger("illegal character '{$matches[0]}'");
         }
         */
-        $first_argument = false;
+        $begin_argument = false;
         while (1) { // 1 Infinite Loop ;)
             $op = substr($expr, $index, 2); // get the first two characters at the current index
             if (preg_match("/^[+\-*\/^_\"<>=%(){\[!~,](?!=|~)/", $op) || preg_match("/\w/", $op)) {
@@ -297,19 +297,20 @@ function nfx($expr)
                 // make sure there was a function
                 if (!preg_match("/^([a-z]\w*)\($/", $stack->last(2), $matches))
                     return $this->trigger("unexpected ','");
-                if ($first_argument) {
-                    $first_argument = false;
-                } else {
-                    $stack->push($stack->pop() + 1); // increment the argument count
-                }
                 $stack->push('('); // put the ( back on, we'll need to pop back to it again
                 $index++;
                 $expecting_op = false;
+                $begin_argument = true;
                 //===============
             } elseif ($op == '(' and !$expecting_op) {
+                if ($begin_argument) {
+                    $begin_argument = false;
+                    if (!$stack->incrementArgument()) {
+                        $this->trigger("unexpected '('");
+                    }
+                }
                 $stack->push('('); // that was easy
                 $index++;
-//                $allow_neg = true;
                 //===============
             } elseif ($ex and !$expecting_op) { // do we now have a function/variable/number?
                 $expecting_op = true;
@@ -321,9 +322,15 @@ function nfx($expr)
                         array_key_exists($matches[1], $this->f) or
                         array_key_exists($matches[1], $this->functions)
                     ) { // it's a func
+                        if ($begin_argument) {
+                            if (!$stack->incrementArgument()) {
+                                $this->trigger("unexpected '('");
+                            }
+                        }
                         $stack->push($val);
                         $stack->push(0);
                         $stack->push('(');
+                        $begin_argument = true;
                         $expecting_op = false;
                     } else { // it's a var w/ implicit multiplication
                         $val = $matches[1];
@@ -332,17 +339,13 @@ function nfx($expr)
                 } else { // it's a plain old var or num
                     $output[] = $val;
                     if (preg_match("/^([a-z]\w*)\($/", $stack->last(3))) {
-                        $first_argument = true;
-                        while (($o2 = $stack->pop()) != '(') {
-                            if (is_null($o2)) return $this->trigger("unexpected error"); // oops, never had a (
-                            else $output[] = $o2; // pop the argument expression stuff and push onto the output
-                        }
-                        // make sure there was a function
-                        if (!preg_match("/^([a-z]\w*)\($/", $stack->last(2), $matches))
-                            return $this->trigger("unexpected error");
+                        if ($begin_argument) {
+                            $begin_argument = false;
 
-                        $stack->push($stack->pop() + 1); // increment the argument count
-                        $stack->push('('); // put the ( back on, we'll need to pop back to it again
+                            if (!$stack->incrementArgument()) {
+                                $this->trigger("unexpected error");
+                            }
+                        }
                     }
                 }
                 $index += strlen($val);
@@ -574,6 +577,26 @@ function pop()
         return null;
     }
 
+    function incrementArgument()
+    {
+        while (($o2 = $this->pop()) != '(') {
+            if (is_null($o2)) {
+                return false; // oops, never had a (
+            } else {
+                $output[] = $o2; // pop the argument expression stuff and push onto the output
+            }
+        }
+        // make sure there was a function
+        if (!preg_match("/^([a-z]\w*)\($/", $this->last(2), $matches)) {
+            return false;
+        }
+
+        $this->push($this->pop() + 1); // increment the argument count
+        $this->push('('); // put the ( back on, we'll need to pop back to it again
+
+        return true;
+    }
+
     function isEmpty()
     {
         return empty($this->stack);

composer.json

@@ -2,7 +2,7 @@
   "name": "optimistex/yii2-expression",
   "description": "Taken from http://www.phpclasses.org/browse/file/11680.html, cred to Miles Kaufmann",
   "type": "yii2-extension",
-  "version": "2.0.1.1",
+  "version": "2.0.1.2",
   "license": "MIT",
   "authors": [
     {

tests/Expression.php

@@ -299,4 +299,39 @@ public function testBug_1()
         $this->assertEquals($expr->evaluate($formula), 364);
     }
     // */
+
+    public function testFormulaWithBrackets()
+    {
+        $e = new Expression();
+        $e->suppress_errors = true;
+        $e->functions = [
+            'min' => function ($v1, $v2) {
+                return min($v1, $v2);
+            },
+            'max' => function ($v1, $v2) {
+                return max($v1, $v2);
+            },
+        ];
+//        $formula = 'max(2,(2+2)*2)';
+
+        $data = [
+            'max(2,(2+2)*2)' => 8,
+            'max((2),(0))' => 2,
+            'max((2+2)*2,(3+2)*2)' => 10,
+            'max((4+2)*2,(3+2)*2)' => 12,
+            'min(max((2+2)*2,(3+2)*2), 1)' => 1,
+            'min(1, max(2,3))' => 1,
+            'min(1, max((4+2)*2,(3+2)*2))' => 1,
+            'min(max((2+2)*2,(3+2)*2), max((4+2)*2,(3+2)*2))' => 10,
+            'max( min((2+2)*2,(3+2)*2), min((4+2)*2,(3+2)*2) )' => 10,
+            '1 + max( min(2-(2+2)*2,(3+2)*2), min((4+2)*2,(3+2)*2) ) - 2' => 9,
+            'max(1,max(2,max(3,4)))' => 4,
+            'max( min(1 + 2, 4), 5)' => 5,
+            'min(2,(2+2)*2)' => 2,
+        ];
+
+        foreach ($data as $formula => $result) {
+            $this->assertEquals($e->evaluate($formula), $result);
+        }
+    }
 }