Update date exception thrown

[VincentLanglet]

Cf https://wiki.php.net/rfc/datetime-exceptions

[VincentLanglet]

I don't think failure are related @isfedorov

[isfedorov]

@VincentLanglet Yes, the failure itself isn't related. Docker image just couldn't build. But still it's not possible to merge the changes.

Could you please rebase your fork and force push changes after that?

[VincentLanglet]

@VincentLanglet Yes, the failure itself isn't related. Docker image just couldn't build. But still it's not possible to merge the changes. Could you please rebase your fork and force push changes after that?

Is it better now ?

I'm getting "Everything is up to date", and if I look at my branch https://github.com/VincentLanglet/phpstorm-stubs/tree/date-exception, there is only one extra commit over master

[isfedorov]

@VincentLanglet Still the same problem. Don't know what can it be, but nevermind, I'll merge it manually, but before that, could you please clarify why you have changed exception to DateException but not DateInvalidTimeZoneException? According to RFC Errors with timezone creation cause an DateInvalidTimeZoneException.

[VincentLanglet]

@VincentLanglet Still the same problem. Don't know what can it be, but nevermind, I'll merge it manually, but before that, could you please clarify why you have changed exception to DateException but not DateInvalidTimeZoneException? According to RFC Errors with timezone creation cause an DateInvalidTimeZoneException.

There are 4 new exceptions which extends DateException:

• DateInvalidTimeZoneException
• DateInvalidOperationException
• DateMalformedStringException
• DateMalformedIntervalStringException

Since I wasn't sure about when they are thrown I wanted to update the stubs in a way I was sure it was correct.

But I re-read the RFC, looked at the implementation and tried on PHP and I would say (cf php/php-src@66a1a91)

• new DateTime and new DateTimeImmutable throws DateMalformedStringException
• new DatetimeZone throws DateInvalidTimeZoneException cf php/php-src@66a1a91#diff-e0064ae8a477c5622338d22a1599df0c688f74739431e400e8803204a19f56dc
• new DateInterval throws DateMalformedIntervalStringException cf php/php-src@66a1a91#diff-759584d0a0a270097725739bd87afc77b229acdce0d747ac8eb96208303f2187
• sub throws DateInvalidOperationException cf php/php-src@66a1a91#diff-26bed3d151aedeff85a88c967bb4a0908c950fd0e5abc62fb6efbdfeb3fc62d9

I updated the PR.

I discovered there is also some other behavior changes and I didn't know how to update the stubs correctly in order to not give a wrong stub for PHP < 8.3 users.

• modify doesn't return false in 8.3 and throws DateMalformedStringException cf php/php-src@66a1a91#diff-c214f4152851ea363816ea9fd989ada2688d4b44a440964765e7285d789f65df
• DateInterval::createFromDateString doesn't return false in 8.3 and throws DateMalformedIntervalStringException

I tried LanguageLevelTypeAware in 0df0010, is it the right way @isfedorov ?

[VincentLanglet]

Sorry for the delay @isfedorov ; I fixed the build

[isfedorov]

@VincentLanglet Thank you! The approach with LanguageLevelTypeAware looks correct in this case but I'd remove return types from method signatures in this case, since return types are already declared by attributes. Could you please update signatures?

[VincentLanglet]

@VincentLanglet Thank you! The approach with LanguageLevelTypeAware looks correct in this case but I'd remove return types from method signatures in this case, since return types are already declared by attributes. Could you please update signatures?

As shown by the CI https://github.com/JetBrains/phpstorm-stubs/actions/runs/9487379401/job/26143867750?pr=1646 @isfedorov I need to keep the method signature to have a green CI. (The previous commit 3a01281 had green tests)

[isfedorov]

@VincentLanglet Actually the failure says that return type of methods modify should contain false in PHP 8.3. And we can check that it's true using simple example https://3v4l.org/IZXbp

[VincentLanglet]

@VincentLanglet Actually the failure says that return type of methods modify should contain false in PHP 8.3. And we can check that it's true using simple example 3v4l.org/IZXbp

I see, I'll try to understand if false is still a possible return type.
I'll send a mail to the PHP internals, to see if it need to be fixed on PHP side or if FALSE is still possible.

[VincentLanglet]

@VincentLanglet Actually the failure says that return type of methods modify should contain false in PHP 8.3. And we can check that it's true using simple example 3v4l.org/IZXbp

The issue is on the PHP side.
Reported and validated here php/php-src#10366 (comment)
The PR is in progress php/php-src#14600

I assume we have to wait for the merge and a new PHP 8.3 release in order to have green tests ?

[VincentLanglet]

@isfedorov I got an anwser.

The return type was buggy in PHP 8.3, but it's fixed and the fix will be landed in PHP 8.4.
See php/php-src#14600 (comment)

I update the LanguageLevelTypeAware attribute then

[VincentLanglet]

Hi @isfedorov the PR should be ready.

The failing build is unrelated, I opened another PR to fix it #1650

[isfedorov]

@VincentLanglet Thank you for your contribution and for the fix of cs-fixer issue!

[ondrejmirtes]

This PR is wrong and I can't update phpstorm-stubs in PHPStan because of it.

@throws DateMalformedStringException is correct only on PHP 8.3+. On older versions @throws Exception would still be correct.

[VincentLanglet]

Hi
I did a similar Pr one year ago and no issue was reported #1554

Since DateMalformedException stub exists in Phpstorm, I'm not sure the PR is creating an issue for Phpstorm users (and it improve php 8.3+ experience).

There will always be a need to decide between new and older php versions. I'm not sure a LanguageAware attribut exists for the exception thrown. Could an adjustment be done on phpstan side ? I'll try to take a look after my day off if needed.

[ondrejmirtes]

I'm going to fix the problems on PHPStan side.

Also: You've forgot to add the exception above DateTimeImmutable::modify().

[VincentLanglet]

I'm going to fix the problems on PHPStan side.

Also: You've forgot to add the exception above DateTimeImmutable::modify().

Thanks, I created #1652 then ; does it require another adjustment on PHPStan side @ondrejmirtes ?

[ondrejmirtes]

No, thanks