fix(toml): handle Infinity/NaN and Date inside inline arrays#7171
fix(toml): handle Infinity/NaN and Date inside inline arrays#7171LeSingh1 wants to merge 2 commits into
Conversation
Inline array stringification used JSON.stringify, which silently turned
Infinity/-Infinity/NaN into 'null' and wrapped Date values in extra
quotes. Both forms broke round-trip through parse():
stringify({x: [Infinity, -Infinity, NaN]}) // x = [null,null,null]
stringify({x: [new Date(0)]}) // x = ["1970-01-01..."]
The same flaw affected #printAsInlineValue, which was the path for
mixed-type arrays — Date values were quoted and non-finite numbers
fell through as String(Infinity) / String(NaN).
Replace the JSON.stringify call in #arrayDeclaration with a per-element
walk through #printAsInlineValue, and teach #printAsInlineValue to emit
TOML's inf / -inf / nan keywords for non-finite numbers and to leave
Date values unquoted to match TOML's datetime literal syntax.
The 'handles mixed array' test pinned the buggy
date = "2022-05-13T00:00:00.000" form inside a nested inline map.
Updated to the correct unquoted form. Four new tests pin the issue's
specific repros (primitive Infinity/-Infinity/NaN, primitive Date,
mixed inf/nan/object, mixed date/object).
Fixes denoland#7162 (partially — the null-in-array cases are left for a
follow-up since the spec disallows null in TOML and the right behavior
is a separate design call).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7171 +/- ##
========================================
Coverage 94.62% 94.62%
========================================
Files 628 630 +2
Lines 51636 51898 +262
Branches 9323 9380 +57
========================================
+ Hits 48863 49111 +248
- Misses 2208 2217 +9
- Partials 565 570 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bartlomieju
left a comment
There was a problem hiding this comment.
Reviewed and verified locally (checked out the branch, ran round-trips against the real parse(), full deno test -A toml/ -> 687 passed, 0 failed). The fix is correct and well-scoped.
Confirmed working:
[Infinity, -Infinity, NaN]->x = [inf,-inf,nan]reparses back to the exact same values.- Date arrays now reparse as
Dateinstances instead of strings. - String/empty/number arrays produce byte-identical output to the old
JSON.stringifypath, so no behavioral drift there.
The change is safe by construction: #getTypeOfArray classifies any array containing a nested array as MIXED, so #arrayDeclaration only ever receives flat primitive arrays.
Updating the nestedArray1 expectation is legitimate -- it pinned the buggy quoted-date form, and the new value genuinely round-trips. A few inline notes below; none are blocking.
| return `"${this.#printDate(value)}"`; | ||
| // TOML datetime literals are unquoted (e.g. 1979-05-27T07:32:00Z). | ||
| // The previous wrapping in quotes caused the value to round-trip as a | ||
| // string instead of a Date (#7162). |
There was a problem hiding this comment.
Confirmed this fixes the type (reparses as Date, not string). Worth being explicit, though, that it does not fix the value off-UTC: #printDate drops the offset suffix, so new Date(0) -> 1970-01-01T00:00:00.000 reparses an hour off in UTC+1 (orig 0ms -> roundtrip -3600000ms).
This is not a regression -- top-level dates via #dateDeclaration already drop the offset identically, so this just makes array dates consistent. It matches the offset issue you deferred in the PR description. No change needed here; just flagging that the "round-trips as a Date" comment is type-accurate but not value-accurate.
| return `${this.#declaration(keys)}${JSON.stringify(value)}`; | ||
| // JSON.stringify(value) turned Infinity / -Infinity / NaN into `null` and | ||
| // wrapped Date values in extra quotes, producing arrays that round-trip | ||
| // incorrectly (#7162). Use the per-element inline formatter instead so |
There was a problem hiding this comment.
Minor (optional): value.map((x) => this.#printAsInlineValue(x)).join(",") now appears in three spots -- here, #printObject (the MIXED branch), and the array case inside #printAsInlineValue itself. Could collapse into a small #inlineArray(value) helper. The duplication predates this PR, so fine to leave, but it's a natural cleanup point now that this path also uses it.
|
|
||
| // https://github.com/denoland/std/issues/7162 — inline array stringification | ||
| // used JSON.stringify, which turns Infinity/-Infinity/NaN into null and | ||
| // wraps Date values in quotes. Both forms broke round-trip through parse(). |
There was a problem hiding this comment.
Suggestion (non-blocking): consider adding a parse(stringify(x)) deep-equality assertion for the non-finite case. The exact-value round-trip is the real win here (verified: [Infinity, -Infinity, NaN] survives intact), and asserting it directly locks in the stringify<->parse contract against future parser changes.
Partial fix for #7162.
#arrayDeclarationrendered primitive arrays by callingJSON.stringify(value), which silently turnedInfinity/-Infinity/NaNinto the literalnulland wrappedDatevalues in extra quotes. Both forms broke round-trip throughparse():The same flaw affected
#printAsInlineValue, which is the path for mixed-type arrays —Datevalues were quoted there too, and non-finite numbers fell through asString(Infinity)/String(NaN)(not valid TOML).The fix is two small edits:
#arrayDeclarationno longer callsJSON.stringify. It maps the array through#printAsInlineValueso primitive arrays use the same TOML literal forms as mixed-type arrays.#printAsInlineValueemitsinf/-inf/nanfor non-finite numbers and leavesDatevalues unquoted, matching TOML's datetime literal syntax.After:
The existing
handles mixed arraytest pinned the buggydate = \"2022-05-13T00:00:00.000\"form inside a nested inline map. Updated to the correct unquoted form (and round-trip verified —parse(stringify(...))returns aDateinstance again instead of a string).Four new tests cover the issue's specific repros.
Not included in this PR:
{x: [null]}/{x: [1, null]}— TOML disallowsnull, and the right behavior (skip vs. throw with a clearer message vs. coerce) is a design call worth its own discussion. Leaving for a follow-up.Z) —#printDatedrops the offset suffix in all TOML output, not just arrays. Restoring it is a broader change touching every Date round-trip and would balloon this PR. Happy to file separately.