Polyfill: Change check to assertion in adjustCalendarDate#3221
Polyfill: Change check to assertion in adjustCalendarDate#3221catamorphism wants to merge 1 commit intotc39:mainfrom
Conversation
This should be guaranteed by the previous call to validateCalendarDate.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3221 +/- ##
=======================================
Coverage 96.75% 96.76%
=======================================
Files 22 22
Lines 10398 10399 +1
Branches 1859 1858 -1
=======================================
+ Hits 10061 10063 +2
+ Misses 289 288 -1
Partials 48 48 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ptomato
left a comment
ptomato
left a comment
There was a problem hiding this comment.
Should probably gain a test262 test instead.
| const largestMonth = this.monthsInYear({ year }); | ||
| if (month < 1 || month > largestMonth) throw new RangeErrorCtor(`Invalid monthCode: ${monthCode}`); | ||
| // The earlier call to validateCalendarDate guarantees this. | ||
| assert(month >= 1 && month <= largestMonth, `invalid monthCode: ${monthCode}`); |
There was a problem hiding this comment.
This isn't quite the same as what's guaranteed in validateCalendarDate. I see there that the number part of the month code is validated to be between 1 and 13 inclusive, but in a 12-month year I can trigger this assertion:
Temporal.PlainDate.from({ calendar: 'hebrew', year: 5781, monthCode: 'M13', day: 1 })There was a problem hiding this comment.
Ah, I see. There's already a test262 test: https://github.com/tc39/test262/blob/main/test/intl402/Temporal/PlainDate/from/invalid-month-codes-hebrew.js
and I see that codecov.io shows this line as green now (not sure why it was red before, as that test was added in Nov. 2025), so I think I can just close this PR.
There was a problem hiding this comment.
Probably it was red because 5781 is a common year and 5779 is a leap year, which happen to be different code paths. So it still might be worth adding a line to that test file. (I'm not sure why the line is green now.)
2 participants
This should be guaranteed by the previous call to validateCalendarDate.