Skip to content

Commit 12f6a70

Browse files
phpstan-botphpstan-bot
committed
Pick the ternary's final scope from the native condition type when treatPhpDocTypesAsCertain is off
- TernaryHandler decided whether to keep only the truthy or only the falsey branch scope from the PHPDoc condition type. With a PHPDoc-only `@return Expr`, `$x === null` is PHPDoc-false, so the truthy branch (where the native type is `null`) was dropped and the native type stayed `mixed~null` after the ternary, making a repeated `$x === null` "always false" even with treatPhpDocTypesAsCertain: false. - Inject the `treatPhpDocTypesAsCertain` parameter and read the native condition type when it is off, like IfHandler, ForHandler, WhileHandler, DoWhileHandler and MatchHandler already do. - Port the change to the turbo-ext C++ twin (TernaryHandler.cpp + regenerated declarations header). - Drop the stale line-20 expectation in ImpossibleInstanceOfRuleWithUncertainPhpDocTypesTest: it was produced by the same scope drop after the line-19 ternary. - Probed if/elseif, switch, match, &&, ||, ??, while and short ternary; only the ternary (long and short) was affected.
1 parent acd0511 commit 12f6a70

6 files changed

Lines changed: 158 additions & 58 deletions

File tree

‎src/Analyser/ExprHandler/TernaryHandler.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use PHPStan\Analyser\TypeSpecifierContext;
2222
use PHPStan\Analyser\VariableFlow;
2323
use PHPStan\Dependency\Dependencies;
24+
use PHPStan\DependencyInjection\AutowiredParameter;
2425
use PHPStan\DependencyInjection\AutowiredService;
2526
use PHPStan\Turbo\ShadowedByTurboExtension;
2627
use PHPStan\Type\BooleanType;
@@ -59,6 +60,8 @@ public function __construct(
5960
private ExpressionResultFactory $expressionResultFactory,
6061
private DefaultNarrowingHelper $defaultNarrowingHelper,
6162
private BooleanNarrowingHelper $booleanNarrowingHelper,
63+
#[AutowiredParameter]
64+
private bool $treatPhpDocTypesAsCertain,
6265
)
6366
{
6467
}
@@ -122,7 +125,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
122125
$this->capturedResults[spl_object_id($expr)] = [$expr, $ternaryCondResult, $ifResult, $elseResult];
123126
}
124127

125-
$condType = $ternaryCondResult->getType();
128+
$condType = $this->treatPhpDocTypesAsCertain ? $ternaryCondResult->getType() : $ternaryCondResult->getNativeType();
126129
if ($condType->isTrue()->yes()) {
127130
$finalScope = $ifTrueScope;
128131
} elseif ($condType->isFalse()->yes()) {

‎tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleWithUncertainPhpDocTypesTest.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,6 @@ public function testTernaryElseDoNotReportPhpDoc(): void
117117
'Instanceof between UnreachableTernaryElseBranchNotPhpDoc\Foo and UnreachableTernaryElseBranchNotPhpDoc\Foo will always evaluate to true.',
118118
17,
119119
],
120-
[
121-
'Instanceof between UnreachableTernaryElseBranchNotPhpDoc\Foo and UnreachableTernaryElseBranchNotPhpDoc\Foo will always evaluate to true.',
122-
20,
123-
],
124120
]);
125121
}
126122

‎tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleWithUncertainPhpDocTypesTest.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Comparison;
4+
5+
use PHPStan\Analyser\RicherScopeGetTypeHelper;
6+
use PHPStan\Rules\Rule;
7+
use PHPStan\Testing\RuleTestCase;
8+
use function array_merge;
9+
10+
/**
11+
* @extends RuleTestCase<StrictComparisonOfDifferentTypesRule>
12+
*/
13+
class StrictComparisonOfDifferentTypesRuleWithUncertainPhpDocTypesTest extends RuleTestCase
14+
{
15+
16+
protected function getRule(): Rule
17+
{
18+
return new StrictComparisonOfDifferentTypesRule(
19+
self::getContainer()->getByType(RicherScopeGetTypeHelper::class),
20+
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
21+
false,
22+
false,
23+
true,
24+
);
25+
}
26+
27+
public static function getAdditionalConfigFiles(): array
28+
{
29+
return array_merge(
30+
parent::getAdditionalConfigFiles(),
31+
[
32+
__DIR__ . '/../uncertain-phpdoc-types.neon',
33+
],
34+
);
35+
}
36+
37+
protected function shouldTreatPhpDocTypesAsCertain(): bool
38+
{
39+
return false;
40+
}
41+
42+
public function testBug15403(): void
43+
{
44+
$this->analyse([__DIR__ . '/data/bug-15403.php'], []);
45+
}
46+
47+
}

