Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion src/Analyser/ExprHandler/Helper/MethodCallReturnTypeHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use PHPStan\DependencyInjection\AutowiredService;
use PHPStan\DependencyInjection\Type\DynamicReturnTypeExtensionRegistryProvider;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use function count;
Expand Down Expand Up @@ -52,7 +53,9 @@
}

$resolvedTypes = [];
foreach ($typeWithMethod->getObjectClassNames() as $className) {
$allClassNames = $typeWithMethod->getObjectClassNames();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$handledClassNames = [];
foreach ($allClassNames as $className) {
if ($normalizedMethodCall instanceof MethodCall) {
foreach ($this->dynamicReturnTypeExtensionRegistryProvider->getRegistry()->getDynamicMethodReturnTypeExtensionsForClass($className) as $dynamicMethodReturnTypeExtension) {
if (!$dynamicMethodReturnTypeExtension->isMethodSupported($methodReflection)) {
Expand All @@ -65,6 +68,7 @@
}

$resolvedTypes[] = $resolvedType;
$handledClassNames[] = $className;
}
} else {
foreach ($this->dynamicReturnTypeExtensionRegistryProvider->getRegistry()->getDynamicStaticMethodReturnTypeExtensionsForClass($className) as $dynamicStaticMethodReturnTypeExtension) {
Expand All @@ -82,11 +86,29 @@
}

$resolvedTypes[] = $resolvedType;
$handledClassNames[] = $className;
}
}
}

