Spec issue in NonISODateSurpasses

[ptomato]

#69 may have introduced an inadvertent observable change, which needs to be reverted.

The replaced prose read:

Add (without constraining) as many years as possible to one, in the direction from one to two, without surpassing two. "Surpassing" here (and in all steps below) means to compare years numerically, then month codes lexicographically, then days numerically; if any of them exceed two in the direction from one to two, then two is surpassed.

Taking the Hebrew calendar dates one = 1 Adar I 5784 (5784-M05L-1, ISO 2024-02-10) and two = 1 Adar 5783 (5783-M06-1, ISO 2023-02-22) the current NonISODateSurpasses algorithm returns false for a candidateYears of -1. However the above prose would seem to suggest it should return true.

h/t @Manishearth

[Manishearth]

cc @anba as well

[sffc]

Here are tests I had written for this:

https://github.com/unicode-org/icu4x/blob/main/components/calendar/tests/arithmetic.rs

I don't have many negative duration tests. It's a TODO item that got punted:

unicode-org/icu4x#7078

I don't think I agree that the old behavior is better. IIRC, the spec I wrote in #69 constrains months according to calendar specific constraining rules, and the surpasses function operates on ordinal months after month codes are resolved.

(This is another example of why we need spec algorithms and not prose. If spec authors can't tell if an algorithm matches the prose, how do we expect implementers to be able to do it?)

[ptomato]

I could just as well say that the prose was clear about the intention while the spec algorithm is opaque. Otherwise we would have spotted this in the code review 😄

The old behaviour seems like it's necessary to support the conclusion we reached in tc39/proposal-temporal#2535. See in particular tc39/proposal-temporal#2535 (comment).

Also see the discussion https://github.com/tc39/proposal-temporal/blob/main/meetings/agenda-minutes-2024-01-25.md#compatibility-with-java-date-arithmetic-2535 where we talked specifically about comparing the month codes lexicographically.

[sffc]

Let's consider a few Hebrew cases:

One \ Two	1 Shevat 5783	1 Adar 5783	1 Nisan 5783
1 Shevat 5784	-P1Y	-P11M	-P10M
1 Adar I 5784	-P1Y1M	Prose: -P12M Spec Alg: -P1Y	-P11M
1 Adar II 5784	-P1Y1M	-P1Y	-P12M
1 Nisan 5784	-P1Y2M	-P1Y1M	-P1Y

And some Chinese cases:

One \ Two	2022-M01-1	2022-M02-1	2022-M03-1
2023-M01-1	-P1Y	-P11M	-P10M
2023-M02-1	-P1Y1M	-P1Y	-P11M
2023-M02L-1	-P1Y1M	-P1Y	-P12M
2023-M03-1	-P1Y2M	-P1Y1M	-P1Y

Note the difference between Hebrew and Chinese behavior, which arises from the leap month being ordered before the normal month in Hebrew but after the normal month in Chinese.

---

From my research (#70), 1 year before Adar I is unambiguously Adar.

---

I should have either been more proactive in highlighting that #69 implemented a change in Hebrew calendar arithmetic or pulled it into a separate pull request. Thinking back four months, the methodology I used in writing #69 was referencing the algorithm in Temporal, rather than the prose in Intl Era Month Code.

[anba]

This issue also shows up in the tests added in tc39/test262#4652 and tc39/test262#4653, which require behaviour which contradicts the current NonISODateSurpasses algorithm from this proposal.