test262 icon indicating copy to clipboard operation
test262 copied to clipboard

option of Calendar.dateAdd is not undefined but null

Open FrankYFTang opened this issue 2 years ago • 6 comments

This test is wrong I think Temporal/Duration/prototype/add/calendar-dateadd-called-with-options-undefined.js

const calendar = TemporalHelpers.calendarDateAddUndefinedOptions();
const timeZone = TemporalHelpers.oneShiftTimeZone(new Temporal.Instant(0n), 3600e9);
const instance = new Temporal.Duration(1, 1, 1, 1);
instance.add(instance, { relativeTo: new Temporal.ZonedDateTime(0n, timeZone, calendar) });

The problem is in step6 of 7.3.18 Temporal.Duration.prototype.add ( other [ , options ] ) https://tc39.es/proposal-temporal/#sec-temporal-addzoneddatetime

6. Let result be ? AddDuration(duration.[[Years]], duration.[[Months]], duration.[[Weeks]], duration.[[Days]], duration.[[Hours]], duration.[[Minutes]], duration.[[Seconds]], duration.[[Milliseconds]], duration.[[Microseconds]], duration.[[Nanoseconds]], other.[[Years]], other.[[Months]], other.[[Weeks]], other.[[Days]], other.[[Hours]], other.[[Minutes]], other.[[Seconds]], other.[[Milliseconds]], other.[[Microseconds]], other.[[Nanoseconds]], relativeTo).

it will call AddDuration. then AddDuration will call AddZonedDateTime in step 7d https://tc39.es/proposal-temporal/#sec-temporal-addduration

d. Let intermediateNs be ? AddZonedDateTime(relativeTo.[[Nanoseconds]], timeZone, calendar, y1, mon1, w1, d1, h1, min1, s1, ms1, mus1, ns1).

and then in AddZonedDateTime https://tc39.es/proposal-temporal/#sec-temporal-addzoneddatetime

1. If options is not present, set options to ! OrdinaryObjectCreate(null).

options will be a null object (not undefined) then

7. Let addedDate be ? CalendarDateAdd(calendar, datePart, dateDuration, options).

so what the Calendar.dateAdd will get in this case is a empty object, not undefined.

FrankYFTang avatar Oct 09 '21 00:10 FrankYFTang

@ptomato @Ms2ger @jugglinmike @ryzokuken

FrankYFTang avatar Oct 09 '21 00:10 FrankYFTang

I think the following tests has the same issue

'built-ins/Temporal/Duration/prototype/add/calendar-dateadd-called-with-options-undefined': [FAIL], 'built-ins/Temporal/Duration/prototype/subtract/calendar-dateadd-called-with-options-undefined': [FAIL], 'built-ins/Temporal/PlainDate/prototype/toZonedDateTime/calendar-dateadd-called-with-options-undefined': [SKIP], 'built-ins/Temporal/PlainDateTime/prototype/toZonedDateTime/calendar-dateadd-called-with-options-undefined': [SKIP], 'built-ins/Temporal/PlainTime/prototype/toZonedDateTime/calendar-dateadd-called-with-options-undefined': [FAIL], 'built-ins/Temporal/TimeZone/prototype/getInstantFor/calendar-dateadd-called-with-options-undefined': [SKIP], 'built-ins/Temporal/ZonedDateTime/prototype/round/calendar-dateadd-called-with-options-undefined': [SKIP], 'built-ins/Temporal/ZonedDateTime/prototype/startOfDay/calendar-dateadd-called-with-options-undefined': [SKIP], 'built-ins/Temporal/ZonedDateTime/prototype/withPlainDate/calendar-dateadd-called-with-options-undefined': [SKIP], 'built-ins/Temporal/ZonedDateTime/prototype/withPlainTime/calendar-dateadd-called-with-options-undefined': [SKIP],

FrankYFTang avatar Oct 09 '21 01:10 FrankYFTang

Ref https://github.com/tc39/proposal-temporal/issues/1685

Ms2ger avatar Oct 11 '21 15:10 Ms2ger

Yes, this was a discrepancy with the spec text. I'm hoping to present that normative change at the December TC39 meeting. I guess it's up to the maintainers whether to remove the tests affected by it in the meantime.

ptomato avatar Oct 15 '21 17:10 ptomato

Yes, this was a discrepancy with the spec text. I'm hoping to present that normative change at the December TC39 meeting. I guess it's up to the maintainers whether to remove the tests affected by it in the meantime.

I suggest we wait till the conclusion from Decemeber TC39 before changing anything.

FrankYFTang avatar Oct 15 '21 19:10 FrankYFTang

I didn't get a chance to finish this for December and I'm hoping to present it at the March TC39 meeting.

ptomato avatar Jan 20 '22 21:01 ptomato