Skip to content

Commit

Permalink
fix: #3301 precedence of % (mod) is higher than * and / (#3311)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nkumawat34
nkumawat34 authored Nov 7, 2024
1 parent b10e316 commit d0f8b2b
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 46 deletions.
3 changes: 1 addition & 2 deletions docs/expressions/syntax.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,8 @@ Operators | Description
`!` | Factorial
`^`, `.^` | Exponentiation
`+`, `-`, `~`, `not` | Unary plus, unary minus, bitwise not, logical not
`%`, `mod` | percentage, modulus
See section below | Implicit multiplication
`*`, `/`, `.*`, `./` | Multiply, divide
`*`, `/`, `.*`, `./`,`%`, `mod` | Multiply, divide , percentage, modulus
`+`, `-` | Add, subtract
`:` | Range
`to`, `in` | Unit conversion
Expand Down
67 changes: 25 additions & 42 deletions src/expression/parse.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -1039,7 +1039,9 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
'*': 'multiply',
'.*': 'dotMultiply',
'/': 'divide',
'./': 'dotDivide'
'./': 'dotDivide',
'%': 'mod',
mod: 'mod'
}

while (true) {
Expand All @@ -1050,8 +1052,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
}
Expand Down Expand Up @@ -1104,7 +1120,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 = []

Expand All @@ -1127,7 +1143,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
Expand All @@ -1148,39 +1164,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
Expand Down
55 changes: 53 additions & 2 deletions test/unit-tests/expression/parse.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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/)
})
Expand All @@ -1375,6 +1375,57 @@ describe('parse', function () {
approxEqual(parseAndEval('8 mod 3'), 2)
})

it('should give equal precedence to % 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 () {
approxEqual(parseAndStringifyWithParens('10 % 4 / 2'), '(10 % 4) / 2')
approxEqual(parseAndStringifyWithParens('10 / 2 % 3'), '(10 / 2) % 3')
})

it('should give equal precedence to mod and * operators', function () {
approxEqual(parseAndStringifyWithParens('8 mod 3 * 2'), '(8 mod 3) * 2')
approxEqual(parseAndStringifyWithParens('8 * 3 mod 5'), '(8 * 3) mod 5')
})

it('should give equal precedence to mod and / operators', function () {
approxEqual(parseAndStringifyWithParens('8 mod 3 / 2'), '(8 mod 3) / 2')
approxEqual(parseAndStringifyWithParens('8 / 3 mod 2'), '(8 / 3) mod 2')
})

it('should give equal precedence to % 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 () {
approxEqual(parseAndStringifyWithParens('10 % 4 ./ 2'), '(10 % 4) ./ 2')
approxEqual(parseAndStringifyWithParens('10 ./ 2 % 3'), '(10 ./ 2) % 3')
})

it('should give equal precedence to mod and .* operators', function () {
approxEqual(parseAndStringifyWithParens('8 mod 3 .* 2'), '(8 mod 3) .* 2')
approxEqual(parseAndStringifyWithParens('8 .* 3 mod 5'), '(8 .* 3) mod 5')
})

it('should give equal precedence to mod and ./ operators', function () {
approxEqual(parseAndStringifyWithParens('8 mod 3 ./ 2'), '(8 mod 3) ./ 2')
approxEqual(parseAndStringifyWithParens('8 ./ 3 mod 2'), '(8 ./ 3) mod 2')
})

it('should evaluate complex expressions with mixed precedence equally', function () {
approxEqual(parseAndStringifyWithParens('10 % 3 * 2 + 4 / 2'), '((10 % 3) * 2) + (4 / 2)')
approxEqual(parseAndStringifyWithParens('8 mod 3 + 2 * 4 - 5'), '((8 mod 3) + (2 * 4)) - 5')
approxEqual(parseAndStringifyWithParens('12 / 4 % 2 .* 5'), '((12 / 4) % 2) .* 5')
})

it('should handle cases with equal precedence among all operators', function () {
approxEqual(parseAndStringifyWithParens('10 % 3 .* 2 ./ 2'), '((10 % 3) .* 2) ./ 2')
approxEqual(parseAndStringifyWithParens('10 ./ 2 % 3 * 2'), '((10 ./ 2) % 3) * 2')
})

it('should parse multiply *', function () {
approxEqual(parseAndEval('4 * 2'), 8)
approxEqual(parseAndEval('8 * 2 * 2'), 32)
Expand Down

0 comments on commit d0f8b2b

Please sign in to comment.