‎tests/PHPStan/Rules/Comparison/data/bug-15403.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15403;
4+
5+
class Expr
6+
{
7+
/** @return string */
8+
public function getTitle() { return 't'; }
9+
/** @return string */
10+
public function getExpression() { return 'e'; }
11+
}
12+
13+
interface Binding
14+
{
15+
/** @return Expr */
16+
public function getCondition();
17+
}
18+
19+
function sig(Binding $b): string
20+
{
21+
$condition = $b->getCondition();
22+
return implode("\0", [
23+
$condition === null ? '' : (string) $condition->getTitle(),
24+
$condition === null ? '' : (string) $condition->getExpression(),
25+
]);
26+
}
27+
28+
function assigned(Binding $b): void
29+
{
30+
$condition = $b->getCondition();
31+
$x = $condition === null ? '' : (string) $condition->getTitle();
32+
$y = $condition === null ? '' : (string) $condition->getExpression();
33+
}
34+
35+
function shortTernary(Binding $b): void
36+
{
37+
$condition = $b->getCondition();
38+
$x = $condition === null ?: 'a';
39+
$y = $condition === null ?: 'b';
40+
}
41+
42+
function negated(Binding $b): void
43+
{
44+
$condition = $b->getCondition();
45+
$x = $condition !== null ? 'a' : 'b';
46+
$y = $condition !== null ? 'a' : 'b';
47+
}

‎turbo-ext/src/TernaryHandler.cpp‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,12 @@ class TernaryHandler
5757
explicit TernaryHandler(zend_object *self) : self(self) {}
5858

5959
/* the constructor body: the promoted properties */
60-
void construct(zval *expressionResultFactory, zval *defaultNarrowingHelper, zval *booleanNarrowingHelper) const
60+
void construct(zval *expressionResultFactory, zval *defaultNarrowingHelper, zval *booleanNarrowingHelper, bool treatPhpDocTypesAsCertain) const
6161
{
6262
pt_write_slot(self, slots::expressionResultFactory, expressionResultFactory);
6363
pt_write_slot(self, slots::defaultNarrowingHelper, defaultNarrowingHelper);
6464
pt_write_slot(self, slots::booleanNarrowingHelper, booleanNarrowingHelper);
65+
zv::ObjRef(self).propAtWrite(slots::treatPhpDocTypesAsCertain, zv::Val::boolean(treatPhpDocTypesAsCertain));
6566
}
6667

6768
/* Mirrors resetFileAnalysisState(). */
@@ -167,7 +168,8 @@ class TernaryHandler
167168
zend_hash_index_update(Z_ARRVAL_P(capturedResults), Z_OBJ_HANDLE_P(expr), &entryValue);
168169
}
169170

