Skip to content

Commit 3daa312

Browse files
ondrejmirtesondrejmirtes
committed
Fix #13303
1 parent 501721f commit 3daa312

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
@@ -3818,7 +3818,7 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self
38183818
$scope->getNamespace(),
38193819
$scope->expressionTypes,
38203820
$scope->nativeExpressionTypes,
3821-
array_merge($specifiedTypes->getNewConditionalExpressionHolders(), $scope->conditionalExpressions),
3821+
$this->mergeConditionalExpressions($specifiedTypes->getNewConditionalExpressionHolders(), $scope->conditionalExpressions),
38223822
$scope->inClosureBindScopeClasses,
38233823
$scope->anonymousFunctionReflection,
38243824
$scope->inFirstLevelStatement,
@@ -4007,6 +4007,25 @@ private function intersectConditionalExpressions(array $otherConditionalExpressi
40074007
return $newConditionalExpressions;
40084008
}
40094009

4010+
/**
4011+
* @param array<string, ConditionalExpressionHolder[]> $newConditionalExpressions
4012+
* @param array<string, ConditionalExpressionHolder[]> $existingConditionalExpressions
4013+
* @return array<string, ConditionalExpressionHolder[]>
4014+
*/
4015+
private function mergeConditionalExpressions(array $newConditionalExpressions, array $existingConditionalExpressions): array
4016+
{
4017+
$result = $existingConditionalExpressions;
4018+
foreach ($newConditionalExpressions as $exprString => $holders) {
4019+
if (!array_key_exists($exprString, $result)) {
4020+
$result[$exprString] = $holders;
4021+
} else {
4022+
$result[$exprString] = array_merge($result[$exprString], $holders);
4023+
}
4024+
}
4025+
4026+
return $result;
4027+
}
4028+
40104029
/**
40114030
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
40124031
* @param array<string, ExpressionTypeHolder> $ourExpressionTypes

‎src/Analyser/NodeScopeResolver.php‎

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

4367+
if (!$hasDefaultCond && !$hasAlwaysTrueCond && $condType->isBoolean()->yes() && $condType->isConstantScalarValue()->yes()) {
4368+
if ($this->isScopeConditionallyImpossible($matchScope)) {
4369+
$hasAlwaysTrueCond = true;
4370+
$matchScope = $matchScope->addTypeToExpression($expr->cond, new NeverType());
4371+
}
4372+
}
4373+
43674374
$isExhaustive = $hasDefaultCond || $hasAlwaysTrueCond;
43684375
if (!$isExhaustive) {
43694376
$remainingType = $matchScope->getType($expr->cond);
@@ -7598,6 +7605,57 @@ private function getFilteringExprForMatchArm(Expr\Match_ $expr, array $condition
75987605
);
75997606
}
76007607

7608+
/**
7609+
* Checks if a scope's conditional expressions form a contradiction,
7610+
* meaning no combination of variable values is possible.
7611+
* Used for match(true) exhaustiveness detection.
7612+
*/
7613+
private function isScopeConditionallyImpossible(MutatingScope $scope): bool
7614+
{
7615+
$boolVars = [];
7616+
foreach ($scope->getDefinedVariables() as $varName) {
7617+
$varType = $scope->getVariableType($varName);
7618+
if ($varType->isBoolean()->yes() && !$varType->isConstantScalarValue()->yes()) {
7619+
$boolVars[] = $varName;
7620+
}
7621+
}
7622+
7623+
if ($boolVars === []) {
7624+
return false;
7625+
}
7626+
7627+
// Check if any boolean variable's both truth values lead to contradictions
7628+
foreach ($boolVars as $varName) {
7629+
$varExpr = new Variable($varName);
7630+
$truthyScope = $scope->filterByTruthyValue($varExpr);
7631+
$falseyScope = $scope->filterByFalseyValue($varExpr);
7632+
7633+
$truthyContradiction = $this->scopeHasNeverVariable($truthyScope, $boolVars);
7634+
$falseyContradiction = $this->scopeHasNeverVariable($falseyScope, $boolVars);
7635+
7636+
if ($truthyContradiction && $falseyContradiction) {
7637+
return true;
7638+
}
7639+
}
7640+
7641+
return false;
7642+
}
7643+
7644+
/**
7645+
* @param string[] $varNames
7646+
*/
7647+
private function scopeHasNeverVariable(MutatingScope $scope, array $varNames): bool
7648+
{
7649+
foreach ($varNames as $varName) {
7650+
$type = $scope->getVariableType($varName);
7651+
if ($type instanceof NeverType) {
7652+
return true;
7653+
}
7654+
}
7655+
7656+
return false;
7657+
}
7658+
76017659
private function inferForLoopExpressions(For_ $stmt, Expr $lastCondExpr, MutatingScope $bodyScope): MutatingScope
76027660
{
76037661
// 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,6 +645,16 @@ 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
$result = new SpecifiedTypes(
649659
$types->getSureTypes(),
650660
$types->getSureNotTypes(),
@@ -653,10 +663,10 @@ public function specifyTypesInCondition(
653663
$result = $result->setAlwaysOverwriteTypes();
654664
}
655665
return $result->setNewConditionalExpressionHolders(array_merge(
656-
$this->processBooleanNotSureConditionalTypes($scope, $leftTypes, $rightTypes),
657-
$this->processBooleanNotSureConditionalTypes($scope, $rightTypes, $leftTypes),
658-
$this->processBooleanSureConditionalTypes($scope, $leftTypes, $rightTypes),
659-
$this->processBooleanSureConditionalTypes($scope, $rightTypes, $leftTypes),
666+
$this->processBooleanNotSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders),
667+
$this->processBooleanNotSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders),
668+
$this->processBooleanSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders),
669+
$this->processBooleanSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders),
660670
))->setRootExpr($expr);
661671
}
662672

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

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

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)