diff --git a/CLAUDE.md b/CLAUDE.md index 10279d2f0d..2e90eb0689 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -295,6 +295,10 @@ Loops are a frequent source of false positives because PHPStan must reason about Match expressions in `NodeScopeResolver` (around line 4154) process each arm and merge the resulting scopes. The critical pattern for variable certainty is: when a match is exhaustive (has a `default` arm or an always-true condition), arm body scopes should be merged only with each other (not with the original pre-match scope). This mirrors how if/else merging works — `$finalScope` starts as `null`, and each branch's scope is merged via `$branchScope->mergeWith($finalScope)`. When the match is NOT exhaustive, the original scope must also participate in the merge (via `$scope->mergeWith($armBodyFinalScope)`) because execution may skip all arms and throw `UnhandledMatchError`. The `mergeVariableHolders()` method in `MutatingScope` uses `ExpressionTypeHolder::createMaybe()` for variables present in only one scope, so merging an arm scope that defines `$x` with the original scope that lacks `$x` degrades certainty to "maybe" — this is the root cause of false "might not be defined" reports for exhaustive match expressions. +### Boolean conditional expressions and match(true) exhaustiveness + +`TypeSpecifier::processBooleanSureConditionalTypes()` and `processBooleanNotSureConditionalTypes()` create `ConditionalExpressionHolder` instances that link type narrowings across expressions in boolean `&&` / `||` conditions. These are used during `filterBySpecifiedTypes()` to propagate type narrowings — for example, if `$a && $b` is false and `$a` is known to be true, then `$b` must be false. These conditional holders are critical for `match(true)` exhaustiveness detection in `isScopeConditionallyImpossible()`. Originally these methods only created conditional holders for `Variable` expressions, which meant `match(true)` exhaustiveness worked for `$b && $c` but not for `$arr['a'] && $arr['b']`. The fix was to allow any expression type (including `ArrayDimFetch`) in these methods, and to use expression string comparison instead of variable name comparison for self-reference detection. The `isScopeConditionallyImpossible` method was also extended to extract boolean-typed offsets from constant array shapes and construct `ArrayDimFetch` expressions for them. + ### GenericClassStringType narrowing and tryRemove `GenericClassStringType` represents `class-string` where `T` is the generic object type. When the generic type is a union (e.g., `class-string`), it's a single `GenericClassStringType` with an inner `UnionType`. This is distinct from `class-string|class-string|class-string` which is a `UnionType` of individual `GenericClassStringType`s. diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 8a807ea745..cc3ff0f2d7 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -7634,33 +7634,54 @@ private function getFilteringExprForMatchArm(Expr\Match_ $expr, array $condition */ private function isScopeConditionallyImpossible(MutatingScope $scope): bool { - $boolVars = []; + $boolExprs = []; foreach ($scope->getDefinedVariables() as $varName) { $varType = $scope->getVariableType($varName); - if (!$varType->isBoolean()->yes() || $varType->isConstantScalarValue()->yes()) { + if ($varType->isBoolean()->yes() && !$varType->isConstantScalarValue()->yes()) { + $boolExprs[] = new Variable($varName); continue; } - $boolVars[] = $varName; + $constantArrays = $varType->getConstantArrays(); + if (count($constantArrays) !== 1) { + continue; + } + foreach ($constantArrays[0]->getKeyTypes() as $i => $keyType) { + $valueType = $constantArrays[0]->getValueTypes()[$i]; + if (!$valueType->isBoolean()->yes() || $valueType->isConstantScalarValue()->yes()) { + continue; + } + $constantStrings = $keyType->getConstantStrings(); + if (count($constantStrings) === 1) { + $boolExprs[] = new Expr\ArrayDimFetch( + new Variable($varName), + new Node\Scalar\String_($constantStrings[0]->getValue()), + ); + } elseif ($keyType->isInteger()->yes()) { + $keyValues = $keyType->getConstantScalarValues(); + if (count($keyValues) === 1 && is_int($keyValues[0])) { + $boolExprs[] = new Expr\ArrayDimFetch( + new Variable($varName), + new Node\Scalar\Int_($keyValues[0]), + ); + } + } + } } - if ($boolVars === []) { + if ($boolExprs === []) { return false; } - // Check if any boolean variable's both truth values lead to contradictions - foreach ($boolVars as $varName) { - $varExpr = new Variable($varName); - - $truthyScope = $scope->filterByTruthyValue($varExpr); - $truthyContradiction = $this->scopeHasNeverVariable($truthyScope, $boolVars); - if (!$truthyContradiction) { + // Check if any boolean expression's both truth values lead to contradictions + foreach ($boolExprs as $boolExpr) { + $truthyScope = $scope->filterByTruthyValue($boolExpr); + if (!$this->scopeHasNeverBooleanExpr($truthyScope, $boolExprs)) { continue; } - $falseyScope = $scope->filterByFalseyValue($varExpr); - $falseyContradiction = $this->scopeHasNeverVariable($falseyScope, $boolVars); - if ($falseyContradiction) { + $falseyScope = $scope->filterByFalseyValue($boolExpr); + if ($this->scopeHasNeverBooleanExpr($falseyScope, $boolExprs)) { return true; } } @@ -7669,12 +7690,12 @@ private function isScopeConditionallyImpossible(MutatingScope $scope): bool } /** - * @param string[] $varNames + * @param Expr[] $boolExprs */ - private function scopeHasNeverVariable(MutatingScope $scope, array $varNames): bool + private function scopeHasNeverBooleanExpr(MutatingScope $scope, array $boolExprs): bool { - foreach ($varNames as $varName) { - $type = $scope->getVariableType($varName); + foreach ($boolExprs as $boolExpr) { + $type = $scope->getType($boolExpr); if ($type instanceof NeverType) { return true; } diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 64f9841cf0..bb673902cb 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -1629,10 +1629,7 @@ private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes { $conditionExpressionTypes = []; foreach ($leftTypes->getSureTypes() as $exprString => [$expr, $type]) { - if (!$expr instanceof Expr\Variable) { - continue; - } - if (!is_string($expr->name)) { + if ($expr instanceof Expr\Variable && !is_string($expr->name)) { continue; } @@ -1645,10 +1642,7 @@ private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes if (count($conditionExpressionTypes) > 0) { $holders = []; foreach ($rightTypes->getSureTypes() as $exprString => [$expr, $type]) { - if (!$expr instanceof Expr\Variable) { - continue; - } - if (!is_string($expr->name)) { + if ($expr instanceof Expr\Variable && !is_string($expr->name)) { continue; } @@ -1658,14 +1652,7 @@ private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes $conditions = $conditionExpressionTypes; foreach ($conditions as $conditionExprString => $conditionExprTypeHolder) { - $conditionExpr = $conditionExprTypeHolder->getExpr(); - if (!$conditionExpr instanceof Expr\Variable) { - continue; - } - if (!is_string($conditionExpr->name)) { - continue; - } - if ($conditionExpr->name !== $expr->name) { + if ($conditionExprString !== $exprString) { continue; } @@ -1696,10 +1683,7 @@ private function processBooleanNotSureConditionalTypes(Scope $scope, SpecifiedTy { $conditionExpressionTypes = []; foreach ($leftTypes->getSureNotTypes() as $exprString => [$expr, $type]) { - if (!$expr instanceof Expr\Variable) { - continue; - } - if (!is_string($expr->name)) { + if ($expr instanceof Expr\Variable && !is_string($expr->name)) { continue; } @@ -1712,10 +1696,7 @@ private function processBooleanNotSureConditionalTypes(Scope $scope, SpecifiedTy if (count($conditionExpressionTypes) > 0) { $holders = []; foreach ($rightTypes->getSureNotTypes() as $exprString => [$expr, $type]) { - if (!$expr instanceof Expr\Variable) { - continue; - } - if (!is_string($expr->name)) { + if ($expr instanceof Expr\Variable && !is_string($expr->name)) { continue; } @@ -1725,14 +1706,7 @@ private function processBooleanNotSureConditionalTypes(Scope $scope, SpecifiedTy $conditions = $conditionExpressionTypes; foreach ($conditions as $conditionExprString => $conditionExprTypeHolder) { - $conditionExpr = $conditionExprTypeHolder->getExpr(); - if (!$conditionExpr instanceof Expr\Variable) { - continue; - } - if (!is_string($conditionExpr->name)) { - continue; - } - if ($conditionExpr->name !== $expr->name) { + if ($conditionExprString !== $exprString) { continue; } diff --git a/tests/PHPStan/Analyser/nsrt/bug-12517.php b/tests/PHPStan/Analyser/nsrt/bug-12517.php new file mode 100644 index 0000000000..dd19547bc3 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-12517.php @@ -0,0 +1,19 @@ +a !== null || $foo->b !== null) { + if ($foo->a === null) { + assertType('null', $foo->a); + assertType('mixed~null', $foo->b); + } + } + } +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-13446.php b/tests/PHPStan/Analyser/nsrt/bug-13446.php new file mode 100644 index 0000000000..f735196cc3 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13446.php @@ -0,0 +1,48 @@ += 8.2 + +namespace Bug13446; + +use function PHPStan\Testing\assertType; + +final readonly class MoneyVO +{ + public function __construct( + public float $value, + public string $currency, + ) {} + + public function add(self $money): self + { + return new self($this->getRoundedValue() + $money->getRoundedValue(), $this->currency); + } + + public function getRoundedValue(): float + { + return round($this->value, 2); + } +} + +final readonly class ContainerCarriageStepPriceVO +{ + public function __construct( + public ?MoneyVO $mainCarriage, + public ?MoneyVO $destinationLocals, + ) {} + + public function getSum(): MoneyVO + { + if ($this->mainCarriage === null && $this->destinationLocals === null) { + return new MoneyVO(0.0, 'EUR'); + } + + if ($this->mainCarriage === null) { + assertType('Bug13446\MoneyVO', $this->destinationLocals); + } + + if ($this->destinationLocals === null) { + assertType('Bug13446\MoneyVO', $this->mainCarriage); + } + + return new MoneyVO(0.0, 'EUR'); + } +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-6486.php b/tests/PHPStan/Analyser/nsrt/bug-6486.php new file mode 100644 index 0000000000..7dedb1c7e9 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-6486.php @@ -0,0 +1,44 @@ +only && !$this->exclude) { + return true; + } + + if ($this->only) { + return Preg::isMatch($this->only, $name); + } + + assertType('non-falsy-string', $this->exclude); + return !Preg::isMatch($this->exclude, $name); + } +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-9155.php b/tests/PHPStan/Analyser/nsrt/bug-9155.php new file mode 100644 index 0000000000..8ff47b325f --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-9155.php @@ -0,0 +1,38 @@ +foo && null === $this->bar) { + return; + } + + if (null === $this->foo) { + assertType('Bug9155\Bar', $this->bar); + } + } +} diff --git a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php index dc7e8c05a7..b9acba0398 100644 --- a/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/MatchExpressionRuleTest.php @@ -446,4 +446,10 @@ public function testBug9534(): void ]); } + #[RequiresPhp('>= 8.0')] + public function testBug13029(): void + { + $this->analyse([__DIR__ . '/data/bug-13029.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Comparison/data/bug-13029.php b/tests/PHPStan/Rules/Comparison/data/bug-13029.php new file mode 100644 index 0000000000..18e956c64c --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-13029.php @@ -0,0 +1,41 @@ += 8.0 + +namespace Bug13029; + +/** + * @param array{a: bool, b: bool} $arr + */ +function matchWithBoolArrayExhaustive(array $arr): int +{ + return match(true) { + $arr['a'] && $arr['b'] => 1, + !$arr['a'] && !$arr['b'] => 2, + !$arr['a'] && $arr['b'] => 3, + $arr['a'] && !$arr['b'] => 4, + }; +} + +/** + * @param array{a: bool, b: bool} $arr + */ +function matchWithBoolArrayExhaustive2(array $arr): int +{ + return match(true) { + $arr['a'] === true && $arr['b'] === true => 1, + $arr['a'] === false && $arr['b'] === false => 2, + $arr['a'] === false && $arr['b'] === true => 3, + $arr['a'] === true && $arr['b'] === false => 4, + }; +} + +/** + * @param array{a: bool, b: bool} $arr + */ +function matchWithBoolArrayAndDefault(array $arr): int +{ + return match(true) { + $arr['a'] && $arr['b'] => 1, + $arr['a'] || $arr['b'] => 2, + default => 3, + }; +}