Skip to content

fix: emit full seconds in Instant and ZonedDateTime toISOString (RFC 3339) - #131

Open
dualfroz wants to merge 1 commit into
brick:mainfrom
dualfroz:dualfroz/fix-toisostring-rfc3339
Open

fix: emit full seconds in Instant and ZonedDateTime toISOString (RFC 3339)#131
dualfroz wants to merge 1 commit into
brick:mainfrom
dualfroz:dualfroz/fix-toisostring-rfc3339

Conversation

@dualfroz

@dualfroz dualfroz commented Sep 5, 2026

Copy link
Copy Markdown

Problem

Fixes #110

Problem

Instant::toISOString() (and therefore Instant::jsonSerialize() / (string)) and
ZonedDateTime::toISOString() emit a compact time format that omits the seconds when they
are zero, e.g.:

2023-11-14T22:14Z            // whole minute
1970-01-01T00:00Z            // epoch
2023-11-14T23:00Z            // whole hour

This is a valid ISO 8601 time, but it is not a valid RFC 3339 date-time. RFC 3339
section 5.6 defines full-time as partial-time time-offset where
partial-time = time-hour ":" time-minute ":" time-second [time-secfrac] — the seconds
component is mandatory, even when it is 00. Instant and ZonedDateTime are the types
meant for interoperability (JSON serialization, wire formats), so they should always emit a
value other systems can parse as RFC 3339. Producing 2023-11-14T22:14Z breaks strict
RFC 3339 parsers.

Root cause

The compact behavior is intentional and documented in
src/LocalTime.php:639 (toISOString() — "the shortest that outputs the full value of the
time"). That is correct for LocalTime and is left untouched.

The bug is that the interop types inherit that compact form:

  • src/ZonedDateTime.php:740 toISOString() built the string from
    $this->localDateTime, whose __toString() delegates to LocalTime::toISOString()
    (the compact formatter).
  • src/Instant.php:362 toISOString() returns (string) ZonedDateTime::ofInstant(...),
    so it inherited the same compact time. jsonSerialize() and __toString() both route
    through toISOString().

The fix

ZonedDateTime::toISOString() now builds the time portion explicitly with a mandatory
HH:MM:SS component (keeping the existing optional fractional-seconds handling), instead of
delegating to LocalTime's compact formatter. Instant::toISOString() is fixed
transitively because it delegates to ZonedDateTime; its PHPDoc now documents the RFC 3339
guarantee. LocalTime is deliberately not changed.

Output after the fix:

2023-11-14T22:14:00Z
1970-01-01T00:00:00Z
2023-11-14T23:00:00Z
2023-11-14T23:14:00+01:00[Europe/Paris]

Fractional seconds and non-zero seconds are unchanged (e.g.
1970-01-01T00:00:00.000123456Z, 1969-12-31T23:59:59Z).

Why the test fixture changes are justified (BC break)

Some existing fixtures pinned the buggy compact output and had to be updated to the
RFC-3339-correct strings — this is the legitimate case of changing a maintainer's tests
because the old expected values encode the bug being fixed:

  • tests/InstantTest.php providerToString: [0, 0, '1970-01-01T00:00Z'] ->
    '1970-01-01T00:00:00Z'.
  • tests/ZonedDateTimeTest.php: the DST plus/minus and interval result strings that landed
    on whole minutes/hours (e.g. 2025-03-30T01:00+01:00[Europe/Prague] ->
    2025-03-30T01:00:00+01:00[Europe/Prague]).
  • tests/IntervalTest.php: one exception-message fixture (1970-01-04T11:20Z ->
    1970-01-04T11:20:00Z).

New assertions were added for whole-minute and whole-hour values to lock in the :00:

  • tests/InstantTest.php: [1700000040, 0, '2023-11-14T22:14:00Z'] and
    [1700002800, 0, '2023-11-14T23:00:00Z'].
  • tests/ZonedDateTimeTest.php: ['2000-01-20T12:34', '-07:00', '2000-01-20T12:34:00-07:00']
    and ['2000-01-20T12:00', 'America/Los_Angeles', '2000-01-20T12:00:00-08:00[America/Los_Angeles]'].

Because the serialized string representation changes, this is technically a backward
compatibility break, matching the maintainer's note on issue #110: "this will target
version 0.8.0 as this is technically a BC break." Parsing is unaffected — the parser still
accepts the compact input form, so round-tripping and existing stored values keep working.

@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.30%. Comparing base (66dab53) to head (9dd1445).

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #131   +/-   ##
=========================================
  Coverage     99.30%   99.30%           
- Complexity     1109     1113    +4     
=========================================
  Files            48       48           
  Lines          2445     2458   +13     
=========================================
+ Hits           2428     2441   +13     
  Misses           17       17           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dualfroz
dualfroz force-pushed the dualfroz/fix-toisostring-rfc3339 branch from 57dd10e to b9042bd Compare September 5, 2026 22:57
…3339)

