Skip to content

Commit 21b91c0

Browse files
phpstan-botclaude
andcommitted
Remove useSubtypeForConditionMatching flag, use two-pass matching instead
Instead of a flag on ConditionalExpressionHolder to control whether condition matching uses equals() or isSuperTypeOf(), use a two-pass approach in filterBySpecifiedTypes(): Pass 1: exact matching via equals() (existing behavior) Pass 2: isSuperTypeOf for condition types with finite types (fallback) Pass 2 only runs when pass 1 found no matches for a target expression. This handles conditional parameter types where the condition is a union (e.g. 'value1'|'value2') that can't match a narrowed type ('value1') via equals(), but should match via isSuperTypeOf. The two-pass approach avoids regressions from using isSuperTypeOf globally: when scope merging creates both a specific condition (e.g. "if $key=2, then $value is Yes") and a broader condition (e.g. "if $key=0|2, then $value is Maybe"), exact matching in pass 1 prevents the broader condition from degrading variable certainty through extremeIdentity. The bug-5051 test expectations are updated to reflect more precise type narrowing: when narrowing $data to a specific constant, PHPStan now correctly determines which branch was taken and narrows related variables accordingly (e.g. $update becomes 'false' instead of 'bool' when $data === 1 because that branch always sets $update = false). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 151b7b8 commit 21b91c0

File tree

3 files changed

+39
-27
lines changed

3 files changed

+39
-27
lines changed

