fix: accept Temporal objects in withCalendar - #34
Open
MidnightDesign wants to merge 1 commit into
Open
Conversation
… fast path)
TC39's ToTemporalCalendarIdentifier has an object fast path: when the argument
is a Temporal date-bearing object (PlainDate, PlainDateTime, PlainMonthDay,
PlainYearMonth, ZonedDateTime) it returns that object's [[Calendar]] rather
than requiring a string. Our withCalendar typed its parameter as `string` and
rejected such objects with a native TypeError, deviating from the spec.
Spec layer (PlainDate/PlainDateTime/ZonedDateTime): widen the parameter to
`mixed` and resolve via the existing CalendarFactory::resolveBagCalendar(),
which already implements the spec branching (object-with-calendarId -> read
slot; string -> parse; non-string non-object -> TypeError; unknown calendar
string -> RangeError).
Porcelain layer: widen the union to accept the five date-bearing value types
alongside Calendar, resolving through a new Calendar::resolve() helper that
returns a Calendar case as-is or reads the value's own calendar.
Unlocks 6 test262 fixtures (withCalendar/calendar-temporal-object{,-objects}
for PlainDate, PlainDateTime, ZonedDateTime); test262 incomplete 417 -> 411,
0 failures. Full `composer check` green: PHPStan 9 / Psalm 1 / Mago clean,
Infection 100% covered MSI (563 mutations, 0 escaped, 0 uncovered).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MidnightDesign
changed the title
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
1 participant
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.
Summary
withCalendardeviated from TC39: it typed its parameter asstringand rejected Temporal objects with a nativeTypeError. The spec'sToTemporalCalendarIdentifierhas an object fast path — when the argument is a Temporal date-bearing object (PlainDate,PlainDateTime,PlainMonthDay,PlainYearMonth,ZonedDateTime), it returns that object's[[Calendar]].This came out of a re-investigation of the test262 "transpiler gap" buckets: it was surfacing as a transpiler artifact (
Argument #1 ($calendar) must be of type string, … PlainDate given) but was a genuine spec bug.Changes
PlainDate,PlainDateTime,ZonedDateTime): widen the parameter tomixed, resolve via the existingCalendarFactory::resolveBagCalendar()(object-with-calendarId→ read slot; string → parse; non-string non-object →TypeError; unknown calendar string →RangeError).Calendar, via a newCalendar::resolve()helper.PlainDateTest::testWithCalendarAcceptsTemporalObject.Per-commit impact
c729ae46— unlocks 6 test262 fixtures (withCalendar/calendar-temporal-object{,-objects}× {PlainDate, PlainDateTime, ZonedDateTime}). test262 incomplete 417 → 411, 0 failures. Fullcomposer checkgreen: PHPStan 9 / Psalm 1 / Mago clean; Infection 100% covered MSI (563 mutations, 0 escaped, 0 uncovered).🤖 Generated with Claude Code