Skip to content

Commit 989d13d

Browse files
ondrejmirtesphpstan-bot
authored andcommitted
Fix #13303
1 parent 107bb2c commit 989d13d

File tree

7 files changed

+157
-5
lines changed

7 files changed

+157
-5
lines changed

src/Analyser/MutatingScope.php

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3799,7 +3799,7 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self
37993799
$scope->getNamespace(),
38003800
$scope->expressionTypes,
38013801
$scope->nativeExpressionTypes,
3802-
array_merge($specifiedTypes->getNewConditionalExpressionHolders(), $scope->conditionalExpressions),
3802+
$this->mergeConditionalExpressions($specifiedTypes->getNewConditionalExpressionHolders(), $scope->conditionalExpressions),
38033803
$scope->inClosureBindScopeClasses,
38043804
$scope->anonymousFunctionReflection,
38053805
$scope->inFirstLevelStatement,
@@ -3988,6 +3988,25 @@ private function intersectConditionalExpressions(array $otherConditionalExpressi
39883988
return $newConditionalExpressions;
39893989
}
39903990

3991+
/**
3992+
* @param array<string, ConditionalExpressionHolder[]> $newConditionalExpressions
3993+
* @param array<string, ConditionalExpressionHolder[]> $existingConditionalExpressions
3994+
* @return array<string, ConditionalExpressionHolder[]>
3995+
*/
3996+
private function mergeConditionalExpressions(array $newConditionalExpressions, array $existingConditionalExpressions): array
3997+
{
3998+
$result = $existingConditionalExpressions;
3999+
foreach ($newConditionalExpressions as $exprString => $holders) {
4000+
if (!array_key_exists($exprString, $result)) {
4001+
$result[$exprString] = $holders;
4002+
} else {
4003+
$result[$exprString] = array_merge($result[$exprString], $holders);
4004+
}
4005+
}
4006+
4007+
return $result;
4008+
}
4009+
39914010
/**
39924011
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
39934012
* @param array<string, ExpressionTypeHolder> $ourExpressionTypes

src/Analyser/NodeScopeResolver.php

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4363,6 +4363,13 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto
43634363
$matchScope = $matchScope->filterByFalseyValue($filteringExpr);
43644364
}
43654365

4366+
if (!$hasDefaultCond && !$hasAlwaysTrueCond && $condType->isBoolean()->yes() && $condType->isConstantScalarValue()->yes()) {
4367+
if ($this->isScopeConditionallyImpossible($matchScope)) {
4368+
$hasAlwaysTrueCond = true;
4369+
$matchScope = $matchScope->addTypeToExpression($expr->cond, new NeverType());
4370+
}
4371+
}
4372+
43664373
if (!$hasDefaultCond && !$hasAlwaysTrueCond) {
43674374
$remainingType = $matchScope->getType($expr->cond);
43684375
if (!$remainingType instanceof NeverType) {
@@ -7579,6 +7586,57 @@ private function getFilteringExprForMatchArm(Expr\Match_ $expr, array $condition
75797586
);
75807587
}
75817588

7589+
/**
7590+
* Checks if a scope's conditional expressions form a contradiction,
7591+
* meaning no combination of variable values is possible.
7592+
* Used for match(true) exhaustiveness detection.
7593+
*/
7594+
private function isScopeConditionallyImpossible(MutatingScope $scope): bool
7595+
{
7596+
$boolVars = [];
7597+
foreach ($scope->getDefinedVariables() as $varName) {
7598+
$varType = $scope->getVariableType($varName);
7599+
if ($varType->isBoolean()->yes() && !$varType->isConstantScalarValue()->yes()) {
7600+
$boolVars[] = $varName;
7601+
}
7602+
}
7603+
7604+
if ($boolVars === []) {
7605+
return false;
7606+
}
7607+
7608+
// Check if any boolean variable's both truth values lead to contradictions
7609+
foreach ($boolVars as $varName) {
7610+
$varExpr = new Variable($varName);
7611+
$truthyScope = $scope->filterByTruthyValue($varExpr);
7612+
$falseyScope = $scope->filterByFalseyValue($varExpr);
7613+
7614+
$truthyContradiction = $this->scopeHasNeverVariable($truthyScope, $boolVars);
7615+
$falseyContradiction = $this->scopeHasNeverVariable($falseyScope, $boolVars);
7616+
7617+
if ($truthyContradiction && $falseyContradiction) {
7618+
return true;
7619+
}
7620+
}
7621+
7622+
return false;
7623+
}
7624+
7625+
/**
7626+
* @param string[] $varNames
7627+
*/
7628+
private function scopeHasNeverVariable(MutatingScope $scope, array $varNames): bool
7629+
{
7630+
foreach ($varNames as $varName) {
7631+
$type = $scope->getVariableType($varName);
7632+
if ($type instanceof NeverType) {
7633+
return true;
7634+
}
7635+
}
7636+
7637+
return false;
7638+
}
7639+
75827640
private function inferForLoopExpressions(For_ $stmt, Expr $lastCondExpr, MutatingScope $bodyScope): MutatingScope
75837641
{
75847642
// infer $items[$i] type from for ($i = 0; $i < count($items); $i++) {...}

src/Analyser/SpecifiedTypes.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,16 @@ public function unionWith(SpecifiedTypes $other): self
188188
$result = $result->setAlwaysOverwriteTypes();
189189
}
190190

