Skip to content

Commit 42c0937

Browse files
VincentLangletVincentLanglet
authored
Track more expressions (#5596)
1 parent 196eba6 commit 42c0937

9 files changed

Lines changed: 249 additions & 58 deletions

File tree

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3917,7 +3917,7 @@ private function tryProcessUnrolledConstantArrayForeach(
39173917
$matchedNativeArrays = count($nativeConstantArrays) === count($constantArrays) ? $nativeConstantArrays : null;
39183918

39193919
$valueVarName = $stmt->valueVar->name;
3920-
$keyVarName = $stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name) ? $stmt->keyVar->name : null;
3920+
$keyVarName = $stmt->keyVar instanceof Variable ? $stmt->keyVar->name : null;
39213921

39223922
$allBodyScopes = [];
39233923
$allChainScopes = [];
@@ -4051,10 +4051,7 @@ private function enterForeach(MutatingScope $scope, ExpressionResultStorage $sto
40514051
($stmt->valueVar instanceof Variable && is_string($stmt->valueVar->name))
40524052
&& ($stmt->keyVar === null || ($stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name)))
40534053
) {
4054-
$keyVarName = null;
4055-
if ($stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name)) {
4056-
$keyVarName = $stmt->keyVar->name;
4057-
}
4054+
$keyVarName = $stmt->keyVar instanceof Variable ? $stmt->keyVar->name : null;
40584055
$scope = $scope->enterForeach(
40594056
$originalScope,
40604057
$stmt->expr,

‎src/Analyser/TypeSpecifier.php‎

Lines changed: 35 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
use PHPStan\Type\UnionType;
8484
use function array_key_exists;
8585
use function array_key_first;
86+
use function array_keys;
8687
use function array_last;
8788
use function array_map;
8889
use function array_merge;
@@ -596,7 +597,7 @@ public function specifyTypesInCondition(
596597
}
597598

598599
return $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope);
599-
} elseif ($expr instanceof FuncCall && !($expr->name instanceof Name)) {
600+
} elseif ($expr instanceof FuncCall) {
600601
$specifiedTypes = $this->specifyTypesFromCallableCall($context, $expr, $scope);
601602
if ($specifiedTypes !== null) {
602603
return $specifiedTypes;
@@ -754,10 +755,10 @@ public function specifyTypesInCondition(
754755
$result = $result->setAlwaysOverwriteTypes();
755756
}
756757
return $result->setNewConditionalExpressionHolders(array_merge(
757-
$this->processBooleanNotSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders),
758-
$this->processBooleanNotSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders),
759-
$this->processBooleanSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders),
760-
$this->processBooleanSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders),
758+
$this->processBooleanNotSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, $rightScope),
759+
$this->processBooleanNotSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, $scope),
760+
$this->processBooleanSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, $rightScope),
761+
$this->processBooleanSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, $scope),
761762
))->setRootExpr($expr);
762763
}
763764

@@ -804,10 +805,10 @@ public function specifyTypesInCondition(
804805
$result = $result->setAlwaysOverwriteTypes();
805806
}
806807
return $result->setNewConditionalExpressionHolders(array_merge(
807-
$this->processBooleanNotSureConditionalTypes($scope, $leftTypes, $rightTypes),
808-
$this->processBooleanNotSureConditionalTypes($scope, $rightTypes, $leftTypes),
809-
$this->processBooleanSureConditionalTypes($scope, $leftTypes, $rightTypes),
810-
$this->processBooleanSureConditionalTypes($scope, $rightTypes, $leftTypes),
808+
$this->processBooleanNotSureConditionalTypes($scope, $leftTypes, $rightTypes, $rightScope),
809+
$this->processBooleanNotSureConditionalTypes($scope, $rightTypes, $leftTypes, $scope),
810+
$this->processBooleanSureConditionalTypes($scope, $leftTypes, $rightTypes, $rightScope),
811+
$this->processBooleanSureConditionalTypes($scope, $rightTypes, $leftTypes, $scope),
811812
))->setRootExpr($expr);
812813
}
813814

