Skip to content

Commit fadf8d6

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Compare ternary arm values, not their truthiness, in exact narrowing contexts
`(cond ? if : else) !== false` was narrowed through the boolean rewrite `(cond && if) || (!cond && else)` with the `!== false` context passed to the arms, which reads `0 !== false` as `0` being truthy. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LuUxEfBoMtVfZmJsUxA85s
1 parent 6e0238d commit fadf8d6

3 files changed

Lines changed: 125 additions & 11 deletions

File tree

‎src/Analyser/ExprHandler/TernaryHandler.php‎

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@
55
use PhpParser\Node\Expr;
66
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
77
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
8+
use PhpParser\Node\Expr\BinaryOp\Identical;
9+
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
10+
use PhpParser\Node\Expr\ConstFetch;
811
use PhpParser\Node\Expr\Ternary;
12+
use PhpParser\Node\Name;
913
use PhpParser\Node\Stmt;
1014
use PHPStan\Analyser\ExpressionContext;
1115
use PHPStan\Analyser\ExpressionResult;
@@ -85,19 +89,54 @@ public function specifyTypes(TypeSpecifier $typeSpecifier, Scope $scope, Expr $e
8589
return $typeSpecifier->specifyDefaultTypes($scope, $expr, $context);
8690
}
8791

88-
if ($expr->if !== null) {
89-
$conditionExpr = new BooleanOr(
90-
new BooleanAnd($expr->cond, $expr->if),
91-
new BooleanAnd(new Expr\BooleanNot($expr->cond), $expr->else),
92-
);
93-
} else {
94-
$conditionExpr = new BooleanOr(
95-
$expr->cond,
96-
new BooleanAnd(new Expr\BooleanNot($expr->cond), $expr->else),
97-
);
92+
if ($context === TypeSpecifierContext::createTruthy() || $context === TypeSpecifierContext::createFalsey()) {
93+
if ($expr->if !== null) {
94+
$conditionExpr = new BooleanOr(
95+
new BooleanAnd($expr->cond, $expr->if),
96+
new BooleanAnd(new Expr\BooleanNot($expr->cond), $expr->else),
97+
);
98+
} else {
99+
$conditionExpr = new BooleanOr(
100+
$expr->cond,
101+
new BooleanAnd(new Expr\BooleanNot($expr->cond), $expr->else),
102+
);
103+
}
104+
105+
return $typeSpecifier->specifyTypesInCondition($scope, $conditionExpr, $context)->setRootExpr($expr);
106+
}
107+
108+
// An exact context (`=== true`, `!== false`, ...) is about the value of the taken arm, not
109+
// its truthiness - the boolean rewrite above would read `0 !== false` as `0` being truthy.
110+
$ifComparison = $this->createArmComparison($expr->if ?? $expr->cond, $context);
111+
$elseComparison = $this->createArmComparison($expr->else, $context);
112+
if ($ifComparison === null || $elseComparison === null) {
113+
return $typeSpecifier->specifyDefaultTypes($scope, $expr, $context);
114+
}
115+
116+
$conditionExpr = new BooleanOr(
117+
new BooleanAnd($expr->cond, $ifComparison),
118+
new BooleanAnd(new Expr\BooleanNot($expr->cond), $elseComparison),
119+
);
120+
121+
return $typeSpecifier->specifyTypesInCondition($scope, $conditionExpr, TypeSpecifierContext::createTruthy())->setRootExpr($expr);
122+
}
123+
124+
private function createArmComparison(Expr $arm, TypeSpecifierContext $context): ?Expr
125+
{
126+
if ($context === TypeSpecifierContext::createTrue()) {
127+
return new Identical($arm, new ConstFetch(new Name('true')));
128+
}
129+
if ($context === TypeSpecifierContext::createFalse()) {
130+
return new Identical($arm, new ConstFetch(new Name('false')));
131+
}
132+
if ($context === TypeSpecifierContext::createTrue()->negate()) {
133+
return new NotIdentical($arm, new ConstFetch(new Name('true')));
134+
}
135+
if ($context === TypeSpecifierContext::createFalse()->negate()) {
136+
return new NotIdentical($arm, new ConstFetch(new Name('false')));
98137
}
99138

100-
return $typeSpecifier->specifyTypesInCondition($scope, $conditionExpr, $context)->setRootExpr($expr);
139+
return null;
101140
}
102141

103142
public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15226;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
/** @param array<mixed> $options */
8+
function test(array $options): void {
9+
$select_if_unique = isset($options['select']);
10+
$force = $select_if_unique ? null : ($options['force'] ?? null);
11+
if (isset($force)) {
12+
assertType('false', $select_if_unique); // ok: false
13+
}
14+
assertType('bool', $select_if_unique); // should be: bool
15+
16+
}

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug15307;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
function f(): int|false
10+
{
11+
return rand(0, 9999) ?: false;
12+
}
13+
14+
$a = false;
15+
16+
if (rand(0,1) == 1) {
17+
$a = true;
18+
}
19+
20+
$b = !$a ? f() : 0;
21+
22+
if ($b === false) {
23+
// this check erronously changes the type of a to false;
24+
// it *is* false, but only inside of this branch.
25+
} elseif ($a || $b > 99) {
26+
// if execution ends up here, $a might very well stil lbe true
27+
assertType('bool', $a);
28+
}
29+
30+
function exactContexts(bool $a, int|bool $x): void
31+
{
32+
if ((!$a ? f() : 0) !== false) {
33+
assertType('bool', $a);
34+
} else {
35+
assertType('false', $a);
36+
}
37+
38+
if ((!$a ? f() : 0) === false) {
39+
assertType('false', $a);
40+
} else {
41+
assertType('bool', $a);
42+
}
43+
44+
if (($a ? $x : true) === true) {
45+
assertType('bool', $a);
46+
assertType('bool|int', $x);
47+
} else {
48+
assertType('true', $a);
49+
assertType('int|false', $x);
50+
}
51+
52+
if (($a ? $x : 1) !== true) {
53+
assertType('bool', $a);
54+
assertType('bool|int', $x);
55+
} else {
56+
assertType('true', $a);
57+
assertType('true', $x);
58+
}
59+
}

0 commit comments

Comments
 (0)