Add opt-in BigInt support, throw when parsing out of bounds values#74
Conversation
Greptile SummaryThis PR adds opt-in
Confidence Score: 3/5Not safe to merge as-is — any TOML file with an explicitly positive-signed integer will throw an unexpected SyntaxError at the BigInt() conversion step. The dec_integer_text grammar rule preserves the TOML '+' sign prefix, which worked fine with parseInt but silently breaks with BigInt. The existing 'integer formats' test exercises a = +99 and would fail, and real-world TOML files with +99-style integers would throw unintelligible SyntaxErrors. The fix is a one-line change in the PEG source and the generated parser. src/toml.pegjs (dec_integer_text sign handling) and lib/parser.js (peg$f46/peg$f47) need the + sign stripped before BigInt() is called.
|
| Filename | Overview |
|---|---|
| src/toml.pegjs | Switches integer parsing from parseInt to BigInt; the dec_integer_text rule preserves the '+' sign in its return value, causing BigInt('+99') to throw SyntaxError for any positively-signed TOML integer. |
| lib/parser.js | Generated parser reflects the same '+'-sign bug from the PEG source (peg$f46/peg$f47 preserve '+', then peg$f45 passes it to BigInt()). |
| lib/compiler.js | Adds reduceInteger() that validates the int64 range and the JS safe-integer range with clear error messages; logic and boundary comparisons look correct. |
| test/test_toml.js | Adds thorough new test suites for integer range and bigint mode; the pre-existing "integer formats" test (a = +99) will expose the BigInt('+') regression once run. |
| test/spec-test.js | Moves valid/integer/long from KNOWN_FAILURES to a BIGINT_TESTS list and re-runs it with { bigint: true }; bigint handling in toTaggedJSON looks correct. |
| index.d.ts | Adds bigint?: boolean to the options type with accurate JSDoc; no issues. |
Comments Outside Diff (1)
-
src/toml.pegjs, line 265-267 (link)BigInt()does not accept a leading+sign —BigInt('+99')throws aSyntaxError. TOML allows explicitly positive-signed decimal integers (e.g.a = +99,a = +0), and thedec_integer_textrule preserves the+in its return value via(sign || '') + ..., so every positively-signed integer literal will throw during parsing. The existing"integer formats"test intest_toml.js(line 262) exercisesa = +99and would fail. The float rules already handle this correctly by using(sign === '-' ? '-' : '')— the same treatment should be applied here.
Reviews (1): Last reviewed commit: "Add opt-in BigInt support, throw when pa..." | Re-trigger Greptile
This PR addresses the issue of out-of-range integers. TOML supports 64-bit integer values, but JavaScript does not.
This PR introduces a breaking change that causes the parser to throw if it encounters an integer outside of JavaScript's safe integer range; a new parser option,
bigint, can be set totrueto cause all integers to be converted toBigInts.Closes #28