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.

Sign up for GitHub

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

Jump to bottom

Normative: Validate receiver fields before fields of any param objects #2478

Merged
merged 3 commits into from
Feb 1, 2023
Merged

Normative: Validate receiver fields before fields of any param objects #2478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
354ce1f
Normative: Don't read 'timeZone' property in ZonedDateTime.p.with
ptomato Jan 18, 2023
7aaab13
Normative: Validate receiver fields before fields of any param objects
ptomato Jan 16, 2023
f25466a
Update test262
ptomato Feb 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions polyfill/lib/ecmascript.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3883,12 +3883,12 @@ export const ES = ObjectAssign({}, ES2022, {
const settings = ES.GetDifferenceSettings(operation, options, 'date', ['week', 'day'], 'month', 'year');

const fieldNames = ES.CalendarFields(calendar, ['monthCode', 'year']);
const otherFields = ES.PrepareTemporalFields(other, fieldNames, []);
otherFields.day = 1;
const otherDate = ES.CalendarDateFromFields(calendar, otherFields);
const thisFields = ES.PrepareTemporalFields(yearMonth, fieldNames, []);
thisFields.day = 1;
const thisDate = ES.CalendarDateFromFields(calendar, thisFields);
const otherFields = ES.PrepareTemporalFields(other, fieldNames, []);
otherFields.day = 1;
const otherDate = ES.CalendarDateFromFields(calendar, otherFields);

const untilOptions = ObjectCreate(null);
ES.CopyDataProperties(untilOptions, options, []);
Expand Down
5 changes: 2 additions & 3 deletions polyfill/lib/plaindate.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,15 @@ export class PlainDate {
throw new TypeError('invalid argument');
}
ES.RejectObjectWithCalendarOrTimeZone(temporalDateLike);
options = ES.GetOptionsObject(options);

const calendar = GetSlot(this, CALENDAR);
const fieldNames = ES.CalendarFields(calendar, ['day', 'month', 'monthCode', 'year']);
const partialDate = ES.PrepareTemporalFields(temporalDateLike, fieldNames, 'partial');
let fields = ES.PrepareTemporalFields(this, fieldNames, []);
const partialDate = ES.PrepareTemporalFields(temporalDateLike, fieldNames, 'partial');
fields = ES.CalendarMergeFields(calendar, fields, partialDate);
fields = ES.PrepareTemporalFields(fields, fieldNames, []);

options = ES.GetOptionsObject(options);

return ES.CalendarDateFromFields(calendar, fields, options);
}
withCalendar(calendar) {
Expand Down
2 changes: 1 addition & 1 deletion polyfill/lib/plaindatetime.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ export class PlainDateTime {
'second',
'year'
]);
const partialDateTime = ES.PrepareTemporalFields(temporalDateTimeLike, fieldNames, 'partial');
let fields = ES.PrepareTemporalFields(this, fieldNames, []);
const partialDateTime = ES.PrepareTemporalFields(temporalDateTimeLike, fieldNames, 'partial');
fields = ES.CalendarMergeFields(calendar, fields, partialDateTime);
fields = ES.PrepareTemporalFields(fields, fieldNames, []);
const { year, month, day, hour, minute, second, millisecond, microsecond, nanosecond } =
Expand Down
4 changes: 2 additions & 2 deletions polyfill/lib/plainmonthday.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ export class PlainMonthDay {
throw new TypeError('invalid argument');
}
ES.RejectObjectWithCalendarOrTimeZone(temporalMonthDayLike);
options = ES.GetOptionsObject(options);

const calendar = GetSlot(this, CALENDAR);
const fieldNames = ES.CalendarFields(calendar, ['day', 'month', 'monthCode', 'year']);
const partialMonthDay = ES.PrepareTemporalFields(temporalMonthDayLike, fieldNames, 'partial');
let fields = ES.PrepareTemporalFields(this, fieldNames, []);
const partialMonthDay = ES.PrepareTemporalFields(temporalMonthDayLike, fieldNames, 'partial');
fields = ES.CalendarMergeFields(calendar, fields, partialMonthDay);
fields = ES.PrepareTemporalFields(fields, fieldNames, []);

options = ES.GetOptionsObject(options);
return ES.CalendarMonthDayFromFields(calendar, fields, options);
}
equals(other) {
Expand Down
4 changes: 2 additions & 2 deletions polyfill/lib/plaintime.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ export class PlainTime {
throw new TypeError('invalid argument');
}
ES.RejectObjectWithCalendarOrTimeZone(temporalTimeLike);

const partialTime = ES.ToTemporalTimeRecord(temporalTimeLike, 'partial');
options = ES.GetOptionsObject(options);
const overflow = ES.ToTemporalOverflow(options);

const partialTime = ES.ToTemporalTimeRecord(temporalTimeLike, 'partial');

const fields = ES.ToTemporalTimeRecord(this);
let { hour, minute, second, millisecond, microsecond, nanosecond } = ObjectAssign(fields, partialTime);
({ hour, minute, second, millisecond, microsecond, nanosecond } = ES.RegulateTime(
Expand Down
5 changes: 2 additions & 3 deletions polyfill/lib/plainyearmonth.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,15 @@ export class PlainYearMonth {
throw new TypeError('invalid argument');
}
ES.RejectObjectWithCalendarOrTimeZone(temporalYearMonthLike);
options = ES.GetOptionsObject(options);

const calendar = GetSlot(this, CALENDAR);
const fieldNames = ES.CalendarFields(calendar, ['month', 'monthCode', 'year']);
const partialYearMonth = ES.PrepareTemporalFields(temporalYearMonthLike, fieldNames, 'partial');
let fields = ES.PrepareTemporalFields(this, fieldNames, []);
const partialYearMonth = ES.PrepareTemporalFields(temporalYearMonthLike, fieldNames, 'partial');
fields = ES.CalendarMergeFields(calendar, fields, partialYearMonth);
fields = ES.PrepareTemporalFields(fields, fieldNames, []);

options = ES.GetOptionsObject(options);

return ES.CalendarYearMonthFromFields(calendar, fields, options);
}
add(temporalDurationLike, options = undefined) {
Expand Down
8 changes: 3 additions & 5 deletions polyfill/lib/zoneddatetime.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ export class ZonedDateTime {
throw new TypeError('invalid zoned-date-time-like');
}
ES.RejectObjectWithCalendarOrTimeZone(temporalZonedDateTimeLike);
options = ES.GetOptionsObject(options);

const calendar = GetSlot(this, CALENDAR);
const fieldNames = ES.CalendarFields(calendar, [
Expand All @@ -195,14 +196,11 @@ export class ZonedDateTime {
'year'
]);
ES.Call(ArrayPrototypePush, fieldNames, ['offset']);
let fields = ES.PrepareTemporalFields(this, fieldNames, ['offset']);
const partialZonedDateTime = ES.PrepareTemporalFields(temporalZonedDateTimeLike, fieldNames, 'partial');

ES.Call(ArrayPrototypePush, fieldNames, ['timeZone']);
let fields = ES.PrepareTemporalFields(this, fieldNames, ['timeZone', 'offset']);
fields = ES.CalendarMergeFields(calendar, fields, partialZonedDateTime);
fields = ES.PrepareTemporalFields(fields, fieldNames, ['timeZone', 'offset']);
fields = ES.PrepareTemporalFields(fields, fieldNames, ['offset']);

options = ES.GetOptionsObject(options);
const disambiguation = ES.ToTemporalDisambiguation(options);
const offset = ES.ToTemporalOffset(options, 'prefer');

Expand Down
2 changes: 1 addition & 1 deletion polyfill/test262
Open in desktop
Submodule test262 updated 74 files
+27 −8 test/built-ins/Temporal/Calendar/from/calendar-temporal-object.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/dateAdd/argument-propertybag-calendar-case-insensitive.js
+25 −0 test/built-ins/Temporal/Calendar/prototype/dateUntil/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/day/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/dayOfWeek/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/dayOfYear/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/daysInMonth/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/daysInWeek/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/daysInYear/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/inLeapYear/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/month/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/monthCode/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/monthsInYear/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/weekOfYear/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/Calendar/prototype/year/argument-propertybag-calendar-case-insensitive.js
+20 −0 test/built-ins/Temporal/Calendar/prototype/yearOfWeek/argument-propertybag-calendar-case-insensitive.js
+1 −1 test/built-ins/Temporal/Instant/prototype/toZonedDateTime/calendar-case-insensitive.js
+27 −8 test/built-ins/Temporal/Instant/prototype/toZonedDateTime/calendar-temporal-object.js
+26 −9 test/built-ins/Temporal/Now/plainDate/calendar-temporal-object.js
+27 −10 test/built-ins/Temporal/Now/plainDateTime/calendar-temporal-object.js
+26 −9 test/built-ins/Temporal/Now/zonedDateTime/calendar-temporal-object.js
+26 −9 test/built-ins/Temporal/PlainDate/calendar-temporal-object.js
+22 −0 test/built-ins/Temporal/PlainDate/compare/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainDate/from/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainDate/prototype/equals/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainDate/prototype/since/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainDate/prototype/until/argument-propertybag-calendar-case-insensitive.js
+10 −10 test/built-ins/Temporal/PlainDate/prototype/with/order-of-operations.js
+1 −1 test/built-ins/Temporal/PlainDate/prototype/withCalendar/calendar-case-insensitive.js
+27 −8 test/built-ins/Temporal/PlainDate/prototype/withCalendar/calendar-temporal-object.js
+27 −10 test/built-ins/Temporal/PlainDateTime/calendar-temporal-object.js
+22 −0 test/built-ins/Temporal/PlainDateTime/compare/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainDateTime/from/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainDateTime/prototype/equals/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainDateTime/prototype/since/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainDateTime/prototype/until/argument-propertybag-calendar-case-insensitive.js
+10 −10 test/built-ins/Temporal/PlainDateTime/prototype/with/order-of-operations.js
+1 −1 test/built-ins/Temporal/PlainDateTime/prototype/withCalendar/calendar-case-insensitive.js
+27 −8 test/built-ins/Temporal/PlainDateTime/prototype/withCalendar/calendar-temporal-object.js
+7 −3 .../built-ins/Temporal/PlainDateTime/prototype/withPlainDate/argument-propertybag-calendar-case-insensitive.js
+26 −9 test/built-ins/Temporal/PlainMonthDay/calendar-temporal-object.js
+7 −3 test/built-ins/Temporal/PlainMonthDay/from/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainMonthDay/prototype/equals/argument-propertybag-calendar-case-insensitive.js
+6 −6 test/built-ins/Temporal/PlainMonthDay/prototype/with/order-of-operations.js
+7 −3 test/built-ins/Temporal/PlainTime/prototype/toPlainDateTime/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainTime/prototype/toZonedDateTime/argument-propertybag-calendar-case-insensitive.js
+4 −4 test/built-ins/Temporal/PlainTime/prototype/with/order-of-operations.js
+26 −9 test/built-ins/Temporal/PlainYearMonth/calendar-temporal-object.js
+22 −0 test/built-ins/Temporal/PlainYearMonth/compare/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainYearMonth/from/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainYearMonth/prototype/equals/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/PlainYearMonth/prototype/since/argument-propertybag-calendar-case-insensitive.js
+9 −7 test/built-ins/Temporal/PlainYearMonth/prototype/since/order-of-operations.js
+7 −3 test/built-ins/Temporal/PlainYearMonth/prototype/until/argument-propertybag-calendar-case-insensitive.js
+9 −7 test/built-ins/Temporal/PlainYearMonth/prototype/until/order-of-operations.js
+8 −8 test/built-ins/Temporal/PlainYearMonth/prototype/with/order-of-operations.js
+7 −3 test/built-ins/Temporal/TimeZone/prototype/getInstantFor/argument-propertybag-calendar-case-insensitive.js
+1 −1 test/built-ins/Temporal/TimeZone/prototype/getPlainDateTimeFor/calendar-case-insensitive.js
+28 −10 test/built-ins/Temporal/TimeZone/prototype/getPlainDateTimeFor/calendar-temporal-object.js
+7 −3 ...lt-ins/Temporal/TimeZone/prototype/getPossibleInstantsFor/argument-propertybag-calendar-case-insensitive.js
+26 −9 test/built-ins/Temporal/ZonedDateTime/calendar-temporal-object.js
+25 −0 test/built-ins/Temporal/ZonedDateTime/compare/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/ZonedDateTime/from/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/ZonedDateTime/prototype/equals/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/ZonedDateTime/prototype/since/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/built-ins/Temporal/ZonedDateTime/prototype/until/argument-propertybag-calendar-case-insensitive.js
+0 −1 test/built-ins/Temporal/ZonedDateTime/prototype/with/copies-merge-fields-object.js
+33 −32 test/built-ins/Temporal/ZonedDateTime/prototype/with/order-of-operations.js
+1 −1 test/built-ins/Temporal/ZonedDateTime/prototype/withCalendar/calendar-case-insensitive.js
+27 −8 test/built-ins/Temporal/ZonedDateTime/prototype/withCalendar/calendar-temporal-object.js
+7 −3 .../built-ins/Temporal/ZonedDateTime/prototype/withPlainDate/argument-propertybag-calendar-case-insensitive.js
+3 −1 test/intl402/DateTimeFormat/prototype/format/proleptic-gregorian-calendar.js
+7 −3 test/intl402/Temporal/Calendar/prototype/era/argument-propertybag-calendar-case-insensitive.js
+7 −3 test/intl402/Temporal/Calendar/prototype/eraYear/argument-propertybag-calendar-case-insensitive.js
4 changes: 2 additions & 2 deletions spec/plaindate.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,11 +391,11 @@ <h1>Temporal.PlainDate.prototype.with ( _temporalDateLike_ [ , _options_ ] )</h1
1. If Type(_temporalDateLike_) is not Object, then
1. Throw a *TypeError* exception.
1. Perform ? RejectObjectWithCalendarOrTimeZone(_temporalDateLike_).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _calendar_ be _temporalDate_.[[Calendar]].
1. Let _fieldNames_ be ? CalendarFields(_calendar_, « *"day"*, *"month"*, *"monthCode"*, *"year"* »).
1. Let _partialDate_ be ? PrepareTemporalFields(_temporalDateLike_, _fieldNames_, ~partial~).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _fields_ be ? PrepareTemporalFields(_temporalDate_, _fieldNames_, «»).
1. Let _partialDate_ be ? PrepareTemporalFields(_temporalDateLike_, _fieldNames_, ~partial~).
1. Set _fields_ to ? CalendarMergeFields(_calendar_, _fields_, _partialDate_).
1. Set _fields_ to ? PrepareTemporalFields(_fields_, _fieldNames_, «»).
1. Return ? CalendarDateFromFields(_calendar_, _fields_, _options_).
Expand Down
4 changes: 2 additions & 2 deletions spec/plaindatetime.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,11 +394,11 @@ <h1>Temporal.PlainDateTime.prototype.with ( _temporalDateTimeLike_ [ , _options_
1. If Type(_temporalDateTimeLike_) is not Object, then
1. Throw a *TypeError* exception.
1. Perform ? RejectObjectWithCalendarOrTimeZone(_temporalDateTimeLike_).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _calendar_ be _dateTime_.[[Calendar]].
1. Let _fieldNames_ be ? CalendarFields(_calendar_, « *"day"*, *"hour"*, *"microsecond"*, *"millisecond"*, *"minute"*, *"month"*, *"monthCode"*, *"nanosecond"*, *"second"*, *"year"* »).
1. Let _partialDateTime_ be ? PrepareTemporalFields(_temporalDateTimeLike_, _fieldNames_, ~partial~).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _fields_ be ? PrepareTemporalFields(_dateTime_, _fieldNames_, «»).
1. Let _partialDateTime_ be ? PrepareTemporalFields(_temporalDateTimeLike_, _fieldNames_, ~partial~).
1. Set _fields_ to ? CalendarMergeFields(_calendar_, _fields_, _partialDateTime_).
1. Set _fields_ to ? PrepareTemporalFields(_fields_, _fieldNames_, «»).
1. Let _result_ be ? InterpretTemporalDateTimeFields(_calendar_, _fields_, _options_).
Expand Down
4 changes: 2 additions & 2 deletions spec/plainmonthday.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,11 @@ <h1>Temporal.PlainMonthDay.prototype.with ( _temporalMonthDayLike_ [ , _options_
1. If Type(_temporalMonthDayLike_) is not Object, then
1. Throw a *TypeError* exception.
1. Perform ? RejectObjectWithCalendarOrTimeZone(_temporalMonthDayLike_).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _calendar_ be _monthDay_.[[Calendar]].
1. Let _fieldNames_ be ? CalendarFields(_calendar_, « *"day"*, *"month"*, *"monthCode"*, *"year"* »).
1. Let _partialMonthDay_ be ? PrepareTemporalFields(_temporalMonthDayLike_, _fieldNames_, ~partial~).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _fields_ be ? PrepareTemporalFields(_monthDay_, _fieldNames_, «»).
1. Let _partialMonthDay_ be ? PrepareTemporalFields(_temporalMonthDayLike_, _fieldNames_, ~partial~).
1. Set _fields_ to ? CalendarMergeFields(_calendar_, _fields_, _partialMonthDay_).
1. Set _fields_ to ? PrepareTemporalFields(_fields_, _fieldNames_, «»).
1. Return ? CalendarMonthDayFromFields(_calendar_, _fields_, _options_).
Expand Down
2 changes: 1 addition & 1 deletion spec/plaintime.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,9 @@ <h1>Temporal.PlainTime.prototype.with ( _temporalTimeLike_ [ , _options_ ] )</h1
1. If Type(_temporalTimeLike_) is not Object, then
1. Throw a *TypeError* exception.
1. Perform ? RejectObjectWithCalendarOrTimeZone(_temporalTimeLike_).
1. Let _partialTime_ be ? ToTemporalTimeRecord(_temporalTimeLike_, ~partial~).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _overflow_ be ? ToTemporalOverflow(_options_).
1. Let _partialTime_ be ? ToTemporalTimeRecord(_temporalTimeLike_, ~partial~).
Comment on lines -236 to +238
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's debatable whether we'd want to make this change to PlainTime, which doesn't use PrepareTemporalFields, but it seems most consistent with the form suggested by Richard in #2462 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍
But if this is awkward and options never affects the interpretation of the first argument, then it would also be coherent to move every Set _options_ to ? GetOptionsObject(_options_). to follow full consumption (with constituent validation) of the first argument by PrepareTemporalFields/ToTemporalTimeRecord/etc.—although that is not my preference because it's not clear to me in that case whether GetOptionsObject should precede or follow integration of receiver and argument data in e.g. CalendarMergeFields.

1. If _partialTime_.[[Hour]] is not *undefined*, then
1. Let _hour_ be _partialTime_.[[Hour]].
1. Else,
Expand Down
10 changes: 5 additions & 5 deletions spec/plainyearmonth.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,11 @@ <h1>Temporal.PlainYearMonth.prototype.with ( _temporalYearMonthLike_ [ , _option
1. If Type(_temporalYearMonthLike_) is not Object, then
1. Throw a *TypeError* exception.
1. Perform ? RejectObjectWithCalendarOrTimeZone(_temporalYearMonthLike_).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _calendar_ be _yearMonth_.[[Calendar]].
1. Let _fieldNames_ be ? CalendarFields(_calendar_, « *"month"*, *"monthCode"*, *"year"* »).
1. Let _partialYearMonth_ be ? PrepareTemporalFields(_temporalYearMonthLike_, _fieldNames_, ~partial~).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _fields_ be ? PrepareTemporalFields(_yearMonth_, _fieldNames_, «»).
1. Let _partialYearMonth_ be ? PrepareTemporalFields(_temporalYearMonthLike_, _fieldNames_, ~partial~).
1. Set _fields_ to ? CalendarMergeFields(_calendar_, _fields_, _partialYearMonth_).
1. Set _fields_ to ? PrepareTemporalFields(_fields_, _fieldNames_, «»).
1. Return ? CalendarYearMonthFromFields(_calendar_, _fields_, _options_).
Expand Down Expand Up @@ -611,12 +611,12 @@ <h1>
1. If ? CalendarEquals(_calendar_, _other_.[[Calendar]]) is *false*, throw a *RangeError* exception.
1. Let _settings_ be ? GetDifferenceSettings(_operation_, _options_, ~date~, &laquo; *"week"*, *"day"* &raquo;, *"month"*, *"year"*).
1. Let _fieldNames_ be ? CalendarFields(_calendar_, « *"monthCode"*, *"year"* »).
1. Let _otherFields_ be ? PrepareTemporalFields(_other_, _fieldNames_, «»).
1. Perform ! CreateDataPropertyOrThrow(_otherFields_, *"day"*, *1*<sub>𝔽</sub>).
1. Let _otherDate_ be ? CalendarDateFromFields(_calendar_, _otherFields_).
1. Let _thisFields_ be ? PrepareTemporalFields(_yearMonth_, _fieldNames_, «»).
1. Perform ! CreateDataPropertyOrThrow(_thisFields_, *"day"*, *1*<sub>𝔽</sub>).
1. Let _thisDate_ be ? CalendarDateFromFields(_calendar_, _thisFields_).
1. Let _otherFields_ be ? PrepareTemporalFields(_other_, _fieldNames_, «»).
1. Perform ! CreateDataPropertyOrThrow(_otherFields_, *"day"*, *1*<sub>𝔽</sub>).
1. Let _otherDate_ be ? CalendarDateFromFields(_calendar_, _otherFields_).
1. Let _untilOptions_ be OrdinaryObjectCreate(*null*).
1. Perform ? CopyDataProperties(_untilOptions_, _settings_.[[Options]], « »).
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is curious that this step can throw; is that a bug in failure to properly consume options earlier on?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think it's a bug - we have to perform CopyDataProperties on options at some point, so that we can pass an options object to CalendarDateUntil with a modified largestUnit property. options is a user-provided object so it may have accessor properties.

Copy link
Collaborator

@gibson042 gibson042 Jan 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Right, I'm saying that the bug is consuming it so deep in the spec algorithms rather than in initial processing—and in particular in performing CopyDataProperties after GetDifferenceSettings has read several properties on its own.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could see this going either way - it'd be good to eliminate the duplicate reads, but on the other hand we'd introduce an inconsistency where we'd copy the options object in the since/until methods of PlainDate, PlainDateTime, PlainYearMonth, and ZonedDateTime; and not copy it in the since/until methods of PlainTime and Instant.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't options be copied in all since/until methods?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only the ones where a modified options object needs to be passed on to calendar.dateUntil(). Otherwise, we just read only the needed properties in alphabetical order.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, either that serves as explanation of the inconsistency or we'd copy options in all since/until methods even though some technically don't need it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you have a concrete proposal for this? If we can manage to write that additional change before TC39's review deadline, I wouldn't be opposed to including it here. Otherwise I think it can be done as part of #2289 or left as is.

Copy link
Collaborator

@gibson042 gibson042 Jan 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Concretely, I'm proposing 1. Perform ? CopyDataProperties(_optionsCopy_, _options_, « »). in every DifferenceTemporal<Type> operation immediately after validating other and use of optionsCopy rather than options in every subsequent step. But I'm also comfortable with a compromise in which that change is omitted in DifferenceTemporalInstant and DifferenceTemporalPlainTime, ideally replaced with a NOTE step explaining its omission.

I agree that it is logically part of the fixes for #2289, but will still try to put up a narrow PR since it has been specifically identified (and in fact already has a comment: #2289 (comment) ).

1. Perform ! CreateDataPropertyOrThrow(_untilOptions_, *"largestUnit"*, _settings_.[[LargestUnit]]).
Expand Down
7 changes: 3 additions & 4 deletions spec/zoneddatetime.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,15 +581,14 @@ <h1>Temporal.ZonedDateTime.prototype.with ( _temporalZonedDateTimeLike_ [ , _opt
1. If Type(_temporalZonedDateTimeLike_) is not Object, then
1. Throw a *TypeError* exception.
1. Perform ? RejectObjectWithCalendarOrTimeZone(_temporalZonedDateTimeLike_).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _calendar_ be _zonedDateTime_.[[Calendar]].
1. Let _fieldNames_ be ? CalendarFields(_calendar_, « *"day"*, *"hour"*, *"microsecond"*, *"millisecond"*, *"minute"*, *"month"*, *"monthCode"*, *"nanosecond"*, *"second"*, *"year"* »).
1. Append *"offset"* to _fieldNames_.
1. Let _fields_ be ? PrepareTemporalFields(_zonedDateTime_, _fieldNames_, « *"offset"* »).
1. Let _partialZonedDateTime_ be ? PrepareTemporalFields(_temporalZonedDateTimeLike_, _fieldNames_, ~partial~).
1. Append *"timeZone"* to _fieldNames_.
1. Let _fields_ be ? PrepareTemporalFields(_zonedDateTime_, _fieldNames_, « *"timeZone"*, *"offset"* »).
1. Set _fields_ to ? CalendarMergeFields(_calendar_, _fields_, _partialZonedDateTime_).
1. Set _fields_ to ? PrepareTemporalFields(_fields_, _fieldNames_, « *"timeZone"*, *"offset"* »).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Set _fields_ to ? PrepareTemporalFields(_fields_, _fieldNames_, « *"offset"* »).
1. NOTE: The following steps read options and perform independent validation in alphabetical order (ToTemporalDisambiguation reads *"disambiguation"*, ToTemporalOffset reads *"offset"*, and InterpretTemporalDateTimeFields reads *"overflow"*).
1. Let _disambiguation_ be ? ToTemporalDisambiguation(_options_).
1. Let _offset_ be ? ToTemporalOffset(_options_, *"prefer"*).
Expand Down