Report invalid DateInterval constructor arguments at analysis time#5587
Merged
staabm merged 7 commits intophpstan:2.1.xfrom May 5, 2026
Merged
Conversation
staabm
reviewed
May 3, 2026
VincentLanglet
changed the title
DateInterval and DateTimeZone constructor arguments at analysis timeDateInterval constructor arguments at analysis time
staabm
reviewed
May 3, 2026
staabm
force-pushed
the
create-pull-request/patch-v8z1qut
branch
from
070b036 to
c0929f9
Compare
staabm
reviewed
May 3, 2026
| /** | ||
| * @implements Rule<Node\Expr\New_> | ||
| */ | ||
| #[RegisteredRule(level: 5)] |
Contributor
There was a problem hiding this comment.
should it be bleeding edge only?
Collaborator
Author
There was a problem hiding this comment.
Done. The DateIntervalInstantiationRule is now bleeding edge only:
- Removed
#[RegisteredRule(level: 5)]from the rule class - Added
checkDateIntervalConstructorfeature toggle (defaultfalseinconfig.neon,trueinbleedingEdge.neon) - Added the toggle to the parameters schema
- Registered the rule as a service in
config.level5.neonwith aconditionalTagsentry gated on the feature toggle
All tests pass and make phpstan reports no errors.
…s at analysis time - Add DateIntervalInstantiationRule that validates constant duration strings passed to the DateInterval constructor and reports errors for invalid ones - Add DateTimeZoneInstantiationRule that validates constant timezone strings passed to the DateTimeZone constructor and reports errors for invalid ones - Both rules follow the same pattern as the existing DateTimeInstantiationRule: extract constant strings, attempt construction, report exceptions as errors - Both registered at level 5, matching DateTimeInstantiationRule
The DateTimeZone validation rule is less valuable than the DateInterval case since DateInterval durations are more commonly known at analysis time. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On PHP < 8.1, DateInterval::__construct() prefixes the exception message with "DateInterval::__construct(): ", while PHP 8.1+ uses a shorter format. Gate the existing test with RequiresPhp(>= 8.1.0) and add a separate testLegacyPhp method for older PHP versions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
staabm
force-pushed
the
create-pull-request/patch-v8z1qut
branch
from
c2c3f59 to
a71fa23
Compare
Comment on lines
+53
to
+54
| // normalize PHP7 errors to PHP8 format | ||
| str_replace('DateInterval::__construct(): ', '', $e->getMessage()), |
Contributor
There was a problem hiding this comment.
The same difference exist for other instantiation but we don't normalize the errors.
https://3v4l.org/Yt4EU#v7.4.33
I think we should be consistent for now.
Collaborator
Author
There was a problem hiding this comment.
Done. Removed the str_replace normalization from DateIntervalInstantiationRule so error messages are passed through as-is from PHP's DateInterval constructor — consistent with how other rules handle version-specific error messages. Test expectations now use a $prefix variable based on PHP_VERSION_ID to handle the DateInterval::__construct(): prefix present on PHP < 8.1.
Be consistent with other rules by not normalizing PHP exception messages. Split test expectations by PHP version instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
VincentLanglet
approved these changes
May 5, 2026
Contributor
|
I didn't realize the bot will handle my review @staabm (#5587 (comment)) Please confirm you're ok with it before merging thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PHPStan did not report errors when
DateIntervalwas constructed with an invalid duration string (e.g.new DateInterval('1M')instead ofnew DateInterval('P1M')). While there was already aDateIntervalConstructorThrowTypeExtensionthat refined the throw type, there was no rule to actually report the error to the user.Fixes phpstan/phpstan#14442
see https://3v4l.org/MBaPe#veol