191+
$conditionalExpressionHolders = $this->newConditionalExpressionHolders;
192+
foreach ($other->newConditionalExpressionHolders as $exprString => $holders) {
193+
if (!array_key_exists($exprString, $conditionalExpressionHolders)) {
194+
$conditionalExpressionHolders[$exprString] = $holders;
195+
} else {
196+
$conditionalExpressionHolders[$exprString] = array_merge($conditionalExpressionHolders[$exprString], $holders);
197+
}
198+
}
199+
$result->newConditionalExpressionHolders = $conditionalExpressionHolders;
200+
191201
return $result->setRootExpr($rootExpr);
192202
}
193203

src/Analyser/TypeSpecifier.php

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -645,14 +645,24 @@ public function specifyTypesInCondition(
645645
$rightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, $context)->setRootExpr($expr);
646646
$types = $context->true() ? $leftTypes->unionWith($rightTypes) : $leftTypes->normalize($scope)->intersectWith($rightTypes->normalize($rightScope));
647647
if ($context->false()) {
648+
$leftTypesForHolders = $leftTypes;
649+
$rightTypesForHolders = $rightTypes;
650+
if ($context->truthy()) {
651+
if ($leftTypesForHolders->getSureTypes() === [] && $leftTypesForHolders->getSureNotTypes() === []) {
652+
$leftTypesForHolders = $this->specifyTypesInCondition($scope, $expr->left, TypeSpecifierContext::createFalsey())->setRootExpr($expr);
653+
}
654+
if ($rightTypesForHolders->getSureTypes() === [] && $rightTypesForHolders->getSureNotTypes() === []) {
655+
$rightTypesForHolders = $this->specifyTypesInCondition($rightScope, $expr->right, TypeSpecifierContext::createFalsey())->setRootExpr($expr);
656+
}
657+
}
648658
return (new SpecifiedTypes(
649659
$types->getSureTypes(),
650660
$types->getSureNotTypes(),
651661
))->setNewConditionalExpressionHolders(array_merge(
652-
$this->processBooleanNotSureConditionalTypes($scope, $leftTypes, $rightTypes),
653-
$this->processBooleanNotSureConditionalTypes($scope, $rightTypes, $leftTypes),
654-
$this->processBooleanSureConditionalTypes($scope, $leftTypes, $rightTypes),
655-
$this->processBooleanSureConditionalTypes($scope, $rightTypes, $leftTypes),
662+
$this->processBooleanNotSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders),
663+
$this->processBooleanNotSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders),
664+
$this->processBooleanSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders),
665+
$this->processBooleanSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders),
656666
))->setRootExpr($expr);
657667
}
658668

tests/PHPStan/Rules/Classes/ImpossibleInstanceOfRuleTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,11 @@ public function testBug3632(): void
494494

495495
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
496496
$this->analyse([__DIR__ . '/data/bug-3632.php'], [
497+
[
498+
'Instanceof between null and Bug3632\NiceClass will always evaluate to false.',
499+
32,
500+
$tipText,
501+
],
497502
[
498503
'Instanceof between Bug3632\NiceClass and Bug3632\NiceClass will always evaluate to true.',
499504
36,

tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,4 +405,15 @@ public function testBug13048(): void
405405
$this->analyse([__DIR__ . '/data/bug-13048.php'], []);
406406
}
407407

408+
#[RequiresPhp('>= 8.0')]
409+
public function testBug13303(): void
410+
{
411+
$this->analyse([__DIR__ . '/data/bug-13303.php'], [
412+
[
413+
'Match expression does not handle remaining value: true',
414+
34,
415+
],
416+
]);
417+
}
418+
408419
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php // lint >= 8.0
2+
3+
namespace Bug13303;
4+
5+
function a(bool $b, bool $c): int {
6+
return match(true) {
7+
$b && $c => 1,
8+
!$b && !$c => 2,
9+
!$b && $c => 3,
10+
$b && !$c => 4,
11+
};
12+
}
13+
14+
function b(bool $b, bool $c): int {
15+
return match(true) {
16+
$b && $c,
17+
!$b && !$c => 1,
18+
!$b && $c,
19+
$b && !$c => 2,
20+
};
21+
}
22+
23+
function c(bool $b, bool $c): int {
24+
return match(true) {
25+
$b === true && $c === true => 1,
26+
$b === false && $c === false => 2,
27+
$b === false && $c === true => 3,
28+
$b === true && $c === false => 4,
29+
};
30+
}
31+
32+
function d(bool $b, bool $c): int {
33+
// Not exhaustive - should still report error
34+
return match(true) {
35+
$b && $c => 1,
36+
!$b && !$c => 2,
37+
!$b && $c => 3,
38+
};
39+
}

0 commit comments

Comments
 (0)