Skip to content

fix: keep a leading + sign in the number spelling before hooks see it - #160

Merged
dpranke merged 4 commits into
dpranke:mainfrom
rahul05ranjan:fix-plus-sign-spelling
Nov 2, 2026
Merged

dpranke merged 4 commits into
dpranke:mainfrom
rahul05ranjan:fix-plus-sign-spelling

Conversation

@rahul05ranjan

Copy link
Copy Markdown
Contributor

What's wrong

The JSON5 grammar accepts a leading + on numbers (+Infinity was added in #63), but num_literal's + branch consumes the sign and returns the unsigned token alone — the - branch keeps its sign. So the parse_int / parse_float / parse_constant hooks receive a spelling that differs from the document, breaking the "passed the integer string as written" contract documented in loads()/parse() (and the as-written parse_constant semantics established around -NaN in #152/#157):

>>> json5.loads('+NaN', parse_constant=str)       # expected '+NaN'
'NaN'
>>> json5.loads('+Infinity', parse_constant=str)  # expected '+Infinity'
'Infinity'
>>> json5.loads('+0xff', parse_int=str)           # expected '+0xff'
'0xff'
>>> json5.loads('+5', parse_int=str)              # expected '+5'
'5'
>>> json5.loads('+1.5', parse_float=str)          # expected '+1.5'
'1.5'

The fix

  • json5/json5.g: the + branch returns '+' + n, as the - branch does.
  • json5/parser.py (generated): the same one-token change in _num_literal__c1_, mirroring _num_literal__c0_.
  • json5/lib.py: _walk_ast's hex-prefix test and the default _parse_int helper strip either sign (v[:1] in ('-', '+') / s.lstrip('+-')) instead of only -, so a +0xff token reaches the hex branch and parses as 255 rather than falling through to base-10 int() and raising.
  • tests/lib_test.py: assert the + spellings arrive at the hooks (+5, +0xff for parse_int; +1.5 for parse_float; +Infinity/+NaN for parse_constant) and +0xff → 255 as a value.

Values are unchanged everywhere (+1 → 1, +0xf → 15, +Infinity → inf all keep passing); only the hook spellings become as-written.

Verification

System python3, no dependencies beyond the package's own sources:

  • Full upstream suite with the changes applied: python3 -m unittest discover -s tests → 80 tests OK (including the new + assertions).
  • Pre-fix proof of the dropped sign, per hook (see A leading + sign is dropped from the number spelling before parse_int/parse_float/parse_constant see it #159 for the full transcript).
  • Value battery: +5→5, +0xff→255, +0X2a→42, +.5→0.5, +1e3→1000.0, +Infinity→inf, +NaN→nan, plus -/unsigned forms unchanged.
  • Error battery unchanged: + 5, ++5, +-5, +, +Infinit, +Nan still reject with the same messages.
  • parser.py is generated code; the change mirrors the - branch's generated form exactly so it matches what a regeneration from the updated .g would produce.

Fixes #159

rahul05ranjan and others added 4 commits November 2, 2026 15:29
The JSON5 num_literal rule consumed a leading + but returned the
unsigned token alone, while - was kept, so parse_int, parse_float and
parse_constant received a spelling that differed from the document:
loads('+NaN', parse_constant=str) gave 'NaN' and
loads('+0xff', parse_int=str) gave '0xff'. Keep the + in the token,
mirror the generated parser, and extend the two sign checks in lib.py
(the hex-prefix test in _walk_ast and _parse_int) so +0xff parses as
255 instead of falling through to base 10.

Values are unchanged; only hook spellings become as-written.

Fixes dpranke#159

Co-authored-by: rahul05ranjan <11793455+rahul05ranjan@users.noreply.github.com>
Regenerated-parser equivalent of the json5.g change: the + branch of
num_literal now returns the sign with the token, as the - branch does.

Part of dpranke#159

Co-authored-by: rahul05ranjan <11793455+rahul05ranjan@users.noreply.github.com>
_walk_ast and the default _parse_int helper only stripped - before the
hex-prefix test; a +0xff token would fall through to base-10 int() and
raise. Strip either sign, so the + forms of hex literals parse the same
way the - forms do.

Part of dpranke#159

Co-authored-by: rahul05ranjan <11793455+rahul05ranjan@users.noreply.github.com>
parse_int/parse_float/parse_constant now see '+5', '+0xff', '+1.5',
'+Infinity' and '+NaN' with the sign, matching the - spellings that
were already asserted.

Part of dpranke#159

Co-authored-by: rahul05ranjan <11793455+rahul05ranjan@users.noreply.github.com>

@dpranke dpranke left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, nice work :).

@dpranke
dpranke merged commit 36dd657 into dpranke:main Nov 2, 2026
9 checks passed
@rahul05ranjan

Copy link
Copy Markdown
Contributor Author

Thanks for merging this one too — appreciate the quick turnaround on both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A leading + sign is dropped from the number spelling before parse_int/parse_float/parse_constant see it

2 participants