Skip to content

Conversation

@jessealama
Copy link
Collaborator

In #1753 there was some discussion of what to do about dates with an extended year equal to "-000000". It was decided to reject such cases. Here's a simple solution to that.

Alternatives considered:

  • Change the regexp for the year part of dates. This would have the desired effect of rejecting "-000000" as ungrammatical. This alternative wasn't chosen because it seems to me that the invalidity is a domain decision, not a grammar decision. Thus, "-000000" remains grammatically valid and thus potentially meaningful; we just reject this one case by a domain convention.

closes #1753

@codecov
Copy link

codecov bot commented Dec 28, 2021

Codecov Report

Merging #1992 (82240b6) into main (3343a66) will decrease coverage by 0.35%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1992      +/-   ##
==========================================
- Coverage   94.97%   94.62%   -0.36%     
==========================================
  Files          19       19              
  Lines       10990    10992       +2     
  Branches     1750     1598     -152     
==========================================
- Hits        10438    10401      -37     
- Misses        535      573      +38     
- Partials       17       18       +1     
Flag Coverage Δ
test262 80.91% <0.00%> (+1.65%) ⬆️
tests 89.13% <0.00%> (-0.75%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
polyfill/lib/ecmascript.mjs 95.30% <100.00%> (+0.02%) ⬆️
polyfill/lib/plainmonthday.mjs 82.97% <0.00%> (-6.39%) ⬇️
polyfill/lib/plaindate.mjs 84.23% <0.00%> (-5.95%) ⬇️
polyfill/lib/plainyearmonth.mjs 93.41% <0.00%> (-2.70%) ⬇️
polyfill/lib/plaintime.mjs 89.14% <0.00%> (-0.20%) ⬇️
polyfill/lib/plaindatetime.mjs 97.44% <0.00%> (-0.15%) ⬇️
polyfill/lib/intrinsicclass.mjs 55.40% <0.00%> (+4.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3343a66...82240b6. Read the comment docs.

@ptomato ptomato added the spec-text Specification text involved label Jan 11, 2022
@jessealama
Copy link
Collaborator Author

closing because this change belongs in PR #1992

@jessealama jessealama closed this Jan 12, 2022
@jessealama jessealama reopened this Jan 12, 2022
@ptomato ptomato marked this pull request as ready for review January 12, 2022 19:39
Copy link
Collaborator

@ptomato ptomato left a comment

Choose a reason for hiding this comment

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

Yes, this is all that's needed! We will need to pull in test262 tests when they are merged there, but this can land separately from that.

@ptomato ptomato changed the title Reject '-000000' as extended year Normative: Reject '-000000' as extended year Jan 12, 2022
@ptomato
Copy link
Collaborator

ptomato commented Jan 12, 2022

Just for the record: This is a normative change, but it was already "pre-approved" at the October 2021 meeting of TC39: https://github.com/tc39/notes/blob/master/meetings/2021-10/oct-25.md#clarify-validity-of-negative-expanded-year-0

In [1] we raised the question of whether "-000000" should be
accepted as an extended year.

The discussion there was focused on parsing ISO 8601 dates,
where "-000000" shows up as the year part of a potentially
complicated string. But what about simpler strings, more
precisely, year-month strings like "2022-01"?

I think it is reasonable to infer from the discussion in [1]
that "-000000" should be rejected as the year part of
Temporal PlainYearMonths. If that's right, for consistency's
sake, we ought to change `ParseTemporalYearMonthString` so
that "-000000" gets rejected, just as "-000000" is supposed
to be rejected when parsing date(-time) strings more
complicated than year-months (and which use a different
abstract operation).

Temporal has an abstract operation,
`ParseTemporalYearMonthString`, for parsing year-month
strings. It is simpler than parsing the full ISO 8601
grammar. As it currently stands, "-000000" sneaks through
and is not rejected. Thus, "-000000-07" is not rejected as a
Temporal PlainYearMonth.

1: tc39#1753
@jessealama
Copy link
Collaborator Author

The PR has been extended to handle the case of -000000 showing up when parsing PlainYearMonths.

jessealama and others added 2 commits January 31, 2022 17:29
Co-authored-by: Jordan Harband <[email protected]>
Co-authored-by: Jordan Harband <[email protected]>
@jessealama
Copy link
Collaborator Author

@ljharb I didn't know about SameValue -- thanks!

@ptomato
Copy link
Collaborator

ptomato commented Jan 31, 2022

I reviewed the update, I think this is ready to go now.

@ptomato ptomato merged commit e60ef9e into tc39:main Jan 31, 2022
12wrigja added a commit to 12wrigja/temporal-polyfill that referenced this pull request Mar 4, 2022
12wrigja added a commit to 12wrigja/temporal-polyfill that referenced this pull request Mar 4, 2022
12wrigja added a commit to 12wrigja/temporal-polyfill that referenced this pull request Mar 4, 2022
toJSON.

Reject '-000000' as an extended year value.

Bump test262.

Port of tc39/proposal-temporal#1995.

Port of tc39/proposal-temporal#1992
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this pull request May 10, 2022
https://bugs.webkit.org/show_bug.cgi?id=240263

Reviewed by Yusuke Suzuki.

As of the following two PRs, -000000 is officially disallowed as a representation of the year zero in ISO date strings.
tc39/ecma262#2550
tc39/proposal-temporal#1992

This patch implements the change for Temporal and Date alike.

* test262/expectations.yaml:
Mark 24 test cases as passing.

* runtime/ISO8601.cpp:
(JSC::ISO8601::parseDate):

* wtf/DateMath.cpp:
(WTF::parseES5DatePortion):

Canonical link: https://commits.webkit.org/250432@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293996 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

spec-text Specification text involved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Is "-000000" a valid extended year format?

3 participants