Skip to content

Commit ed816ea

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 172c271 commit ed816ea

2 files changed

Lines changed: 147 additions & 0 deletions

File tree

‎src/Analyser/TypeSpecifier.php‎

Lines changed: 36 additions & 0 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(),
@@ -747,6 +759,10 @@ public function specifyTypesInCondition(
747759
$this->processBooleanConditionalTypes($scope, $rightTypesForHolders, false, $leftTypesForHolders, false, $scope),
748760
$this->processBooleanConditionalTypes($scope, $leftTypesForHolders, true, $rightTypesForHolders, true, $rightScope),
749761
$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

@@ -800,6 +816,10 @@ public function specifyTypesInCondition(
800816
$this->processBooleanConditionalTypes($scope, $rightTypes, false, $leftTypes, false, $scope),
801817
$this->processBooleanConditionalTypes($scope, $leftTypes, true, $rightTypes, true, $rightScope),
802818
$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

@@ -2143,6 +2163,22 @@ private function isTrackableExpression(Expr $expr): bool
21432163
|| $expr instanceof Expr\StaticPropertyFetch;
21442164
}
21452165

2166+
private function allExpressionsTrackable(SpecifiedTypes $types): bool
2167+
{
2168+
foreach ($types->getSureTypes() as [$expr]) {
2169+
if (!$this->isTrackableExpression($expr)) {
2170+
return false;
2171+
}
2172+
}
2173+
foreach ($types->getSureNotTypes() as [$expr]) {
2174+
if (!$this->isTrackableExpression($expr)) {
2175+
return false;
2176+
}
2177+
}
2178+
2179+
return $types->getSureTypes() !== [] || $types->getSureNotTypes() !== [];
2180+
}
2181+
21462182
/**
21472183
* Flatten a deep BooleanOr chain into leaf expressions and process them
21482184
* without recursive filterByFalseyValue calls. This reduces O(n^2) to O(n)

‎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)