Skip to content

Commit f5c8d2e

Browse files
ondrejmirtesclaude
authored and
phpstan-bot
committed
Fix instanceof stored in variables not narrowing types in OR conditions
- Changed MutatingScope::addConditionalExpressions to merge instead of replace, so multiple instanceof assignments on the same target don't overwrite each other's conditional expression holders - Added resolveConditionalExpressions in TypeSpecifier to resolve scope conditional expressions during BooleanOr processing, enabling type narrowing to flow through variable-stored instanceof results - Added getConditionalExpressions() public getter to MutatingScope - Updated test expectations in multi-assign.php and bug-7716.php to reflect the now-correct transitive type narrowing - New regression test in tests/PHPStan/Analyser/nsrt/bug-9519.php Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b70fb0f commit f5c8d2e

File tree

5 files changed

+162
-8
lines changed

5 files changed

+162
-8
lines changed

‎src/Analyser/MutatingScope.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3274,7 +3274,11 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self
32743274
public function addConditionalExpressions(string $exprString, array $conditionalExpressionHolders): self
32753275
{
32763276
$conditionalExpressions = $this->conditionalExpressions;
3277-
$conditionalExpressions[$exprString] = $conditionalExpressionHolders;
3277+
if (isset($conditionalExpressions[$exprString])) {
3278+
$conditionalExpressions[$exprString] = array_merge($conditionalExpressions[$exprString], $conditionalExpressionHolders);
3279+
} else {
3280+
$conditionalExpressions[$exprString] = $conditionalExpressionHolders;
3281+
}
32783282
return $this->scopeFactory->create(
32793283
$this->context,
32803284
$this->isDeclareStrictTypes(),
@@ -3295,6 +3299,14 @@ public function addConditionalExpressions(string $exprString, array $conditional
32953299
);
32963300
}
32973301

3302+
/**
3303+
* @return array<string, ConditionalExpressionHolder[]>
3304+
*/
3305+
public function getConditionalExpressions(): array
3306+
{
3307+
return $this->conditionalExpressions;
3308+
}
3309+
32983310
public function exitFirstLevelStatements(): self
32993311
{
33003312
if (!$this->inFirstLevelStatement) {

‎src/Analyser/TypeSpecifier.php‎

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -756,7 +756,9 @@ public function specifyTypesInCondition(
756756
) {
757757
$types = $leftTypes->normalize($scope);
758758
} else {
759-
$types = $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope));
759+
$leftNormalized = $this->resolveConditionalExpressions($scope, $leftTypes->normalize($scope));
760+
$rightNormalized = $this->resolveConditionalExpressions($rightScope, $rightTypes->normalize($rightScope));
761+
$types = $leftNormalized->intersectWith($rightNormalized);
760762
}
761763
} else {
762764
$types = $leftTypes->unionWith($rightTypes);
@@ -2098,6 +2100,59 @@ private function processBooleanNotSureConditionalTypes(Scope $scope, SpecifiedTy
20982100
return [];
20992101
}
21002102

2103+
private function resolveConditionalExpressions(MutatingScope $scope, SpecifiedTypes $specifiedTypes): SpecifiedTypes
2104+
{
2105+
$sureTypes = $specifiedTypes->getSureTypes();
2106+
$specifiedExpressions = [];
2107+
foreach ($sureTypes as $exprString => [$expr, $type]) {
2108+
$specifiedExpressions[$exprString] = ExpressionTypeHolder::createYes($expr, $type);
2109+
}
2110+
2111+
if (count($specifiedExpressions) === 0) {
2112+
return $specifiedTypes;
2113+
}
2114+
2115+
$additionalSureTypes = [];
2116+
foreach ($scope->getConditionalExpressions() as $conditionalExprString => $conditionalExpressions) {
2117+
foreach ($conditionalExpressions as $conditionalExpression) {
2118+
foreach ($conditionalExpression->getConditionExpressionTypeHolders() as $holderExprString => $conditionalTypeHolder) {
2119+
if (!array_key_exists($holderExprString, $specifiedExpressions) || !$specifiedExpressions[$holderExprString]->equals($conditionalTypeHolder)) {
2120+
continue 2;
2121+
}
2122+
}
2123+
2124+
$holder = $conditionalExpression->getTypeHolder();
2125+
if (isset($additionalSureTypes[$conditionalExprString])) {
2126+
$additionalSureTypes[$conditionalExprString] = [
2127+
$holder->getExpr(),
2128+
TypeCombinator::intersect($additionalSureTypes[$conditionalExprString][1], $holder->getType()),
2129+
];
2130+
} else {
2131+
$additionalSureTypes[$conditionalExprString] = [$holder->getExpr(), $holder->getType()];
2132+
}
2133+
}
2134+
}
2135+
2136+
if (count($additionalSureTypes) === 0) {
2137+
return $specifiedTypes;
2138+
}
2139+
2140+
foreach ($additionalSureTypes as $additionalExprString => [$additionalExpr, $additionalType]) {
2141+
if (isset($sureTypes[$additionalExprString])) {
2142+
$sureTypes[$additionalExprString] = [$additionalExpr, TypeCombinator::union($sureTypes[$additionalExprString][1], $additionalType)];
2143+
} else {
2144+
$sureTypes[$additionalExprString] = [$additionalExpr, $additionalType];
2145+
}
2146+
}
2147+
2148+
$result = new SpecifiedTypes($sureTypes, $specifiedTypes->getSureNotTypes());
2149+
if ($specifiedTypes->shouldOverwrite()) {
2150+
$result = $result->setAlwaysOverwriteTypes();
2151+
}
2152+
2153+
return $result->setRootExpr($specifiedTypes->getRootExpr());
2154+
}
2155+
21012156
/**
21022157
* @return array{Expr, ConstantScalarType, Type}|null
21032158
*/

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public function sayHello(array $array): int
1515
$hasBar = isset($array['bar']) && $array['bar'] > 1;
1616

1717
if ($hasFoo) {
18-
assertType('array{foo?: int, bar?: int}', $array);
18+
assertType('array{foo: int, bar?: int}', $array);
1919
assertType('int<2, max>', $array['foo']);
2020
return $array['foo'];
2121
}
@@ -44,7 +44,7 @@ public function sayHello2(array $array): int
4444
}
4545

