Skip to content
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

Is the yearAbsent assertion in ParseISODateTime correct? #3045

Open
trflynn89 opened this issue Nov 20, 2024 · 3 comments
Open

Is the yearAbsent assertion in ParseISODateTime correct? #3045

trflynn89 opened this issue Nov 20, 2024 · 3 comments
Labels
editorial spec-text Specification text involved

Comments

@trflynn89
Copy link
Contributor

In ParseISODateTime, step 4.a.ii.4 states:

4. If goal is TemporalMonthDayString and parseResult does not contain a DateYear Parse Node, then
   a. Assert: goal is the last element of allowedFormats.
   b. Set yearAbsent to true.

In the PlainMonthDay/from/argument-propertybag-calendar-iso-string.js test262 test, we create a Temporal.PlainMonthDay of the form:

Temporal.PlainMonthDay.from({ monthCode: "M11", day: 18, calendar: "01-01" });

Since calendar is a String, we enter ParseTemporalCalendarString. Its first step is:

1. Let parseResult be Completion(ParseISODateTime(string, « TemporalDateTimeString[+Zoned], TemporalDateTimeString[~Zoned], TemporalInstantString, TemporalTimeString, TemporalMonthDayString, TemporalYearMonthString »)).

So the last item in allowedFormats is TemporalYearMonthString. However, the calendar string "01-01" can be parsed as a TemporalMonthDayString, and in that case, the DateYear parse node will be empty. We will then fail the assertion that the parsed goal is the last allowed format.

@trflynn89
Copy link
Contributor Author

Aha, the string "01-1" can actually also be parsed as a TemporalTimeString, with hour "01" and UTC offset "-01". So we then don't hit the assertion.

@trflynn89
Copy link
Contributor Author

Actually, I'm back to thinking this is potentially a bug - "01-01" shouldn't be parsed as a TemporalTimeString due to the ambiguity rules:

https://tc39.es/proposal-temporal/#sec-temporal-iso8601grammar-static-semantics-early-errors

* It is a Syntax Error if ParseText(Time DateTimeUTCOffset[~Z], DateSpecMonthDay) is a Parse Node.

@trflynn89 trflynn89 reopened this Nov 23, 2024
@ptomato
Copy link
Collaborator

ptomato commented Nov 26, 2024

I agree with this reading, thanks for catching it. This is a regression from ce94055. It could be that we need to swap the order of TemporalYearMonthString and TemporalMonthDayString in ParseTemporalCalendarString and ParseTemporalTimeZoneString, but on the other hand I'm not sure I remember what the assertion is guaranteeing in the first place. Maybe it was to ensure that the yearAbsent code path is only hit in cases where a month-day string is really supposed to be accepted?

@ptomato ptomato added bug spec-text Specification text involved editorial and removed bug labels Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial spec-text Specification text involved
Projects
None yet
Development

No branches or pull requests

2 participants