Skip to content

Commit 1262b29

Browse files
takaramstaabm
authored andcommitted
Invalidate static expressions when a non-static expression is called
1 parent 925f29c commit 1262b29

File tree

4 files changed

+135
-1
lines changed

4 files changed

+135
-1
lines changed

src/Analyser/MutatingScope.php

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3656,6 +3656,67 @@ private function invalidateMethodsOnExpression(Expr $expressionToInvalidate): se
36563656
);
36573657
}
36583658

3659+
public function invalidateStaticMembers(string $className): self
3660+
{
3661+
if (!$this->reflectionProvider->hasClass($className)) {
3662+
return $this;
3663+
}
3664+
3665+
$classReflection = $this->reflectionProvider->getClass($className);
3666+
$classNamesToInvalidate = [strtolower($className)];
3667+
foreach ($classReflection->getParents() as $parentClass) {
3668+
$classNamesToInvalidate[] = strtolower($parentClass->getName());
3669+
}
3670+
3671+
$expressionTypes = $this->expressionTypes;
3672+
$nativeExpressionTypes = $this->nativeExpressionTypes;
3673+
$invalidated = false;
3674+
$nodeFinder = new NodeFinder();
3675+
foreach ($expressionTypes as $exprString => $exprTypeHolder) {
3676+
$expr = $exprTypeHolder->getExpr();
3677+
$found = $nodeFinder->findFirst([$expr], static function (Node $node) use ($classNamesToInvalidate): bool {
3678+
if (!$node instanceof Expr\StaticCall && !$node instanceof Expr\StaticPropertyFetch) {
3679+
return false;
3680+
}
3681+
if (!$node->class instanceof Name || !$node->class->isFullyQualified()) {
3682+
return false;
3683+
}
3684+
3685+
return in_array($node->class->toLowerString(), $classNamesToInvalidate, true);
3686+
});
3687+
if ($found === null) {
3688+
continue;
3689+
}
3690+
3691+
unset($expressionTypes[$exprString]);
3692+
unset($nativeExpressionTypes[$exprString]);
3693+
$invalidated = true;
3694+
}
3695+
3696+
if (!$invalidated) {
3697+
return $this;
3698+
}
3699+
3700+
return $this->scopeFactory->create(
3701+
$this->context,
3702+
$this->isDeclareStrictTypes(),
3703+
$this->getFunction(),
3704+
$this->getNamespace(),
3705+
$expressionTypes,
3706+
$nativeExpressionTypes,
3707+
$this->conditionalExpressions,
3708+
$this->inClosureBindScopeClasses,
3709+
$this->anonymousFunctionReflection,
3710+
$this->inFirstLevelStatement,
3711+
$this->currentlyAssignedExpressions,
3712+
$this->currentlyAllowedUndefinedExpressions,
3713+
[],
3714+
$this->afterExtractCall,
3715+
$this->parentScope,
3716+
$this->nativeTypesPromoted,
3717+
);
3718+
}
3719+
36593720
private function setExpressionCertainty(Expr $expr, TrinaryLogic $certainty): self
36603721
{
36613722
if ($this->hasExpressionType($expr)->no()) {

src/Analyser/NodeScopeResolver.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3215,9 +3215,16 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
32153215
$scope = $result->getScope();
32163216

32173217
if ($methodReflection !== null) {
3218-
if ($methodReflection->getName() === '__construct' || $methodReflection->hasSideEffects()->yes()) {
3218+
$hasSideEffects = $methodReflection->hasSideEffects()->yes();
3219+
if ($hasSideEffects || $methodReflection->getName() === '__construct') {
32193220
$this->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage);
32203221
$scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass());
3222+
if ($hasSideEffects) {
3223+
$classNames = $scope->getType($normalizedExpr->var)->getObjectClassNames();
3224+
foreach ($classNames as $className) {
3225+
$scope = $scope->invalidateStaticMembers($className);
3226+
}
3227+
}
32213228
}
32223229
if ($parametersAcceptor !== null && !$methodReflection->isStatic()) {
32233230
$selfOutType = $methodReflection->getSelfOutType();

tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,4 +1056,9 @@ public function testBug11609(): void
10561056
]);
10571057
}
10581058

1059+
public function testBug13416(): void
1060+
{
1061+
$this->analyse([__DIR__ . '/data/bug-13416.php'], []);
1062+
}
1063+
10591064
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?php declare(strict_types=1);
2+
3+
namespace Bug13416;
4+
5+
class MyRecord {
6+
/** @var array<int, self> */
7+
private static array $storage = [];
8+
9+
/** @phpstan-impure */
10+
public function insert(): void {
11+
self::$storage[] = $this;
12+
}
13+
14+
/**
15+
* @return array<int, self>
16+
* @phpstan-impure
17+
*/
18+
public static function find(): array {
19+
return self::$storage;
20+
}
21+
}
22+
23+
class AnotherRecord extends MyRecord {}
24+
25+
class PHPStanMinimalBug {
26+
public function testMinimalBug(): void {
27+
$msg1 = new MyRecord();
28+
$msg1->insert();
29+
30+
assert(
31+
count(MyRecord::find()) === 1,
32+
'should have 1 record initially'
33+
);
34+
35+
$msg2 = new MyRecord();
36+
$msg2->insert();
37+
38+
assert(
39+
count(MyRecord::find()) === 2,
40+
'should have 2 messages after adding one'
41+
);
42+
}
43+
44+
public function testMinimalBugChildClass(): void {
45+
$msg1 = new AnotherRecord();
46+
$msg1->insert();
47+
48+
assert(
49+
count(MyRecord::find()) === 1,
50+
'should have 1 record initially'
51+
);
52+
53+
$msg2 = new AnotherRecord();
54+
$msg2->insert();
55+
56+
assert(
57+
count(MyRecord::find()) === 2,
58+
'should have 2 messages after adding one'
59+
);
60+
}
61+
}

0 commit comments

Comments
 (0)