Instant::toISOString() and ZonedDateTime::toISOString() emitted a compact
time that dropped zero seconds (e.g. 2023-11-14T22:14Z), which is valid
ISO 8601 but not valid RFC 3339. Section 5.6 of RFC 3339 requires the full
HH:MM:SS time even when the seconds are zero. These types are meant for
interoperability, so they must always emit RFC 3339 compliant output.

Build the time part of ZonedDateTime::toISOString() explicitly with a
mandatory seconds component instead of delegating to LocalTime's compact
formatter. Instant delegates to ZonedDateTime, so it is fixed transitively;
jsonSerialize() and __toString() route through toISOString(). LocalTime's
documented compact format is intentional and left unchanged.

This changes the serialized string representation and is therefore a BC
break; fixtures that pinned the compact output are updated accordingly.
@dualfroz
dualfroz force-pushed the dualfroz/fix-toisostring-rfc3339 branch from b9042bd to 9dd1445 Compare September 5, 2026 23:04
Comment on lines +881 to +884
['2025-03-30T01:30:00+01:00[Europe/Prague]', 3000, '2025-03-30T03:20:00+02:00[Europe/Prague]'],
['2025-03-30T01:30:00+01:00[Europe/Prague]', 6000, '2025-03-30T04:10:00+02:00[Europe/Prague]'],
['2025-03-30T03:20:00+02:00[Europe/Prague]', -3000, '2025-03-30T01:30:00+01:00[Europe/Prague]'],
['2025-03-30T04:10:00+02:00[Europe/Prague]', -6000, '2025-03-30T01:30:00+01:00[Europe/Prague]'],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Input does not need to be modified:

Suggested change
['2025-03-30T01:30:00+01:00[Europe/Prague]', 3000, '2025-03-30T03:20:00+02:00[Europe/Prague]'],
['2025-03-30T01:30:00+01:00[Europe/Prague]', 6000, '2025-03-30T04:10:00+02:00[Europe/Prague]'],
['2025-03-30T03:20:00+02:00[Europe/Prague]', -3000, '2025-03-30T01:30:00+01:00[Europe/Prague]'],
['2025-03-30T04:10:00+02:00[Europe/Prague]', -6000, '2025-03-30T01:30:00+01:00[Europe/Prague]'],
['2025-03-30T01:30:00+01[Europe/Prague]', 3000, '2025-03-30T03:20:00+02:00[Europe/Prague]'],
['2025-03-30T01:30:00+01[Europe/Prague]', 6000, '2025-03-30T04:10:00+02:00[Europe/Prague]'],
['2025-03-30T03:20:00+02[Europe/Prague]', -3000, '2025-03-30T01:30:00+01:00[Europe/Prague]'],
['2025-03-30T04:10:00+02[Europe/Prague]', -6000, '2025-03-30T01:30:00+01:00[Europe/Prague]'],

Comment on lines +851 to +854
['2025-03-30T01:30:00+01:00[Europe/Prague]', 50, '2025-03-30T03:20:00+02:00[Europe/Prague]'],
['2025-03-30T01:30:00+01:00[Europe/Prague]', 100, '2025-03-30T04:10:00+02:00[Europe/Prague]'],
['2025-03-30T03:20:00+02:00[Europe/Prague]', -50, '2025-03-30T01:30:00+01:00[Europe/Prague]'],
['2025-03-30T04:10:00+02:00[Europe/Prague]', -100, '2025-03-30T01:30:00+01:00[Europe/Prague]'],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Input does not need to be modified:

Suggested change
['2025-03-30T01:30:00+01:00[Europe/Prague]', 50, '2025-03-30T03:20:00+02:00[Europe/Prague]'],
['2025-03-30T01:30:00+01:00[Europe/Prague]', 100, '2025-03-30T04:10:00+02:00[Europe/Prague]'],
['2025-03-30T03:20:00+02:00[Europe/Prague]', -50, '2025-03-30T01:30:00+01:00[Europe/Prague]'],
['2025-03-30T04:10:00+02:00[Europe/Prague]', -100, '2025-03-30T01:30:00+01:00[Europe/Prague]'],
['2025-03-30T01:30:00+01[Europe/Prague]', 50, '2025-03-30T03:20:00+02:00[Europe/Prague]'],
['2025-03-30T01:30:00+01[Europe/Prague]', 100, '2025-03-30T04:10:00+02:00[Europe/Prague]'],
['2025-03-30T03:20:00+02[Europe/Prague]', -50, '2025-03-30T01:30:00+01:00[Europe/Prague]'],
['2025-03-30T04:10:00+02[Europe/Prague]', -100, '2025-03-30T01:30:00+01:00[Europe/Prague]'],

@BenMorel

BenMorel commented Sep 6, 2026

Copy link
Copy Markdown
Member

2 nitpicks, otherwise LGTM @dualfroz!

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.

Instant::toISOString() is not RFC 3339 compliant

2 participants