fix(parser): correct pure percent addition and preserve relative perc…

[philpeng1]

Fixes #3563

Summary

• Pure percent additions now behave arithmetically: 10% + 20% == 0.3.
• Existing “relative percent” semantics on non-percent left-hand sides are preserved: 50 + 20% + 10% == 66.
• Modulo behavior remains intact outside the percent-addition context.

Problem

• Previously, pure percent sums like 10% + 20% were parsed/modulo’d/compounded unintuitively (see issue/discussion).
• The expected behavior (per the discussion) is:
  Pure percent sums add arithmetically.
  A ± B% keeps the existing relative semantics when A is not a pure-percent expression (e.g., numbers/variables/units).
  Do not break modulo (%) or other operators.

Changes

• Parser (src/expression/parse.js):
  Added helper isPurePercentageExpression to identify pure percentage expressions, including nested parentheses, unary +/- wrappers, and add/sub trees.
  Prefer unary percent for the RHS when the LHS is a pure-percent sum by setting a temporary parser flag (state.preferUnaryPercentAfterPlus). This prevents mis-parsing % as modulo in chains like 10% + 20% + x.
  Retain “relative percent” semantics for non-percent LHS by transforming A ± B% into A ± (A * B%) only when the LHS is not pure percent.
  Guard parseMultiplyDivideModulus against creating mod (%) when we are in a context that must treat the upcoming % as unary percent.
  Convert a misparsed “mod” shape like A % (B%) directly followed by + or - into unary percentage on A (A/100), matching the intended semantics.

Tests (test/unit-tests/expression/parse.test.js):

• Added/adjusted coverage for:
  Pure percent arithmetic and chaining:
  10% + 20%, 10% - 20%, 10% + 20% + 30%, 10% + 50% - 20%, etc.

• Relative percent on numbers and variables:
  50 + 20% + 10% == 66
  x + 20% + 10%, x - 10% - 20%

• Grouping and associativity:
  10% + 20% + x
  (10%) + (20%), (10% + 20%) + 30%, 10% + (20% + 30%)

• Multiplication with grouped percents:
  x * (10% + 20%)

• Units with % on the right:
  10 cm + 20% == 12 cm

• Modulo/unary interactions remain valid:
  100-3% (unary %)
  3%-100 (mod)
  11%-3 (mod with bitwise-not)

Behavioral notes

• Pure percent sums add arithmetically.
• When LHS is not pure percent, percent remains relative to LHS (existing behavior).
• Modulo is unchanged outside these specific percent-addition contexts.
• Ambiguous constructs like 10% + 20 % 3 will prefer unary percent on 20 in this targeted context; users can force modulo via mod(20,3) or parentheses if desired.

I had a lotta fun trying to solve this issue. I’m happy to adjust implementation details, test coverage, or semantics per maintainer feedback and coding style preferences.

[gwhitney]

Thanks so much for your submission! Sorry it's taken a few days to get to it.

• Ambiguous constructs like 10% + 20 % 3 will prefer unary percent on 20 in this targeted context

You didn't put in a test on this case -- is your proposed parse

10% + 20 % 3 == 10% + (20%)3 == 10% + 20%*3 == 10% + 60% = 70% = 0.7

?
Is that what we actually want here? What about 20 % 3 + 10 % ? It seems to me that should likely be interpreted symmetrically, otherwise it's too confusing. So either they should both be 70%, or the first should be 2.1 (10% is .1 and 20 mod 3 is 2) and the latter should be 2.2 (20 mod 3 is 2, plus 10% is 2.2).

I will admit to some concern that the code added to the parser is intricate and may make an already fragile hand-written parser noticeably more tricky to navigate. I will continue to review the PR -- perhaps your solution is how we will need to go to solve this (I would love to solve it once and for all, since it has been a recurring thorn in mathjs' side).

The collection of tests is great, we should extend them even more to include as many cases as we can make a reasonable decision on the interpretation of, if possible.

[gwhitney]

On:

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

I do not think these are the correct/desired parses/behaviors. It seems to me that
10 + 20% + 10% is (10+20%) + 10% is 12 + 10% is 13.2, and correspondingly
10 + (20% + 10%) is 10 + 30% is 13.

So your two examples should be 65 and 130, respectively.

[gwhitney]

As the items I raised above for this PR motivated me to give the alternative proposal of making % a dimensionless unit a try in #3590, (comparatively) reviewing/merging this PR and/or that one will have to revert to @josdejong, although of course @ericman314's thoughts and input, as well as yours and anyone else who's interested, would be welcome as well.

[josdejong]

@philpeng1 did you see the feedback of Glen?

[philpeng1]

hi, sorry for the late response! Been dealing with some other stuff. Also, another apology for the messy commits.

• Implemented requested grouping changes (50 + (20% + 10%) = 65; x + (20% + 10%) = 130) and added symmetric modulo/percent cases (10% + 20 % 3 = 2.1; 20 % 3 + 10% = 2.2).
• Parser only prefers unary percent after a pure-percent LHS when the next token is +/-, so modulo parses normally elsewhere.
• I’m happy to adjust or pivot to the Unit-based approach (refactor: Handle percent parsing by making it a Unit #3590) if preferred, also is a great idea.