diff --git a/HISTORY.md b/HISTORY.md index 377754e129..520f693eea 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,6 +2,10 @@ # unreleased changes since 15.1.0 +- Breaking: #3621 change the precedence of the range operator `:` to be between + assignment and conditional expressions. For example, `a:b==c:d` now parses as + `a:(b==c):d`, and `true ? a = 1 : 7` now requires parentheses as + `true ? (a = 1) : 7`. - Fix: #3578 interpret empty true-expr of conditional as error (#3581). Thanks @gwhitney. - Fix: #3597 added nullish type definitions (#3601). Thanks @Ayo1984. diff --git a/docs/expressions/syntax.md b/docs/expressions/syntax.md index 2b8b39d8cc..f24ae5946e 100644 --- a/docs/expressions/syntax.md +++ b/docs/expressions/syntax.md @@ -123,7 +123,6 @@ Operators | Description See section below | Implicit multiplication `*`, `/`, `.*`, `./`,`%`, `mod` | Multiply, divide, modulus `+`, `-` | Add, subtract -`:` | Range `to`, `in` | Unit conversion `<<`, `>>`, `>>>` | Bitwise left shift, bitwise right arithmetic shift, bitwise right logical shift `==`, `!=`, `<`, `>`, `<=`, `>=` | Relational @@ -134,6 +133,7 @@ See section below | Implicit multiplication `xor` | Logical xor `or` | Logical or (lazily evaluated) `?`, `:` | Conditional expression +`:` | Range `=` | Assignment `,` | Parameter and column separator `;` | Row separator diff --git a/src/expression/operators.js b/src/expression/operators.js index 8076f70aeb..588ba99365 100644 --- a/src/expression/operators.js +++ b/src/expression/operators.js @@ -24,6 +24,9 @@ export const properties = [ AssignmentNode: {}, FunctionAssignmentNode: {} }, + { // range + RangeNode: {} + }, { // conditional expression ConditionalNode: { latexLeftParens: false, @@ -135,9 +138,6 @@ export const properties = [ associativeWith: [] } }, - { // range - RangeNode: {} - }, { // addition, subtraction 'OperatorNode:add': { op: '+', diff --git a/src/expression/parse.js b/src/expression/parse.js index 7be4f883ee..46b8790cf1 100644 --- a/src/expression/parse.js +++ b/src/expression/parse.js @@ -210,8 +210,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ index: 0, // current index in expr token: '', // current token tokenType: TOKENTYPE.NULL, // type of the token - nestingLevel: 0, // level of nesting inside parameters, used to ignore newline characters - conditionalLevel: null // when a conditional is being parsed, the level of the conditional is stored here + nestingLevel: 0 // level of nesting inside parameters, used to ignore newline characters } } @@ -680,7 +679,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ function parseAssignment (state) { let name, args, value, valid - const node = parseConditional(state) + const node = parseRange(state) if (state.token === '=') { if (isSymbolNode(node)) { @@ -724,6 +723,51 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ return node } + /** + * parse range, "start:end", "start:step:end", ":", "start:", ":end", etc + * @return {Node} node + * @private + */ + function parseRange (state) { + let node + const params = [] + + if (state.token === ':') { + // implicit start of range = 1 (one-based) + node = new ConstantNode(1) + } else { + // explicit start + node = parseConditional(state) + } + + if (state.token === ':') { + params.push(node) + + // parse step and end + while (state.token === ':' && params.length < 3) { // eslint-disable-line no-unmodified-loop-condition + getTokenSkipNewline(state) + + if (state.token === ')' || state.token === ']' || state.token === ',' || state.token === '') { + // implicit end + params.push(new SymbolNode('end')) + } else { + // explicit end + params.push(parseConditional(state)) + } + } + + if (params.length === 3) { + // params = [start, step, end] + node = new RangeNode(params[0], params[2], params[1]) // start, end, step + } else { // length === 2 + // params = [start, end] + node = new RangeNode(params[0], params[1]) // start, end + } + } + + return node + } + /** * conditional operation * @@ -738,26 +782,18 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ let node = parseLogicalOr(state) while (state.token === '?') { // eslint-disable-line no-unmodified-loop-condition - // set a conditional level, the range operator will be ignored as long - // as conditionalLevel === state.nestingLevel. - const prev = state.conditionalLevel - state.conditionalLevel = state.nestingLevel getTokenSkipNewline(state) const condition = node - const trueExpr = parseAssignment(state) + const trueExpr = parseConditional(state) if (state.token !== ':') throw createSyntaxError(state, 'False part of conditional expression expected') - state.conditionalLevel = null getTokenSkipNewline(state) - const falseExpr = parseAssignment(state) // Note: check for conditional operator again, right associativity + const falseExpr = parseConditional(state) // Note: check for conditional operator again, right associativity node = new ConditionalNode(condition, trueExpr, falseExpr) - - // restore the previous conditional level - state.conditionalLevel = prev } return node @@ -928,7 +964,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ function parseConversion (state) { let node, name, fn, params - node = parseRange(state) + node = parseAddSubtract(state) const operators = { to: 'to', @@ -946,7 +982,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ node = new OperatorNode('*', 'multiply', [node, new SymbolNode('in')], true) } else { // operator 'a to b' or 'a in b' - params = [node, parseRange(state)] + params = [node, parseAddSubtract(state)] node = new OperatorNode(name, fn, params) } } @@ -954,61 +990,6 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ return node } - /** - * parse range, "start:end", "start:step:end", ":", "start:", ":end", etc - * @return {Node} node - * @private - */ - function parseRange (state) { - let node - const params = [] - - if (state.token === ':') { - if (state.conditionalLevel === state.nestingLevel) { - // we are in the midst of parsing a conditional operator, so not - // a range, but rather an empty true-expr, which is considered a - // syntax error - throw createSyntaxError( - state, - 'The true-expression of a conditional operator may not be empty') - } else { - // implicit start of range = 1 (one-based) - node = new ConstantNode(1) - } - } else { - // explicit start - node = parseAddSubtract(state) - } - - if (state.token === ':' && (state.conditionalLevel !== state.nestingLevel)) { - // we ignore the range operator when a conditional operator is being processed on the same level - params.push(node) - - // parse step and end - while (state.token === ':' && params.length < 3) { // eslint-disable-line no-unmodified-loop-condition - getTokenSkipNewline(state) - - if (state.token === ')' || state.token === ']' || state.token === ',' || state.token === '') { - // implicit end - params.push(new SymbolNode('end')) - } else { - // explicit end - params.push(parseAddSubtract(state)) - } - } - - if (params.length === 3) { - // params = [start, step, end] - node = new RangeNode(params[0], params[2], params[1]) // start, end, step - } else { // length === 2 - // params = [start, end] - node = new RangeNode(params[0], params[1]) // start, end - } - } - - return node - } - /** * add or subtract * @return {Node} node diff --git a/test/unit-tests/expression/node/ConditionalNode.test.js b/test/unit-tests/expression/node/ConditionalNode.test.js index f4451ea230..cf53adf394 100644 --- a/test/unit-tests/expression/node/ConditionalNode.test.js +++ b/test/unit-tests/expression/node/ConditionalNode.test.js @@ -347,6 +347,10 @@ describe('ConditionalNode', function () { assert.strictEqual(n.toTex(), '\\begin{cases} { a=2}, &\\quad{\\text{if }\\;true}\\\\{\\mathrm{b}=3}, &\\quad{\\text{otherwise}}\\end{cases}') }) + it('should LaTeX a ConditionalNode with RangeNode branches', function () { + assert.strictEqual(math.parse('a ? (b:c) : (d:e)').toTex(), '\\begin{cases} {\\left(\\mathrm{b}: c\\right)}, &\\quad{\\text{if }\\; a}\\\\{\\left( d: e\\right)}, &\\quad{\\text{otherwise}}\\end{cases}') + }) + it('should LaTeX a ConditionalNode with custom toTex', function () { // Also checks if the custom functions get passed on to the children const customFunction = function (node, options) { diff --git a/test/unit-tests/expression/node/RangeNode.test.js b/test/unit-tests/expression/node/RangeNode.test.js index 97e7b2056d..aff2522b56 100644 --- a/test/unit-tests/expression/node/RangeNode.test.js +++ b/test/unit-tests/expression/node/RangeNode.test.js @@ -276,7 +276,7 @@ describe('RangeNode', function () { const n = new RangeNode(o1, o1, o2) - assert.strictEqual(n.toString(), '1 + 2:(1 < 2):1 + 2') + assert.strictEqual(n.toString(), '1 + 2:1 < 2:1 + 2') }) it('should stringify a RangeNode with a RangeNode', function () { @@ -374,6 +374,10 @@ describe('RangeNode', function () { assert.strictEqual(n.toTex(), '0:2:10') }) + it('should LaTeX a RangeNode with a ConditionalNode', function () { + assert.strictEqual(math.parse('(true ? 3 : -1) : 2 : 5').toTex(), '\\left(\\begin{cases} {3}, &\\quad{\\text{if }\\;true}\\\\{-1}, &\\quad{\\text{otherwise}}\\end{cases}\\right):2:5') + }) + it('should LaTeX a RangeNode with custom toTex', function () { // Also checks if the custom functions get passed on to the children const customFunction = function (node, options) { diff --git a/test/unit-tests/expression/operators.test.js b/test/unit-tests/expression/operators.test.js index 7abc1e2fee..c3b31a701f 100644 --- a/test/unit-tests/expression/operators.test.js +++ b/test/unit-tests/expression/operators.test.js @@ -3,6 +3,7 @@ import math from '../../../src/defaultInstance.js' import { getAssociativity, getPrecedence, isAssociativeWith, getOperator } from '../../../src/expression/operators.js' const OperatorNode = math.OperatorNode const AssignmentNode = math.AssignmentNode +const RangeNode = math.RangeNode const SymbolNode = math.SymbolNode const ConstantNode = math.ConstantNode const Node = math.Node @@ -17,10 +18,14 @@ describe('operators', function () { const n2 = new OperatorNode('??', 'nullish', [a, b]) const n3 = new OperatorNode('or', 'or', [a, b]) const n4 = math.parse("M'") + const n5 = new RangeNode(a, b) + const n6 = math.parse('a ? b : c') assert.strictEqual(getPrecedence(n1, 'keep'), 0) + assert.strictEqual(getPrecedence(n5, 'keep'), 1) + assert.strictEqual(getPrecedence(n6, 'keep'), 2) assert.strictEqual(getPrecedence(n2, 'keep'), 17) // nullish coalescing - assert.strictEqual(getPrecedence(n3, 'keep'), 2) // logical or + assert.strictEqual(getPrecedence(n3, 'keep'), 3) // logical or assert.strictEqual(getPrecedence(n4, 'keep'), 19) }) diff --git a/test/unit-tests/expression/parse.test.js b/test/unit-tests/expression/parse.test.js index 444247244f..1aedc05963 100644 --- a/test/unit-tests/expression/parse.test.js +++ b/test/unit-tests/expression/parse.test.js @@ -2205,16 +2205,17 @@ describe('parse', function () { }) it( - 'should allow a range with implicit start as the false expr', + 'should require parentheses for a range with implicit start as the false expr', function () { - assert.strictEqual(parseAndEval('true?0::3'), 0) - assert.deepStrictEqual(parseAndEval('false?0::3'), parseAndEval(':3')) + assert.throws(() => parseAndEval('false?0::3'), SyntaxError) + assert.deepStrictEqual(parseAndEval('false?0:(:3)'), parseAndEval(':3')) }) it('should parse : (range)', function () { assert.ok(parseAndEval('2:5') instanceof Matrix) assert.deepStrictEqual(parseAndEval('2:5'), math.matrix([2, 3, 4, 5])) assert.deepStrictEqual(parseAndEval('10:-2:0'), math.matrix([10, 8, 6, 4, 2, 0])) + assert.deepStrictEqual(parseAndEval('1:2:10'), math.matrix([1, 3, 5, 7, 9])) assert.deepStrictEqual(parseAndEval('2:4.0'), math.matrix([2, 3, 4])) assert.deepStrictEqual(parseAndEval('2:4.5'), math.matrix([2, 3, 4])) assert.deepStrictEqual(parseAndEval('2:4.1'), math.matrix([2, 3, 4])) @@ -2361,6 +2362,34 @@ describe('parse', function () { assert.deepStrictEqual(parseAndEval('false ? 1:2:6'), math.matrix([2, 3, 4, 5, 6])) }) + it('should parse range with lower precedence than conditional operator (#3621)', function () { + const node = math.parse('true ? 3 : -1 : 2 : 5') + + assert(node instanceof RangeNode) + assert(node.start instanceof ConditionalNode) + assert.strictEqual(node.start.toString(), 'true ? 3 : (-1)') + assert.strictEqual(node.step.toString(), '2') + assert.strictEqual(node.end.toString(), '5') + assert.deepStrictEqual(node.compile().evaluate(), math.matrix([3, 5])) + }) + + it('should keep conditional lower than logical or', function () { + const node = math.parse('false or true ? 1 : 2') + + assert(node instanceof ConditionalNode) + assert.strictEqual(node.condition.toString(), 'false or true') + assert.strictEqual(node.compile().evaluate(), 1) + }) + + it('should parse logical or with higher precedence than range (#3621)', function () { + const node = math.parse('false or 1:3') + + assert(node instanceof RangeNode) + assert.strictEqual(node.start.toString(), 'false or 1') + assert.strictEqual(node.end.toString(), '3') + assert.deepStrictEqual(node.compile().evaluate(), math.matrix([1, 2, 3])) + }) + it('should respect precedence between left/right shift and relational operators', function () { assert.strictEqual(parseAndEval('32 >> 4 == 2'), true) assert.strictEqual(parseAndEval('2 == 32 >> 4'), true) @@ -2415,12 +2444,14 @@ describe('parse', function () { assert.strictEqual(node.falseExpr.toString(), 'a < b') }) - it('should respect precedence of conditional operator and range operator', function () { + it('should respect precedence of range operator and conditional operator', function () { const node = math.parse('a ? b : c : d') - assert(node instanceof ConditionalNode) - assert.strictEqual(node.condition.toString(), 'a') - assert.strictEqual(node.trueExpr.toString(), 'b') - assert.strictEqual(node.falseExpr.toString(), 'c:d') + assert(node instanceof RangeNode) + assert(node.start instanceof ConditionalNode) + assert.strictEqual(node.start.condition.toString(), 'a') + assert.strictEqual(node.start.trueExpr.toString(), 'b') + assert.strictEqual(node.start.falseExpr.toString(), 'c') + assert.strictEqual(node.end.toString(), 'd') }) it('should respect precedence of conditional operator and range operator (2)', function () { @@ -2441,9 +2472,10 @@ describe('parse', function () { it('should respect precedence of range operator and relational operators', function () { const node = math.parse('a:b == c:d') - assert(node instanceof OperatorNode) - assert.strictEqual(node.args[0].toString(), 'a:b') - assert.strictEqual(node.args[1].toString(), 'c:d') + assert(node instanceof RangeNode) + assert.strictEqual(node.start.toString(), 'a') + assert.strictEqual(node.step.toString(), 'b == c') + assert.strictEqual(node.end.toString(), 'd') }) it('should respect precedence of range operator and operator plus and minus', function () { @@ -2467,6 +2499,61 @@ describe('parse', function () { assert.strictEqual(node.args[1].toString(), 'c') }) + it('should respect precedence of "to" operator and range operator', function () { + const node1 = math.parse('a:b to c') + assert(node1 instanceof RangeNode) + assert.strictEqual(node1.start.toString(), 'a') + assert.strictEqual(node1.end.toString(), 'b to c') + + const node2 = math.parse('a to b:c') + assert(node2 instanceof RangeNode) + assert.strictEqual(node2.start.toString(), 'a to b') + assert.strictEqual(node2.end.toString(), 'c') + }) + + it('should keep assignment lower than range and conditional operators (#3621)', function () { + assert.throws(function () { parseAndEval('true ? a = 1 : 7') }, SyntaxError) + + const scope = {} + assert.strictEqual(parseAndEval('true ? (a = 1) : 7', scope), 1) + assert.strictEqual(scope.a, 1) + + assert.deepStrictEqual(parseAndEval('a = 1:5', scope), math.matrix([1, 2, 3, 4, 5])) + assert.deepStrictEqual(scope.a, math.matrix([1, 2, 3, 4, 5])) + }) + + it('should still parse ranges in indexes and function arguments (#3621)', function () { + const scope = { + A: [1, 2, 3, 4], + f: x => x + } + + assert.deepStrictEqual(parseAndEval('A[1:3]', scope), [1, 2, 3]) + assert.deepStrictEqual(parseAndEval('f(1:3)', scope), math.matrix([1, 2, 3])) + }) + + it('should keep nested conditional operators right-associative (#3621)', function () { + const node = math.parse('a ? b : c ? d : e') + + assert(node instanceof ConditionalNode) + assert.strictEqual(node.condition.toString(), 'a') + assert.strictEqual(node.trueExpr.toString(), 'b') + assert(node.falseExpr instanceof ConditionalNode) + assert.strictEqual(node.falseExpr.toString(), 'c ? d : e') + }) + + it('should round trip range with conditional start (#3621)', function () { + const node = math.parse('(true ? 3 : -1) : 2 : 5') + const stringified = node.toString() + const reparsed = math.parse(stringified) + + assert.strictEqual(reparsed.toString(), stringified) + assert.strictEqual( + reparsed.toString({ parenthesis: 'all' }), + node.toString({ parenthesis: 'all' }) + ) + }) + // TODO: extensively test operator precedence }) })