Skip to content

Commit 7489a09

Browse files
authored
Fix invalidating static property access after impure call
1 parent e6d9f0c commit 7489a09

17 files changed

+321
-8
lines changed

src/Analyser/MutatingScope.php

+11
Original file line numberDiff line numberDiff line change
@@ -4379,6 +4379,17 @@ private function shouldInvalidateExpression(string $exprStringToInvalidate, Expr
43794379
$nodeFinder = new NodeFinder();
43804380
$expressionToInvalidateClass = get_class($exprToInvalidate);
43814381
$found = $nodeFinder->findFirst([$expr], function (Node $node) use ($expressionToInvalidateClass, $exprStringToInvalidate): bool {
4382+
if (
4383+
$exprStringToInvalidate === '$this'
4384+
&& $node instanceof Name
4385+
&& (
4386+
in_array($node->toLowerString(), ['self', 'static', 'parent'], true)
4387+
|| ($this->getClassReflection() !== null && $this->getClassReflection()->is($this->resolveName($node)))
4388+
)
4389+
) {
4390+
return true;
4391+
}
4392+
43824393
if (!$node instanceof $expressionToInvalidateClass) {
43834394
return false;
43844395
}

src/Analyser/NodeScopeResolver.php

+11-4
Original file line numberDiff line numberDiff line change
@@ -2597,6 +2597,13 @@ static function (): void {
25972597
$throwPoints[] = ThrowPoint::createImplicit($scope, $expr);
25982598
}
25992599

2600+
if (
2601+
$parametersAcceptor instanceof ClosureType && count($parametersAcceptor->getImpurePoints()) > 0
2602+
&& $scope->isInClass()
2603+
) {
2604+
$scope = $scope->invalidateExpression(new Variable('this'), true);
2605+
}
2606+
26002607
if (
26012608
$functionReflection !== null
26022609
&& in_array($functionReflection->getName(), ['json_encode', 'json_decode'], true)
@@ -3022,13 +3029,13 @@ static function (): void {
30223029

30233030
if (
30243031
$methodReflection !== null
3025-
&& !$methodReflection->isStatic()
30263032
&& (
30273033
$methodReflection->hasSideEffects()->yes()
3028-
|| $methodReflection->getName() === '__construct'
3034+
|| (
3035+
!$methodReflection->isStatic()
3036+
&& $methodReflection->getName() === '__construct'
3037+
)
30293038
)
3030-
&& $scopeFunction instanceof MethodReflection
3031-
&& !$scopeFunction->isStatic()
30323039
&& $scope->isInClass()
30333040
&& $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName())
30343041
) {

tests/PHPStan/Analyser/nsrt/bug-12902-non-strict.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ public function __construct()
7272
assertNativeType('int', self::$i);
7373

7474
$this->impureCall();
75-
assertType('int', self::$i); // should be float|int
76-
assertNativeType('int', self::$i); // should be float|int
75+
assertType('float|int', self::$i);
76+
assertNativeType('float|int', self::$i);
7777
}
7878

7979
public function doFoo(): void {

tests/PHPStan/Analyser/nsrt/bug-12902.php

+29-2
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ public function __construct()
7272
assertNativeType('int', self::$i);
7373

7474
$this->impureCall();
75-
assertType('int', self::$i); // should be float|int
76-
assertNativeType('int', self::$i); // should be float|int
75+
assertType('float|int', self::$i);
76+
assertNativeType('float|int', self::$i);
7777
}
7878

7979
public function doFoo(): void {
@@ -85,6 +85,33 @@ public function doFoo(): void {
8585
public function impureCall(): void {}
8686
}
8787

88+
class BaseClass
89+
{
90+
static protected int|float $i;
91+
}
92+
93+
class UsesBaseClass extends BaseClass
94+
{
95+
public function __construct()
96+
{
97+
parent::$i = getInt();
98+
assertType('int', parent::$i);
99+
assertNativeType('int', parent::$i);
100+
101+
$this->impureCall();
102+
assertType('float|int', parent::$i);
103+
assertNativeType('float|int', parent::$i);
104+
}
105+
106+
public function doFoo(): void {
107+
assertType('float|int', parent::$i);
108+
assertNativeType('float|int', parent::$i);
109+
}
110+
111+
/** @phpstan-impure */
112+
public function impureCall(): void {}
113+
}
114+
88115
function getInt(): int {
89116
return 1;
90117
}

tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php

+5
Original file line numberDiff line numberDiff line change
@@ -930,4 +930,9 @@ public function testBug12593(): void
930930
$this->analyse([__DIR__ . '/data/bug-12593.php'], []);
931931
}
932932

933+
public function testBug3747(): void
934+
{
935+
$this->analyse([__DIR__ . '/data/bug-3747.php'], []);
936+
}
937+
933938
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
namespace Bug3747;
4+
5+
class X {
6+
7+
/** @var array<string,string> $x */
8+
private static array $x;
9+
10+
public function y(): void {
11+
12+
self::$x = [];
13+
14+
$this->z();
15+
16+
echo self::$x['foo'];
17+
18+
}
19+
20+
private function z(): void {
21+
self::$x['foo'] = 'bar';
22+
}
23+
24+
}
25+
26+
$x = new X();
27+
$x->y();

tests/PHPStan/Rules/Comparison/IfConstantConditionRuleTest.php

+12
Original file line numberDiff line numberDiff line change
@@ -173,4 +173,16 @@ public function testBug4912(): void
173173
$this->analyse([__DIR__ . '/data/bug-4912.php'], []);
174174
}
175175

176+
public function testBug4864(): void
177+
{
178+
$this->treatPhpDocTypesAsCertain = true;
179+
$this->analyse([__DIR__ . '/data/bug-4864.php'], []);
180+
}
181+
182+
public function testBug8926(): void
183+
{
184+
$this->treatPhpDocTypesAsCertain = true;
185+
$this->analyse([__DIR__ . '/data/bug-8926.php'], []);
186+
}
187+
176188
}

tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php

+5
Original file line numberDiff line numberDiff line change
@@ -1011,4 +1011,9 @@ public function testBug12748(): void
10111011
$this->analyse([__DIR__ . '/data/bug-12748.php'], []);
10121012
}
10131013

1014+
public function testBug11019(): void
1015+
{
1016+
$this->analyse([__DIR__ . '/data/bug-11019.php'], []);
1017+
}
1018+
10141019
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
namespace Bug11019;
4+
5+
class HelloWorld
6+
{
7+
public static int $a;
8+
9+
public function reset(): void
10+
{
11+
static::$a = rand(1,10);
12+
}
13+
14+
public function sayHello(): void
15+
{
16+
$this->reset();
17+
assert(static::$a === 1);
18+
$this->reset();
19+
assert(static::$a === 1);
20+
}
21+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
namespace Bug4864;
4+
5+
class Example
6+
{
7+
/** @var mixed */
8+
private $value;
9+
private bool $isHandled;
10+
11+
public function fetchValue(callable $f): void
12+
{
13+
$this->isHandled = false;
14+
$this->value = null;
15+
16+
(function () {
17+
$this->isHandled = true;
18+
$this->value = 'value';
19+
})();
20+
21+
if ($this->isHandled) {
22+
$f($this->value);
23+
}
24+
}
25+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
namespace Bug8926;
4+
5+
class Foo {
6+
private bool $test;
7+
8+
/** @param int[] $arr */
9+
function success(array $arr) : void {
10+
$test = false;
11+
(function($arr) use(&$test) {
12+
$test = count($arr) == 1;
13+
})($arr);
14+
15+
if ($test) {
16+
echo "...\n";
17+
}
18+
}
19+
20+
/** @param int[] $arr */
21+
function error(array $arr) : void {
22+
$this->test = false;
23+
(function($arr) {
24+
$this->test = count($arr) == 1;
25+
})($arr);
26+
27+
28+
if ($this->test) {
29+
echo "...\n";
30+
}
31+
}
32+
}

tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php

+15
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,19 @@ public function testBug6922b(): void
5555
$this->analyse([__DIR__ . '/data/bug-6922b.php'], []);
5656
}
5757

58+
public function testBug8523(): void
59+
{
60+
$this->analyse([__DIR__ . '/data/bug-8523.php'], []);
61+
}
62+
63+
public function testBug8523b(): void
64+
{
65+
$this->analyse([__DIR__ . '/data/bug-8523b.php'], []);
66+
}
67+
68+
public function testBug8523c(): void
69+
{
70+
$this->analyse([__DIR__ . '/data/bug-8523c.php'], []);
71+
}
72+
5873
}

tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php

+14
Original file line numberDiff line numberDiff line change
@@ -1232,4 +1232,18 @@ public function testBug1O580(): void
12321232
]);
12331233
}
12341234