170-
zv::Val condType = pt_expression_result_get_type(ternaryCondResult.raw());
171+
bool treatPhpDocTypesAsCertain = Z_TYPE_P(OBJ_PROP_NUM(self, slots::treatPhpDocTypesAsCertain)) == IS_TRUE;
172+
zv::Val condType = treatPhpDocTypesAsCertain ? pt_expression_result_get_type(ternaryCondResult.raw()) : pt_expression_result_get_native_type(ternaryCondResult.raw());
171173
if (UNEXPECTED(condType.isUndef())) return zv::Val();
172174
zval *finalScope;
173175
zv::Val mergedScope;
@@ -952,8 +954,9 @@ PT_MINIT_REGISTRATION(pt_register_ternary_handler)
952954
* service by reflecting the constructor */
953955
cls.method(sigs::__construct, [](INTERNAL_FUNCTION_PARAMETERS) {
954956
zval *expressionResultFactory, *defaultNarrowingHelper, *booleanNarrowingHelper;
955-
if (!zp::parse<zp::Obj, zp::Obj, zp::Obj>(execute_data, expressionResultFactory, defaultNarrowingHelper, booleanNarrowingHelper)) RETURN_THROWS();
956-
TernaryHandler(Z_OBJ_P(ZEND_THIS)).construct(expressionResultFactory, defaultNarrowingHelper, booleanNarrowingHelper);
957+
bool treatPhpDocTypesAsCertain;
958+
if (!zp::parse<zp::Obj, zp::Obj, zp::Obj, zp::Bool>(execute_data, expressionResultFactory, defaultNarrowingHelper, booleanNarrowingHelper, treatPhpDocTypesAsCertain)) RETURN_THROWS();
959+
TernaryHandler(Z_OBJ_P(ZEND_THIS)).construct(expressionResultFactory, defaultNarrowingHelper, booleanNarrowingHelper, treatPhpDocTypesAsCertain);
957960
});
958961

959962
cls.method(sigs::supports, [](INTERNAL_FUNCTION_PARAMETERS) {

‎turbo-ext/src/generated/TernaryHandler.h‎

Lines changed: 53 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ inline constexpr uint32_t capturedResults = 0;
1414
inline constexpr uint32_t expressionResultFactory = 1;
1515
inline constexpr uint32_t defaultNarrowingHelper = 2;
1616
inline constexpr uint32_t booleanNarrowingHelper = 3;
17+
inline constexpr uint32_t treatPhpDocTypesAsCertain = 4;
1718
} // namespace slot
1819

1920
inline void declareClass(reg::Class &cls)
@@ -29,6 +30,7 @@ inline void declareProperties(reg::Class &cls)
2930
cls.property("expressionResultFactory", ZEND_ACC_PRIVATE, reg::PropertyKind::Typed, 0, "PHPStan\\Analyser\\ExpressionResultFactory");
3031
cls.property("defaultNarrowingHelper", ZEND_ACC_PRIVATE, reg::PropertyKind::Typed, 0, "PHPStan\\Analyser\\ExprHandler\\Helper\\DefaultNarrowingHelper");
3132
cls.property("booleanNarrowingHelper", ZEND_ACC_PRIVATE, reg::PropertyKind::Typed, 0, "PHPStan\\Analyser\\ExprHandler\\Helper\\BooleanNarrowingHelper");
33+
cls.property("treatPhpDocTypesAsCertain", ZEND_ACC_PRIVATE, reg::PropertyKind::Typed, MAY_BE_BOOL);
3234
}
3335

