Skip to content

Commit 11dddc2

Browse files
ondrejmirtesclaude
andcommitted
Project stored-boolean narrowings through || truthy
`if (A || B)` truthy where `A` and `B` are booleans backed by stored conditional-expression holders (e.g. `$isA = $obj instanceof ClassA; $isB = $obj instanceof ClassB;`) needs to apply the held narrowings of the *target* (`$obj`) to the OR-truthy scope as the union of the per-arm narrowings. specifyTypesInCondition for each arm only sees the boolean variable itself, so the `$obj` narrowing was invisible until a later inner check pinned one of the booleans down. For each conditional-holder target we now resolve its type in the left-truthy and right-truthy filtered scopes, and when both narrow it strictly below the original we add a sure type carrying the union to the OR-truthy result. Gated to keep certainty unchanged: only project when the target stays Yes-defined in all three scopes — `if (empty($a['bar']))` for instance must leave `$a` Maybe-defined because `empty()` tolerates undefined offsets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6885529 commit 11dddc2

3 files changed

Lines changed: 149 additions & 0 deletions

File tree

src/Analyser/MutatingScope.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3348,6 +3348,14 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self
33483348
);
33493349
}
33503350

3351+
/**
3352+
* @return array<string, ConditionalExpressionHolder[]>
3353+
*/
3354+
public function getConditionalExpressions(): array
3355+
{
3356+
return $this->conditionalExpressions;
3357+
}
3358+
33513359
/**
33523360
* @param ConditionalExpressionHolder[] $conditionalExpressionHolders
33533361
*/

