feat: give unary % operator (percentage) higher precedence than binary % operator (modulus) (#3432)

Author: kiprobinsonknack

docs/expressions/syntax.md

@@ -114,8 +114,9 @@ Operators                         | Description
 `!`                               | Factorial
 `^`, `.^`                         | Exponentiation
 `+`, `-`, `~`, `not`              | Unary plus, unary minus, bitwise not, logical not
+`%`                               | Unary percentage
 See section below                 | Implicit multiplication
-`*`, `/`, `.*`, `./`,`%`, `mod`   | Multiply, divide , percentage, modulus
+`*`, `/`, `.*`, `./`,`%`, `mod`   | Multiply, divide, modulus
 `+`, `-`                          | Add, subtract
 `:`                               | Range
 `to`, `in`                        | Unit conversion
@@ -134,8 +135,8 @@ See section below                 | Implicit multiplication
 `\n`, `;`                         | Statement separators
 
 Lazy evaluation is used where logically possible for bitwise and logical
-operators. In the following example, the value of `x` will not even be 
-evaluated because it cannot effect the final result: 
+operators. In the following example, the value of `x` will not even be
+evaluated because it cannot effect the final result:
 ```js
 math.evaluate('false and x')        // false, no matter what x equals
 ```
@@ -652,7 +653,7 @@ at 1, when the end is undefined, the range will end at the end of the matrix.
 
 There is a context variable `end` available as well to denote the end of the
 matrix. This variable cannot be used in multiple nested indices. In that case,
-`end` will be resolved as the end of the innermost matrix. To solve this, 
+`end` will be resolved as the end of the innermost matrix. To solve this,
 resolving of the nested index needs to be split in two separate operations.
 
 *IMPORTANT: matrix indexes and ranges work differently from the math.js indexes

src/expression/parse.js

@@ -1001,7 +1001,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
   function parseAddSubtract (state) {
     let node, name, fn, params
 
-    node = parseMultiplyDivideModulusPercentage(state)
+    node = parseMultiplyDivideModulus(state)
 
     const operators = {
       '+': 'add',
@@ -1012,7 +1012,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
       fn = operators[name]
 
       getTokenSkipNewline(state)
-      const rightNode = parseMultiplyDivideModulusPercentage(state)
+      const rightNode = parseMultiplyDivideModulus(state)
       if (rightNode.isPercentage) {
         params = [node, new OperatorNode('*', 'multiply', [node, rightNode])]
       } else {
@@ -1025,11 +1025,11 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
   }
 
   /**
-   * multiply, divide, modulus, percentage
+   * multiply, divide, modulus
    * @return {Node} node
    * @private
    */
-  function parseMultiplyDivideModulusPercentage (state) {
+  function parseMultiplyDivideModulus (state) {
     let node, last, name, fn
 
     node = parseImplicitMultiplication(state)
@@ -1053,17 +1053,8 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
         getTokenSkipNewline(state)
 
         if (name === '%' && state.tokenType === TOKENTYPE.DELIMITER && state.token !== '(') {
-          // If the expression contains only %, then treat that as /100
-          if (state.token !== '' && operators[state.token]) {
-            const left = new OperatorNode('/', 'divide', [node, new ConstantNode(100)], false, true)
-            name = state.token
-            fn = operators[name]
-            getTokenSkipNewline(state)
-            last = parseImplicitMultiplication(state)
-
-            node = new OperatorNode(name, fn, [left, last])
-          } else { node = new OperatorNode('/', 'divide', [node, new ConstantNode(100)], false, true) }
-          // return node
+          // This % cannot be interpreted as a modulus, and it wasn't handled by parseUnaryPostfix
+          throw createSyntaxError(state, 'Unexpected operator %')
         } else {
           last = parseImplicitMultiplication(state)
           node = new OperatorNode(name, fn, [node, last])
@@ -1120,7 +1111,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
    * @private
    */
   function parseRule2 (state) {
-    let node = parseUnary(state)
+    let node = parseUnaryPercentage(state)
     let last = node
     const tokenStates = []
 
@@ -1143,7 +1134,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
             // Rewind once and build the "number / number" node; the symbol will be consumed later
             Object.assign(state, tokenStates.pop())
             tokenStates.pop()
-            last = parseUnary(state)
+            last = parseUnaryPercentage(state)
             node = new OperatorNode('/', 'divide', [node, last])
           } else {
             // Not a match, so rewind
@@ -1164,6 +1155,30 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
     return node
   }
 
+  /**
+   * Unary percentage operator (treated as `value / 100`)
+   * @return {Node} node
+   * @private
+   */
+  function parseUnaryPercentage (state) {
+    let node = parseUnary(state)
+
+    if (state.token === '%') {
+      const previousState = Object.assign({}, state)
+      getTokenSkipNewline(state)
+
+      if (state.tokenType === TOKENTYPE.DELIMITER && state.token !== '(') {
+        // This is unary postfix %, then treat that as /100
+        node = new OperatorNode('/', 'divide', [node, new ConstantNode(100)], false, true)
+      } else {
+        // Not a match, so rewind
+        Object.assign(state, previousState)
+      }
+    }
+
+    return node
+  }
+
   /**
    * Unary plus and minus, and logical and bitwise not
    * @return {Node} node

test/unit-tests/expression/parse.test.js

@@ -1370,18 +1370,29 @@ describe('parse', function () {
     })
 
     it('should parse % with division', function () {
-      approxEqual(parseAndEval('100/50%'), 0.02) // should be treated as ((100/50)%)
-      approxEqual(parseAndEval('100/50%*2'), 0.04) // should be treated as ((100/50)%))×2
+      approxEqual(parseAndEval('100/50%'), 200) // should be treated as 100/(50%)
+      approxEqual(parseAndEval('100/50%*2'), 400) // should be treated as (100/(50%))×2
       approxEqual(parseAndEval('50%/100'), 0.005)
+      approxEqual(parseAndEval('50%(13)'), 11) // should be treated as 50 % (13)
       assert.throws(function () { parseAndEval('50%(/100)') }, /Value expected/)
     })
 
-    it('should parse % with addition', function () {
+    it('should parse unary % before division, binary % with division', function () {
+      approxEqual(parseAndEval('10/200%%3'), 2) // should be treated as (10/(200%))%3
+    })
+
+    it('should reject repeated unary percentage operators', function () {
+      assert.throws(function () { math.parse('17%%') }, /Unexpected operator %/)
+      assert.throws(function () { math.parse('17%%*5') }, /Unexpected operator %/)
+      assert.throws(function () { math.parse('10/200%%%3') }, /Unexpected operator %/)
+    })
+
+    it('should parse unary % with addition', function () {
       approxEqual(parseAndEval('100+3%'), 103)
       approxEqual(parseAndEval('3%+100'), 100.03)
     })
 
-    it('should parse % with subtraction', function () {
+    it('should parse unary % with subtraction', function () {
       approxEqual(parseAndEval('100-3%'), 97)
       approxEqual(parseAndEval('3%-100'), -99.97)
     })
@@ -1390,12 +1401,12 @@ describe('parse', function () {
       approxEqual(parseAndEval('8 mod 3'), 2)
     })
 
-    it('should give equal precedence to % and * operators', function () {
+    it('should give equal precedence to binary % and * operators', function () {
       approxEqual(parseAndStringifyWithParens('10 % 3 * 2'), '(10 % 3) * 2')
       approxEqual(parseAndStringifyWithParens('10 * 3 % 4'), '(10 * 3) % 4')
     })
 
-    it('should give equal precedence to % and / operators', function () {
+    it('should give equal precedence to binary % and / operators', function () {
       approxEqual(parseAndStringifyWithParens('10 % 4 / 2'), '(10 % 4) / 2')
       approxEqual(parseAndStringifyWithParens('10 / 2 % 3'), '(10 / 2) % 3')
     })
@@ -1410,12 +1421,12 @@ describe('parse', function () {
       approxEqual(parseAndStringifyWithParens('8 / 3 mod 2'), '(8 / 3) mod 2')
     })
 
-    it('should give equal precedence to % and .* operators', function () {
+    it('should give equal precedence to binary % and .* operators', function () {
       approxEqual(parseAndStringifyWithParens('10 % 3 .* 2'), '(10 % 3) .* 2')
       approxEqual(parseAndStringifyWithParens('10 .* 3 % 4'), '(10 .* 3) % 4')
     })
 
-    it('should give equal precedence to % and ./ operators', function () {
+    it('should give equal precedence to binary % and ./ operators', function () {
       approxEqual(parseAndStringifyWithParens('10 % 4 ./ 2'), '(10 % 4) ./ 2')
       approxEqual(parseAndStringifyWithParens('10 ./ 2 % 3'), '(10 ./ 2) % 3')
     })