From 695df1647da39d33f05073a8d252900309f7d8cb Mon Sep 17 00:00:00 2001 From: Sarath Francis Date: Wed, 3 Jun 2026 03:17:50 -0400 Subject: [PATCH] fix: keep implicit multiplication round-trippable after a symbol or accessor When stringifying an implicit multiplication, a ConstantNode that follows an unparenthesized factor was wrapped in parentheses so that adjacent numbers such as `4 4` (which is not valid input) would be serialized recoverably as `4 (4)`. However, when the preceding factor ends with a SymbolNode or AccessorNode the parenthesized ConstantNode reparses as a function call or index access rather than an implicit multiplication. For example `parse('x 2').toString()` returned `'x (2)'`, which parses back as the function call `x(2)` instead of `x * 2`, so the node no longer round-tripped and evaluating the serialized form threw or produced a different value. The same applied to `a b 2` (-> `a b (2)`, parsed as `a * b(2)`) and `A[1] 2` (-> `A[1] (2)`, parsed as the index access `A[1](2)`). Only force the parentheses when the preceding factor does not end with a symbol or accessor, in which case the bare ConstantNode is unambiguous and recoverable. --- HISTORY.md | 4 ++ src/expression/node/OperatorNode.js | 42 ++++++++++++++++++- .../expression/node/OperatorNode.test.js | 29 +++++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index deb73875db..81d5b30a02 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,6 +2,10 @@ # unpublished changes since 15.2.0 +- Fix: `Node.toString` of an implicit multiplication no longer wraps a + `ConstantNode` that follows a `SymbolNode` or `AccessorNode` in parentheses, + which made the output reparse as a function call or index access + (for example `x 2` was stringified as `x (2)`, which parses as `x(2)`). - Docs: fix the browser example `rocket_trajectory_optimization.html` (#3654). Thanks @dvd101x. diff --git a/src/expression/node/OperatorNode.js b/src/expression/node/OperatorNode.js index 6890db6ece..818379f619 100644 --- a/src/expression/node/OperatorNode.js +++ b/src/expression/node/OperatorNode.js @@ -1,4 +1,4 @@ -import { isNode, isConstantNode, isOperatorNode, isParenthesisNode } from '../../utils/is.js' +import { isAccessorNode, isConstantNode, isFunctionNode, isNode, isOperatorNode, isParenthesisNode, isSymbolNode } from '../../utils/is.js' import { map } from '../../utils/array.js' import { createSubScope } from '../../utils/scope.js' import { escape } from '../../utils/string.js' @@ -32,6 +32,39 @@ export const createOperatorNode = /* #__PURE__ */ factory(name, dependencies, ({ return false } + /** + * Returns true if the string representation of the expression ends with a + * SymbolNode or AccessorNode, i.e. something that a directly following + * parenthesis would bind to as a function invocation or matrix index + * (for example `x` in `a b x` or `A[1]` in `c A[1]`). In that situation a + * parenthesized ConstantNode must NOT be appended for an implicit + * multiplication, since e.g. `x (2)` would reparse as the function call + * `x(2)` instead of the implicit multiplication `x * 2`. + * @param {Node} expression + * @param {string} parenthesis + * @return {boolean} + */ + function endsWithSymbolOrAccessor (expr, parenthesis) { + // A parenthesized expression ends with ')', which the parser treats as an + // implicit multiplication rather than a function call, so it is safe. + if (isParenthesisNode(expr)) return false + if (isSymbolNode(expr) || isAccessorNode(expr)) return true + // FunctionNode also ends with ')', and `)(...)` is implicit multiplication. + if (isFunctionNode(expr)) return false + if (isOperatorNode(expr)) { + if (expr.args.length === 1) { + // a prefix unary operator (like `-a`) prints its operand last and can + // expose a symbol/accessor, whereas a postfix one (like `a!`) ends with + // the operator symbol and is therefore safe + return getAssociativity(expr, parenthesis) === 'right' && + endsWithSymbolOrAccessor(expr.args[0], parenthesis) + } + // binary (and n-ary) operators print their right-most operand last + return endsWithSymbolOrAccessor(expr.args[expr.args.length - 1], parenthesis) + } + return false + } + /** * Calculate which parentheses are necessary. Gets an OperatorNode * (which is the root of the tree) and an Array of Nodes @@ -226,11 +259,16 @@ export const createOperatorNode = /* #__PURE__ */ factory(name, dependencies, ({ // of ConstantNode. // In that case, parenthesize ConstantNodes that follow an unparenthesized // expression, even though they normally wouldn't be printed. + // The exception is when the preceding factor ends with a SymbolNode or + // AccessorNode: there a parenthesized ConstantNode would reparse as a + // function call or index access (e.g. `x (2)` parses as `x(2)`), so the + // ConstantNode must be left unparenthesized to keep the round-trip intact. if (args.length >= 2 && root.getIdentifier() === 'OperatorNode:multiply' && root.implicit && parenthesis !== 'all' && implicit === 'hide') { for (let i = 1; i < result.length; ++i) { if (startsWithConstant(args[i], parenthesis) && !result[i - 1] && - (parenthesis !== 'keep' || !isParenthesisNode(args[i - 1]))) { + (parenthesis !== 'keep' || !isParenthesisNode(args[i - 1])) && + !endsWithSymbolOrAccessor(args[i - 1], parenthesis)) { result[i] = true } } diff --git a/test/unit-tests/expression/node/OperatorNode.test.js b/test/unit-tests/expression/node/OperatorNode.test.js index 812c12bd11..a3c377e2f7 100644 --- a/test/unit-tests/expression/node/OperatorNode.test.js +++ b/test/unit-tests/expression/node/OperatorNode.test.js @@ -693,6 +693,35 @@ describe('OperatorNode', function () { } }) + it('should not parenthesize a ConstantNode in an implicit multiplication when the preceding factor is a SymbolNode or AccessorNode', function () { + // A leading SymbolNode or AccessorNode followed by a parenthesized + // ConstantNode reparses as a function call / index access instead of an + // implicit multiplication, e.g. "x (2)" parses as the call "x(2)". + // Therefore the ConstantNode must not be parenthesized in those cases. + const xtimes2 = new OperatorNode('*', 'multiply', [xsym, two], true) + const abtimes2 = new OperatorNode('*', 'multiply', + [new OperatorNode('*', 'multiply', [asym, bsym], true), two], true) + const accessor = math.parse('A[1]') + const accessorTimes2 = new OperatorNode('*', 'multiply', [accessor, two], true) + + for (const paren of ['auto', 'keep']) { + for (const expr of [xtimes2, abtimes2, accessorTimes2]) { + const estring = expr.toString({ parenthesis: paren, implicit: 'hide' }) + const reparsed = math.parse(estring) + // Parsing the serialized form must reproduce the exact same grouping + assert.strictEqual( + reparsed.toString({ parenthesis: 'all' }), + expr.toString({ parenthesis: 'all' }), + `round-trip mismatch for ${estring} (parenthesis: ${paren})`) + } + } + + // Spot-check the concrete serialized strings + assert.strictEqual(xtimes2.toString({ implicit: 'hide' }), 'x 2') + assert.strictEqual(abtimes2.toString({ implicit: 'hide' }), 'a b 2') + assert.strictEqual(accessorTimes2.toString({ implicit: 'hide' }), 'A[1] 2') + }) + it('should HTML implicit multiplications between ConstantNodes with parentheses', function () { const z = math.parse('(3)x') const a = math.parse('(4)(4)(4)(4)')