Skip to content

refactor: Handle percent parsing by making it a Unit #3590

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

Draft
gwhitney wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

refactor: Handle percent parsing by making it a Unit #3590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
06be095
refactor: Solve percent parsing by making it a Unit
gwhitney Nov 6, 2025
345d500
refactor: Elimintate OperatorNode `isPercentage` field
gwhitney Nov 6, 2025
b69873b
chore: minor comment and layout adjustments for clarity
gwhitney Nov 6, 2025
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
21 changes: 8 additions & 13 deletions src/expression/node/OperatorNode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,8 @@ export const createOperatorNode = /* #__PURE__ */ factory(name, dependencies, ({
* @param {string} fn Function name, for example 'add'
* @param {Node[]} args Operator arguments
* @param {boolean} [implicit] Is this an implicit multiplication?
* @param {boolean} [isPercentage] Is this an percentage Operation?
*/
constructor (op, fn, args, implicit, isPercentage) {
constructor (op, fn, args, implicit) {
super()
// validate input
if (typeof op !== 'string') {
Expand All @@ -266,7 +265,6 @@ export const createOperatorNode = /* #__PURE__ */ factory(name, dependencies, ({
}

this.implicit = (implicit === true)
this.isPercentage = (isPercentage === true)
this.op = op
this.fn = fn
this.args = args || []
Expand Down Expand Up @@ -355,8 +353,7 @@ export const createOperatorNode = /* #__PURE__ */ factory(name, dependencies, ({
for (let i = 0; i < this.args.length; i++) {
args[i] = this._ifNode(callback(this.args[i], 'args[' + i + ']', this))
}
return new OperatorNode(
this.op, this.fn, args, this.implicit, this.isPercentage)
return new OperatorNode(this.op, this.fn, args, this.implicit)
}

/**
Expand All @@ -365,7 +362,7 @@ export const createOperatorNode = /* #__PURE__ */ factory(name, dependencies, ({
*/
clone () {
return new OperatorNode(
this.op, this.fn, this.args.slice(0), this.implicit, this.isPercentage)
this.op, this.fn, this.args.slice(0), this.implicit)
}

/**
Expand Down Expand Up @@ -472,8 +469,7 @@ export const createOperatorNode = /* #__PURE__ */ factory(name, dependencies, ({
op: this.op,
fn: this.fn,
args: this.args,
implicit: this.implicit,
isPercentage: this.isPercentage
implicit: this.implicit
}
}

Expand All @@ -483,16 +479,15 @@ export const createOperatorNode = /* #__PURE__ */ factory(name, dependencies, ({
* An object structured like
* ```
* {"mathjs": "OperatorNode",
* "op": "+", "fn": "add", "args": [...],
* "implicit": false,
* "isPercentage":false}
* "op": "+", "fn": "add",
* "args": [...],
* "implicit": false}
* ```
* where mathjs is optional
* @returns {OperatorNode}
*/
static fromJSON (json) {
return new OperatorNode(
json.op, json.fn, json.args, json.implicit, json.isPercentage)
return new OperatorNode(json.op, json.fn, json.args, json.implicit)
}

/**
Expand Down
138 changes: 83 additions & 55 deletions src/expression/parse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,11 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
not: true
}

const UNIT_DELIMITERS = {
in: true,
'%': true
}

const CONSTANTS = {
true: true,
false: false,
Expand Down Expand Up @@ -941,14 +946,9 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({

getTokenSkipNewline(state)

if (name === 'in' && '])},;'.includes(state.token)) {
// end of expression -> this is the unit 'in' ('inch')
node = new OperatorNode('*', 'multiply', [node, new SymbolNode('in')], true)
} else {
// operator 'a to b' or 'a in b'
params = [node, parseRange(state)]
node = new OperatorNode(name, fn, params)
}
// operator 'a to b' or 'a in b'
params = [node, parseRange(state)]
node = new OperatorNode(name, fn, params)
}

return node
Expand Down Expand Up @@ -1006,25 +1006,18 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
* @private
*/
function parseAddSubtract (state) {
let node, name, fn, params

node = parseMultiplyDivideModulus(state)
let node = parseMultiplyDivideModulus(state)

const operators = {
'+': 'add',
'-': 'subtract'
}
while (hasOwnProperty(operators, state.token)) {
name = state.token
fn = operators[name]
const name = state.token
const fn = operators[name]

getTokenSkipNewline(state)
const rightNode = parseMultiplyDivideModulus(state)
if (rightNode.isPercentage) {
params = [node, new OperatorNode('*', 'multiply', [node, rightNode])]
} else {
params = [node, rightNode]
}
const params = [node, parseMultiplyDivideModulus(state)]
node = new OperatorNode(name, fn, params)
}

Expand Down Expand Up @@ -1079,9 +1072,71 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
last = node

while (true) {
if ((state.tokenType === TOKENTYPE.SYMBOL) ||
(state.token === 'in' && isConstantNode(node)) ||
(state.token === 'in' && isOperatorNode(node) && node.fn === 'unaryMinus' && isConstantNode(node.args[0])) ||
// The idiosyncrasies of `%` in the mathjs language are handled
// here (and only here) because percent is treated as a dimensionless
// unit. Hence, we first encounter the possibility of the character `%`
// when we attempt an implicit multiplication. At this point, if we
// happen to be looking at `%`, we decide once and for all if it is a
// unit symbol or the 'mod' operator.
// Note this approach handles disambiguation of `in` as a unit or
// conversion operator in this same place, although unfortunately
// with somewhat different special cases.

// First determine if a unit delimiter is being used as a unit or
// a delimiter (in the former case it is implicit multiplication,
// in latter case not).
let delimiterAsUnit = state.token in UNIT_DELIMITERS
if (delimiterAsUnit) {
// Here we check if we are in a pattern in which this token should
// _not_ in fact be interpreted as a unit.

// First try looking ahead one token for general cases to disambiguate
const saveState = Object.assign({}, state)
getTokenSkipNewline(state)
if (state.token === '(' ||
state.tokenType === TOKENTYPE.NUMBER ||
state.tokenType === TOKENTYPE.SYMBOL ||
// Now check special cases for `in`:
// Parsing `5 in in`, the first `in` is a unit, second is operator
(saveState.token === 'in' &&
!(isConstantNode(node) ||
(isOperatorNode(node) && node.fn === 'unaryMinus')) &&
state.token in UNIT_DELIMITERS)
) {
delimiterAsUnit = false
} else if (saveState.token === '%') { // Now check special cases for %
// Prevent doubled percent
if (isOperatorNode(node) &&
(node.fn === 'mod' ||
(isSymbolNode(node.args[1]) && node.args[1].name === '%'))
) {
delimiterAsUnit = false
// So now % is an operator. If the next token is a
// UNIT_DELIMITER, that is a syntax error:
if (state.token in UNIT_DELIMITERS) {
throw createSyntaxError(
state, `Unexpected token '${state.token}' after '%'.`)
}
} else if (state.token !== '%') {
// only treat the '%' of '3 % +[EXPR]' or '3% - [EXPR]' as percent
// if '+[EXPR]' or '- [EXPR]' is a percentage.
// Otherwise, this % is the modulo operator, e.g.
// 3 % +100 == 3 and 3 % -100 == -97
try {
const rhs = parseImplicitMultiplication(state)
if (!(isOperatorNode(rhs) && rhs.implicit &&
rhs.args[1].name in { percent: true, '%': true })
) {
delimiterAsUnit = false
}
} catch {}
}
}
Object.assign(state, saveState)
}

// Now we can go ahead and check for implicit multiplication:
if ((state.tokenType === TOKENTYPE.SYMBOL) || delimiterAsUnit ||
(state.tokenType === TOKENTYPE.NUMBER &&
!isConstantNode(last) &&
(!isOperatorNode(last) || last.op === '!')) ||
Expand All @@ -1090,6 +1145,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
//
// symbol: implicit multiplication like '2a', '(2+3)a', 'a b'
// number: implicit multiplication like '(2+3)2'
// units: implicit multiplication like '5 m', '5 in', or '5%'
// parenthesis: implicit multiplication like '2(3+4)', '(3+4)(1+2)'
last = parseRule2(state)
node = new OperatorNode('*', 'multiply', [node, last], true /* implicit */)
Expand All @@ -1111,7 +1167,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
* @private
*/
function parseRule2 (state) {
let node = parseUnaryPercentage(state)
let node = parseUnary(state)
let last = node
const tokenStates = []

Expand All @@ -1134,7 +1190,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 = parseUnaryPercentage(state)
last = parseUnary(state)
node = new OperatorNode('/', 'divide', [node, last])
} else {
// Not a match, so rewind
Expand All @@ -1155,36 +1211,6 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
return node
}

/**
* Unary percentage operator (treated as `value / 100`)
* @return {Node} node
* @private
*/
function parseUnaryPercentage (state) {
let node = parseUnary(state)

if (state.token === '%') {
const previousState = Object.assign({}, state)
getTokenSkipNewline(state)
// We need to decide if this is a unary percentage % or binary modulo %
// So we attempt to parse a unary expression at this point.
// If it fails, then the only possibility is that this is a unary percentage.
// If it succeeds, then we presume that this must be binary modulo, since the
// only things that parseUnary can handle are _higher_ precedence than unary %.
try {
parseUnary(state)
// Not sure if we could somehow use the result of that parseUnary? Without
// further analysis/testing, safer just to discard and let the parse proceed
Object.assign(state, previousState)
} catch {
// Not seeing a term at this point, so was a unary %
node = new OperatorNode('/', 'divide', [node, new ConstantNode(100)], false, true)
}
}

return node
}

/**
* Unary plus and minus, and logical and bitwise not
* @return {Node} node
Expand Down Expand Up @@ -1357,7 +1383,9 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({
let node, name

if (state.tokenType === TOKENTYPE.SYMBOL ||
(state.tokenType === TOKENTYPE.DELIMITER && state.token in NAMED_DELIMITERS)) {
(state.tokenType === TOKENTYPE.DELIMITER &&
(state.token in UNIT_DELIMITERS || state.token in NAMED_DELIMITERS))
) {
name = state.token

getToken(state)
Expand Down
Loading