fix: #3621 give range lower precedence than conditional expressions (breaking)

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.

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

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: '+',

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,69 +982,14 @@ 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)
       }
     }
 
     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

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) {

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) {

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)
   })