Fix phpstan/phpstan#11073: Nullsafe operator chaining by phpstan-bot · Pull Request #5407 · phpstan/phpstan-src

phpstan-bot · 2026-04-05T22:37:45Z

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

Fixed src/Type/Php/DateTimeModifyReturnTypeExtension.php to strip null from the callee type before returning it as the return type of modify()
Updated assertion in tests/PHPStan/Analyser/nsrt/date-format.php (null was incorrectly part of modify() return type)
Added rule test in tests/PHPStan/Rules/Methods/data/bug-11073.php and tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
Added type inference test in tests/PHPStan/Analyser/nsrt/bug-11073.php

Root 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 included null from the callee expression's type. When NullsafeOperatorHelper::getNullsafeShortcircuitedExprRespectingScope converts a NullsafeMethodCall to a regular MethodCall for 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 see DateTimeImmutable|null instead of DateTimeImmutable.

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

Rule test verifies that $date?->modify('+1 year')->setTime(23, 59, 59) produces no errors when $date is ?DateTimeImmutable
NSRT test verifies the full chain's type is correctly inferred as DateTimeImmutable|null

Fixes phpstan/phpstan#11073

VincentLanglet · 2026-04-05T22:52:31Z

src/Type/Php/DateTimeModifyReturnTypeExtension.php

 		} elseif ($hasDateTime) {
-			return $scope->getType($methodCall->var);
+			$callerType = $scope->getType($methodCall->var);
+			if (TypeCombinator::containsNull($callerType)) {


Remove the check containsNull since it's already in removeNull

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.

VincentLanglet · 2026-04-05T22:54:51Z

tests/PHPStan/Analyser/nsrt/date-format.php


 function (?\DateTimeImmutable $d): void {
-	assertType('DateTimeImmutable|null', $d->modify('+1 day'));
+	assertType('DateTimeImmutable', $d->modify('+1 day'));


Add an assertType on $d?->modify('+1 day')

Tests background task confirmed complete as well — 11733 tests passing. All done.

tests/PHPStan/Analyser/nsrt/date-format.php

VincentLanglet

The test

class Foo extends \DateTimeImmutable {}
class Bar {
	/** @return string */
	public function modify($string) {}
}

function (Foo|Bar $d): void {
	assertType('DateFormatReturnType\Foo|string', $d->modify('+1 day'));
};

is not working.

Two things:

Rather than calling removeNull it might be better to intersect with DateTimeImmutable to keep only Foo from the dynamicReturnTypeExtension
There is a bug in dynamicReturnTypeExtension which does not the union of the return types

phpstan-bot · 2026-04-06T08:23:22Z

All done — every background task has completed successfully. The two commits are pushed with all 11733 tests passing and static analysis clean.

VincentLanglet

I'm affraid recomputing type with $classType = new ObjectType($className); will lose some precisions (like Generics), would it be possible to recompute the "reduced" $typeWithMethod and ParametersAcceptorSelector instead without a foreach loop ?

src/Type/Php/DateTimeModifyReturnTypeExtension.php

phpstan-bot · 2026-04-06T08:56:00Z

Pushed. The change replaces the per-class-name new ObjectType($className) loop with a single filtered $remainingType computed by removing handled classes from the original $typeWithMethod. This preserves generic type information (and any other type details) that would be lost when constructing plain ObjectType instances. All 11733 tests and static analysis pass.

VincentLanglet · 2026-04-06T09:42:17Z

src/Analyser/ExprHandler/Helper/MethodCallReturnTypeHelper.php


 		$resolvedTypes = [];
-		foreach ($typeWithMethod->getObjectClassNames() as $className) {
+		$allClassNames = array_unique($typeWithMethod->getObjectClassNames());


Remove the array_unique call, getObjectClassNames already returns unique class names

Already handled — tests passed and the commit has been pushed.

staabm · 2026-04-06T10:08:59Z

src/Analyser/ExprHandler/Helper/MethodCallReturnTypeHelper.php


 		$resolvedTypes = [];
-		foreach ($typeWithMethod->getObjectClassNames() as $className) {
+		$allClassNames = $typeWithMethod->getObjectClassNames();


does this change have also an impact on StaticCall which needs tests?

Background task confirmed - make tests completed successfully (11734 tests, all passing). Everything was already committed and pushed.

staabm · 2026-04-06T10:10:50Z

src/Analyser/ExprHandler/Helper/MethodCallReturnTypeHelper.php

 		}

 		if (count($resolvedTypes) > 0) {
+			if (count($allClassNames) !== count($handledClassNames)) {


does this change need testing outside of the DateTime* use-case?
it feels pretty generic

- 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

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>

The previous non-nullsafe call narrows the type of $d, which could affect the nullsafe assertion on the next line. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…urnTypeExtension Instead of removeNull, filter out non-DateTime types from the caller type to correctly handle union types like Foo|Bar where only Foo extends DateTimeImmutable. Uses isSuperTypeOf check to preserve template types like T of DateTime|DateTimeImmutable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…extensions When a union type like Foo|Bar has a dynamic return type extension for Foo but not Bar, the dispatch previously discarded Bar's native return type entirely. Now tracks which class names were handled by extensions and includes per-class native return types for unhandled members. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…lasses Preserve generic type information by filtering the original $typeWithMethod to remove handled classes, rather than creating plain ObjectType instances which lose generics and other type details. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ersect Replace the manual loop filtering DateTimeInterface subtypes with a single TypeCombinator::intersect() call as suggested in review. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

getObjectClassNames() already returns unique class names, so the array_unique wrapper is redundant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Tests the generic fallback logic in MethodCallReturnTypeHelper for static method calls on union types where only some classes are handled by dynamic return type extensions. Uses BackedEnum::from() as the extension-handled case and a plain class as the unhandled fallback case. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

VincentLanglet reviewed Apr 5, 2026

View reviewed changes

VincentLanglet reviewed Apr 6, 2026

View reviewed changes

tests/PHPStan/Analyser/nsrt/date-format.php Show resolved Hide resolved

staabm reviewed Apr 6, 2026

View reviewed changes

tests/PHPStan/Analyser/nsrt/date-format.php Show resolved Hide resolved

VincentLanglet reviewed Apr 6, 2026

View reviewed changes

src/Type/Php/DateTimeModifyReturnTypeExtension.php Show resolved Hide resolved

VincentLanglet reviewed Apr 6, 2026

View reviewed changes

VincentLanglet approved these changes Apr 6, 2026

View reviewed changes

VincentLanglet requested a review from staabm April 6, 2026 10:03

VincentLanglet self-assigned this Apr 6, 2026

staabm reviewed Apr 6, 2026

View reviewed changes

VincentLanglet and others added 15 commits April 6, 2026 14:25

Remove redundant containsNull check before removeNull

46bd07f

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>

Add assertType for nullsafe modify call in date-format test

682945a

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add assertType for modify() on union DateTimeImmutable subclasses

fd75369

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Separate nullsafe modify assertion into its own function

c1d83e8

The previous non-nullsafe call narrows the type of $d, which could affect the nullsafe assertion on the next line. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add failing test

9ae64dc

Simplify DateTimeModifyReturnTypeExtension to use TypeCombinator::int…

3371556

…ersect Replace the manual loop filtering DateTimeInterface subtypes with a single TypeCombinator::intersect() call as suggested in review. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Improvements

64fcece

Remove unnecessary array_unique call from getObjectClassNames()

7e4fdb1

getObjectClassNames() already returns unique class names, so the array_unique wrapper is redundant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix and tests

fe58d31

Fix

c6ac861

VincentLanglet force-pushed the create-pull-request/patch-nsdr3no branch from 155c637 to c6ac861 Compare April 6, 2026 12:33

Conversation

phpstan-bot commented Apr 5, 2026

Summary

Changes

Root cause

Test

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented Apr 6, 2026

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phpstan-bot commented Apr 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants