Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BugFix3301: Precedence of % (mod) is higher than * and / #3311

Merged
merged 4 commits into from
Nov 7, 2024
Merged

BugFix3301: Precedence of % (mod) is higher than * and / #3311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6ded46a
BugFix3301: Precedence of % (mod) is higher than * and /
nkumawat34 Nov 4, 2024
0c27d03
BugFix3301: Precedence of % (mod) is higher than * and /
nkumawat34 Nov 7, 2024
b414dde
BugFix3301: Precedence of % (mod) is higher than * and /
nkumawat34 Nov 7, 2024
7007c77
Merge branch 'bugFix-3301' of https://github.com/nkumawat34/mathjs in…
nkumawat34 Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions docs/expressions/syntax.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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