Skip to content

Commit 6de7bd1

Browse files
VincentLangletVincentLanglet
committed
Create cross-kind conditional expression holders in BooleanAnd/BooleanOr type specifier with truthy fallback for isset()
- Add `processBooleanNotSureWithSureConditionalTypes()` which builds conditions from sureNotTypes and holders from sureTypes (cross-kind pairing) - Add `processBooleanSureWithNotSureConditionalTypes()` which builds conditions from sureTypes and holders from sureNotTypes (reverse cross-kind pairing) - Call both new functions (with both argument orderings) in BooleanAnd falsey and BooleanOr truthy conditional holder creation - Add truthy fallback: when left/right falsey types produce empty SpecifiedTypes (e.g. isset() on non-constant array dim fetch), recompute in truthy context and swap sureTypes/sureNotTypes to derive conditions for holders - Guard the truthy fallback with `allExpressionsTrackable()` to prevent over-narrowing when non-trackable expressions (method calls) are involved - Fixes patterns like `if ($a && !is_string($y)) { throw; }` followed by `if ($a) { /* $y is now string */ }` and `if (isset($data['x']) && !is_string($data['x'])) { throw; }` followed by `$data['x'] ?? ''` being correctly inferred as string
1 parent 723081d commit 6de7bd1

2 files changed

Lines changed: 173 additions & 70 deletions

File tree

‎src/Analyser/TypeSpecifier.php‎

Lines changed: 62 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,18 @@ public function specifyTypesInCondition(
735735
$rightTypesForHolders = $this->specifyTypesInCondition($rightScope, $expr->right, TypeSpecifierContext::createFalsey())->setRootExpr($expr);
736736
}
737737
}
738+
if ($leftTypesForHolders->getSureTypes() === [] && $leftTypesForHolders->getSureNotTypes() === []) {
739+
$truthyLeftTypes = $this->specifyTypesInCondition($scope, $expr->left, TypeSpecifierContext::createTruthy());
740+
if ($this->allExpressionsTrackable($truthyLeftTypes)) {
741+
$leftTypesForHolders = new SpecifiedTypes($truthyLeftTypes->getSureNotTypes(), $truthyLeftTypes->getSureTypes());
742+
}
743+
}
744+
if ($rightTypesForHolders->getSureTypes() === [] && $rightTypesForHolders->getSureNotTypes() === []) {
745+
$truthyRightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, TypeSpecifierContext::createTruthy());
746+
if ($this->allExpressionsTrackable($truthyRightTypes)) {
747+
$rightTypesForHolders = new SpecifiedTypes($truthyRightTypes->getSureNotTypes(), $truthyRightTypes->getSureTypes());
748+
}
749+
}
738750
$result = new SpecifiedTypes(
739751
$types->getSureTypes(),
740752
$types->getSureNotTypes(),
@@ -743,10 +755,14 @@ public function specifyTypesInCondition(
743755
$result = $result->setAlwaysOverwriteTypes();
744756
}
745757
return $result->setNewConditionalExpressionHolders(array_merge(
746-
$this->processBooleanNotSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, $rightScope),
747-
$this->processBooleanNotSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, $scope),
748-
$this->processBooleanSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, $rightScope),
749-
$this->processBooleanSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, $scope),
758+
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, false, $rightTypesForHolders, false, $rightScope),
759+
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, false, $leftTypesForHolders, false, $scope),
760+
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, true, $rightTypesForHolders, true, $rightScope),
761+
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, true, $leftTypesForHolders, true, $scope),
762+
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, false, $rightTypesForHolders, true, $rightScope),
763+
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, false, $leftTypesForHolders, true, $scope),
764+
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, true, $rightTypesForHolders, false, $rightScope),
765+
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, true, $leftTypesForHolders, false, $scope),
750766
))->setRootExpr($expr);
751767
}
752768

@@ -796,10 +812,14 @@ public function specifyTypesInCondition(
796812
$result = $result->setAlwaysOverwriteTypes();
797813
}
798814
return $result->setNewConditionalExpressionHolders(array_merge(
799-
$this->processBooleanNotSureConditionalTypes($scope, $leftTypes, $rightTypes, $rightScope),
800-
$this->processBooleanNotSureConditionalTypes($scope, $rightTypes, $leftTypes, $scope),
801-
$this->processBooleanSureConditionalTypes($scope, $leftTypes, $rightTypes, $rightScope),
802-
$this->processBooleanSureConditionalTypes($scope, $rightTypes, $leftTypes, $scope),
815+
$this->processBooleanConditionalTypes($scope, $leftTypes, false, $rightTypes, false, $rightScope),
816+
$this->processBooleanConditionalTypes($scope, $rightTypes, false, $leftTypes, false, $scope),
817+
$this->processBooleanConditionalTypes($scope, $leftTypes, true, $rightTypes, true, $rightScope),
818+
$this->processBooleanConditionalTypes($scope, $rightTypes, true, $leftTypes, true, $scope),
819+
$this->processBooleanConditionalTypes($scope, $leftTypes, false, $rightTypes, true, $rightScope),
820+
$this->processBooleanConditionalTypes($scope, $rightTypes, false, $leftTypes, true, $scope),
821+
$this->processBooleanConditionalTypes($scope, $leftTypes, true, $rightTypes, false, $rightScope),
822+
$this->processBooleanConditionalTypes($scope, $rightTypes, true, $leftTypes, false, $scope),
803823
))->setRootExpr($expr);
804824
}
805825

