-
Notifications
You must be signed in to change notification settings - Fork 156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Editorial: Use early errors and parameterization to simplify ISO 8601 grammar #2766
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2766 +/- ##
==========================================
+ Coverage 96.51% 96.57% +0.06%
==========================================
Files 23 23
Lines 12432 12243 -189
Branches 2258 2268 +10
==========================================
- Hits 11999 11824 -175
+ Misses 374 358 -16
- Partials 59 61 +2 ☔ View full report in Codecov by Sentry. |
f9cf3f0
to
67fb2fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed (in person!), and we found the above improvements. But more importantly, no issues.
Dates such as 2019-02-29, 2024-09-31, etc., can be rejected at early error time. (This changes the order in which some errors are thrown, but since they are all RangeError, it is not observable from the spec's point of view.)
Previously the grammar relied on bizarre one-off productions such as TimeSpecWithOptionalOffsetNotAmbiguous and DateMonthWithThirtyOneDays, to prevent time strings such as 0119 (01:19 but ambiguous with 19th January) or 1524-08 (15:24 at -08:00 from UTC, but ambiguous with August of 1524) from parsing correctly without a time designator. By using early errors, these confusing productions are no longer necessary to achieve the desired result. Closes: #1984
TimeSeparator[Extended] is defined in ECMA-262 and we can use it here instead of notation like `:`?. We define a new production Time which is a TimeSpec that is either fully basic or fully extended. In AnnotatedTime we keep it split out when there is no TimeDesignator present, because the early error only applies to the [~Extended] case.
67fb2fd
to
c3eb4cf
Compare
Similar to TimeSeparator[Extended] in ECMA-262, introduce DateSeparator, and Date to denote a DateSpec either fully in basic format or fully in extended format.
…owed In Plain types, the UTC designator 'Z' is not allowed in the string. This was previously expressed with explicit checks in the ParseTemporal... AOs, but can be more cleanly expressed with a [Z] parameter to the DateTimeUTCOffset production.
…meterization Another place where we can deduplicate parts of the grammar.
Instead of two separate productions UTCOffsetSubMinutePrecision and UTCOffsetMinutePrecision, we can have one UTCOffset production with a SubMinutePrecision parameter.
c3eb4cf
to
c507068
Compare
In the ISO 8601 grammar, we can use early errors and parameterization to avoid such bizarre productions like TimeHourNotThirtyOneDayMonth and TimeSpecWithOptionalOffsetNotAmbiguous, etc.
Best reviewed commit by commit.
Closes: #1984