4646
if ($hasBar) {
47-
assertType('array{foo?: int, bar?: int}', $array);
47+
assertType('array{foo?: int, bar: int}', $array);
4848
assertType('int<2, max>', $array['bar']);
4949
return $array['bar'];
5050
}

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

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug9519;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class ClassA {
8+
public function sayHello(): void
9+
{
10+
echo 'Hello';
11+
}
12+
}
13+
14+
class ClassB {
15+
public function sayHello(): void
16+
{
17+
echo 'Hello';
18+
}
19+
}
20+
21+
function test1(mixed $obj): void {
22+
$isA = $obj instanceof ClassA;
23+
$isB = $obj instanceof ClassB;
24+
25+
if ($isA || $isB) {
26+
assertType('Bug9519\ClassA|Bug9519\ClassB', $obj);
27+
}
28+
}
29+
30+
function test2(mixed $obj): void {
31+
// Direct instanceof in condition should work (and already does)
32+
if (($obj instanceof ClassA) || ($obj instanceof ClassB)) {
33+
assertType('Bug9519\ClassA|Bug9519\ClassB', $obj);
34+
}
35+
}
36+
37+
function test3(mixed $obj): void {
38+
$isA = $obj instanceof ClassA;
39+
$isB = $obj instanceof ClassB;
40+
41+
if ($isA) {
42+
assertType('Bug9519\ClassA', $obj);
43+
}
44+
45+
if ($isB) {
46+
assertType('Bug9519\ClassB', $obj);
47+
}
48+
}
49+
50+
interface SomeInterface {
51+
public function test(): void;
52+
}
53+
54+
class ObjectClass {
55+
}
56+
57+
class OtherClass extends ObjectClass {
58+
}
59+
60+
/**
61+
* @template T of object
62+
* @param class-string<T> $class_name
63+
* @return T
64+
*/
65+
function getObject(string $class_name): object {
66+
return new $class_name;
67+
}
68+
69+
function test4(): void {
70+
$obj = getObject(ObjectClass::class);
71+
$is_other = $obj instanceof OtherClass;
72+
$is_interface = $obj instanceof SomeInterface;
73+
74+
if ($is_interface) {
75+
assertType('Bug9519\ObjectClass&Bug9519\SomeInterface', $obj);
76+
}
77+
}
78+
79+
function test5(): void {
80+
$obj = getObject(ObjectClass::class);
81+
$is_interface = $obj instanceof SomeInterface;
82+
$is_other = $obj instanceof OtherClass;
83+
84+
if ($is_interface) {
85+
assertType('Bug9519\ObjectClass&Bug9519\SomeInterface', $obj);
86+
}
87+
}

‎tests/PHPStan/Analyser/nsrt/multi-assign.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,12 @@ function (bool $b): void {
5555
function (bool $b): void {
5656
$foo = $bar = $baz = $b;
5757
if ($bar) {
58-
assertType('bool', $b);
58+
assertType('true', $b);
5959
assertType('bool', $foo);
6060
assertType('true', $bar);
6161
assertType('bool', $baz);
6262
} else {
63-
assertType('bool', $b);
63+
assertType('false', $b);
6464
assertType('bool', $foo);
6565
assertType('false', $bar);
6666
assertType('bool', $baz);
@@ -70,12 +70,12 @@ function (bool $b): void {
7070
function (bool $b): void {
7171
$foo = $bar = $baz = $b;
7272
if ($baz) {
73-
assertType('bool', $b);
73+
assertType('true', $b);
7474
assertType('bool', $foo);
7575
assertType('bool', $bar);
7676
assertType('true', $baz);
7777
} else {
78-
assertType('bool', $b);
78+
assertType('false', $b);
7979
assertType('bool', $foo);
8080
assertType('bool', $bar);
8181
assertType('false', $baz);

0 commit comments

Comments
 (0)