Skip to content

Commit 5e6b44b

Browse files
phpstan-botVincentLangletclaude
authored
Include null in return type of Closure::bindTo() and Closure::bind() dynamic return type extensions (#5675)
Co-authored-by: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5f35fb0 commit 5e6b44b

8 files changed

Lines changed: 123 additions & 17 deletions

src/Type/Php/ClosureBindDynamicReturnTypeExtension.php

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@
77
use PHPStan\Analyser\Scope;
88
use PHPStan\DependencyInjection\AutowiredService;
99
use PHPStan\Reflection\MethodReflection;
10+
use PHPStan\TrinaryLogic;
11+
use PHPStan\Type\BenevolentUnionType;
1012
use PHPStan\Type\ClosureType;
1113
use PHPStan\Type\DynamicStaticMethodReturnTypeExtension;
14+
use PHPStan\Type\NullType;
1215
use PHPStan\Type\Type;
1316

1417
#[AutowiredService]
@@ -27,12 +30,29 @@ public function isStaticMethodSupported(MethodReflection $methodReflection): boo
2730

2831
public function getTypeFromStaticMethodCall(MethodReflection $methodReflection, StaticCall $methodCall, Scope $scope): ?Type
2932
{
30-
$closureType = $scope->getType($methodCall->getArgs()[0]->value);
33+
$args = $methodCall->getArgs();
34+
if (!isset($args[0])) {
35+
return null;
36+
}
37+
38+
$closureType = $scope->getType($args[0]->value);
3139
if (!($closureType instanceof ClosureType)) {
3240
return null;
3341
}
3442

35-
return $closureType;
43+
if ($closureType->isStaticClosure()->yes()) {
44+
$newThisIsNull = isset($args[1]) ? $scope->getType($args[1]->value)->isNull() : TrinaryLogic::createYes();
45+
if ($newThisIsNull->yes()) {
46+
return $closureType;
47+
}
48+
if ($newThisIsNull->no()) {
49+
return new NullType();
50+
}
51+
52+
return new BenevolentUnionType([$closureType, new NullType()]);
53+
}
54+
55+
return new BenevolentUnionType([$closureType, new NullType()]);
3656
}
3757

3858
}

src/Type/Php/ClosureBindToDynamicReturnTypeExtension.php

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@
77
use PHPStan\Analyser\Scope;
88
use PHPStan\DependencyInjection\AutowiredService;
99
use PHPStan\Reflection\MethodReflection;
10+
use PHPStan\TrinaryLogic;
11+
use PHPStan\Type\BenevolentUnionType;
1012
use PHPStan\Type\ClosureType;
1113
use PHPStan\Type\DynamicMethodReturnTypeExtension;
14+
use PHPStan\Type\NullType;
1215
use PHPStan\Type\Type;
1316

1417
#[AutowiredService]
@@ -32,7 +35,20 @@ public function getTypeFromMethodCall(MethodReflection $methodReflection, Method
3235
return null;
3336
}
3437

35-
return $closureType;
38+
if ($closureType->isStaticClosure()->yes()) {
39+
$args = $methodCall->getArgs();
40+
$newThisIsNull = isset($args[0]) ? $scope->getType($args[0]->value)->isNull() : TrinaryLogic::createYes();
41+
if ($newThisIsNull->yes()) {
42+
return $closureType;
43+
}
44+
if ($newThisIsNull->no()) {
45+
return new NullType();
46+
}
47+
48+
return new BenevolentUnionType([$closureType, new NullType()]);
49+
}
50+
51+
return new BenevolentUnionType([$closureType, new NullType()]);
3652
}
3753

3854
}

tests/PHPStan/Analyser/AnalyserIntegrationTest.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -473,13 +473,14 @@ public function testBug4734(): void
473473
{
474474
// false positive
475475
$errors = $this->runAnalyse(__DIR__ . '/data/bug-4734.php');
476-
$this->assertCount(5, $errors); // could be 3
476+
$this->assertCount(6, $errors); // could be 4
477477

478478
$this->assertSame('Static property Bug4734\Foo::$httpMethodParameterOverride (bool) is never assigned false so the property type can be changed to true.', $errors[0]->getMessage()); // should not error
479479
$this->assertSame('Property Bug4734\Foo::$httpMethodParameterOverride2 (bool) is never assigned false so the property type can be changed to true.', $errors[1]->getMessage()); // should not error
480480
$this->assertSame('Unsafe access to private property Bug4734\Foo::$httpMethodParameterOverride through static::.', $errors[2]->getMessage());
481-
$this->assertSame('Access to an undefined static property static(Bug4734\Foo)::$httpMethodParameterOverride3.', $errors[3]->getMessage());
482-
$this->assertSame('Access to an undefined property Bug4734\Foo::$httpMethodParameterOverride4.', $errors[4]->getMessage());
481+
$this->assertSame('Trying to invoke null but it\'s not a callable.', $errors[3]->getMessage()); // binding a static closure to an object returns null
482+
$this->assertSame('Access to an undefined static property static(Bug4734\Foo)::$httpMethodParameterOverride3.', $errors[4]->getMessage());
483+
$this->assertSame('Access to an undefined property Bug4734\Foo::$httpMethodParameterOverride4.', $errors[5]->getMessage());
483484
}
484485

