Bug fix 3301:Precedence of % (mod) is higher than * and /

src/expression/Parser.js

@@ -2,9 +2,9 @@ import { factory } from '../utils/factory.js'
 import { createEmptyMap, toObject } from '../utils/map.js'
 
 const name = 'Parser'
-const dependencies = ['evaluate']
+const dependencies = ['evaluate', 'parse']
 
-export const createParserClass = /* #__PURE__ */ factory(name, dependencies, ({ evaluate }) => {
+export const createParserClass = /* #__PURE__ */ factory(name, dependencies, ({ evaluate, parse }) => {
   /**
    * @constructor Parser
    * Parser contains methods to evaluate or parse expressions, and has a number
@@ -112,12 +112,32 @@ export const createParserClass = /* #__PURE__ */ factory(name, dependencies, ({
     return this.scope
   }
 
+  function isValidVariableName (name) {
+    if (name.length === 0) { return false }
+
+    for (let i = 0; i < name.length; i++) {
+      const cPrev = name.charAt(i - 1)
+      const c = name.charAt(i)
+      const cNext = name.charAt(i + 1)
+      const valid = parse.isAlpha(c, cPrev, cNext) || (i > 0 && parse.isDigit(c))
+
+      if (!valid) {
+        return false
+      }
+    }
+
+    return true
+  }
+
   /**
    * Set a symbol (a function or variable) by name from the parsers scope.
    * @param {string} name
    * @param {* | undefined} value
    */
   Parser.prototype.set = function (name, value) {
+    if (!isValidVariableName(name)) {
+      throw new Error(`Invalid variable name: '${name}'. Variable names must follow the specified rules.`)
+    }
     this.scope.set(name, value)
     return value
   }

src/expression/parse.js

@@ -1001,7 +1001,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
   function parseAddSubtract (state) {
     let node, name, fn, params
 
-    node = parseMultiplyDivide(state)
+    node = parseMultiplyDivideModulusPercentage(state)
 
     const operators = {
       '+': 'add',
@@ -1012,7 +1012,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
       fn = operators[name]
 
       getTokenSkipNewline(state)
-      const rightNode = parseMultiplyDivide(state)
+      const rightNode = parseMultiplyDivideModulusPercentage(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
+   * multiply, divide, modulus, percentage
    * @return {Node} node
    * @private
    */
-  function parseMultiplyDivide (state) {
+  function parseMultiplyDivideModulusPercentage (state) {
     let node, last, name, fn
 
     node = parseImplicitMultiplication(state)
@@ -1039,9 +1039,10 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
       '*': 'multiply',
       '.*': 'dotMultiply',
       '/': 'divide',
-      './': 'dotDivide'
+      './': 'dotDivide',
+      '%': 'mod',
+      mod: 'mod'
     }
-
     while (true) {
       if (hasOwnProperty(operators, state.token)) {
         // explicit operators
@@ -1050,8 +1051,22 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
 
         getTokenSkipNewline(state)
 
-        last = parseImplicitMultiplication(state)
-        node = new OperatorNode(name, fn, [node, last])
+        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
+        } else {
+          last = parseImplicitMultiplication(state)
+          node = new OperatorNode(name, fn, [node, last])
+        }
       } else {
         break
       }
@@ -1104,7 +1119,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
    * @private
    */
   function parseRule2 (state) {
-    let node = parseModulusPercentage(state)
+    let node = parseUnary(state)
     let last = node
     const tokenStates = []
 
@@ -1127,7 +1142,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 = parseModulusPercentage(state)
+            last = parseUnary(state)
             node = new OperatorNode('/', 'divide', [node, last])
           } else {
             // Not a match, so rewind
@@ -1148,39 +1163,6 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
     return node
   }
 
-  /**
-   * modulus and percentage
-   * @return {Node} node
-   * @private
-   */
-  function parseModulusPercentage (state) {
-    let node, name, fn, params
-
-    node = parseUnary(state)
-
-    const operators = {
-      '%': 'mod',
-      mod: 'mod'
-    }
-
-    while (hasOwnProperty(operators, state.token)) {
-      name = state.token
-      fn = operators[name]
-
-      getTokenSkipNewline(state)
-
-      if (name === '%' && state.tokenType === TOKENTYPE.DELIMITER && state.token !== '(') {
-        // If the expression contains only %, then treat that as /100
-        node = new OperatorNode('/', 'divide', [node, new ConstantNode(100)], false, true)
-      } else {
-        params = [node, parseUnary(state)]
-        node = new OperatorNode(name, fn, params)
-      }
-    }
-
-    return node
-  }
-
   /**
    * Unary plus and minus, and logical and bitwise not
    * @return {Node} node

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

@@ -128,6 +128,24 @@ describe('parser', function () {
     assert.strictEqual(parser.evaluate('pi'), Math.PI)
   })
 
+  it('should validate variable names', function () {
+    const parser = new Parser()
+
+    // Valid variable names
+    assert.strictEqual(parser.set('validVar', 42), 42)
+    assert.strictEqual(parser.evaluate('validVar'), 42)
+    assert.strictEqual(parser.set('_underscoreVar', 10), 10)
+    assert.strictEqual(parser.evaluate('_underscoreVar'), 10)
+    assert.strictEqual(parser.set('var123', 100), 100)
+    assert.strictEqual(parser.evaluate('var123'), 100)
+
+    // Invalid variable names
+    assert.throws(() => parser.set('123var', 5), /Invalid variable name/)
+    assert.throws(() => parser.set('var-with-hyphen', 5), /Invalid variable name/)
+    assert.throws(() => parser.set('var with space', 5), /Invalid variable name/)
+    assert.throws(() => parser.set('@specialChar', 5), /Invalid variable name/)
+  })
+
   describe('security', function () {
     it('should return undefined when accessing what appears to be inherited properties', function () {
       try {

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

@@ -1355,8 +1355,8 @@ describe('parse', function () {
     })
 
     it('should parse % with division', function () {
-      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('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('50%/100'), 0.005)
       assert.throws(function () { parseAndEval('50%(/100)') }, /Value expected/)
     })