Skip to content

Commit

Permalink
Normative: Fix inconsistency in order of observable operations
Browse files Browse the repository at this point in the history 
Previously, the monthDayFromFields() method of non-ISO calendars behaved
differently from dateFromFields() and yearMonthFromFields(), as well as
monthDayFromFields() of the ISO calendar. The latter checked the overflow
option before accessing the fields, and the former checked the overflow
option afterwards.

This normative change fixes this inconsistency. The general principle is
that arguments to a function should be validated in order, so we go with
accessing the fields before checking the overflow option.

Requires an update to the reference code, which previously consistently
checked the overflow option first.
  • Loading branch information
@ptomato
ptomato committed Aug 25, 2022
1 parent e938a3e commit 875c5bb
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 11 deletions.
12 changes: 6 additions & 6 deletions polyfill/lib/calendar.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,24 +233,24 @@ DefineIntrinsic('Temporal.Calendar.from', Calendar.from);

impl['iso8601'] = {
dateFromFields(fields, options, calendar) {
const overflow = ES.ToTemporalOverflow(options);
fields = ES.PrepareTemporalFields(fields, ['day', 'month', 'monthCode', 'year'], ['year', 'day']);
const overflow = ES.ToTemporalOverflow(options);
fields = resolveNonLunisolarMonth(fields);
let { year, month, day } = fields;
({ year, month, day } = ES.RegulateISODate(year, month, day, overflow));
return ES.CreateTemporalDate(year, month, day, calendar);
},
yearMonthFromFields(fields, options, calendar) {
const overflow = ES.ToTemporalOverflow(options);
fields = ES.PrepareTemporalFields(fields, ['month', 'monthCode', 'year'], ['year']);
const overflow = ES.ToTemporalOverflow(options);
fields = resolveNonLunisolarMonth(fields);
let { year, month } = fields;
({ year, month } = ES.RegulateISOYearMonth(year, month, overflow));
return ES.CreateTemporalYearMonth(year, month, calendar, /* referenceISODay = */ 1);
},
monthDayFromFields(fields, options, calendar) {
const overflow = ES.ToTemporalOverflow(options);
fields = ES.PrepareTemporalFields(fields, ['day', 'month', 'monthCode', 'year'], ['day']);
const overflow = ES.ToTemporalOverflow(options);
if (fields.month !== undefined && fields.year === undefined && fields.monthCode === undefined) {
throw new TypeError('either year or monthCode required with month');
}
Expand Down Expand Up @@ -1874,35 +1874,35 @@ const helperDangi = ObjectAssign({}, { ...helperChinese, id: 'dangi' });
*/
const nonIsoGeneralImpl = {
dateFromFields(fields, options, calendar) {
const overflow = ES.ToTemporalOverflow(options);
const cache = new OneObjectCache();
const fieldNames = this.fields(['day', 'month', 'monthCode', 'year']);
ES.Call(ArrayPrototypeSort, fieldNames, []);
fields = ES.PrepareTemporalFields(fields, fieldNames, []);
const overflow = ES.ToTemporalOverflow(options);
const { year, month, day } = this.helper.calendarToIsoDate(fields, overflow, cache);
const result = ES.CreateTemporalDate(year, month, day, calendar);
cache.setObject(result);
return result;
},
yearMonthFromFields(fields, options, calendar) {
const overflow = ES.ToTemporalOverflow(options);
const cache = new OneObjectCache();
const fieldNames = this.fields(['month', 'monthCode', 'year']);
ES.Call(ArrayPrototypeSort, fieldNames, []);
fields = ES.PrepareTemporalFields(fields, fieldNames, []);
const overflow = ES.ToTemporalOverflow(options);
const { year, month, day } = this.helper.calendarToIsoDate({ ...fields, day: 1 }, overflow, cache);
const result = ES.CreateTemporalYearMonth(year, month, calendar, /* referenceISODay = */ day);
cache.setObject(result);
return result;
},
monthDayFromFields(fields, options, calendar) {
const overflow = ES.ToTemporalOverflow(options);
const cache = new OneObjectCache();
// For lunisolar calendars, either `monthCode` or `year` must be provided
// because `month` is ambiguous without a year or a code.
const fieldNames = this.fields(['day', 'month', 'monthCode', 'year']);
ES.Call(ArrayPrototypeSort, fieldNames, []);
fields = ES.PrepareTemporalFields(fields, fieldNames, []);
const overflow = ES.ToTemporalOverflow(options);
const { year, month, day } = this.helper.monthDayFromFields(fields, overflow, cache);
// `year` is a reference year where this month/day exists in this calendar
const result = ES.CreateTemporalMonthDay(month, day, calendar, /* referenceISOYear = */ year);
Expand Down
6 changes: 3 additions & 3 deletions spec/calendar.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -572,8 +572,8 @@ <h1>ISODateFromFields ( _fields_, _options_ )</h1>
</p>
<emu-alg>
1. Assert: Type(_fields_) is Object.
1. Let _overflow_ be ? ToTemporalOverflow(_options_).
1. Set _fields_ to ? PrepareTemporalFields(_fields_, « *"day"*, *"month"*, *"monthCode"*, *"year"* », « *"year"*, *"day"* »).
1. Let _overflow_ be ? ToTemporalOverflow(_options_).
1. Let _year_ be ! Get(_fields_, *"year"*).
1. Assert: Type(_year_) is Number.
1. Let _month_ be ? ResolveISOMonth(_fields_).
Expand All @@ -591,8 +591,8 @@ <h1>ISOYearMonthFromFields ( _fields_, _options_ )</h1>
</p>
<emu-alg>
1. Assert: Type(_fields_) is Object.
1. Let _overflow_ be ? ToTemporalOverflow(_options_).
1. Set _fields_ to ? PrepareTemporalFields(_fields_, « *"month"*, *"monthCode"*, *"year"* », « *"year"* »).
1. Let _overflow_ be ? ToTemporalOverflow(_options_).
1. Let _year_ be ! Get(_fields_, *"year"*).
1. Assert: Type(_year_) is Number.
1. Let _month_ be ? ResolveISOMonth(_fields_).
Expand All @@ -612,8 +612,8 @@ <h1>ISOMonthDayFromFields ( _fields_, _options_ )</h1>
</p>
<emu-alg>
1. Assert: Type(_fields_) is Object.
1. Let _overflow_ be ? ToTemporalOverflow(_options_).
1. Set _fields_ to ? PrepareTemporalFields(_fields_, « *"day"*, *"month"*, *"monthCode"*, *"year"* », « *"day"* »).
1. Let _overflow_ be ? ToTemporalOverflow(_options_).
1. Let _month_ be ! Get(_fields_, *"month"*).
1. Let _monthCode_ be ! Get(_fields_, *"monthCode"*).
1. Let _year_ be ! Get(_fields_, *"year"*).
Expand Down
4 changes: 2 additions & 2 deletions spec/intl.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1649,10 +1649,10 @@ <h1>Temporal.Calendar.prototype.dateFromFields ( _fields_ [ , _options_ ] )</h1>
1. If _calendar_.[[Identifier]] is *"iso8601"*, then
1. Let _result_ be ? ISODateFromFields(_fields_, _options_).
1. Else,
1. Let _overflow_ be ? ToTemporalOverflow(_options_).
1. Let _fieldNames_ be CalendarDateFields(_calendar_.[[Identifier]], « *"day"*, *"month"*, *"monthCode"*, *"year"* »).
1. Sort _fieldNames_ into the same order as if an Array of the same values had been sorted using %Array.prototype.sort% with *undefined* as _comparefn_.
1. Set _fields_ to ? PrepareTemporalFields(_fields_, _fieldNames_, « »).
1. Let _overflow_ be ? ToTemporalOverflow(_options_).
1. Let _result_ be ? CalendarDateToISO(_calendar_.[[Identifier]], _fields_, _overflow_).
1. Return ? CreateTemporalDate(_result_.[[Year]], _result_.[[Month]], _result_.[[Day]], _calendar_).
</emu-alg>
Expand All @@ -1672,10 +1672,10 @@ <h1>Temporal.Calendar.prototype.yearMonthFromFields ( _fields_ [ , _options_ ] )
1. If _calendar_.[[Identifier]] is *"iso8601"*, then
1. Let _result_ be ? ISOYearMonthFromFields(_fields_, _options_).
1. Else,
1. Let _overflow_ be ? ToTemporalOverflow(_options_).
1. Let _fieldNames_ be CalendarDateFields(_calendar_.[[Identifier]], « *"month"*, *"monthCode"*, *"year"* »).
1. Sort _fieldNames_ into the same order as if an Array of the same values had been sorted using %Array.prototype.sort% with *undefined* as _comparefn_.
1. Set _fields_ to ? PrepareTemporalFields(_fields_, _fieldNames_, « »).
1. Let _overflow_ be ? ToTemporalOverflow(_options_).
1. Let _result_ be ? CalendarDateToISO(_calendar_.[[Identifier]], _fields_, _overflow_).
1. Set _result_.[[ReferenceISODay]] to _result_.[[Day]].
1. Return ? CreateTemporalYearMonth(_result_.[[Year]], _result_.[[Month]], _calendar_, _result_.[[ReferenceISODay]]).
Expand Down

0 comments on commit 875c5bb

Please sign in to comment.