@@ -2079,14 +2080,11 @@ private function augmentBooleanOrTruthyWithConditionalHolders(MutatingScope $sco
20792080
/**
20802081
* @return array<string, ConditionalExpressionHolder[]>
20812082
*/
2082-
private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes $leftTypes, SpecifiedTypes $rightTypes): array
2083+
private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes $leftTypes, SpecifiedTypes $rightTypes, Scope $rightScope): array
20832084
{
20842085
$conditionExpressionTypes = [];
20852086
foreach ($leftTypes->getSureTypes() as $exprString => [$expr, $type]) {
2086-
if (!$expr instanceof Expr\Variable) {
2087-
continue;
2088-
}
2089-
if (!is_string($expr->name)) {
2087+
if (!$this->isTrackableExpression($expr)) {
20902088
continue;
20912089
}
20922090

@@ -2105,10 +2103,7 @@ private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes
21052103
if (count($conditionExpressionTypes) > 0) {
21062104
$holders = [];
21072105
foreach ($rightTypes->getSureTypes() as $exprString => [$expr, $type]) {
2108-
if (!$expr instanceof Expr\Variable) {
2109-
continue;
2110-
}
2111-
if (!is_string($expr->name)) {
2106+
if (!$this->isTrackableExpression($expr)) {
21122107
continue;
21132108
}
21142109

@@ -2117,28 +2112,21 @@ private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes
21172112
}
21182113

21192114
$conditions = $conditionExpressionTypes;
2120-
foreach ($conditions as $conditionExprString => $conditionExprTypeHolder) {
2121-
$conditionExpr = $conditionExprTypeHolder->getExpr();
2122-
if (!$conditionExpr instanceof Expr\Variable) {
2123-
continue;
2124-
}
2125-
if (!is_string($conditionExpr->name)) {
2115+
foreach (array_keys($conditions) as $conditionExprString) {
2116+
if ($conditionExprString !== $exprString) {
21262117
continue;
21272118
}
2128-
if ($conditionExpr->name !== $expr->name) {
2129-
continue;
2130-
}
2131-
21322119
unset($conditions[$conditionExprString]);
21332120
}
21342121

21352122
if (count($conditions) === 0) {
21362123
continue;
21372124
}
21382125

2126+
$targetScope = $expr instanceof Expr\Variable ? $scope : $rightScope;
21392127
$holder = new ConditionalExpressionHolder(
21402128
$conditions,
2141-
ExpressionTypeHolder::createYes($expr, TypeCombinator::intersect($scope->getType($expr), $type)),
2129+
ExpressionTypeHolder::createYes($expr, TypeCombinator::intersect($targetScope->getType($expr), $type)),
21422130
);
21432131
$holders[$exprString][$holder->getKey()] = $holder;
21442132
}
@@ -2149,6 +2137,17 @@ private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes
21492137
return [];
21502138
}
21512139

2140+
private function isTrackableExpression(Expr $expr): bool
2141+
{
2142+
if ($expr instanceof Expr\Variable) {
2143+
return is_string($expr->name);
2144+
}
2145+
2146+
return $expr instanceof Expr\PropertyFetch
2147+
|| $expr instanceof Expr\ArrayDimFetch
2148+
|| $expr instanceof Expr\StaticPropertyFetch;
2149+
}
2150+
21522151
/**
21532152
* Flatten a deep BooleanOr chain into leaf expressions and process them
21542153
* without recursive filterByFalseyValue calls. This reduces O(n^2) to O(n)
@@ -2279,14 +2278,11 @@ private function specifyTypesForFlattenedBooleanAnd(
22792278
/**
22802279
* @return array<string, ConditionalExpressionHolder[]>
22812280
*/
2282-
private function processBooleanNotSureConditionalTypes(Scope $scope, SpecifiedTypes $leftTypes, SpecifiedTypes $rightTypes): array
2281+
private function processBooleanNotSureConditionalTypes(Scope $scope, SpecifiedTypes $leftTypes, SpecifiedTypes $rightTypes, Scope $rightScope): array
22832282
{
22842283
$conditionExpressionTypes = [];
22852284
foreach ($leftTypes->getSureNotTypes() as $exprString => [$expr, $type]) {
2286-
if (!$expr instanceof Expr\Variable) {
2287-
continue;
2288-
}
2289-
if (!is_string($expr->name)) {
2285+
if (!$this->isTrackableExpression($expr)) {
22902286
continue;
22912287
}
22922288

@@ -2299,10 +2295,7 @@ private function processBooleanNotSureConditionalTypes(Scope $scope, SpecifiedTy
22992295
if (count($conditionExpressionTypes) > 0) {
23002296
$holders = [];
23012297
foreach ($rightTypes->getSureNotTypes() as $exprString => [$expr, $type]) {
2302-
if (!$expr instanceof Expr\Variable) {
2303-
continue;
2304-
}
2305-
if (!is_string($expr->name)) {
2298+
if (!$this->isTrackableExpression($expr)) {
23062299
continue;
23072300
}
23082301

@@ -2311,28 +2304,21 @@ private function processBooleanNotSureConditionalTypes(Scope $scope, SpecifiedTy
23112304
}
23122305

23132306
$conditions = $conditionExpressionTypes;
2314-
foreach ($conditions as $conditionExprString => $conditionExprTypeHolder) {
2315-
$conditionExpr = $conditionExprTypeHolder->getExpr();
2316-
if (!$conditionExpr instanceof Expr\Variable) {
2307+
foreach (array_keys($conditions) as $conditionExprString) {
2308+
if ($conditionExprString !== $exprString) {
23172309
continue;
23182310
}
2319-
if (!is_string($conditionExpr->name)) {
2320-
continue;
2321-
}
2322-
if ($conditionExpr->name !== $expr->name) {
2323-
continue;
2324-
}
2325-
23262311
unset($conditions[$conditionExprString]);
23272312
}
23282313

23292314
if (count($conditions) === 0) {
23302315
continue;
23312316
}
23322317

2318+
$targetScope = $expr instanceof Expr\Variable ? $scope : $rightScope;
23332319
$holder = new ConditionalExpressionHolder(
23342320
$conditions,
2335-
ExpressionTypeHolder::createYes($expr, TypeCombinator::remove($scope->getType($expr), $type)),
2321+
ExpressionTypeHolder::createYes($expr, TypeCombinator::remove($targetScope->getType($expr), $type)),
23362322
);
23372323
$holders[$exprString][$holder->getKey()] = $holder;
23382324
}

‎src/Type/Constant/ConstantArrayTypeBuilder.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,6 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
120120
}
121121

122122
if ($offsetType === null) {
123-
if (count($this->nextAutoIndexes) === 0) {
124-
return;
125-
}
126-
127123
$newAutoIndexes = $optional ? $this->nextAutoIndexes : [];
128124
$hasOptional = false;
129125
foreach ($this->keyTypes as $i => $keyType) {

‎tests/PHPStan/Analyser/nsrt/bug-12517.php‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<?php
2+
3+
namespace Bug12517;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class HelloWorld
8+
{
9+
public function sayHello(\stdClass $foo): void
10+
{
11+
if ($foo->a !== null || $foo->b !== null) {
12+
if ($foo->a === null) {
13+
assertType('null', $foo->a);
14+
assertType('mixed~null', $foo->b);
15+
}
16+
}
17+
18+
$a = $foo->a;
19+
$b = $foo->b;
20+
if ($a !== null || $b !== null) {
21+
if ($a === null) {
22+
assertType('null', $a);
23+
assertType('mixed~null', $b);
24+
}
25+
}
26+
}
27+
}
28+
29+
class Test
30+
{
31+
/** @var mixed */
32+
public static $a = null;
33+
/** @var mixed */
34+
public static $b = null;
35+
36+
public function sayHello(): void
37+
{
38+
if (Test::$a !== null || Test::$b !== null) {
39+
if (Test::$a === null) {
40+
assertType('null', Test::$a);
41+
assertType('mixed~null', Test::$b);
42+
}
43+
}
44+
45+
$a = Test::$a;
46+
$b = Test::$b;
47+
if ($a !== null || $b !== null) {
48+
if ($a === null) {
49+
assertType('null', $a);
50+
assertType('mixed~null', $b);
51+
}
52+
}
53+
}
54+
}
55+
56+
class WithArray
57+
{
58+
public function sayHello(array $array): void
59+
{
60+
if ($array['a'] !== null || $array['b'] !== null) {
61+
if ($array['a'] === null) {
62+
assertType('null', $array['a']);
63+
assertType('mixed~null', $array['b']);
64+
}
65+
}
66+
67+
$a = $array['a'];
68+
$b = $array['b'];
69+
if ($a !== null || $b !== null) {
70+
if ($a === null) {
71+
assertType('null', $a);
72+
assertType('mixed~null', $b);
73+
}
74+
}
75+
}
76+
}

‎tests/PHPStan/Analyser/nsrt/pr-5596.php‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
namespace Pr5596;
4+
5+
use PhpParser\Node\Expr\CallLike;
6+
use PhpParser\Node\Expr\FuncCall;
7+
use PhpParser\Node\Expr\MethodCall;
8+
use PhpParser\Node\Expr\New_;
9+
use PhpParser\Node\Expr\StaticCall;
10+
use PhpParser\Node\Identifier;
11+
use PhpParser\Node\Name;
12+
use PHPStan\Analyser\Scope;
13+
14+
use function PHPStan\Testing\assertType;
15+
16+
class Test
17+
{
18+
private function whitelistAllowedCallables(
19+
CallLike $node,
20+
Scope $scope
21+
): void
22+
{
23+
if ($node instanceof MethodCall && $node->name instanceof Identifier) {
24+
assertType('PhpParser\Node\Expr\MethodCall', $node);
25+
assertType('PhpParser\Node\Identifier', $node->name);
26+
assertType('PhpParser\Node\Expr', $node->var);
27+
} elseif ($node instanceof StaticCall && $node->name instanceof Identifier && $node->class instanceof Name) {
28+
assertType('PhpParser\Node\Expr\StaticCall', $node);
29+
assertType('PhpParser\Node\Identifier', $node->name);
30+
assertType('PhpParser\Node\Name', $node->class);
31+
} elseif ($node instanceof New_ && $node->class instanceof Name) {
32+
assertType('PhpParser\Node\Expr\New_', $node);
33+
assertType('PhpParser\Node\Name', $node->class);
34+
} elseif ($node instanceof FuncCall && $node->name instanceof Name) {
35+
assertType('PhpParser\Node\Expr\FuncCall', $node);
36+
assertType('PhpParser\Node\Name', $node->name);
37+
} elseif ($node instanceof FuncCall) {
38+
assertType('PhpParser\Node\Expr\FuncCall', $node);
39+
assertType('PhpParser\Node\Expr', $node->name);
40+
}
41+
}
42+
}

‎tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4009,6 +4009,14 @@ public function testBug10422(): void
40094009
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-10422.php'], []);
40104010
}
40114011

4012+
public function testBug9155(): void
4013+
{
4014+
$this->checkThisOnly = false;
4015+
$this->checkNullables = true;
4016+
$this->checkUnionTypes = true;
4017+
$this->analyse([__DIR__ . '/data/bug-9155.php'], []);
4018+
}
4019+
40124020
public function testBug13272(): void
40134021
{
40144022
$this->checkThisOnly = false;

‎tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -975,6 +975,12 @@ public function testBug12558(): void
975975
]);
976976
}
977977

978+
public function testBug6486(): void
979+
{
980+
$this->checkThisOnly = false;
981+
$this->analyse([__DIR__ . '/data/bug-6486.php'], []);
982+
}
983+
978984
#[RequiresPhp('>= 8.5.0')]
979985
public function testPipeOperator(): void
980986
{

0 commit comments

Comments
 (0)