From b7942dd790be717865cd71eb45fc7b1350ac6cd6 Mon Sep 17 00:00:00 2001 From: Phil Peng Date: Wed, 29 Oct 2025 19:15:25 -0500 Subject: [PATCH 1/3] fix(parser): correct pure percent addition and preserve relative percent semantics; add tests --- src/expression/parse.js | 60 +++++++++++++++++++++--- test/unit-tests/expression/parse.test.js | 52 ++++++++++++++++++++ 2 files changed, 106 insertions(+), 6 deletions(-) diff --git a/src/expression/parse.js b/src/expression/parse.js index 7be4f883ee..2a00967e70 100644 --- a/src/expression/parse.js +++ b/src/expression/parse.js @@ -617,6 +617,19 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ return node } + function isPurePercentageExpression (n) { + if (!n) return false + if (n.isPercentage) return true + if (n.type === 'ParenthesisNode') return isPurePercentageExpression(n.content) + if (isOperatorNode(n) && (n.fn === 'unaryPlus' || n.fn === 'unaryMinus') && n.args && n.args.length === 1) { + return isPurePercentageExpression(n.args[0]) + } + if (isOperatorNode(n) && (n.fn === 'add' || n.fn === 'subtract') && n.args && n.args.length === 2) { + return isPurePercentageExpression(n.args[0]) && isPurePercentageExpression(n.args[1]) + } + return false + } + /** * Parse a block with expressions. Expressions can be separated by a newline * character '\n', or by a semicolon ';'. In case of a semicolon, no output @@ -1019,6 +1032,13 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ node = parseMultiplyDivideModulus(state) + if (isOperatorNode(node) && node.fn === 'mod' && node.args && node.args.length === 2) { + const rhs = node.args[1] + if ((state.token === '+' || state.token === '-') && isPurePercentageExpression(rhs)) { + node = new OperatorNode('/', 'divide', [node.args[0], new ConstantNode(100)], false, true) + } + } + const operators = { '+': 'add', '-': 'subtract' @@ -1028,8 +1048,13 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ fn = operators[name] getTokenSkipNewline(state) + const savedPrefer = state.preferUnaryPercentAfterPlus + if (isPurePercentageExpression(node)) { + state.preferUnaryPercentAfterPlus = true + } const rightNode = parseMultiplyDivideModulus(state) - if (rightNode.isPercentage) { + state.preferUnaryPercentAfterPlus = savedPrefer + if (rightNode.isPercentage && !isPurePercentageExpression(node)) { params = [node, new OperatorNode('*', 'multiply', [node, rightNode])] } else { params = [node, rightNode] @@ -1062,6 +1087,9 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ while (true) { if (hasOwnProperty(operators, state.token)) { + if (state.token === '%' && state.preferUnaryPercentAfterPlus) { + break + } // explicit operators name = state.token fn = operators[name] @@ -1169,17 +1197,37 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ * @return {Node} node * @private */ - function parseUnaryPercentage (state) { + function parseUnaryPercentage (state, lookaheadMode) { let node = parseUnary(state) if (state.token === '%') { const previousState = Object.assign({}, state) getTokenSkipNewline(state) + if (state.preferUnaryPercentAfterPlus) { + node = new OperatorNode('/', 'divide', [node, new ConstantNode(100)], false, true) + return node + } + if (lookaheadMode) { + node = new OperatorNode('/', 'divide', [node, new ConstantNode(100)], false, true) + return node + } // 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 %. + // So we attempt a controlled lookahead only for + or - to support cases like + // 10% + 20% and 10% + (20% + 30%). + if (state.token === '+' || state.token === '-') { + const lookAheadState = Object.assign({}, state) + getTokenSkipNewline(lookAheadState) + try { + const laNode = parseUnaryPercentage(lookAheadState, true) + if (isPurePercentageExpression(laNode)) { + node = new OperatorNode('/', 'divide', [node, new ConstantNode(100)], false, true) + return node + } + } catch (err) { + node = new OperatorNode('/', 'divide', [node, new ConstantNode(100)], false, true) + return node + } + } try { parseUnary(state) // Not sure if we could somehow use the result of that parseUnary? Without diff --git a/test/unit-tests/expression/parse.test.js b/test/unit-tests/expression/parse.test.js index 444247244f..0494a48610 100644 --- a/test/unit-tests/expression/parse.test.js +++ b/test/unit-tests/expression/parse.test.js @@ -1668,6 +1668,58 @@ describe('parse', function () { assert.strictEqual(parseAndEval('3%+100'), 3) // treat as 3 mod 100 }) + it('should add and subtract percentages intuitively', function () { + approxEqual(parseAndEval('10% + 20%'), 0.3) + approxEqual(parseAndEval('10% - 20%'), -0.1) + approxEqual(parseAndEval('10% + 20% + 30%'), 0.6) + approxEqual(parseAndEval('10% + 50% - 20%'), 0.4) + }) + + it('should preserve relative percentage on numbers', function () { + approxEqual(parseAndEval('50 + 20% + 10%'), 66) + }) + + it('should compound percentages on variables', function () { + const scope = { x: 10 } + approxEqual(parseAndEval('x + 20% + 10%', scope), 13.2) + approxEqual(parseAndEval('x - 10% - 20%', scope), 7.2) + }) + + it('should keep percent sums before adding a variable', function () { + const scope = { x: 1 } + approxEqual(parseAndEval('10% + 20% + x', scope), 1.3) + approxEqual(parseAndEval('10% - 20% - x', scope), -1.1) + }) + + it('should support parentheses with percentages', function () { + approxEqual(parseAndEval('(10%) + (20%)'), 0.3) + approxEqual(parseAndEval('10% + (20%)'), 0.3) + approxEqual(parseAndEval('(10% + 20%) + 30%'), 0.6) + approxEqual(parseAndEval('10% + (20% + 30%)'), 0.6) + }) + + it('should add more pure percentages arithmetically', function () { + approxEqual(parseAndEval('50% + 20%'), 0.7) + approxEqual(parseAndEval('10% + 20% - 30%'), 0.0) + approxEqual(parseAndEval('10% + 20% + 30% + 40%'), 1.0) + }) + + it('should combine percentages inside multiplication and with parentheses', function () { + const scope = { x: 10 } + approxEqual(parseAndEval('x * (10% + 20%)', scope), 3) + approxEqual(parseAndEval('(10% + 20%) * x', scope), 3) + }) + + it('should preserve semantics when grouping percentage additions explicitly', function () { + approxEqual(parseAndEval('50 + (20% + 10%)'), 50.3) + const scope = { x: 100 } + approxEqual(parseAndEval('x + (20% + 10%)', scope), 100.3) + }) + + it('should support units with percentages on the right-hand side', function () { + approxDeepEqual(parseAndEval('10 cm + 20%'), new Unit(12, 'cm')) + }) + it('should parse unary % with subtraction', function () { approxEqual(parseAndEval('100-3%'), 97) assert.strictEqual(parseAndEval('3%-100'), -97) // treat as 3 mod -100 From 821cbf7c8f147a8bb0d1e3ac79766cf5bdf3272e Mon Sep 17 00:00:00 2001 From: Phil Peng Date: Sun, 23 Nov 2025 22:41:57 -0600 Subject: [PATCH 2/3] parser: prefer unary percent after pure percent LHS; keep modulo symmetry; update grouping/ambiguous tests --- src/expression/parse.js | 12 +----------- test/unit-tests/expression/parse.test.js | 10 ++++++++-- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/expression/parse.js b/src/expression/parse.js index 2a00967e70..5ad9d6bffd 100644 --- a/src/expression/parse.js +++ b/src/expression/parse.js @@ -1032,13 +1032,6 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ node = parseMultiplyDivideModulus(state) - if (isOperatorNode(node) && node.fn === 'mod' && node.args && node.args.length === 2) { - const rhs = node.args[1] - if ((state.token === '+' || state.token === '-') && isPurePercentageExpression(rhs)) { - node = new OperatorNode('/', 'divide', [node.args[0], new ConstantNode(100)], false, true) - } - } - const operators = { '+': 'add', '-': 'subtract' @@ -1054,7 +1047,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ } const rightNode = parseMultiplyDivideModulus(state) state.preferUnaryPercentAfterPlus = savedPrefer - if (rightNode.isPercentage && !isPurePercentageExpression(node)) { + if ((rightNode.isPercentage || isPurePercentageExpression(rightNode)) && !isPurePercentageExpression(node)) { params = [node, new OperatorNode('*', 'multiply', [node, rightNode])] } else { params = [node, rightNode] @@ -1087,9 +1080,6 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ while (true) { if (hasOwnProperty(operators, state.token)) { - if (state.token === '%' && state.preferUnaryPercentAfterPlus) { - break - } // explicit operators name = state.token fn = operators[name] diff --git a/test/unit-tests/expression/parse.test.js b/test/unit-tests/expression/parse.test.js index 0494a48610..1a7a38ed36 100644 --- a/test/unit-tests/expression/parse.test.js +++ b/test/unit-tests/expression/parse.test.js @@ -1708,12 +1708,13 @@ describe('parse', function () { const scope = { x: 10 } approxEqual(parseAndEval('x * (10% + 20%)', scope), 3) approxEqual(parseAndEval('(10% + 20%) * x', scope), 3) + approxEqual(parseAndEval('x * 10% * 20%', scope), 0.2) }) it('should preserve semantics when grouping percentage additions explicitly', function () { - approxEqual(parseAndEval('50 + (20% + 10%)'), 50.3) + approxEqual(parseAndEval('50 + (20% + 10%)'), 65) const scope = { x: 100 } - approxEqual(parseAndEval('x + (20% + 10%)', scope), 100.3) + approxEqual(parseAndEval('x + (20% + 10%)', scope), 130) }) it('should support units with percentages on the right-hand side', function () { @@ -1730,6 +1731,11 @@ describe('parse', function () { assert.strictEqual(parseAndEval('11%~-3'), 1) // equivalent to 11 mod 2 }) + it('should interpret modulo vs percent in mixed sums symmetrically', function () { + approxEqual(parseAndEval('10% + 20 % 3'), 2.1) + approxEqual(parseAndEval('20 % 3 + 10%'), 2.2) + }) + it('should parse operator mod', function () { approxEqual(parseAndEval('8 mod 3'), 2) }) From 6ba0d3ba0981cc290f38e08aced1ac745ad3f99b Mon Sep 17 00:00:00 2001 From: Phil Peng Date: Sun, 23 Nov 2025 22:49:17 -0600 Subject: [PATCH 3/3] chore:format to LF line endings --- package-lock.json | 16 ++++++++++++++++ src/expression/parse.js | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index 4ccb555ac9..87074bbd64 100644 --- a/package-lock.json +++ b/package-lock.json @@ -117,6 +117,7 @@ "integrity": "sha512-e7jT4DxYvIDLk1ZHmU/m/mB19rex9sv0c2ftBtjSBv+kVM/902eh0fINUzD7UwLLNR+jU585GxUJ8/EBfAM5fw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.27.1", "@babel/generator": "^7.28.5", @@ -2301,6 +2302,7 @@ "integrity": "sha512-FXx2pKgId/WyYo2jXw63kk7/+TY7u7AziEJxJAnSFzHlqTAS3Ync6SvgYAN/k4/PQpnnVuzoMuVnByKK2qp0ag==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@types/estree": "*", "@types/json-schema": "*" @@ -2365,6 +2367,7 @@ "integrity": "sha512-ixiWrCSRi33uqBMRuICcKECW7rtgY43TbsHDpM2XK7lXispd48opW+0IXrBVxv9NMhaz/Ue9kyj6r3NTVyXm8A==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~6.20.0" } @@ -2422,6 +2425,7 @@ "integrity": "sha512-6m1I5RmHBGTnUGS113G04DMu3CpSdxCAU/UvtjNWL4Nuf3MW9tQhiJqRlHzChIkhy6kZSAQmc+I1bcGjE3yNKg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.46.3", "@typescript-eslint/types": "8.46.3", @@ -2836,6 +2840,7 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -3820,6 +3825,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.8.9", "caniuse-lite": "^1.0.30001746", @@ -5369,6 +5375,7 @@ "deprecated": "This version is no longer supported. Please see https://eslint.org/version-support for other options.", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.6.1", @@ -5454,6 +5461,7 @@ "integrity": "sha512-NSWl5BFQWEPi1j4TjVNItzYV7dZXZ+wP6I6ZhrBGpChQhZRUaElihE9uRRkcbRnNb76UMKDF3r+WTmNcGPKsqw==", "dev": true, "license": "MIT", + "peer": true, "bin": { "eslint-config-prettier": "bin/cli.js" }, @@ -5569,6 +5577,7 @@ "integrity": "sha512-whOE1HFo/qJDyX4SnXzP4N6zOWn79WhnCUY/iDR0mPfQZO8wcYE4JClzI2oZrhBnnMUCBCHZhO6VQyoBU95mZA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@rtsao/scc": "^1.1.0", "array-includes": "^3.1.9", @@ -5684,6 +5693,7 @@ "integrity": "sha512-6TyDmZ1HXoFQXnhCTUjVFULReoBPOAjpuiKELMkeP40yffI/1ZRO+d9ug/VC6fqISo2WkuIBk3cvuRPALaWlOQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.4.0", "builtins": "^5.0.1", @@ -5797,6 +5807,7 @@ "integrity": "sha512-57Zzfw8G6+Gq7axm2Pdo3gW/Rx3h9Yywgn61uE/3elTCOePEHVrn2i5CdfBwA1BLK0Q0WqctICIUSqXZW/VprQ==", "dev": true, "license": "ISC", + "peer": true, "engines": { "node": "^12.22.0 || ^14.17.0 || >=16.0.0" }, @@ -8504,6 +8515,7 @@ "integrity": "sha512-LrtUxbdvt1gOpo3gxG+VAJlJAEMhbWlM4YrFQgql98FwF7+K8K12LYO4hnDdUkNjeztYrOXEMqgTajSWgmtI/w==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@colors/colors": "1.5.0", "body-parser": "^1.19.0", @@ -10523,6 +10535,7 @@ "integrity": "sha512-I7AIg5boAr5R0FFtJ6rCfD+LFsWHp81dolrFD8S79U9tb8Az2nGrJncnMSnys+bpQJfRUzqs9hnA81OAA3hCuQ==", "dev": true, "license": "MIT", + "peer": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -11240,6 +11253,7 @@ "integrity": "sha512-B/gBuNg5SiMTrPkC+A2+cW0RszwxYmn6VYxB/inlBStS5nx6xHIt/ehKRhIMhqusl7a8LjQoZnjCs5vhwxOQ1g==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "fast-deep-equal": "^3.1.3", "fast-uri": "^3.0.1", @@ -12620,6 +12634,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -13194,6 +13209,7 @@ "integrity": "sha512-7h/weGm9d/ywQ6qzJ+Xy+r9n/3qgp/thalBbpOi5i223dPXKi04IBtqPN9nTd+jBc7QKfvDbaBnFipYp4sJAUQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@types/eslint-scope": "^3.7.7", "@types/estree": "^1.0.8", diff --git a/src/expression/parse.js b/src/expression/parse.js index 5ad9d6bffd..0049c8a008 100644 --- a/src/expression/parse.js +++ b/src/expression/parse.js @@ -1193,7 +1193,7 @@ export const createParse = /* #__PURE__ */ factory(name, dependencies, ({ if (state.token === '%') { const previousState = Object.assign({}, state) getTokenSkipNewline(state) - if (state.preferUnaryPercentAfterPlus) { + if (state.preferUnaryPercentAfterPlus && (state.token === '+' || state.token === '-')) { node = new OperatorNode('/', 'divide', [node, new ConstantNode(100)], false, true) return node }