AddDurationToOrSubtractDurationFromPlainYearMonth: Move GetOptionsObject closer to the top

[anba]

AddDurationToOrSubtractDurationFromPlainYearMonth should probably call GetOptionsObject earlier to validate the user-supplied input before performing other user-visible operations. Maybe move it directly before the BalanceTimeDuration call in step 3?

[ptomato]

This ordering was intentional — the design principle followed here is, perform validation on each argument in order (temporalDurationLike, then options). So, personally I'd prefer not changing anything here, especially as we are trying to keep changes as rare as possible this late in stage 3, but we can discuss it in the next champions meeting.

[anba]

I wouldn't count performing CalendarFields, PrepareTemporalFields, CalendarDateFromFields, etc. as part of the arguments validation of temporalDurationLike, though. And all these steps happen before GetOptionsObject is called.

[ptomato]

We discussed this in the Temporal champions meeting today (2023-11-09). Whether this adheres to the design guideline of processing each argument in order, is debatable. (We are a bit vague in the design guideline about what exactly is included in "argument processing".) But we don't see a compelling motivation to change it this late in stage 3 just for consistency unless there were a demonstration of how this causes a real problem, or how it is gratuitously different from the rest of Temporal.

(To the best of our knowledge, the only difference for end users is in the exception that might get thrown if one of the steps fails.)

[anba]

I don't think "stage 3" is a valid argument for dismissing this issue, because the relevant algorithm was changed only recently (573de94). Before that commit, GetOptionsObject was called directly after BalanceTimeDuration. Looking through the commit history:

• AddDurationToOrSubtractDurationFromPlainYearMonth (née AddTemporalPlainYearMonth) was first added in Editorial: Refactor add/subtract to common AOs #2166. GetOptionsObject was called early in this commit.
• Editorial: Simplify object snapshotting  #2586 added a second call to GetOptionsObject.
• Editorial: Various fixes #2670 removed the second call to GetOptionsObject. This makes Editorial: Various fixes #2670 actually a normative change!

Because #2670 is actually a normative change, but was incorrectly marked as editorial, I think it's okay to add a another editorial change to revert the AddDurationToOrSubtractDurationFromPlainYearMonth changes from #2670 and instead remove the second call to GetOptionsObject. Then everything can still be marked as editorial and no normative changes are necessary.

[ptomato]

Thanks, that is a compelling reason to me. If a normative change was incorrectly landed as editorial, then we should revert it.

[ptomato]

Note, to revert to the previous observable behaviour, GetOptionsObject would end up as step 4, directly after BalanceTimeDuration, not directly before it.

[ptomato]

(Which is fine with me, personally. I think BalanceTimeDuration is arguably validation of the duration argument to add/subtract, which comes before options.)

[anba]

This is also fine with me, because BalanceTimeDuration can't trigger any user-controlled code, so the only difference is which error is thrown (RangeError from BalanceTimeDuration or TypeError from GetOptionsObject). But it seems with #2727 applied, BalanceTimeDuration can no longer thrown an error, so it doesn't actually matter if GetOptionsObject is called before or after an infallible BalanceTimeDuration.