Carbon: sub(x months) from first day of year - one more day outta nowhere

[jurchiks]

I just stumbled upon a weird bug when subtracting a month from the first day of any year.

The code that showcases this:

$firstDayOfYear = CarbonImmutable::parse('2021-01-01 00:00:00.000000'); // Works for any year.

echo $firstDayOfYear
    ->subMonth() // Can be any number of months.
    ->diffForHumans($firstDayOfYear, null/*default syntax*/, false/*short format off*/, 7/*max parts*/);

I expected to get:

1 month before

But I actually get:

1 month 1 day before

Carbon version: master branch, forked just prior to making this issue (0d87f3d34cd9464e6807df1e41aa5a8ce87adee3)
PHP version: 7.4.12

[kylekatarnls]

Hello,

When I try the code your provide in https://try-carbon.herokuapp.com/ I get 1 month before

Can you reproduce it in the live demo? Or try to find particular settings you may have on your date setup.

Thanks.

[jurchiks]

That is very weird, because I have no custom date setup.
Tried changing the locale but that does nothing, no idea what else to look for.

[kylekatarnls]

I can reproduce if use UTC as default timezone and create a date with GMT > 0 such as 'Europe/Paris', I'm not sure if it's the expected behavior.

[kylekatarnls]

Reproduced with pure PHP:

$firstDayOfYear = new DateTimeImmutable('2021-01-01 00:00:00.000000', new DateTimeZone('Europe/Paris'));

echo $firstDayOfYear
    ->modify('-1 month')
    ->diff($firstDayOfYear)
    ->format('%m month %d day');

1 month 1 day

There is no DST in Paris in December so I can't tell why PHP behaves like this. But in such case we rely on the inherited native PHP DateTime(Immutable) class. And when we try to work-around the native behavior we often get more troubles somewhere else.

Sure thing is: you always get less unexpected result sticking to UTC timezone. Try to do a maximum of the calculation with UTC and only use user timezone at the very last moment to format for the user or parsing user input.

[jurchiks]

My php.ini date.timezone = "Europe/Riga" and I am not passing in a timezone to Carbon. I was hoping timezones would have nothing to do with this...

I am not doing this in any project, just noticed this weirdness while doing a PR.

[kylekatarnls]

There is few no advantages in having a City timezone as the default setting, and a lot of advantages in using UTC instead. You can see my detailed recommendations on this here:
https://medium.com/@kylekatarnls/always-use-utc-dates-and-times-8a8200ca3164

[jurchiks]

Ok, I reproduced this in the live demo with the following code:

$firstDayOfYear = CarbonImmutable::parse('2021-01-01 00:00:00.000000');
$intervalOneEach = CarbonInterval::create(1, 1, 1, 1, 1, 1, 1);

echo $firstDayOfYear->sub($intervalOneEach)->diffForHumans($firstDayOfYear, null, false, 7);

[kylekatarnls]

Again this matches the PHP native behavior:

$firstDayOfYear = new DateTimeImmutable('2021-01-01 00:00:00.000000');
$intervalOneEach = new DateInterval('P1Y1M1DT1H1M1S');

echo $firstDayOfYear->sub($intervalOneEach)->diff($firstDayOfYear)->format('%Y years %M months %d days %h hours %m minutes % seconds');

... 2 days ...

I can't say why it's so, likely a bug, but it's on the PHP side, so to be reported on https://bugs.php.net/ as there is pretty not much I could do from the Carbon side.

[jurchiks]

Welp, that's a RIP then. PHP bugs take forever to fix :D

[jurchiks]

https://bugs.php.net/bug.php?id=80696

[jurchiks]

And now they replied saying it's not a bug :D

[kylekatarnls]

Explanation sounds legit to me, you can't get 1 month = 30 and 31 days in the same time, and each time you choose, you make someone unhappy. It would be worse to make interval ordered and say 1M1D is different from 1D1M, and so either your sub or add would produce different order? That would be even more random this way. Then if not on a start/end of month, results will be un-understandable.

Remember it's not our, or PHP or even timelib fault. Month duration inconsistency is the original bug.

[jurchiks]

You can't custom-order the interval though, it has a fixed order.

[kylekatarnls]

That's the point. Reverse operation of add 1 month then add 1 day, is subtract 1 day then subtract 1 month as requinix explained on the PHP ticket, subtract 1 month then subtract 1 day produces a different result than the operation, so when diff() output 1 month + 1 day it's an interval you must proceed in month-then-day order using addition from earlier date to get the latest date as a result and it can only work in this order, it can't match in the same time the addition and the subtraction operation and date interval does not "remember" how you created it, it's start/end-agnostic.

[kylekatarnls]

See:

echo Carbon::parse('2021-01-01')->subMonth()->subDay()->addDay()->addMonth()->format('Y-m-d') . "\n"; // 2021-01-01
echo Carbon::parse('2021-01-01')->subMonth()->subDay()->addMonth()->addDays(2)->format('Y-m-d') . "\n"; // 2021-01-01

Second case is the reason why comparing 2020-11-30 and 2021-01-01 gives you 1 month and 2 days.

[jurchiks]

I meant that internally they could have changed it so that adding an interval with months + days would work in the opposite order as subtracting the same interval. But, as always, people will complain, can't argue with that.

[kylekatarnls]

so that adding an interval with months + days would work in the opposite order as subtracting the same interval

In that case you would get A - B != - (B - A) And I guess the priority was to get the same components of the DateInterval (only a different sign) when comparing 2 dates, no matter you $a->diff($b) or $b->diff($a) and this sounds very reasonable to me.

[jurchiks]

I'm not saying that anyone's wrong, what I am saying is that this is a fukken mess.
You obviously wouldn't expect the outcome that I got with my code.