@@ -2066,18 +2086,25 @@ private function augmentDisjunctionTypes(
20662086
/**
20672087
* @return array<string, ConditionalExpressionHolder[]>
20682088
*/
2069-
private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes $leftTypes, SpecifiedTypes $rightTypes, Scope $rightScope): array
2089+
private function processBooleanConditionalTypes(Scope $scope, SpecifiedTypes $conditionSpecifiedTypes, bool $conditionsFromSureTypes, SpecifiedTypes $holderSpecifiedTypes, bool $holdersFromSureTypes, Scope $rightScope): array
20702090
{
2091+
$conditionTypes = $conditionsFromSureTypes ? $conditionSpecifiedTypes->getSureTypes() : $conditionSpecifiedTypes->getSureNotTypes();
2092+
$holderTypes = $holdersFromSureTypes ? $holderSpecifiedTypes->getSureTypes() : $holderSpecifiedTypes->getSureNotTypes();
2093+
20712094
$conditionExpressionTypes = [];
2072-
foreach ($leftTypes->getSureTypes() as $exprString => [$expr, $type]) {
2095+
foreach ($conditionTypes as $exprString => [$expr, $type]) {
20732096
if (!$this->isTrackableExpression($expr)) {
20742097
continue;
20752098
}
20762099

2077-
$scopeType = $scope->getType($expr);
2078-
$conditionType = TypeCombinator::remove($scopeType, $type);
2079-
if ($scopeType->equals($conditionType)) {
2080-
continue;
2100+
if ($conditionsFromSureTypes) {
2101+
$scopeType = $scope->getType($expr);
2102+
$conditionType = TypeCombinator::remove($scopeType, $type);
2103+
if ($scopeType->equals($conditionType)) {
2104+
continue;
2105+
}
2106+
} else {
2107+
$conditionType = TypeCombinator::intersect($scope->getType($expr), $type);
20812108
}
20822109

20832110
$conditionExpressionTypes[$exprString] = ExpressionTypeHolder::createYes(
@@ -2088,7 +2115,7 @@ private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes
20882115

20892116
if (count($conditionExpressionTypes) > 0) {
20902117
$holders = [];
2091-
foreach ($rightTypes->getSureTypes() as $exprString => [$expr, $type]) {
2118+
foreach ($holderTypes as $exprString => [$expr, $type]) {
20922119
if (!$this->isTrackableExpression($expr)) {
20932120
continue;
20942121
}
@@ -2110,9 +2137,12 @@ private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes
21102137
}
21112138

21122139
$targetScope = $expr instanceof Expr\Variable ? $scope : $rightScope;
2140+
$holderType = $holdersFromSureTypes
2141+
? TypeCombinator::intersect($targetScope->getType($expr), $type)
2142+
: TypeCombinator::remove($targetScope->getType($expr), $type);
21132143
$holder = new ConditionalExpressionHolder(
21142144
$conditions,
2115-
ExpressionTypeHolder::createYes($expr, TypeCombinator::intersect($targetScope->getType($expr), $type)),
2145+
ExpressionTypeHolder::createYes($expr, $holderType),
21162146
);
21172147
$holders[$exprString][$holder->getKey()] = $holder;
21182148
}
@@ -2134,6 +2164,22 @@ private function isTrackableExpression(Expr $expr): bool
21342164
|| $expr instanceof Expr\StaticPropertyFetch;
21352165
}
21362166

2167+
private function allExpressionsTrackable(SpecifiedTypes $types): bool
2168+
{
2169+
foreach ($types->getSureTypes() as [$expr]) {
2170+
if (!$this->isTrackableExpression($expr)) {
2171+
return false;
2172+
}
2173+
}
2174+
foreach ($types->getSureNotTypes() as [$expr]) {
2175+
if (!$this->isTrackableExpression($expr)) {
2176+
return false;
2177+
}
2178+
}
2179+
2180+
return $types->getSureTypes() !== [] || $types->getSureNotTypes() !== [];
2181+
}
2182+
21372183
/**
21382184
* Flatten a deep BooleanOr chain into leaf expressions and process them
21392185
* without recursive filterByFalseyValue calls. This reduces O(n^2) to O(n)
@@ -2261,60 +2307,6 @@ private function specifyTypesForFlattenedBooleanAnd(
22612307
return (new SpecifiedTypes($sureTypes, $sureNotTypes))->setRootExpr($expr);
22622308
}
22632309

2264-
/**
2265-
* @return array<string, ConditionalExpressionHolder[]>
2266-
*/
2267-
private function processBooleanNotSureConditionalTypes(Scope $scope, SpecifiedTypes $leftTypes, SpecifiedTypes $rightTypes, Scope $rightScope): array
2268-
{
2269-
$conditionExpressionTypes = [];
2270-
foreach ($leftTypes->getSureNotTypes() as $exprString => [$expr, $type]) {
2271-
if (!$this->isTrackableExpression($expr)) {
2272-
continue;
2273-
}
2274-
2275-
$conditionExpressionTypes[$exprString] = ExpressionTypeHolder::createYes(
2276-
$expr,
2277-
TypeCombinator::intersect($scope->getType($expr), $type),
2278-
);
2279-
}
2280-
2281-
if (count($conditionExpressionTypes) > 0) {
2282-
$holders = [];
2283-
foreach ($rightTypes->getSureNotTypes() as $exprString => [$expr, $type]) {
2284-
if (!$this->isTrackableExpression($expr)) {
2285-
continue;
2286-
}
2287-
2288-
if (!isset($holders[$exprString])) {
2289-
$holders[$exprString] = [];
2290-
}
2291-
2292-
$conditions = $conditionExpressionTypes;
2293-
foreach (array_keys($conditions) as $conditionExprString) {
2294-
if ($conditionExprString !== $exprString) {
2295-
continue;
2296-
}
2297-
unset($conditions[$conditionExprString]);
2298-
}
2299-
2300-
if (count($conditions) === 0) {
2301-
continue;
2302-
}
2303-
2304-
$targetScope = $expr instanceof Expr\Variable ? $scope : $rightScope;
2305-
$holder = new ConditionalExpressionHolder(
2306-
$conditions,
2307-
ExpressionTypeHolder::createYes($expr, TypeCombinator::remove($targetScope->getType($expr), $type)),
2308-
);
2309-
$holders[$exprString][$holder->getKey()] = $holder;
2310-
}
2311-
2312-
return $holders;
2313-
}
2314-
2315-
return [];
2316-
}
2317-
23182310
/**
23192311
* @return array{Expr, ConstantScalarType, Type}|null
23202312
*/

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

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug10644;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
/**
8+
* @param array<string, mixed> $data
9+
*/
10+
function testIssetCoalesce(array $data): void
11+
{
12+
if (isset($data['subtitle']) && !is_string($data['subtitle'])) {
13+
throw new \InvalidArgumentException('Subtitle must be a string');
14+
}
15+
16+
if (isset($data['subtitle'])) {
17+
assertType("string", $data['subtitle']);
18+
}
19+
assertType("string", $data['subtitle'] ?? '');
20+
}
21+
22+
function testSimpleBool(bool $a, mixed $y): void
23+
{
24+
if ($a && !is_string($y)) {
25+
throw new \Exception();
26+
}
27+
28+
if ($a) {
29+
assertType("string", $y);
30+
}
31+
assertType("mixed", $y);
32+
}
33+
34+
function testSimpleInt(bool $a, mixed $y): void
35+
{
36+
if ($a && !is_int($y)) {
37+
throw new \Exception();
38+
}
39+
40+
if ($a) {
41+
assertType("int", $y);
42+
}
43+
}
44+
45+
function testSimpleArray(bool $a, mixed $y): void
46+
{
47+
if ($a && !is_array($y)) {
48+
throw new \Exception();
49+
}
50+
51+
if ($a) {
52+
assertType("array<mixed, mixed>", $y);
53+
}
54+
}
55+
56+
function testNotNull(?int $x, mixed $y): void
57+
{
58+
if ($x !== null && !is_string($y)) {
59+
throw new \Exception();
60+
}
61+
62+
if ($x !== null) {
63+
assertType("string", $y);
64+
}
65+
}
66+
67+
function testInstanceof(mixed $x, mixed $y): void
68+
{
69+
if ($x instanceof \stdClass && !is_int($y)) {
70+
throw new \Exception();
71+
}
72+
73+
if ($x instanceof \stdClass) {
74+
assertType("int", $y);
75+
}
76+
}
77+
78+
/**
79+
* @param array<string, mixed> $data
80+
*/
81+
function testIssetMultipleKeys(array $data): void
82+
{
83+
if (isset($data['a']) && !is_string($data['a'])) {
84+
throw new \Exception();
85+
}
86+
if (isset($data['b']) && !is_int($data['b'])) {
87+
throw new \Exception();
88+
}
89+
90+
if (isset($data['a'])) {
91+
assertType("string", $data['a']);
92+
}
93+
if (isset($data['b'])) {
94+
assertType("int", $data['b']);
95+
}
96+
assertType("string", $data['a'] ?? '');
97+
assertType("int", $data['b'] ?? 0);
98+
}
99+
100+
/**
101+
* @param array<string, mixed> $data
102+
*/
103+
function testArrayKeyExists(array $data): void
104+
{
105+
if (array_key_exists('subtitle', $data) && !is_string($data['subtitle'])) {
106+
throw new \Exception();
107+
}
108+
if (array_key_exists('subtitle', $data)) {
109+
assertType("string", $data['subtitle']);
110+
}
111+
}

0 commit comments

Comments
 (0)