1235+
public function testBug4443(): void
1236+
{
1237+
if (PHP_VERSION_ID < 80000) {
1238+
$this->markTestSkipped('Test requires PHP 8.0.');
1239+
}
1240+
1241+
$this->analyse([__DIR__ . '/data/bug-4443.php'], [
1242+
[
1243+
'Method Bug4443\HelloWorld::getArray() should return array<mixed> but returns array<mixed>|null.',
1244+
22,
1245+
],
1246+
]);
1247+
}
1248+
12351249
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
namespace Bug4443;
4+
5+
class HelloWorld
6+
{
7+
/** @var array<mixed> */
8+
private static ?array $arr = null;
9+
10+
private static function setup(): void
11+
{
12+
self::$arr = null;
13+
}
14+
15+
/** @return array<mixed> */
16+
public static function getArray(): array
17+
{
18+
if (self::$arr === null) {
19+
self::$arr = [];
20+
self::setup();
21+
}
22+
return self::$arr;
23+
}
24+
}
25+
26+
HelloWorld::getArray();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
namespace Bug8523;
4+
5+
class HelloWorld
6+
{
7+
public static ?HelloWorld $instance = null;
8+
9+
public static function bazz(): void
10+
{
11+
self::$instance = null;
12+
}
13+
14+
public function foo(): void
15+
{
16+
self::$instance = new HelloWorld();
17+
18+
self::bazz();
19+
20+
self::$instance?->foo();
21+
}
22+
23+
public function bar(): void
24+
{
25+
self::$instance = null;
26+
}
27+
28+
public function baz(): void
29+
{
30+
self::$instance = new HelloWorld();
31+
32+
$this->bar();
33+
34+
self::$instance?->foo();
35+
}
36+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug8523b;
4+
5+
class HelloWorld
6+
{
7+
public static ?HelloWorld $instance = null;
8+
9+
public function save(): void {
10+
self::$instance = new HelloWorld();
11+
12+
$callback = static function(): void {
13+
self::$instance = null;
14+
};
15+
16+
$callback();
17+
18+
var_dump(self::$instance);
19+
20+
self::$instance?->save();
21+
}
22+
}
23+
24+
(new HelloWorld())->save();

0 commit comments

Comments
 (0)