swiftlang / swift-foundation

The Foundation project
Apache License 2.0
2.39k stars 157 forks source link

`Calendar.date(from:)` incorrectly sets `UCAL_DAY_OF_MONTH` #532

Open davedelong opened 6 months ago

davedelong commented 6 months ago

I was trying to iterate through all the weeks in a month, like so:

for w in 1 ... 5 {
    let components = DateComponents(year: 2024, month: 1, weekOfMonth: w)
    let date = Calendar.current.date(from: components)!
    print(date.debugDescription)
}

However, this printed some unexpected results:

2024-01-01 07:00:00 +0000
2024-01-01 07:00:00 +0000
2024-01-01 07:00:00 +0000
2024-01-01 07:00:00 +0000
2024-01-01 07:00:00 +0000

After some digging into the implementation, I discovered that the date(from:) method is incorrectly setting the UCAL_DAY_OF_MONTH field on the cleared UCalendar, which is messing up the calculation.

I verified this by testing the logic myself:

UErrorCode error = U_ZERO_ERROR;
UCalendar *ucalendar = ucal_open(NULL, 0, "en_US_POSIX", UCAL_GREGORIAN, &error);

for (int32_t week = 1; week <= 5; week++) {
    ucal_clear(ucalendar);

    ucal_set(ucalendar, UCAL_ERA, 1);
    ucal_set(ucalendar, UCAL_IS_LEAP_MONTH, 0);
    ucal_set(ucalendar, UCAL_YEAR, 2024);
    ucal_set(ucalendar, UCAL_MONTH, 0); // 0 = January
    ucal_set(ucalendar, UCAL_WEEK_OF_MONTH, week);
//    ucal_set(ucalendar, UCAL_DAY_OF_MONTH, 1);

    ucal_set(ucalendar, UCAL_HOUR_OF_DAY, 0);
    ucal_set(ucalendar, UCAL_MINUTE, 0);
    ucal_set(ucalendar, UCAL_SECOND, 0);
    ucal_set(ucalendar, UCAL_MILLISECOND, 0);

    UDate udate = ucal_getMillis(ucalendar, &error);

    NSDate *d = [NSDate dateWithTimeIntervalSince1970:udate / 1000];
    printf("%s\n", d.debugDescription);
}
ucal_close(ucalendar);

When the UCAL_DAY_OF_MONTH field is set, this prints:

2024-01-01 07:00:00 +0000
2024-01-01 07:00:00 +0000
2024-01-01 07:00:00 +0000
2024-01-01 07:00:00 +0000
2024-01-01 07:00:00 +0000

When the line is commented out, it prints:

2023-12-31 07:00:00 +0000
2024-01-07 07:00:00 +0000
2024-01-14 07:00:00 +0000
2024-01-21 07:00:00 +0000
2024-01-28 07:00:00 +0000

Setting the UCAL_DAY_OF_MONTH field on the UCalendar causes operations around finding the weekOfMonth and weekOfYear to fail and return the first week of the month/year, instead of the desired week.

davedelong commented 6 months ago

In terms of fixing this, I would suggest looking at the incoming DateComponents and determining its overall granularity: nothing on my DateComponents is more granular than a week, so it strikes me as unusual that the default behavior is to fully set the fields on the UCalendar all the way down to the milliseconds.

Shouldn't the code only be setting the UCalendar fields that correspond to the specified components in the DateComponents?

davedelong commented 6 months ago

Indeed, this works as expected:

for (int32_t week = 1; week <= 5; week++) {
    ucal_clear(ucalendar);

    ucal_set(ucalendar, UCAL_YEAR, 2024);
    ucal_set(ucalendar, UCAL_MONTH, 0);
    ucal_set(ucalendar, UCAL_WEEK_OF_MONTH, week);

    UDate udate = ucal_getMillis(ucalendar, &error);
    NSDate *d = [NSDate dateWithTimeIntervalSince1970:udate / 1000];
    printf("%s\n", d.debugDescription.UTF8String);
}

Foundation already conditionally resets fields on the UCalendar based on what's in the incoming DateComponents… why does it also hard-code certain values?