<i18n dev> RFR: 8225641: Calendar.roll(int field) does not work correctly for WEEK_OF_YEAR [v2]

[Justin Lu]

On Wed, 15 Mar 2023 19:47:24 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> Justin Lu has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Test all possible min week and first day of week combos for bugged date
>>  - Add safety check that amount is positive
>
> src/java.base/share/classes/java/util/GregorianCalendar.java line 1314:
> 
>> 1312:                     // current DAY_OF_WEEK
>> 1313:                     if (newWeekOfYear == 1 && isInvalidWeek1()) {
>> 1314:                         newWeekOfYear+=1;
> 
> is `+1` always correct? Does it work when the amount is negative?

When a week is rolled down to 1 (and it does not have a valid day_of_week), the existing code would appropriately adjust min, so that the new week set would be 52. However, yes, it should only be incremented if amount is positive, but there does not need to be a case for when amount is negative.

> test/jdk/java/util/Calendar/RollToMinWeek.java line 95:
> 
>> 93: 
>> 94:     private static Calendar buildCalendar(int day, int month, int year, Locale locale) {
>> 95:         Calendar calendar = Calendar.getInstance(locale);
> 
> If the fix is `GregorianCalendar` specific, should it check the type?

Yes, I have changed it so that it is intentionally set as Gregorian, and also tested for being Gregorian.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13031#discussion_r1147922575
PR Review Comment: https://git.openjdk.org/jdk/pull/13031#discussion_r1147924484