src/Analyser/TypeSpecifier.php

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
use PHPStan\Type\TypeTraverser;
8383
use PHPStan\Type\UnionType;
8484
use function array_key_exists;
85+
use function array_key_first;
8586
use function array_last;
8687
use function array_map;
8788
use function array_merge;
@@ -783,6 +784,7 @@ public function specifyTypesInCondition(
783784
$types = $leftTypes->normalize($scope);
784785
} else {
785786
$types = $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope));
787+
$types = $this->augmentBooleanOrTruthyWithConditionalHolders($scope, $rightScope, $expr, $types);
786788
}
787789
} else {
788790
$types = $leftTypes->unionWith($rightTypes);
@@ -1941,6 +1943,81 @@ private function specifyTypesFromCallableCall(TypeSpecifierContext $context, Fun
19411943
return $this->specifyTypesFromAsserts($context, $call, $asserts, $parametersAcceptor, $scope);
19421944
}
19431945

1946+
/**
1947+
* For `if ($a || $b)` truthy, expressions narrowed by stored conditional
1948+
* holders (e.g. `$a = $obj instanceof ClassA;` records "when `$a` is
1949+
* truthy, `$obj` is `ClassA`") need to be projected into the OR-truthy
1950+
* scope as the union of the per-arm narrowings. specifyTypesInCondition
1951+
* for each arm only looks at the boolean variable itself, so the held
1952+
* narrowing of `$obj` would otherwise be invisible until a later check
1953+
* pins one of the booleans down.
1954+
*
1955+
* For each conditional-holder target $T:
1956+
* - resolve $T's type in the left-truthy and right-truthy filtered scopes
1957+
* - if both narrow $T strictly below the original, add `$T : leftT|rightT`
1958+
* as a sure type to the OR-truthy result
1959+
*
1960+
* The asymmetric case (one arm narrows, the other doesn't) is intentionally
1961+
* skipped: in the OR-truthy scope the arm that didn't narrow could still be
1962+
* the truthy one, so the sound result is the original (unnarrowed) type.
1963+
*/
1964+
private function augmentBooleanOrTruthyWithConditionalHolders(MutatingScope $scope, MutatingScope $rightScope, BooleanOr|LogicalOr $expr, SpecifiedTypes $types): SpecifiedTypes
1965+
{
1966+
$leftTruthyScope = $scope->filterByTruthyValue($expr->left);
1967+
$rightTruthyScope = $rightScope->filterByTruthyValue($expr->right);
1968+
1969+
$seen = [];
1970+
foreach ([$scope, $rightScope] as $sourceScope) {
1971+
foreach ($sourceScope->getConditionalExpressions() as $exprString => $holders) {
1972+
if (isset($seen[$exprString])) {
1973+
continue;
1974+
}
1975+
if ($holders === []) {
1976+
continue;
1977+
}
1978+
$seen[$exprString] = true;
1979+
$targetExpr = $holders[array_key_first($holders)]->getTypeHolder()->getExpr();
1980+
1981+
// Only project when the target stays Yes-defined in the original
1982+
// scope and in both filtered branches. A sure type implicitly
1983+
// raises certainty to Yes, which would wrongly upgrade Maybe-defined
1984+
// variables — `if (empty($a['bar']))` for instance leaves `$a`
1985+
// Maybe-defined because `empty()` tolerates undefined offsets.
1986+
if (!$scope->hasExpressionType($targetExpr)->yes()) {
1987+
continue;
1988+
}
1989+
if (!$leftTruthyScope->hasExpressionType($targetExpr)->yes()) {
1990+
continue;
1991+
}
1992+
if (!$rightTruthyScope->hasExpressionType($targetExpr)->yes()) {
1993+
continue;
1994+
}
1995+
1996+
$origType = $scope->getType($targetExpr);
1997+
$leftType = $leftTruthyScope->getType($targetExpr);
1998+
$rightType = $rightTruthyScope->getType($targetExpr);
1999+
2000+
$leftNarrowed = !$leftType->equals($origType) && $origType->isSuperTypeOf($leftType)->yes();
2001+
$rightNarrowed = !$rightType->equals($origType) && $origType->isSuperTypeOf($rightType)->yes();
2002+
2003+
if (!$leftNarrowed || !$rightNarrowed) {
2004+
continue;
2005+
}
2006+
2007+
$unionType = TypeCombinator::union($leftType, $rightType);
2008+
if ($unionType->equals($origType)) {
2009+
continue;
2010+
}
2011+
2012+
$types = $types->unionWith(
2013+
$this->create($targetExpr, $unionType, TypeSpecifierContext::createTrue(), $scope),
2014+
);
2015+
}
2016+
}
2017+
2018+
return $types;
2019+
}
2020+
19442021
/**
19452022
* @return array<string, ConditionalExpressionHolder[]>
19462023
*/
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<?php
2+
3+
namespace Bug9519;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
class ClassA {}
8+
class ClassB {}
9+
10+
function instanceofVariants(mixed $obj): void
11+
{
12+
$isA = $obj instanceof ClassA;
13+
$isB = $obj instanceof ClassB;
14+
15+
if ($isA || $isB) {
16+
assertType('Bug9519\\ClassA|Bug9519\\ClassB', $obj);
17+
}
18+
19+
// Sanity check: the equivalent inline form has always worked, so the
20+
// stored-boolean form should produce the same narrowing.
21+
if (($obj instanceof ClassA) || ($obj instanceof ClassB)) {
22+
assertType('Bug9519\\ClassA|Bug9519\\ClassB', $obj);
23+
}
24+
}
25+
26+
/**
27+
* Three-way OR over stored booleans — every arm narrows the same target.
28+
*/
29+
class ClassC {}
30+
31+
function threeWayInstanceof(mixed $obj): void
32+
{
33+
$isA = $obj instanceof ClassA;
34+
$isB = $obj instanceof ClassB;
35+
$isC = $obj instanceof ClassC;
36+
37+
if ($isA || $isB || $isC) {
38+
assertType('Bug9519\\ClassA|Bug9519\\ClassB|Bug9519\\ClassC', $obj);
39+
}
40+
}
41+
42+
/**
43+
* Different narrowing kinds across the OR's arms — `null !==` on the left,
44+
* `instanceof` on the right.
45+
*/
46+
function mixedNarrowingKinds(?ClassA $a, mixed $b): void
47+
{
48+
$aNotNull = $a !== null;
49+
$bIsB = $b instanceof ClassB;
50+
51+
if ($aNotNull || $bIsB) {
52+
// Inside the truthy branch we don't know which arm fired, so each
53+
// target keeps the union of (narrowed-when-its-arm-fired)
54+
// and (original-when-the-other-arm-fired).
55+
assertType(
56+
'Bug9519\\ClassA|null',
57+
$a,
58+
);
59+
assertType(
60+
'mixed',
61+
$b,
62+
);
63+
}
64+
}

0 commit comments

Comments
 (0)