Skip to content

Fix forgetting static property access after impure call #3950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Apr 20, 2025
Merged
11 changes: 11 additions & 0 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -4379,6 +4379,17 @@ private function shouldInvalidateExpression(string $exprStringToInvalidate, Expr
$nodeFinder = new NodeFinder();
$expressionToInvalidateClass = get_class($exprToInvalidate);
$found = $nodeFinder->findFirst([$expr], function (Node $node) use ($expressionToInvalidateClass, $exprStringToInvalidate): bool {
if (
$exprStringToInvalidate === '$this'
&& $node instanceof Name
&& (
in_array($node->toLowerString(), ['self', 'static', 'parent'], true)
|| ($this->getClassReflection() !== null && $this->getClassReflection()->is($this->resolveName($node)))
)
) {
return true;
}

if (!$node instanceof $expressionToInvalidateClass) {
return false;
}
Expand Down
9 changes: 7 additions & 2 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -2597,6 +2597,13 @@ static function (): void {
$throwPoints[] = ThrowPoint::createImplicit($scope, $expr);
}

if (
$parametersAcceptor instanceof ClosureType && count($parametersAcceptor->getImpurePoints()) > 0
&& $scope->isInClass()
Comment on lines +2601 to +2602
Copy link
Contributor Author

@staabm staabm Apr 20, 2025

Choose a reason for hiding this comment

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

last open question: do we want to invalidate 'this' in this case for regular function calls with side-effects?
we only handle Closure at this point right now

see #3950 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Scratch that. A function is pretty unlikely to affect $this or self

) {
$scope = $scope->invalidateExpression(new Variable('this'), true);
}

if (
$functionReflection !== null
&& in_array($functionReflection->getName(), ['json_encode', 'json_decode'], true)
Expand Down Expand Up @@ -3022,13 +3029,11 @@ static function (): void {

if (
$methodReflection !== null
&& !$methodReflection->isStatic()
&& (
$methodReflection->hasSideEffects()->yes()
|| $methodReflection->getName() === '__construct'
)
&& $scopeFunction instanceof MethodReflection
&& !$scopeFunction->isStatic()
&& $scope->isInClass()
&& $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName())
) {
Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Analyser/nsrt/bug-12902-non-strict.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ public function __construct()
assertNativeType('int', self::$i);

$this->impureCall();
assertType('int', self::$i); // should be float|int
assertNativeType('int', self::$i); // should be float|int
assertType('float|int', self::$i);
assertNativeType('float|int', self::$i);
}

public function doFoo(): void {
Expand Down
31 changes: 29 additions & 2 deletions tests/PHPStan/Analyser/nsrt/bug-12902.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ public function __construct()
assertNativeType('int', self::$i);

$this->impureCall();
assertType('int', self::$i); // should be float|int
assertNativeType('int', self::$i); // should be float|int
assertType('float|int', self::$i);
assertNativeType('float|int', self::$i);
}

public function doFoo(): void {
Expand All @@ -85,6 +85,33 @@ public function doFoo(): void {
public function impureCall(): void {}
}

class BaseClass
{
static protected int|float $i;
}

class UsesBaseClass extends BaseClass
{
public function __construct()
{
parent::$i = getInt();
assertType('int', parent::$i);
assertNativeType('int', parent::$i);

$this->impureCall();
assertType('float|int', parent::$i);
assertNativeType('float|int', parent::$i);
}

public function doFoo(): void {
assertType('float|int', parent::$i);
assertNativeType('float|int', parent::$i);
}

/** @phpstan-impure */
public function impureCall(): void {}
}

function getInt(): int {
return 1;
}
Original file line number Diff line number Diff line change
Expand Up @@ -930,4 +930,9 @@ public function testBug12593(): void
$this->analyse([__DIR__ . '/data/bug-12593.php'], []);
}

public function testBug3747(): void
{
$this->analyse([__DIR__ . '/data/bug-3747.php'], []);
}

}
27 changes: 27 additions & 0 deletions tests/PHPStan/Rules/Arrays/data/bug-3747.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace Bug3747;

class X {

/** @var array<string,string> $x */
private static array $x;

public function y(): void {

self::$x = [];

$this->z();

echo self::$x['foo'];

}

private function z(): void {
self::$x['foo'] = 'bar';
}

}

$x = new X();
$x->y();
Original file line number Diff line number Diff line change
Expand Up @@ -1011,4 +1011,9 @@ public function testBug12748(): void
$this->analyse([__DIR__ . '/data/bug-12748.php'], []);
}

public function testBug11019(): void
{
$this->analyse([__DIR__ . '/data/bug-11019.php'], []);
}

}
21 changes: 21 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-11019.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

namespace Bug11019;

class HelloWorld
{
public static int $a;

public function reset(): void
{
static::$a = rand(1,10);
}

public function sayHello(): void
{
$this->reset();
assert(static::$a === 1);
$this->reset();
assert(static::$a === 1);
}
}
15 changes: 15 additions & 0 deletions tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,19 @@ public function testBug6922b(): void
$this->analyse([__DIR__ . '/data/bug-6922b.php'], []);
}

public function testBug8523(): void
{
$this->analyse([__DIR__ . '/data/bug-8523.php'], []);
}

public function testBug8523b(): void
{
$this->analyse([__DIR__ . '/data/bug-8523b.php'], []);
}

public function testBug8523c(): void
{
$this->analyse([__DIR__ . '/data/bug-8523c.php'], []);
}

}
36 changes: 36 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-8523.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

namespace Bug8523;

class HelloWorld
{
public static ?HelloWorld $instance = null;

public static function bazz(): void
{
self::$instance = null;
}

public function foo(): void
{
self::$instance = new HelloWorld();

self::bazz();

self::$instance?->foo();
}

public function bar(): void
{
self::$instance = null;
}

public function baz(): void
{
self::$instance = new HelloWorld();

$this->bar();

self::$instance?->foo();
}
}
24 changes: 24 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-8523b.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php declare(strict_types = 1);

namespace Bug8523b;

class HelloWorld
{
public static ?HelloWorld $instance = null;

public function save(): void {
self::$instance = new HelloWorld();

$callback = static function(): void {
self::$instance = null;
};

$callback();

var_dump(self::$instance);

self::$instance?->save();
}
}

(new HelloWorld())->save();
26 changes: 26 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-8523c.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php declare(strict_types = 1);

namespace Bug8523c;

use function PHPStan\Testing\assertType;

class HelloWorld
{
public static ?HelloWorld $instance = null;

public function save(): void {
HelloWorld::$instance = new HelloWorld();

$callback = static function(): void {
HelloWorld::$instance = null;
};

$callback();

var_dump(HelloWorld::$instance);

HelloWorld::$instance?->save();
}
}

(new HelloWorld())->save();
Loading