if (count($resolvedTypes) > 0) {
if (count($allClassNames) !== count($handledClassNames)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$remainingType = $typeWithMethod;
foreach ($handledClassNames as $handledClassName) {
$remainingType = TypeCombinator::remove($remainingType, new ObjectType($handledClassName));
}
if ($remainingType->hasMethod($methodName)->yes()) {

Check warning on line 100 in src/Analyser/ExprHandler/Helper/MethodCallReturnTypeHelper.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ foreach ($handledClassNames as $handledClassName) { $remainingType = TypeCombinator::remove($remainingType, new ObjectType($handledClassName)); } - if ($remainingType->hasMethod($methodName)->yes()) { + if (!$remainingType->hasMethod($methodName)->no()) { $remainingMethod = $remainingType->getMethod($methodName, $scope); $remainingParametersAcceptor = ParametersAcceptorSelector::selectFromArgs( $scope,

Check warning on line 100 in src/Analyser/ExprHandler/Helper/MethodCallReturnTypeHelper.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ foreach ($handledClassNames as $handledClassName) { $remainingType = TypeCombinator::remove($remainingType, new ObjectType($handledClassName)); } - if ($remainingType->hasMethod($methodName)->yes()) { + if (!$remainingType->hasMethod($methodName)->no()) { $remainingMethod = $remainingType->getMethod($methodName, $scope); $remainingParametersAcceptor = ParametersAcceptorSelector::selectFromArgs( $scope,
$remainingMethod = $remainingType->getMethod($methodName, $scope);
$remainingParametersAcceptor = ParametersAcceptorSelector::selectFromArgs(
$scope,
$methodCall->getArgs(),
$remainingMethod->getVariants(),
$remainingMethod->getNamedArgumentsVariants(),
);
$resolvedTypes[] = $remainingParametersAcceptor->getReturnType();
}
}

return VoidToNullTypeTransformer::transform(TypeCombinator::union(...$resolvedTypes), $methodCall);
}

Expand Down
28 changes: 25 additions & 3 deletions src/Type/Php/DateTimeModifyReturnTypeExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\DynamicMethodReturnTypeExtension;
use PHPStan\Type\NeverType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\TypeTraverser;
use PHPStan\Type\UnionType;
use Throwable;
use function count;

Expand All @@ -39,11 +42,12 @@

public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type
{
if (count($methodCall->getArgs()) < 1) {
$args = $methodCall->getArgs();
if (count($args) < 1) {
return null;
}

$valueType = $scope->getType($methodCall->getArgs()[0]->value);
$valueType = $scope->getType($args[0]->value);
$constantStrings = $valueType->getConstantStrings();

$hasFalse = false;
Expand Down Expand Up @@ -77,7 +81,25 @@

return null;
} elseif ($hasDateTime) {
return $scope->getType($methodCall->var);
$callerType = $scope->getType($methodCall->var);

$dateTimeInterfaceType = new ObjectType(DateTimeInterface::class);
if ($dateTimeInterfaceType->isSuperTypeOf($callerType)->yes()) {

Check warning on line 87 in src/Type/Php/DateTimeModifyReturnTypeExtension.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\IsSuperTypeOfCalleeAndArgumentMutator": @@ @@ $callerType = $scope->getType($methodCall->var); $dateTimeInterfaceType = new ObjectType(DateTimeInterface::class); - if ($dateTimeInterfaceType->isSuperTypeOf($callerType)->yes()) { + if ($callerType->isSuperTypeOf($dateTimeInterfaceType)->yes()) { return $callerType; }

Check warning on line 87 in src/Type/Php/DateTimeModifyReturnTypeExtension.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\IsSuperTypeOfCalleeAndArgumentMutator": @@ @@ $callerType = $scope->getType($methodCall->var); $dateTimeInterfaceType = new ObjectType(DateTimeInterface::class); - if ($dateTimeInterfaceType->isSuperTypeOf($callerType)->yes()) { + if ($callerType->isSuperTypeOf($dateTimeInterfaceType)->yes()) { return $callerType; }
return $callerType;
}

return TypeTraverser::map(
Copy link
Copy Markdown
Contributor

@staabm staabm Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw: the fact that we need such a hack here makes me ferl we likely need it somewhere else, as this logic does not look specific for DateTimeInterface

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's related to the fact we're trying to return $scope->getType($methodCall->var) which might not be only a DateTime.

Most of the dynamic returntype extension are doing

ParametersAcceptorSelector::selectFromArgs($scope, $args, $variants)->getReturnType();

and then remove the unwanted return type (like False) here.

But it breaks a non regression test about Template for DateTime, because someone use modify on

T of DateTime|DateTimeImmutable

$scope->getType($methodCall->var) was the fix used to avoid this, but it has the defaut to not filter non-datetime types. There is only 4 extension with this code

<img width="1291" height="198" alt="image" src="https://github.com/user-attachments/assets/dc649a3b-b1d6-4a1a-bcb5-0123c6970dc7" />

And other extensions seems to handle such cases (with sometimes simpler checks)

$callerType,
static function (Type $type, callable $traverse) use ($dateTimeInterfaceType): Type {
if ($type instanceof UnionType) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need support for IntersectionType in the TypeTraverser callable?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because if you get a type DateTimeInterface&Foo the DateTimeInterface will be a superTypeOf DateTimeInterface and it can be return like this.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All done — both the final test run (11775 tests passing) and static analysis (no errors) have completed. The branch is rebased on origin/2.2.x and pushed.

return $traverse($type);
}
if ($dateTimeInterfaceType->isSuperTypeOf($type)->yes()) {
return $type;
}
return new NeverType();
},
);
}

if ($this->phpVersion->hasDateTimeExceptions()) {
Expand Down
34 changes: 34 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-11073.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php // lint >= 8.0

declare(strict_types = 1);

namespace Bug11073Nsrt;

use DateTimeImmutable;
use function PHPStan\Testing\assertType;

class HelloWorld
{
public function sayHello(?DateTimeImmutable $date): void
{
assertType('DateTimeImmutable|null', $date?->modify('+1 year')->setTime(23, 59, 59));
}
}

class Foo
{
public function getCode(): bool { return false; }
}

class HelloWorld2
{
public function sayHello(\Throwable|Foo $foo): void
{
assertType('bool|int|string', $foo->getCode());
}

public function sayHello2(\LogicException|Foo $foo): void
{
assertType('bool|int', $foo->getCode());
}
}
24 changes: 23 additions & 1 deletion tests/PHPStan/Analyser/nsrt/date-format.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,27 @@ function (\DateTimeImmutable $dt, string $s): void {
};

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

};

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

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

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

function foo2(Foo|Bar2 $d): void {
assertType('DateFormatReturnType\Foo|string|false', $d->modify('+1 day'));
};
27 changes: 27 additions & 0 deletions tests/PHPStan/Analyser/nsrt/method-call-return-type-fallback.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php // lint >= 8.1

namespace MethodCallReturnTypeFallback;

use function PHPStan\Testing\assertType;

enum Suit: string {
case Hearts = 'hearts';
case Diamonds = 'diamonds';
}

class MyClass {
/** @return self */
public static function from(string $value): self {
return new self();
}
}

/** @param class-string<Suit>|class-string<MyClass> $class */
function testStaticCallOnUnionWithConstant(string $class): void {
assertType('MethodCallReturnTypeFallback\MyClass|MethodCallReturnTypeFallback\Suit::Hearts', $class::from('hearts'));
}

/** @param class-string<Suit>|class-string<MyClass> $class */
function testStaticCallOnUnionWithVariable(string $class, string $value): void {
assertType('MethodCallReturnTypeFallback\MyClass|MethodCallReturnTypeFallback\Suit', $class::from($value));
}
9 changes: 9 additions & 0 deletions tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3945,6 +3945,15 @@ public function testBug7369(): void
]);
}

#[RequiresPhp('>= 8.0')]
public function testBug11073(): void
{
$this->checkThisOnly = false;
$this->checkNullables = true;
$this->checkUnionTypes = true;
$this->analyse([__DIR__ . '/data/bug-11073.php'], []);
}

public function testBug11463(): void
{
$this->checkThisOnly = false;
Expand Down
15 changes: 15 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-11073.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php // lint >= 8.0

declare(strict_types = 1);

namespace Bug11073;

use DateTimeImmutable;

class HelloWorld
{
public function sayHello(?DateTimeImmutable $date): ?DateTimeImmutable
{
return $date?->modify('+1 year')->setTime(23, 59, 59);
}
}
Loading