On Thu, 30 Mar 2023 21:33:16 GMT, Justin Lu <j...@openjdk.org> wrote:
>> This PR fixes the bug which occurred when `Calendar.roll(WEEK_OF_YEAR)`
>> rolled into a minimal first week with an invalid `WEEK_OF_YEAR` and
>> `DAY_OF_WEEK` combo.
>>
>> For example, Rolling _Monday, 30 December 2019_ by 1 week produced _Monday,
>> 31 December 2018_, which is incorrect. This is because `WEEK_OF_YEAR` is
>> rolled from 52 to 1, and the original `DAY_OF_WEEK` is 1. However, there is
>> no Monday in week 1 of 2019. This is exposed when a future method calls
>> `Calendar.complete()`, which eventually calculates a `fixedDate` with the
>> invalid `WEEK_OF_YEAR` and `DAY_OF_WEEK` combo.
>>
>> To prevent this, a check is added for rolls into week 1, which determines if
>> the first week is minimal. If it is indeed minimal, then it is checked if
>> `DAY_OF_WEEK` exists in that week, if not, `WEEK_OF_YEAR` must be
>> incremented by one.
>>
>> After the fix, Rolling _Monday, 30 December 2019_ by 1 week produces
>> _Monday, 7 January 2019_
>
> Justin Lu has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - dayInMinWeek should range check input and throw exception
> - Tweak static cal builder
src/java.base/share/classes/java/util/GregorianCalendar.java line 3025:
> 3023: if (endDay > 7 || endDay < 1 || startDay > 7 || startDay < 1) {
> 3024: throw new IllegalArgumentException("Start day or end day is
> not " +
> 3025: "a valid day of the week");
Sorry, I take my previous comment back. I think we can simply rely on the
return value from `getFirstDayOfWeek()` as it is well-checked, so no need to
check the input here. Otherwise the thrown IAE woudl have to end up in
`add()/roll()` public methods, which cannot be spec'ed with these internal
arguments.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13031#discussion_r1153877382
