fix: accept basic-format inline offsets in date-time-string time zone ids - #89
Merged
Conversation
… ids An inline UTC offset in a date-time-string time zone identifier was only recognized when spelled with a colon, so "2021-08-19T1730-0700" raised "bare datetime without Z, offset, or bracket" while the extended spelling of the same offset resolved. ISO 8601 allows both.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
TimeZoneHelper::normalizeTimezoneId()recognized an inline UTC offset in a date-time-string time zone identifier only when it was written with a colon:ISO 8601 allows both the extended (
±HH:MM) and the basic (±HHMM) spelling of an offset, and TC39 accepts either. test262 lists both in the valid set ofrelativeto-propertybag-timezone-string-datetime.js; that fixture passed only because therelativeTopath validated the string and then ignored the zone it named.The fix
One regex.
([+\-]\d{2}:\d{2})becomes([+\-])(\d{2}):?(\d{2}), and the three groups are reassembled into the canonical extended form, so the return value is unchanged for input that already parsed.Scope
Deliberately not touched:
Instant.php:713,Instant.php:931andRelativeTo.php:854each carry their own copy of this parsing, and all three still reject the basic spelling — which is whyInstant::toZonedDateTimeISO('2021-08-19T1730-0700')still throws. That duplication is pre-existing and belongs in its own change.Verification
PHPStan level 9, Psalm, mago lint/analyze/format all clean. Full suite: 12396 tests, 1032460 assertions, 299 incomplete — identical to
masteron all three counts.Split from #82, which is blocked on this: once #82 resolves the zone a
relativeTobag names instead of discarding it, the tworelativeto-propertybag-timezone-string-datetimefixtures fail without this fix.