src/Analyser/ConditionalExpressionHolder.php

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ final class ConditionalExpressionHolder
1717
public function __construct(
1818
private array $conditionExpressionTypeHolders,
1919
private ExpressionTypeHolder $typeHolder,
20-
private bool $useSubtypeForConditionMatching = false,
2120
)
2221
{
2322
if (count($conditionExpressionTypeHolders) === 0) {
@@ -38,11 +37,6 @@ public function getTypeHolder(): ExpressionTypeHolder
3837
return $this->typeHolder;
3938
}
4039

41-
public function useSubtypeForConditionMatching(): bool
42-
{
43-
return $this->useSubtypeForConditionMatching;
44-
}
45-
4640
public function getKey(): string
4741
{
4842
$parts = [];

src/Analyser/MutatingScope.php

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,12 +1760,12 @@ private function enterFunctionLike(
17601760

17611761
$holder = new ConditionalExpressionHolder([
17621762
$parameterType->getParameterName() => ExpressionTypeHolder::createYes(new Variable($targetParameterName), TypeCombinator::intersect($targetParameter->getType(), $parameterType->getTarget())),
1763-
], ExpressionTypeHolder::createYes(new Variable($parameter->getName()), $ifType), true);
1763+
], ExpressionTypeHolder::createYes(new Variable($parameter->getName()), $ifType));
17641764
$conditionalTypes['$' . $parameter->getName()][$holder->getKey()] = $holder;
17651765

17661766
$holder = new ConditionalExpressionHolder([
17671767
$parameterType->getParameterName() => ExpressionTypeHolder::createYes(new Variable($targetParameterName), TypeCombinator::remove($targetParameter->getType(), $parameterType->getTarget())),
1768-
], ExpressionTypeHolder::createYes(new Variable($parameter->getName()), $elseType), true);
1768+
], ExpressionTypeHolder::createYes(new Variable($parameter->getName()), $elseType));
17691769
$conditionalTypes['$' . $parameter->getName()][$holder->getKey()] = $holder;
17701770
}
17711771
}
@@ -3216,28 +3216,46 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self
32163216
if (array_key_exists($conditionalExprString, $conditions)) {
32173217
continue;
32183218
}
3219+
3220+
// Pass 1: exact matching via equals()
32193221
foreach ($conditionalExpressions as $conditionalExpression) {
32203222
foreach ($conditionalExpression->getConditionExpressionTypeHolders() as $holderExprString => $conditionalTypeHolder) {
3221-
if (!array_key_exists($holderExprString, $specifiedExpressions)) {
3222-
continue 2;
3223-
}
3224-
$specifiedHolder = $specifiedExpressions[$holderExprString];
3225-
if (!$specifiedHolder->getCertainty()->equals($conditionalTypeHolder->getCertainty())) {
3223+
if (!array_key_exists($holderExprString, $specifiedExpressions) || !$specifiedExpressions[$holderExprString]->equals($conditionalTypeHolder)) {
32263224
continue 2;
32273225
}
3228-
if ($conditionalExpression->useSubtypeForConditionMatching()) {
3229-
if (!$conditionalTypeHolder->getType()->isSuperTypeOf($specifiedHolder->getType())->yes()) {
3226+
}
3227+
3228+
$conditions[$conditionalExprString][] = $conditionalExpression;
3229+
$specifiedExpressions[$conditionalExprString] = $conditionalExpression->getTypeHolder();
3230+
}
3231+
3232+
// Pass 2: for condition types with finite types, use isSuperTypeOf
3233+
// This handles cases like conditional parameter types where the condition
3234+
// is a union (e.g. 'value1'|'value2') that won't match a narrowed type
3235+
// (e.g. 'value1') via equals(), but should match via isSuperTypeOf.
3236+
// Only attempted when pass 1 found no matches, to avoid conflicts with
3237+
// broader conditions that have lower certainty from scope merging.
3238+
if (!array_key_exists($conditionalExprString, $conditions)) {
3239+
foreach ($conditionalExpressions as $conditionalExpression) {
3240+
foreach ($conditionalExpression->getConditionExpressionTypeHolders() as $holderExprString => $conditionalTypeHolder) {
3241+
if (!array_key_exists($holderExprString, $specifiedExpressions)) {
32303242
continue 2;
32313243
}
3232-
} else {
3233-
if (!$specifiedHolder->equals($conditionalTypeHolder)) {
3244+
$specifiedHolder = $specifiedExpressions[$holderExprString];
3245+
if (!$specifiedHolder->getCertainty()->equals($conditionalTypeHolder->getCertainty())) {
3246+
continue 2;
3247+
}
3248+
if (count($conditionalTypeHolder->getType()->getFiniteTypes()) === 0) {
3249+
continue 2;
3250+
}
3251+
if (!$conditionalTypeHolder->getType()->isSuperTypeOf($specifiedHolder->getType())->yes()) {
32343252
continue 2;
32353253
}
32363254
}
3237-
}
32383255

3239-
$conditions[$conditionalExprString][] = $conditionalExpression;
3240-
$specifiedExpressions[$conditionalExprString] = $conditionalExpression->getTypeHolder();
3256+
$conditions[$conditionalExprString][] = $conditionalExpression;
3257+
$specifiedExpressions[$conditionalExprString] = $conditionalExpression->getTypeHolder();
3258+
}
32413259
}
32423260
}
32433261
}

tests/PHPStan/Analyser/nsrt/bug-5051.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,35 +60,35 @@ public function testWithBooleans($data): void
6060
assertType('bool', $update);
6161
} else {
6262
assertType('1|2', $data);
63-
assertType('bool', $update);
63+
assertType('false', $update);
6464
}
6565

6666
if ($data === 1) {
67-
assertType('bool', $update);
68-
assertType('bool', $foo);
67+
assertType('false', $update);
68+
assertType('false', $foo);
6969
} else {
7070
assertType('bool', $update);
7171
assertType('bool', $foo);
7272
}
7373

7474
if ($data === 2) {
75-
assertType('bool', $update);
76-
assertType('bool', $foo);
75+
assertType('false', $update);
76+
assertType('false', $foo);
7777
} else {
7878
assertType('bool', $update);
7979
assertType('bool', $foo);
8080
}
8181

8282
if ($data === 3) {
83-
assertType('bool', $update);
83+
assertType('false', $update);
8484
assertType('true', $foo);
8585
} else {
8686
assertType('bool', $update);
8787
assertType('bool', $foo);
8888
}
8989

9090
if ($data === 1 || $data === 2) {
91-
assertType('bool', $update);
91+
assertType('false', $update);
9292
assertType('false', $foo);
9393
} else {
9494
assertType('bool', $update);

0 commit comments

Comments
 (0)