485486
#[RequiresPhp('>= 8.1.0')]
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug5009;
6+
7+
use Closure;
8+
use function PHPStan\Testing\assertType;
9+
10+
$foo = function (): void {};
11+
$bar = $foo->bindTo(null);
12+
assertType('((Closure(): void)|null)', $bar);
13+
14+
$baz = Closure::bind($foo, null);
15+
assertType('((Closure(): void)|null)', $baz);
16+
17+
$newThis = new \stdClass();
18+
$bound = $foo->bindTo($newThis);
19+
assertType('((Closure(): void)|null)', $bound);
20+
21+
$staticBound = Closure::bind($foo, $newThis);
22+
assertType('((Closure(): void)|null)', $staticBound);
23+
24+
$bound2 = $foo->bindTo($newThis, 'stdClass');
25+
assertType('((Closure(): void)|null)', $bound2);
26+
27+
$static = static function (): void {};
28+
$boundStatic = $static->bindTo($newThis);
29+
assertType('null', $boundStatic);
30+
31+
$boundStaticNull = $static->bindTo(null);
32+
assertType('static-Closure(): void', $boundStaticNull);
33+
34+
$staticBound2 = Closure::bind($static, $newThis);
35+
assertType('null', $staticBound2);
36+
37+
$staticBoundNull = Closure::bind($static, null);
38+
assertType('static-Closure(): void', $staticBoundNull);
39+
40+
/** @var \stdClass|null $maybeNull */
41+
$maybeNull = null;
42+
$boundMaybe = $foo->bindTo($maybeNull);
43+
assertType('((Closure(): void)|null)', $boundMaybe);
44+
45+
$staticBoundMaybe = $static->bindTo($maybeNull);
46+
assertType('((static-Closure(): void)|null)', $staticBoundMaybe);

tests/PHPStan/Analyser/nsrt/closure-return-type-extensions.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@
1111

1212
$newThis = new class {};
1313
$boundClosure = $closure->bindTo($newThis);
14-
assertType('Closure(object): true', $boundClosure);
14+
assertType('((Closure(object): true)|null)', $boundClosure);
1515

1616
$staticallyBoundClosure = \Closure::bind($closure, $newThis);
17-
assertType('Closure(object): true', $staticallyBoundClosure);
17+
assertType('((Closure(object): true)|null)', $staticallyBoundClosure);
1818

1919
$returnType = $closure->call($newThis, new class {});
2020
assertType('true', $returnType);
2121

2222
$staticallyBoundClosureCaseInsensitive = \closure::bind($closure, $newThis);
23-
assertType('Closure(object): true', $staticallyBoundClosureCaseInsensitive);
23+
assertType('((Closure(object): true)|null)', $staticallyBoundClosureCaseInsensitive);

tests/PHPStan/Analyser/nsrt/closure-static-type.php

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,37 +26,37 @@ public function doFoo(): void
2626
public function doBindTo(): void
2727
{
2828
$static = static function (): void {};
29-
assertType('static-Closure(): void', $static->bindTo($this));
29+
assertType('null', $static->bindTo($this));
3030

3131
$nonStatic = function (): void {};
32-
assertType('Closure(): void', $nonStatic->bindTo($this));
32+
assertType('((Closure(): void)|null)', $nonStatic->bindTo($this));
3333
}
3434

3535
public function doBind(): void
3636
{
3737
$static = static function (): void {};
38-
assertType('static-Closure(): void', Closure::bind($static, $this));
38+
assertType('null', Closure::bind($static, $this));
3939

4040
$nonStatic = function (): void {};
41-
assertType('Closure(): void', Closure::bind($nonStatic, $this));
41+
assertType('((Closure(): void)|null)', Closure::bind($nonStatic, $this));
4242
}
4343

4444
/**
4545
* @param Closure(): void $unknownClosure
4646
*/
4747
public function doUnknown(Closure $unknownClosure): void
4848
{
49-
assertType('Closure(): void', $unknownClosure->bindTo($this));
50-
assertType('Closure(): void', Closure::bind($unknownClosure, $this));
49+
assertType('((Closure(): void)|null)', $unknownClosure->bindTo($this));
50+
assertType('((Closure(): void)|null)', Closure::bind($unknownClosure, $this));
5151
}
5252

5353
public function doFromCallable(): void
5454
{
5555
$fn = Closure::fromCallable(static function (): void {});
56-
assertType('static-Closure(): void', $fn->bindTo($this));
56+
assertType('null', $fn->bindTo($this));
5757

5858
$fn2 = Closure::fromCallable(function (): void {});
59-
assertType('Closure(): void', $fn2->bindTo($this));
59+
assertType('((Closure(): void)|null)', $fn2->bindTo($this));
6060
}
6161

6262
}

tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,11 @@ public function testNullCoalesceInGlobalScope(): void
244244
]);
245245
}
246246

247+
public function testBug5009(): void
248+
{
249+
$this->analyse([__DIR__ . '/data/bug-5009.php'], []);
250+
}
251+
247252
public function testBug5933(): void
248253
{
249254
$this->analyse([__DIR__ . '/data/bug-5933.php'], []);
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug5009;
4+
5+
use Closure;
6+
7+
class Test
8+
{
9+
protected Closure $callback;
10+
11+
/**
12+
* @param Closure(): void $callback
13+
*/
14+
public function __construct(Closure $callback)
15+
{
16+
$this->callback = $callback->bindTo($this, $this) ?? $callback;
17+
}
18+
}

0 commit comments

Comments
 (0)