3436
/* the string and parameter tables the signatures below index into (see reg::Sig) */
@@ -42,57 +44,59 @@ inline constexpr char strings[] =
4244
"PHPStan\\Analyser\\ExprHandler\\Helper\\DefaultNarrowingHelper\0" /* 112 */
4345
"booleanNarrowingHelper\0" /* 171 */
4446
"PHPStan\\Analyser\\ExprHandler\\Helper\\BooleanNarrowingHelper\0" /* 194 */
45-
"__construct\0" /* 253 */
46-
"expr\0" /* 265 */
47-
"PhpParser\\Node\\Expr\0" /* 270 */
48-
"supports\0" /* 290 */
49-
"PhpParser\\Node\\Expr\\Ternary\0" /* 299 */
50-
"getCapturedResults\0" /* 327 */
51-
"nodeScopeResolver\0" /* 346 */
52-
"PHPStan\\Analyser\\NodeScopeResolver\0" /* 364 */
53-
"stmt\0" /* 399 */
54-
"PhpParser\\Node\\Stmt\0" /* 404 */
55-
"scope\0" /* 424 */
56-
"PHPStan\\Analyser\\MutatingScope\0" /* 430 */
57-
"storage\0" /* 461 */
58-
"PHPStan\\Analyser\\ExpressionResultStorage\0" /* 469 */
59-
"nodeCallback\0" /* 510 */
60-
"context\0" /* 523 */
61-
"PHPStan\\Analyser\\ExpressionContext\0" /* 531 */
62-
"PHPStan\\Analyser\\ExpressionResult\0" /* 566 */
63-
"processExpr\0" /* 600 */
64-
"isDisjunction\0" /* 612 */
65-
"armExpr\0" /* 626 */
66-
"armResult\0" /* 634 */
67-
"processingScope\0" /* 644 */
68-
"evaluatedScope\0" /* 660 */
69-
"armContext\0" /* 675 */
70-
"PHPStan\\Analyser\\TypeSpecifierContext\0" /* 686 */
71-
"createArmOperand"; /* 724 */
47+
"treatPhpDocTypesAsCertain\0" /* 253 */
48+
"__construct\0" /* 279 */
49+
"expr\0" /* 291 */
50+
"PhpParser\\Node\\Expr\0" /* 296 */
51+
"supports\0" /* 316 */
52+
"PhpParser\\Node\\Expr\\Ternary\0" /* 325 */
53+
"getCapturedResults\0" /* 353 */
54+
"nodeScopeResolver\0" /* 372 */
55+
"PHPStan\\Analyser\\NodeScopeResolver\0" /* 390 */
56+
"stmt\0" /* 425 */
57+
"PhpParser\\Node\\Stmt\0" /* 430 */
58+
"scope\0" /* 450 */
59+
"PHPStan\\Analyser\\MutatingScope\0" /* 456 */
60+
"storage\0" /* 487 */
61+
"PHPStan\\Analyser\\ExpressionResultStorage\0" /* 495 */
62+
"nodeCallback\0" /* 536 */
63+
"context\0" /* 549 */
64+
"PHPStan\\Analyser\\ExpressionContext\0" /* 557 */
65+
"PHPStan\\Analyser\\ExpressionResult\0" /* 592 */
66+
"processExpr\0" /* 626 */
67+
"isDisjunction\0" /* 638 */
68+
"armExpr\0" /* 652 */
69+
"armResult\0" /* 660 */
70+
"processingScope\0" /* 670 */
71+
"evaluatedScope\0" /* 686 */
72+
"armContext\0" /* 701 */
73+
"PHPStan\\Analyser\\TypeSpecifierContext\0" /* 712 */
74+
"createArmOperand"; /* 750 */
7275
inline constexpr reg::PackedArg args[] = {
7376
reg::packed(0, MAY_BE_VOID), /* resetFileAnalysisState return */
7477
reg::packed(24, 0, 48), /* __construct $expressionResultFactory */
7578
reg::packed(89, 0, 112), /* __construct $defaultNarrowingHelper */
7679
reg::packed(171, 0, 194), /* __construct $booleanNarrowingHelper */
77-
reg::packed(265, 0, 270), /* supports $expr */
80+
reg::packed(253, MAY_BE_BOOL), /* __construct $treatPhpDocTypesAsCertain */
81+
reg::packed(291, 0, 296), /* supports $expr */
7882
reg::packed(0, MAY_BE_BOOL), /* supports return */
79-
reg::packed(265, 0, 299), /* getCapturedResults $expr */
83+
reg::packed(291, 0, 325), /* getCapturedResults $expr */
8084
reg::packed(0, MAY_BE_NULL | MAY_BE_ARRAY), /* getCapturedResults return */
81-
reg::packed(346, 0, 364), /* processExpr $nodeScopeResolver */
82-
reg::packed(399, 0, 404), /* processExpr $stmt */
83-
reg::packed(265, 0, 270), /* processExpr $expr */
84-
reg::packed(424, 0, 430), /* processExpr $scope */
85-
reg::packed(461, 0, 469), /* processExpr $storage */
86-
reg::packed(510, MAY_BE_CALLABLE), /* processExpr $nodeCallback */
87-
reg::packed(523, 0, 531), /* processExpr $context */
88-
reg::packed(0, 0, 566), /* processExpr return */
89-
reg::packed(265, 0, 270), /* isDisjunction $expr */
85+
reg::packed(372, 0, 390), /* processExpr $nodeScopeResolver */
86+
reg::packed(425, 0, 430), /* processExpr $stmt */
87+
reg::packed(291, 0, 296), /* processExpr $expr */
88+
reg::packed(450, 0, 456), /* processExpr $scope */
89+
reg::packed(487, 0, 495), /* processExpr $storage */
90+
reg::packed(536, MAY_BE_CALLABLE), /* processExpr $nodeCallback */
91+
reg::packed(549, 0, 557), /* processExpr $context */
92+
reg::packed(0, 0, 592), /* processExpr return */
93+
reg::packed(291, 0, 296), /* isDisjunction $expr */
9094
reg::packed(0, MAY_BE_BOOL), /* isDisjunction return */
91-
reg::packed(626, 0, 270), /* createArmOperand $armExpr */
92-
reg::packed(634, 0, 566), /* createArmOperand $armResult */
93-
reg::packed(644, 0, 430), /* createArmOperand $processingScope */
94-
reg::packed(660, 0, 430), /* createArmOperand $evaluatedScope */
95-
reg::packed(675, MAY_BE_NULL, 686), /* createArmOperand $armContext */
95+
reg::packed(652, 0, 296), /* createArmOperand $armExpr */
96+
reg::packed(660, 0, 592), /* createArmOperand $armResult */
97+
reg::packed(670, 0, 456), /* createArmOperand $processingScope */
98+
reg::packed(686, 0, 456), /* createArmOperand $evaluatedScope */
99+
reg::packed(701, MAY_BE_NULL, 712), /* createArmOperand $armContext */
96100
reg::packed(0, MAY_BE_ARRAY), /* createArmOperand return */
97101
};
98102
using Sig = reg::Sig<strings, args>;
@@ -101,12 +105,12 @@ using Sig = reg::Sig<strings, args>;
101105
/* the signatures of the methods the class declares itself (a used trait's are in the trait's header) */
102106
namespace sig {
103107
inline constexpr sigtab::Sig resetFileAnalysisState = { { 1 /* resetFileAnalysisState */, 0, 0, 0, 0, ZEND_ACC_PUBLIC } };
104-
inline constexpr sigtab::Sig __construct = { { 253 /* __construct */, 3, 1, 3, reg::NoArg, ZEND_ACC_PUBLIC } };
105-
inline constexpr sigtab::Sig supports = { { 290 /* supports */, 1, 4, 1, 5, ZEND_ACC_PUBLIC } };
106-
inline constexpr sigtab::Sig getCapturedResults = { { 327 /* getCapturedResults */, 1, 6, 1, 7, ZEND_ACC_PUBLIC } };
107-
inline constexpr sigtab::Sig processExpr = { { 600 /* processExpr */, 7, 8, 7, 15, ZEND_ACC_PUBLIC } };
108-
inline constexpr sigtab::Sig isDisjunction = { { 612 /* isDisjunction */, 1, 16, 1, 17, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
109-
inline constexpr sigtab::Sig createArmOperand = { { 724 /* createArmOperand */, 5, 18, 5, 23, ZEND_ACC_PRIVATE } };
108+
inline constexpr sigtab::Sig __construct = { { 279 /* __construct */, 4, 1, 4, reg::NoArg, ZEND_ACC_PUBLIC } };
109+
inline constexpr sigtab::Sig supports = { { 316 /* supports */, 1, 5, 1, 6, ZEND_ACC_PUBLIC } };
110+
inline constexpr sigtab::Sig getCapturedResults = { { 353 /* getCapturedResults */, 1, 7, 1, 8, ZEND_ACC_PUBLIC } };
111+
inline constexpr sigtab::Sig processExpr = { { 626 /* processExpr */, 7, 9, 7, 16, ZEND_ACC_PUBLIC } };
112+
inline constexpr sigtab::Sig isDisjunction = { { 638 /* isDisjunction */, 1, 17, 1, 18, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
113+
inline constexpr sigtab::Sig createArmOperand = { { 750 /* createArmOperand */, 5, 19, 5, 24, ZEND_ACC_PRIVATE } };
110114
} // namespace sig
111115

112116
} // namespace ptdecl::TernaryHandler

0 commit comments

Comments
 (0)