Skip to content

Commit 31d74ad

Browse files
phpstan-botondrejmirtes
authored andcommitted
Fall back to the full branch type for a conditional guard when the partial set difference still overlaps the other branch
- ScopeOps::createConditionalExpressions() derives guard types from TypeCombinator::remove(ours, theirs). When remove() can only subtract part of their type (e.g. null from string|null, but not non-empty-string from string), the remainder (string) still overlaps the other branch, so seeing it later does not prove our branch ran. Such guards produced conditional expressions like "if $title=string then $input->isInteractive() is true". Now the guard falls back to our full type when the remainder is not disjoint from their type, which restores the merged-type comparison that was used before. - Ported the same check to the native mirror in turbo-ext/src/ScopeOps.cpp. - Variants also covered: plain ?string narrowed against non-empty-string|null, and ?array against non-empty-array|null (both were broken). The int<1, max>|null variant already worked, because int<min, 0> is representable; it is kept as coverage.
1 parent 4ef9ab7 commit 31d74ad

3 files changed

Lines changed: 95 additions & 0 deletions

File tree

‎src/Analyser/ScopeOps.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,15 @@ public static function createConditionalExpressions(
505505
if ($remainder instanceof NeverType) {
506506
continue;
507507
}
508+
if (
509+
$remainder !== $holder->getType()
510+
&& !$theirHolder->getType()->isSuperTypeOf($remainder)->no()
511+
) {
512+
// only part of their type was subtracted (e.g. null from
513+
// string|null, but not non-empty-string from string), so the
514+
// remainder still overlaps their branch and cannot prove ours
515+
$remainder = $holder->getType();
516+
}
508517
if ($mergedExpressionTypes[$exprString]->getType()->equals($remainder)) {
509518
// matching this guard later would not discriminate the branches -
510519
// the merged scope already guarantees it
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug15483;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
interface Input
10+
{
11+
public function isInteractive(): bool;
12+
}
13+
14+
function prompt(): ?string
15+
{
16+
return null;
17+
}
18+
19+
function f(Input $input, mixed $option): void
20+
{
21+
$title = \is_string($option) && $option !== '' ? $option : null;
22+
if ($title === null && $input->isInteractive()) {
23+
$title = prompt();
24+
}
25+
if ($title === null) {
26+
return;
27+
}
28+
assertType('bool', $input->isInteractive());
29+
}
30+
31+
function g(Input $input, mixed $option): void
32+
{
33+
$title = \is_string($option) ? $option : null;
34+
if ($title === null && $input->isInteractive()) {
35+
$title = prompt();
36+
}
37+
if ($title === null) {
38+
return;
39+
}
40+
assertType('bool', $input->isInteractive());
41+
}
42+
43+
/** @param non-empty-string|null $title */
44+
function h(Input $input, ?string $title): void
45+
{
46+
if ($title === null && $input->isInteractive()) {
47+
$title = prompt();
48+
}
49+
if ($title === null) {
50+
return;
51+
}
52+
assertType('bool', $input->isInteractive());
53+
}
54+
55+
/** @param int<1, max>|null $count */
56+
function i(Input $input, ?int $count, ?int $other): void
57+
{
58+
if ($count === null && $input->isInteractive()) {
59+
$count = $other;
60+
}
61+
if ($count === null) {
62+
return;
63+
}
64+
assertType('bool', $input->isInteractive());
65+
}
66+
67+
/** @param non-empty-array<int>|null $items */
68+
function j(Input $input, ?array $items, ?array $other): void
69+
{
70+
if ($items === null && $input->isInteractive()) {
71+
$items = $other;
72+
}
73+
if ($items === null) {
74+
return;
75+
}
76+
assertType('bool', $input->isInteractive());
77+
}

‎turbo-ext/src/ScopeOps.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,15 @@ class ScopeOps
603603
zv::Val remainder = typeCombinatorRemove(holderType(holder), holderType(theirHolder));
604604
if (UNEXPECTED(remainder.isUndef())) return zv::Val();
605605
if (remainder.ref().instanceOf(neverTypeCe)) continue;
606+
if (Z_OBJ_P(remainder.raw()) != Z_OBJ_P(holderType(holder))) {
607+
/* a partial subtraction that still overlaps their branch cannot
608+
* prove ours - fall back to our full type, like the twin */
609+
zend_long theirIsSuperTypeOfRemainder;
610+
if (UNEXPECTED(!isSuperTypeOfValue(holderType(theirHolder), remainder.raw(), &theirIsSuperTypeOfRemainder))) return zv::Val();
611+
if (theirIsSuperTypeOfRemainder != PT_TRI_NO) {
612+
remainder = zv::Val::copyOf(zv::Ref(holderType(holder)));
613+
}
614+
}
606615
{
607616
zv::Ref mergedHolder = zv::Ref(mergedSlot).deref();
608617
if (UNEXPECTED(!pt_check_holder(mergedHolder.raw()))) return zv::Val();

0 commit comments

Comments
 (0)