Fix phpstan/phpstan#11073: Nullsafe operator chaining#5407
Open
phpstan-bot wants to merge 3 commits intophpstan:2.1.xfrom
Open
Fix phpstan/phpstan#11073: Nullsafe operator chaining#5407phpstan-bot wants to merge 3 commits intophpstan:2.1.xfrom
phpstan-bot wants to merge 3 commits intophpstan:2.1.xfrom
Conversation
- Fixed DateTimeModifyReturnTypeExtension to strip null from callee type
before returning it as the method's return type
- The extension was returning $scope->getType($methodCall->var) which
includes null from the nullsafe operator, causing "Cannot call method
on DateTimeImmutable|null" false positive for chained calls like
$date?->modify('+1 year')->setTime(23, 59, 59)
- Updated existing date-format.php test assertion (null was incorrectly
expected in modify() return type)
- Added regression tests for the nullsafe chaining scenario
| } elseif ($hasDateTime) { | ||
| return $scope->getType($methodCall->var); | ||
| $callerType = $scope->getType($methodCall->var); | ||
| if (TypeCombinator::containsNull($callerType)) { |
Contributor
There was a problem hiding this comment.
Remove the check containsNull since it's already in removeNull
Collaborator
Author
There was a problem hiding this comment.
Done. Removed the redundant containsNull check as requested by VincentLanglet — TypeCombinator::removeNull() is a no-op when there's no null in the type, so the guard was unnecessary. All tests and static analysis pass.
|
|
||
| function (?\DateTimeImmutable $d): void { | ||
| assertType('DateTimeImmutable|null', $d->modify('+1 day')); | ||
| assertType('DateTimeImmutable', $d->modify('+1 day')); |
Contributor
There was a problem hiding this comment.
Add an assertType on $d?->modify('+1 day')
Collaborator
Author
There was a problem hiding this comment.
Tests background task confirmed complete as well — 11733 tests passing. All done.
TypeCombinator::removeNull() already handles the case where the type does not contain null, making the containsNull guard unnecessary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
2 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
Fixes a false positive where PHPStan reported "Cannot call method setTime() on DateTimeImmutable|null" when using nullsafe operator chaining like
$date?->modify('+1 year')->setTime(23, 59, 59). The nullsafe operator (?->) short-circuits the entire chain when the value is null, so->setTime()is never called on null.Changes
src/Type/Php/DateTimeModifyReturnTypeExtension.phpto strip null from the callee type before returning it as the return type ofmodify()tests/PHPStan/Analyser/nsrt/date-format.php(null was incorrectly part ofmodify()return type)tests/PHPStan/Rules/Methods/data/bug-11073.phpandtests/PHPStan/Rules/Methods/CallMethodsRuleTest.phptests/PHPStan/Analyser/nsrt/bug-11073.phpRoot cause
DateTimeModifyReturnTypeExtension::getTypeFromMethodCall()returned$scope->getType($methodCall->var)as the return type when all argument strings are valid modifiers. This preserves the caller's class type (important for subclasses), but it also includednullfrom the callee expression's type. WhenNullsafeOperatorHelper::getNullsafeShortcircuitedExprRespectingScopeconverts aNullsafeMethodCallto a regularMethodCallfor type checking, the scope still has the underlying variable as nullable. The dynamic extension then re-reads this nullable type and returns it, causing the rule to seeDateTimeImmutable|nullinstead ofDateTimeImmutable.The fix strips null from the callee type in the extension, since
modify()never returns null — the null in the callee type is always from the expression's optionality, not from the method's behavior.Test
$date?->modify('+1 year')->setTime(23, 59, 59)produces no errors when$dateis?DateTimeImmutableDateTimeImmutable|nullFixes phpstan/phpstan#11073