Skip to content

Commit

Permalink
fix(parse): Implement amended "Rule 2"
Browse files Browse the repository at this point in the history
  As per the discussion in #2370, the amended "Rule 2" is
  "when having a division followed by an implicit multiplication, the
   division gets higher precedence over the implicit multiplication when
   (a) the numerator is a constant with optionally a
       prefix operator (-, +, ~), and
   (b) the denominator is a constant."
  This commit implements that behavior and adds tests for it.
  Resolves #2370.
  • Loading branch information
gwhitney committed Apr 30, 2022
1 parent 702ce85 commit e8b8b4e
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/expression/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -1073,10 +1073,19 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
return node
}

function isRule2Node (node) {
return isConstantNode(node) ||
(isOperatorNode(node) &&
node.args.length === 1 &&
isConstantNode(node.args[0]) &&
'-+~'.includes(node.op))
}
/**
* Infamous "rule 2" as described in https://github.com/josdejong/mathjs/issues/792#issuecomment-361065370
* And as amended in https://github.com/josdejong/mathjs/issues/2370#issuecomment-1054052164
* Explicit division gets higher precedence than implicit multiplication
* when the division matches this pattern: [number] / [number] [symbol]
* when the division matches this pattern:
* [unaryPrefixOp]?[number] / [number] [symbol]
* @return {Node} node
* @private
*/
Expand All @@ -1087,7 +1096,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({

while (true) {
// Match the "number /" part of the pattern "number / number symbol"
if (state.token === '/' && isConstantNode(last)) {
if (state.token === '/' && isRule2Node(last)) {
// Look ahead to see if the next token is a number
tokenStates.push(Object.assign({}, state))
getTokenSkipNewline(state)
Expand Down
32 changes: 32 additions & 0 deletions test/unit-tests/expression/node/OperatorNode.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,38 @@ describe('OperatorNode', function () {
)
})

it('should stringify implicit multiplications in a recoverable way', function () {
let coeff = new OperatorNode('/', 'divide',
[new ConstantNode(1), new ConstantNode(2)])
const variable = new SymbolNode('a')
let start = new OperatorNode('*', 'multiply', [coeff, variable], true)
let startString = start.toString()
let finish = math.parse(startString)
assert.strictEqual(
finish.toString({ parenthesis: 'all' }),
start.toString({ parenthesis: 'all' }))
coeff = new OperatorNode('/', 'divide', [
new OperatorNode('-', 'unaryMinus', [new ConstantNode(1)]),
new ConstantNode(2)])
start = new OperatorNode('*', 'multiply', [coeff, variable], true)
startString = start.toString()
finish = math.parse(startString)
assert.strictEqual(
finish.toString({ parenthesis: 'all' }),
start.toString({ parenthesis: 'all' }))
coeff = new OperatorNode('/', 'divide', [
new ConstantNode(1),
new OperatorNode('-', 'unaryMinus', [new ConstantNode(2)])])
start = new OperatorNode('*', 'multiply', [coeff, variable], true)
startString = start.toString()
finish = math.parse(startString)
/* FIXME: this test currently fails, related to #1431:
assert.strictEqual(
finish.toString({ parenthesis: 'all' }),
start.toString({ parenthesis: 'all' }))
*/
})

it('should stringify implicit multiplications between ConstantNodes with parentheses', function () {
const a = math.parse('(4)(4)(4)(4)')
const b = math.parse('4b*4(4)')
Expand Down
5 changes: 5 additions & 0 deletions test/unit-tests/expression/parse.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1338,11 +1338,16 @@ describe('parse', function () {

it('should follow precedence rules for implicit multiplication and division', function () {
assert.strictEqual(parseAndStringifyWithParens('2 / 3 x'), '(2 / 3) x')
assert.strictEqual(parseAndStringifyWithParens('-2/3x'), '((-2) / 3) x')
assert.strictEqual(parseAndStringifyWithParens('2.5 / 5 kg'), '(2.5 / 5) kg')
assert.strictEqual(parseAndStringifyWithParens('2.5 / 5 x y'), '((2.5 / 5) x) y')
assert.strictEqual(parseAndStringifyWithParens('2 x / 5 y'), '(2 x) / (5 y)')
assert.strictEqual(parseAndStringifyWithParens('17 h / 1 h'), '(17 h) / (1 h)')
assert.strictEqual(parseAndStringifyWithParens('1 / 2 x'), '(1 / 2) x')
assert.strictEqual(parseAndStringifyWithParens('+1/2x'), '((+1) / 2) x')
assert.strictEqual(parseAndStringifyWithParens('~1/2x'), '((~1) / 2) x')
assert.strictEqual(parseAndStringifyWithParens('1 / -2 x'), '1 / ((-2) x)')
assert.strictEqual(parseAndStringifyWithParens('-1 / -2 x'), '(-1) / ((-2) x)')
assert.strictEqual(parseAndStringifyWithParens('1 / 2 * x'), '(1 / 2) * x')
assert.strictEqual(parseAndStringifyWithParens('1 / 2 x y'), '((1 / 2) x) y')
assert.strictEqual(parseAndStringifyWithParens('1 / 2 (x y)'), '(1 / 2) (x y)')
Expand Down

0 comments on commit e8b8b4e

Please sign in to comment.