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
15 changes: 11 additions & 4 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,13 @@ static function (): void {

if (
$methodReflection !== null
&& !$methodReflection->isStatic()
&& (
$methodReflection->hasSideEffects()->yes()
|| $methodReflection->getName() === '__construct'
|| (
!$methodReflection->isStatic()
&& $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();
12 changes: 12 additions & 0 deletions tests/PHPStan/Rules/Comparison/IfConstantConditionRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,16 @@ public function testBug4912(): void
$this->analyse([__DIR__ . '/data/bug-4912.php'], []);
}

public function testBug4864(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-4864.php'], []);
}

public function testBug8926(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-8926.php'], []);
}

}
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);
}
}
25 changes: 25 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-4864.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

namespace Bug4864;

class Example
{
/** @var mixed */
private $value;
private bool $isHandled;

public function fetchValue(callable $f): void
{
$this->isHandled = false;
$this->value = null;

(function () {
$this->isHandled = true;
$this->value = 'value';
})();

if ($this->isHandled) {
$f($this->value);
}
}
}
32 changes: 32 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-8926.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace Bug8926;

class Foo {
private bool $test;

/** @param int[] $arr */
function success(array $arr) : void {
$test = false;
(function($arr) use(&$test) {
$test = count($arr) == 1;
})($arr);

if ($test) {
echo "...\n";
}
}

/** @param int[] $arr */
function error(array $arr) : void {
$this->test = false;
(function($arr) {
$this->test = count($arr) == 1;
})($arr);


if ($this->test) {
echo "...\n";
}
}
}
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'], []);
}

}
14 changes: 14 additions & 0 deletions tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1232,4 +1232,18 @@ public function testBug1O580(): void
]);
}

public function testBug4443(): void
{
if (PHP_VERSION_ID < 80000) {
$this->markTestSkipped('Test requires PHP 8.0.');
}

$this->analyse([__DIR__ . '/data/bug-4443.php'], [
[
'Method Bug4443\HelloWorld::getArray() should return array<mixed> but returns array<mixed>|null.',
22,
],
]);
}

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

namespace Bug4443;

class HelloWorld
{
/** @var array<mixed> */
private static ?array $arr = null;

private static function setup(): void
{
self::$arr = null;
}

/** @return array<mixed> */
public static function getArray(): array
{
if (self::$arr === null) {
self::$arr = [];
self::setup();
}
return self::$arr;
}
}

HelloWorld::getArray();
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();
Loading
Loading