Fix local/UTC confusion in dateTimeStamp conversions#156
Open
ajbufort wants to merge 1 commit into
Open
Conversation
xs:date(current-dateTime()) could return a different date than current-date() near UTC midnight, as reported in Paligo#117: the cast from xs:dateTimeStamp to xs:date took the UTC date rather than the date in the value's own timezone. The underlying problem was wider: a NaiveDateTimeWithOffset stores a naive datetime that is local to its offset, and a lexical datetime is also local to its timezone, but several conversions interpreted these naive values as UTC, shifting the value by its offset: - parsing an xs:dateTimeStamp literal used from_utc_datetime, so xs:dateTimeStamp("2025-04-01T23:00:00-01:00") came back as 22:00:00-01:00 - casting xs:dateTime to xs:dateTimeStamp used from_naive_utc_and_offset, shifting the same way - the From<NaiveDateTimeWithOffset> conversion to a chrono DateTime did the same; it now goes through to_date_time_stamp, which was already correct (this conversion currently has no callers in the workspace, so it is fixed for consistency of the public API) - casting xs:dateTimeStamp to xs:date took naive_utc().date() - a dateTimeStamp map key used the local naive datetime while the equivalent dateTime-with-offset key used the UTC-normalized one, so the same instant produced different map keys depending on its type Because chrono stores DateTime<FixedOffset> UTC-normalized internally, a value near chrono's range boundary can be representable as local-plus-offset but not as UTC. The parser and the cast raise FODT0001 for these instead of panicking, with a regression test. Three existing snapshots had captured the buggy shifted values (an identity round trip through xs:dateTimeStamp gaining an hour) and are updated to the now correct output. Fixes Paligo#117. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Fixes #117.
xs:date(current-dateTime())could return a different date thancurrent-date()near UTC midnight, andxs:dateTimeStamp("2025-04-01T23:00:00-01:00")round-tripped to22:00:00-01:00.Root cause
A
NaiveDateTimeWithOffsetstores a naive datetime that is local to its offset - that's the conventionToDateTimeStamp, display, and the parsers forxs:dateTimefollow. A lexical datetime is likewise local to its timezone. But five sites interpreted these naive values as UTC, shifting values by their own offset:date_time_stamp_parserfrom_utc_datetimeon the lexical (local) datetimecast_to_date_time_stamp(fromxs:dateTime)from_naive_utc_and_offseton the stored (local) datetimeFrom<NaiveDateTimeWithOffset> for DateTime<FixedOffset>to_date_time_stampcast_to_date(fromxs:dateTimeStamp)naive_utc().date()- the UTC date, not the date in the value's timezoneMapKeyforxs:dateTimeStampxs:dateTimekeys use the UTC-normalized one, so the same instant produced different map keysThe gYear/gMonth/gDay/time casts already used chrono's local accessors, and the
adjust-*-to-timezonefunctions'naive_utc()is a correct shift idiom - those are untouched.One wrinkle the local interpretation exposes: chrono stores
DateTime<FixedOffset>UTC-normalized internally, so a value near chrono's range boundary can be representable as local-plus-offset but not as UTC (e.g.262142-12-31T23:30:00-01:00).from_local_datetimereturnsNonethere, so the parser and the cast raiseFODT0001rather than unwrap -"262142-12-31T23:30:00-01:00" cast as xs:dateTimeStampis a dynamic error, not a panic, with a regression test. (ToDateTimeStamp::to_date_time_stamphas a pre-existingunwrap()with the same theoretical reach through comparisons of extremexs:dateTimevalues - that predates this change and is untouched here; happy to file it separately.)Testing
xee-xpath/tests/datetime_context.rswith an injectedcurrent_datetimereproduces every case from Oddness surrounding xs:date(xs:current-dateTime()) / xs:dateTimeStamp #117 deterministically (no faketime needed):xs:date(current-dateTime())vscurrent-date()on both sides of UTC midnight, thexs:dateTimeStampstring and cast round trips, and map-key equivalence betweenxs:dateTimeandxs:dateTimeStampfor the same instant.xs:dateTimeStampgaining an hour) and are updated to the now-correct output.cargo fmt,clippy -D warnings, fullcargo testclean.One note for review: the
From<NaiveDateTimeWithOffset> for DateTime<FixedOffset>impl turns out to have no callers anywhere in the workspace (verified by removing it and compiling all targets) - so that particular change can't have broken anything, and equally the reported bugs never flowed through it. I fixed it anyway (delegating to the already-correctto_date_time_stamp) so the public conversion agrees with the type's local-naive semantics rather than silently shifting for the next caller; happy to drop it to a doc comment or remove the impl instead if you prefer.Generated with Claude Code