diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index db31ce7669..7123dd3399 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -2987,7 +2987,7 @@ private static function intersectButNotNever(Type $nativeType, Type $inferredTyp return $result; } - public function enterMatch(Expr\Match_ $expr): self + public function enterMatch(Expr\Match_ $expr, ?Type $condType = null, ?Type $condNativeType = null): self { if ($expr->cond instanceof Variable) { return $this; @@ -3001,8 +3001,8 @@ public function enterMatch(Expr\Match_ $expr): self return $this; } - $type = $this->getType($cond); - $nativeType = $this->getNativeType($cond); + $type = $condType ?? $this->getType($cond); + $nativeType = $condNativeType ?? $this->getNativeType($cond); $condExpr = new AlwaysRememberedExpr($cond, $type, $nativeType); $expr->cond = $condExpr; diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 8a807ea745..d239798ee4 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -4176,13 +4176,14 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto } elseif ($expr instanceof Expr\Match_) { $deepContext = $context->enterDeep(); $condType = $scope->getType($expr->cond); + $condNativeType = $scope->getNativeType($expr->cond); $condResult = $this->processExprNode($stmt, $expr->cond, $scope, $storage, $nodeCallback, $deepContext); $scope = $condResult->getScope(); $hasYield = $condResult->hasYield(); $throwPoints = $condResult->getThrowPoints(); $impurePoints = $condResult->getImpurePoints(); $isAlwaysTerminating = $condResult->isAlwaysTerminating(); - $matchScope = $scope->enterMatch($expr); + $matchScope = $scope->enterMatch($expr, $condType, $condNativeType); $armNodes = []; $hasDefaultCond = false; $hasAlwaysTrueCond = false; @@ -4206,35 +4207,59 @@ function (MutatingScope $scope) use ($stmt, $expr, $nodeCallback, $context, $sto continue; } - $condNodes = []; - $conditionCases = []; - $conditionExprs = []; + // First pass: validate all conditions are enum case references + $validatedConds = []; + $allCondsValid = true; foreach ($arm->conds as $j => $cond) { if (!$cond instanceof Expr\ClassConstFetch) { - continue 2; + $allCondsValid = false; + break; } if (!$cond->class instanceof Name) { - continue 2; + $allCondsValid = false; + break; } if (!$cond->name instanceof Node\Identifier) { - continue 2; + $allCondsValid = false; + break; } $fetchedClassName = $scope->resolveName($cond->class); $loweredFetchedClassName = strtolower($fetchedClassName); if (!array_key_exists($loweredFetchedClassName, $indexedEnumCases)) { - continue 2; + $allCondsValid = false; + break; } + $caseName = $cond->name->toString(); + if (!array_key_exists($caseName, $indexedEnumCases[$loweredFetchedClassName])) { + $allCondsValid = false; + break; + } + $validatedConds[$j] = [ + 'cond' => $cond, + 'loweredFetchedClassName' => $loweredFetchedClassName, + 'caseName' => $caseName, + 'enumCase' => $indexedEnumCases[$loweredFetchedClassName][$caseName], + ]; + } + + if (!$allCondsValid) { + continue; + } + + $condNodes = []; + $conditionCases = []; + $conditionExprs = []; + // Second pass: process validated conditions with side effects + foreach ($validatedConds as $j => $validatedCond) { + $cond = $validatedCond['cond']; + $loweredFetchedClassName = $validatedCond['loweredFetchedClassName']; + $caseName = $validatedCond['caseName']; + $enumCase = $validatedCond['enumCase']; if (!array_key_exists($loweredFetchedClassName, $unusedIndexedEnumCases)) { throw new ShouldNotHappenException(); } - $caseName = $cond->name->toString(); - if (!array_key_exists($caseName, $indexedEnumCases[$loweredFetchedClassName])) { - continue 2; - } - - $enumCase = $indexedEnumCases[$loweredFetchedClassName][$caseName]; $conditionCases[] = $enumCase; $armConditionScope = $matchScope; if (!array_key_exists($caseName, $unusedIndexedEnumCases[$loweredFetchedClassName])) { diff --git a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php index dc7e8c05a7..135cba84e4 100644 --- a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php @@ -152,6 +152,11 @@ public function testEnums(): void 'Match arm comparison between *NEVER* and MatchEnums\DifferentEnum::ONE is always false.', 113, ], + [ + 'Match arm comparison between MatchEnums\Foo::ONE and MatchEnums\Foo::ONE is always true.', + 113, + 'Remove remaining cases below this one and this error will disappear too.', + ], ]); } @@ -446,4 +451,15 @@ public function testBug9534(): void ]); } + #[RequiresPhp('>= 8.0')] + public function testBug11310(): void + { + $this->analyse([__DIR__ . '/data/bug-11310.php'], [ + [ + 'Match arm comparison between int<1, max> and 0 is always false.', + 24, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-11310.php b/tests/PHPStan/Rules/Comparison/data/bug-11310.php new file mode 100644 index 0000000000..38b5daf20d --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-11310.php @@ -0,0 +1,27 @@ += 8.0 + +namespace Bug11310; + +/** @param int<0, max> $i */ +function foo(int $i): void { + echo match ($i++) { + 0 => 'zero', + default => 'default', + }; +} + +/** @param int<0, max> $i */ +function bar(int $i): void { + echo match ($i--) { + 0 => 'zero', + default => 'default', + }; +} + +/** @param int<0, max> $i */ +function baz(int $i): void { + echo match (++$i) { + 0 => 'zero', + default => 'default', + }; +}