diff --git a/src/Analyser/ConditionalExpressionHolderRecipe.php b/src/Analyser/ConditionalExpressionHolderRecipe.php new file mode 100644 index 0000000000..2abb015ece --- /dev/null +++ b/src/Analyser/ConditionalExpressionHolderRecipe.php @@ -0,0 +1,118 @@ + $conditionEntries [exprString, expr, fromSureTypes, type] + * @param list $holderEntries [exprString, expr, type, target type pinned at compose time (null = read the applying scope)] + */ + public function __construct( + private array $conditionEntries, + private array $holderEntries, + private bool $holdersFromSureTypes, + ) + { + } + + /** + * @return array + */ + public function evaluate(MutatingScope $scope): array + { + $conditionExpressionTypes = []; + $droppedNoOpConditions = []; + // the unnarrowed type of each condition expression, for the + // dropped-self-condition complement below + $conditionOriginalTypes = []; + foreach ($this->conditionEntries as [$exprString, $expr, $fromSureTypes, $type]) { + $scopeType = $scope->getType($expr); + $conditionType = $fromSureTypes + ? TypeCombinator::remove($scopeType, $type) + : TypeCombinator::intersect($scopeType, $type); + if ($scopeType->equals($conditionType)) { + $droppedNoOpConditions[$exprString] = true; + continue; + } + + $conditionExpressionTypes[$exprString] = ExpressionTypeHolder::createYes($expr, $conditionType); + $conditionOriginalTypes[$exprString] = $scopeType; + } + + if ($conditionExpressionTypes === []) { + return []; + } + + $holders = []; + foreach ($this->holderEntries as [$exprString, $expr, $type, $pinnedTargetType]) { + // The target's only link to the antecedent was a no-op relation (e.g. + // `$a === $b`) that got dropped, so the antecedent no longer constrains + // it. Projecting a consequent onto it would fire unsoundly. Skip it. + if (array_key_exists($exprString, $droppedNoOpConditions)) { + continue; + } + + $conditions = $conditionExpressionTypes; + $droppedSelfCondition = null; + if (isset($conditions[$exprString])) { + $droppedSelfCondition = $conditions[$exprString]; + unset($conditions[$exprString]); + } + + if ($conditions === []) { + continue; + } + + $targetType = $pinnedTargetType ?? $scope->getType($expr); + $holderType = $this->holdersFromSureTypes + ? TypeCombinator::intersect($targetType, $type) + : TypeCombinator::remove($targetType, $type); + + // The dropped self-condition narrowed the target; without it the + // holder must allow the values it excluded, or it over-narrows when + // only the remaining conditions hold. So union back the complement. + if ($droppedSelfCondition !== null) { + $complement = TypeCombinator::remove($conditionOriginalTypes[$exprString], $droppedSelfCondition->getType()); + if (!$complement instanceof NeverType) { + $holderType = TypeCombinator::union($holderType, $complement); + } + } + + // These boolean-decomposition holders only refine an expression's + // type in a future scope; they must never collapse it to never and + // thereby mark the whole scope unreachable. A never result is an + // artifact (e.g. removing a non-nullable property's full type after + // swapping isset() narrowing), not a real contradiction. + if ($holderType instanceof NeverType && !$targetType instanceof NeverType) { + continue; + } + $holder = new ConditionalExpressionHolder( + $conditions, + ExpressionTypeHolder::createYes($expr, $holderType), + ); + $holders[$exprString] ??= []; + $holders[$exprString][$holder->getKey()] = $holder; + } + + return $holders; + } + +} diff --git a/src/Analyser/DeferredSpecifiedTypesAugment.php b/src/Analyser/DeferredSpecifiedTypesAugment.php new file mode 100644 index 0000000000..f5f97d98af --- /dev/null +++ b/src/Analyser/DeferredSpecifiedTypesAugment.php @@ -0,0 +1,17 @@ + $candidates [target expr, left branch type, right branch type] + */ + public function __construct( + private TypeSpecifier $typeSpecifier, + private array $candidates, + ) + { + } + + public function evaluate(MutatingScope $scope): ?SpecifiedTypes + { + $result = null; + foreach ($this->candidates as [$targetExpr, $leftType, $rightType]) { + if (!$scope->hasExpressionType($targetExpr)->yes()) { + continue; + } + + // the guard above pins the target as tracked on the applying scope + $originalType = $scope->getType($targetExpr); + // re-pinning eagerly priced branch forms of a template-typed subject + // stacks the template inside its own bound (`T of T of ...` - the + // pin intersects with the declared template); its narrowing already + // flows through the operands' exact merge + if (TypeUtils::containsTemplateType($originalType)) { + continue; + } + if ($leftType->equals($originalType) || !$originalType->isSuperTypeOf($leftType)->yes()) { + continue; + } + + if ($rightType->equals($originalType) || !$originalType->isSuperTypeOf($rightType)->yes()) { + continue; + } + + $unionType = TypeCombinator::union($leftType, $rightType); + // a union that covers the whole original type gains no narrowing - + // pinning it would only stack a redundant intersection on the + // expression (e.g. re-wrapping a template type in its own bound) + if ($unionType->isSuperTypeOf($originalType)->yes()) { + continue; + } + + $created = $this->typeSpecifier->create($targetExpr, $unionType, TypeSpecifierContext::createTrue(), $scope); + $result = $result === null ? $created : $result->unionWith($created); + } + + return $result; + } + +} diff --git a/src/Analyser/DisjunctionHolderProjectionAugment.php b/src/Analyser/DisjunctionHolderProjectionAugment.php new file mode 100644 index 0000000000..bc514bc9ae --- /dev/null +++ b/src/Analyser/DisjunctionHolderProjectionAugment.php @@ -0,0 +1,103 @@ + $alternativeKeys expressions the exact either-branch + * merge already constrains - the weaker branch-scope union must not + * be added on top + */ + public function __construct( + private TypeSpecifier $typeSpecifier, + private $leftTruthyScope, + private MutatingScope $leftFalseyScope, + private $rightTruthyScope, + private array $alternativeKeys, + ) + { + } + + public function evaluate(MutatingScope $scope): ?SpecifiedTypes + { + $result = null; + $seen = []; + $leftTruthyScope = null; + $rightTruthyScope = null; + foreach ([$scope, $this->leftFalseyScope] as $sourceScope) { + foreach ($sourceScope->getConditionalExpressions() as $rootExprString => $holders) { + if (isset($seen[$rootExprString])) { + continue; + } + if ($holders === []) { + continue; + } + $seen[$rootExprString] = true; + $targetExpr = $holders[array_key_first($holders)]->getTypeHolder()->getExpr(); + + if (isset($this->alternativeKeys[$rootExprString])) { + continue; + } + + // Only project when the target stays Yes-defined in the original + // scope and in both filtered branches. A sure type implicitly + // raises certainty to Yes, which would wrongly upgrade Maybe-defined + // variables — `if (empty($a['bar']))` for instance leaves `$a` + // Maybe-defined because `empty()` tolerates undefined offsets. + if (!$scope->hasExpressionType($targetExpr)->yes()) { + continue; + } + $leftTruthyScope ??= ($this->leftTruthyScope)(); + $rightTruthyScope ??= ($this->rightTruthyScope)(); + + // the guard above pins the target as tracked on the applying + // scope; the branch scopes are its own filtered views, so their + // reads answer from state (or price the same tracked state) + $origType = $scope->getType($targetExpr); + + $leftType = $leftTruthyScope->getType($targetExpr); + $leftNarrowed = !$leftType->equals($origType) && $origType->isSuperTypeOf($leftType)->yes(); + if (!$leftNarrowed) { + continue; + } + + $rightType = $rightTruthyScope->getType($targetExpr); + $rightNarrowed = !$rightType->equals($origType) && $origType->isSuperTypeOf($rightType)->yes(); + if (!$rightNarrowed) { + continue; + } + + $unionType = TypeCombinator::union($leftType, $rightType); + if ($unionType->equals($origType)) { + continue; + } + + $created = $this->typeSpecifier->create($targetExpr, $unionType, TypeSpecifierContext::createTrue(), $scope); + $result = $result === null ? $created : $result->unionWith($created); + } + } + + return $result; + } + +} diff --git a/src/Analyser/ExprHandler/BooleanAndHandler.php b/src/Analyser/ExprHandler/BooleanAndHandler.php index cad60960e8..e7441dc1ed 100644 --- a/src/Analyser/ExprHandler/BooleanAndHandler.php +++ b/src/Analyser/ExprHandler/BooleanAndHandler.php @@ -29,8 +29,10 @@ use PHPStan\Type\NeverType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; +use function array_filter; use function array_merge; use function array_reverse; +use function array_values; use function is_string; /** @@ -105,10 +107,17 @@ public function specifyTypes(TypeSpecifier $typeSpecifier, Scope $scope, Expr $e if ($context->true()) { $types = $leftTypes->unionWith($rightTypes); } else { - $leftNormalized = $this->conditionalExpressionHolderHelper->toSureTypes($leftTypes, $scope); - $rightNormalized = $this->conditionalExpressionHolderHelper->toSureTypes($rightTypes, $rightScope); $types = $leftTypes->intersectWith($rightTypes); - $types = $this->conditionalExpressionHolderHelper->augmentDisjunctionTypes($scope, $rightScope, $leftNormalized, $rightNormalized, $expr->left, $expr->right, false, $types); + $branchUnionAugment = $this->conditionalExpressionHolderHelper->buildBranchUnionAugment( + $leftTypes, + $rightTypes, + static fn (): MutatingScope => $scope->filterByFalseyValue($expr->left), + static fn (): MutatingScope => $rightScope->filterByFalseyValue($expr->right), + $types, + ); + if ($branchUnionAugment !== null) { + $types = $types->withDeferredAugment($branchUnionAugment); + } } if ($context->false()) { // Consequent (holder) narrowings projected by each holder: these must be @@ -128,37 +137,41 @@ public function specifyTypes(TypeSpecifier $typeSpecifier, Scope $scope, Expr $e // Condition (antecedent) narrowings: when an arm has no falsey narrowing // (e.g. isset() on an array dim fetch), derive the condition from the truthy // narrowing by swapping sure/sureNot types. This swap is only sound for the - // antecedent — processBooleanConditionalTypes inverts it back to the truthy + // antecedent — the holder-recipe evaluation inverts it back to the truthy // narrowing. It must NOT feed the consequent: inverting a comparison's truthy // narrowing (e.g. `$a === $b` narrowing `$a` to `$b`'s broad type) would // over-narrow the consequent (see regression for `$x === $nonConstantString`). + // + // The inverted narrowing stands in for "this side is TRUE", so the + // side's truthy narrowing must be EQUIVALENT to its truth, not just + // implied by it. isset() qualifies: it is exactly the offset's + // non-nullness. A call like non-strict in_array($x, $a) does not - + // its truthy narrowing ($a non-empty) can hold while the call is + // false, and a holder conditioned on it would unsoundly narrow the + // other side (e.g. $x !== null && in_array($x, $a) pinning $x to + // null in a sibling branch where only $a !== [] is known). $leftCondTypes = $leftHolderTypes; $rightCondTypes = $rightHolderTypes; - if ($leftCondTypes->getSureTypes() === [] && $leftCondTypes->getSureNotTypes() === []) { + if ($leftCondTypes->getSureTypes() === [] && $leftCondTypes->getSureNotTypes() === [] && $this->truthinessImpliedByTruthyNarrowing($expr->left)) { $truthyLeftTypes = $typeSpecifier->specifyTypesInCondition($scope, $expr->left, TypeSpecifierContext::createTruthy()); if ($this->allExpressionsTrackable($truthyLeftTypes)) { $leftCondTypes = new SpecifiedTypes($truthyLeftTypes->getSureNotTypes(), $truthyLeftTypes->getSureTypes()); } } - if ($rightCondTypes->getSureTypes() === [] && $rightCondTypes->getSureNotTypes() === []) { + if ($rightCondTypes->getSureTypes() === [] && $rightCondTypes->getSureNotTypes() === [] && $this->truthinessImpliedByTruthyNarrowing($expr->right)) { $truthyRightTypes = $typeSpecifier->specifyTypesInCondition($rightScope, $expr->right, TypeSpecifierContext::createTruthy()); if ($this->allExpressionsTrackable($truthyRightTypes)) { $rightCondTypes = new SpecifiedTypes($truthyRightTypes->getSureNotTypes(), $truthyRightTypes->getSureTypes()); } } - $result = (new SpecifiedTypes( - $types->getSureTypes(), - $types->getSureNotTypes(), - ))->withAlternativeTypesOf($types); - if ($types->shouldOverwrite()) { - $result = $result->setAlwaysOverwriteTypes(); - } - return $result->setNewConditionalExpressionHolders($this->conditionalExpressionHolderHelper->mergeConditionalHolders([ - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $leftCondTypes, $rightHolderTypes, false, true, $rightScope, $expr->right), - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $rightCondTypes, $leftHolderTypes, false, true, $scope, $expr->left), - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $leftCondTypes, $rightHolderTypes, true, true, $rightScope, $expr->right), - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $rightCondTypes, $leftHolderTypes, true, true, $scope, $expr->left), - ]))->setRootExpr($expr); + $result = $types->withoutConditionalExpressionHolders(); + $recipes = [ + $this->conditionalExpressionHolderHelper->buildConditionalHolderRecipe($leftCondTypes, $rightHolderTypes, false, true, $rightScope, $expr->right), + $this->conditionalExpressionHolderHelper->buildConditionalHolderRecipe($rightCondTypes, $leftHolderTypes, false, true, null, $expr->left), + $this->conditionalExpressionHolderHelper->buildConditionalHolderRecipe($leftCondTypes, $rightHolderTypes, true, true, $rightScope, $expr->right), + $this->conditionalExpressionHolderHelper->buildConditionalHolderRecipe($rightCondTypes, $leftHolderTypes, true, true, null, $expr->left), + ]; + return $result->setConditionalExpressionHolderRecipes(array_values(array_filter($recipes)))->setRootExpr($expr); } return $types; @@ -233,6 +246,19 @@ private function specifyTypesForFlattenedBooleanAnd( return (new SpecifiedTypes($sureTypes, $sureNotTypes))->setRootExpr($expr); } + /** + * Whether the side's truthy narrowing is EQUIVALENT to the side being + * true - the requirement for using its inversion as a holder antecedent. + * isset() qualifies: it is exactly the offset's non-nullness. Anything + * else reaching the antecedent-swap fallback (e.g. a non-strict + * in_array() call, whose truthy narrowing only implies a non-empty + * haystack) must not stand in for its own truth. + */ + private function truthinessImpliedByTruthyNarrowing(Expr $side): bool + { + return $side instanceof Expr\Isset_; + } + private function allExpressionsTrackable(SpecifiedTypes $types): bool { foreach ($types->getSureTypes() as [$expr]) { diff --git a/src/Analyser/ExprHandler/BooleanOrHandler.php b/src/Analyser/ExprHandler/BooleanOrHandler.php index fb9914e56c..1ea250d55d 100644 --- a/src/Analyser/ExprHandler/BooleanOrHandler.php +++ b/src/Analyser/ExprHandler/BooleanOrHandler.php @@ -6,6 +6,7 @@ use PhpParser\Node\Expr\BinaryOp\BooleanOr; use PhpParser\Node\Expr\BinaryOp\LogicalOr; use PhpParser\Node\Stmt; +use PHPStan\Analyser\DisjunctionHolderProjectionAugment; use PHPStan\Analyser\ExpressionContext; use PHPStan\Analyser\ExpressionResult; use PHPStan\Analyser\ExpressionResultFactory; @@ -27,10 +28,12 @@ use PHPStan\Type\NeverType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; -use function array_key_first; +use function array_filter; use function array_key_last; +use function array_keys; use function array_merge; use function array_reverse; +use function array_values; use function count; /** @@ -148,37 +151,49 @@ public function specifyTypes(TypeSpecifier $typeSpecifier, Scope $scope, Expr $e if ( $scope->getType($expr->left)->toBoolean()->isFalse()->yes() ) { - $types = $this->conditionalExpressionHolderHelper->toSureTypes($rightTypes, $rightScope); + $types = $rightTypes; } elseif ( $scope->getType($expr->left)->toBoolean()->isTrue()->yes() || $scope->getType($expr->right)->toBoolean()->isFalse()->yes() ) { - $types = $this->conditionalExpressionHolderHelper->toSureTypes($leftTypes, $scope); + $types = $leftTypes; } else { - $leftNormalized = $this->conditionalExpressionHolderHelper->toSureTypes($leftTypes, $scope); - $rightNormalized = $this->conditionalExpressionHolderHelper->toSureTypes($rightTypes, $rightScope); $types = $leftTypes->intersectWith($rightTypes); - $types = $this->augmentBooleanOrTruthyWithConditionalHolders($typeSpecifier, $scope, $rightScope, $expr, $types); - $types = $this->conditionalExpressionHolderHelper->augmentDisjunctionTypes($scope, $rightScope, $leftNormalized, $rightNormalized, $expr->left, $expr->right, true, $types); + $alternativeKeys = []; + foreach (array_keys($types->getAlternativeTypes()) as $alternativeExprString) { + $alternativeKeys[$alternativeExprString] = true; + } + $types = $types->withDeferredAugment(new DisjunctionHolderProjectionAugment( + $typeSpecifier, + static fn (): MutatingScope => $scope->filterByTruthyValue($expr->left), + $rightScope, + static fn (): MutatingScope => $rightScope->filterByTruthyValue($expr->right), + $alternativeKeys, + )); + $branchUnionAugment = $this->conditionalExpressionHolderHelper->buildBranchUnionAugment( + $leftTypes, + $rightTypes, + static fn (): MutatingScope => $scope->filterByTruthyValue($expr->left), + static fn (): MutatingScope => $rightScope->filterByTruthyValue($expr->right), + $types, + ); + if ($branchUnionAugment !== null) { + $types = $types->withDeferredAugment($branchUnionAugment); + } } } else { $types = $leftTypes->unionWith($rightTypes); } if ($context->true()) { - $result = (new SpecifiedTypes( - $types->getSureTypes(), - $types->getSureNotTypes(), - ))->withAlternativeTypesOf($types); - if ($types->shouldOverwrite()) { - $result = $result->setAlwaysOverwriteTypes(); - } - return $result->setNewConditionalExpressionHolders($this->conditionalExpressionHolderHelper->mergeConditionalHolders([ - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes, false, false, $rightScope, $expr->right), - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes, false, false, $scope, $expr->left), - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $leftTypes, $rightTypes, true, false, $rightScope, $expr->right), - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $rightTypes, $leftTypes, true, false, $scope, $expr->left), - ]))->setRootExpr($expr); + $result = $types->withoutConditionalExpressionHolders(); + $recipes = [ + $this->conditionalExpressionHolderHelper->buildConditionalHolderRecipe($leftTypes, $rightTypes, false, false, $rightScope, $expr->right), + $this->conditionalExpressionHolderHelper->buildConditionalHolderRecipe($rightTypes, $leftTypes, false, false, null, $expr->left), + $this->conditionalExpressionHolderHelper->buildConditionalHolderRecipe($leftTypes, $rightTypes, true, false, $rightScope, $expr->right), + $this->conditionalExpressionHolderHelper->buildConditionalHolderRecipe($rightTypes, $leftTypes, true, false, null, $expr->left), + ]; + return $result->setConditionalExpressionHolderRecipes(array_values(array_filter($recipes)))->setRootExpr($expr); } return $types; @@ -262,87 +277,6 @@ private function specifyTypesForFlattenedBooleanOr( return $result->setRootExpr($expr); } - /** - * For `if ($a || $b)` truthy, expressions narrowed by stored conditional - * holders (e.g. `$a = $obj instanceof ClassA;` records "when `$a` is - * truthy, `$obj` is `ClassA`") need to be projected into the OR-truthy - * scope as the union of the per-arm narrowings. specifyTypesInCondition - * for each arm only looks at the boolean variable itself, so the held - * narrowing of `$obj` would otherwise be invisible until a later check - * pins one of the booleans down. - * - * For each conditional-holder target $T: - * - resolve $T's type in the left-truthy and right-truthy filtered scopes - * - if both narrow $T strictly below the original, add `$T : leftT|rightT` - * as a sure type to the OR-truthy result - * - * The asymmetric case (one arm narrows, the other doesn't) is intentionally - * skipped: in the OR-truthy scope the arm that didn't narrow could still be - * the truthy one, so the sound result is the original (unnarrowed) type. - */ - private function augmentBooleanOrTruthyWithConditionalHolders(TypeSpecifier $typeSpecifier, MutatingScope $scope, MutatingScope $rightScope, BooleanOr|LogicalOr $expr, SpecifiedTypes $types): SpecifiedTypes - { - $leftTruthyScope = null; - $rightTruthyScope = null; - - $seen = []; - foreach ([$scope, $rightScope] as $sourceScope) { - foreach ($sourceScope->getConditionalExpressions() as $exprString => $holders) { - if (isset($seen[$exprString])) { - continue; - } - if ($holders === []) { - continue; - } - $seen[$exprString] = true; - $targetExpr = $holders[array_key_first($holders)]->getTypeHolder()->getExpr(); - - // Only project when the target stays Yes-defined in the original - // scope and in both filtered branches. A sure type implicitly - // raises certainty to Yes, which would wrongly upgrade Maybe-defined - // variables — `if (empty($a['bar']))` for instance leaves `$a` - // Maybe-defined because `empty()` tolerates undefined offsets. - if (!$scope->hasExpressionType($targetExpr)->yes()) { - continue; - } - - $leftTruthyScope ??= $scope->filterByTruthyValue($expr->left); - if (!$leftTruthyScope->hasExpressionType($targetExpr)->yes()) { - continue; - } - $rightTruthyScope ??= $rightScope->filterByTruthyValue($expr->right); - if (!$rightTruthyScope->hasExpressionType($targetExpr)->yes()) { - continue; - } - - $origType = $scope->getType($targetExpr); - - $leftType = $leftTruthyScope->getType($targetExpr); - $leftNarrowed = !$leftType->equals($origType) && $origType->isSuperTypeOf($leftType)->yes(); - if (!$leftNarrowed) { - continue; - } - - $rightType = $rightTruthyScope->getType($targetExpr); - $rightNarrowed = !$rightType->equals($origType) && $origType->isSuperTypeOf($rightType)->yes(); - if (!$rightNarrowed) { - continue; - } - - $unionType = TypeCombinator::union($leftType, $rightType); - if ($unionType->equals($origType)) { - continue; - } - - $types = $types->unionWith( - $typeSpecifier->create($targetExpr, $unionType, TypeSpecifierContext::createTrue(), $scope), - ); - } - } - - return $types; - } - public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { $leftResult = $nodeScopeResolver->processExprNode($stmt, $expr->left, $scope, $storage, $nodeCallback, $context->enterDeep()); diff --git a/src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php b/src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php index f0f6b533e3..bc615d90ac 100644 --- a/src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php +++ b/src/Analyser/ExprHandler/Helper/ConditionalExpressionHolderHelper.php @@ -7,18 +7,12 @@ use PhpParser\Node\Expr\BinaryOp\BooleanOr; use PhpParser\Node\Expr\BinaryOp\LogicalAnd; use PhpParser\Node\Expr\BinaryOp\LogicalOr; -use PHPStan\Analyser\ConditionalExpressionHolder; -use PHPStan\Analyser\ExpressionTypeHolder; +use PHPStan\Analyser\ConditionalExpressionHolderRecipe; +use PHPStan\Analyser\DisjunctionBranchUnionAugment; use PHPStan\Analyser\MutatingScope; -use PHPStan\Analyser\Scope; use PHPStan\Analyser\SpecifiedTypes; use PHPStan\Analyser\TypeSpecifier; -use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\DependencyInjection\AutowiredService; -use PHPStan\Type\NeverType; -use PHPStan\Type\TypeCombinator; -use function array_key_exists; -use function count; use function is_string; /** @@ -36,239 +30,161 @@ public function __construct( { } - public function augmentDisjunctionTypes( - MutatingScope $scope, - MutatingScope $rightScope, - SpecifiedTypes $leftNormalized, - SpecifiedTypes $rightNormalized, - Expr $leftExpr, - Expr $rightExpr, - bool $truthy, + /** + * Captures the either-branch union recovery as a deferred augment: the + * branch types are read from the operand-walk filtered scopes here at + * compose time, while the does-it-actually-narrow gates run against the + * applying scope when MutatingScope::filterBySpecifiedTypes() evaluates it. + * + * The filtered scopes are thunks resolved only when there are candidate + * expressions - deriving them per level of a deep boolean chain is + * quadratic. + * + * @param callable(): MutatingScope $leftFilteredScope + * @param callable(): MutatingScope $rightFilteredScope + */ + public function buildBranchUnionAugment( + SpecifiedTypes $leftTypes, + SpecifiedTypes $rightTypes, + callable $leftFilteredScope, + callable $rightFilteredScope, SpecifiedTypes $types, - ): SpecifiedTypes + ): ?DisjunctionBranchUnionAugment { $candidateExprs = []; - foreach ($leftNormalized->getSureTypes() as $exprString => [$exprNode, $type]) { + foreach ($leftTypes->getSureTypes() as $exprString => [$exprNode, $type]) { + $candidateExprs[$exprString] = $exprNode; + } + foreach ($rightTypes->getSureTypes() as $exprString => [$exprNode, $type]) { + $candidateExprs[$exprString] = $exprNode; + } + // sureNot entries constrain their branch too - the old normalize() + // converted them to sure entries before candidates were collected, so a + // sureNot-only narrowing (e.g. the truthy of a bool variable) must also + // contribute its subject. The branch-scope reads below price the subject + // on each filtered scope, where an impossible branch (a holder-fixpoint + // contradiction) collapses to never and drops out of the union. + foreach ($leftTypes->getSureNotTypes() as $exprString => [$exprNode, $type]) { $candidateExprs[$exprString] = $exprNode; } - foreach ($rightNormalized->getSureTypes() as $exprString => [$exprNode, $type]) { + foreach ($rightTypes->getSureNotTypes() as $exprString => [$exprNode, $type]) { $candidateExprs[$exprString] = $exprNode; } $existingSureTypes = $types->getSureTypes(); $existingAlternativeTypes = $types->getAlternativeTypes(); - $viableCandidates = []; + $candidates = []; + $leftScope = null; + $rightScope = null; foreach ($candidateExprs as $exprString => $targetExpr) { if (isset($existingSureTypes[$exprString]) || isset($existingAlternativeTypes[$exprString])) { // an alternative-form entry already encodes the either-branch // union for this expression, deferred to the application point continue; } - if (!$scope->hasExpressionType($targetExpr)->yes()) { - continue; - } - $viableCandidates[$exprString] = $targetExpr; - } - - if ($viableCandidates === []) { - return $types; - } - - if ($truthy) { - $leftFilteredScope = $scope->filterByTruthyValue($leftExpr); - $rightFilteredScope = $rightScope->filterByTruthyValue($rightExpr); - } else { - $leftFilteredScope = $scope->filterByFalseyValue($leftExpr); - $rightFilteredScope = $rightScope->filterByFalseyValue($rightExpr); - } - - foreach ($viableCandidates as $targetExpr) { - if (!$leftFilteredScope->hasExpressionType($targetExpr)->yes()) { + $leftScope ??= $leftFilteredScope(); + $rightScope ??= $rightFilteredScope(); + if (!$leftScope->hasExpressionType($targetExpr)->yes()) { continue; } - if (!$rightFilteredScope->hasExpressionType($targetExpr)->yes()) { + if (!$rightScope->hasExpressionType($targetExpr)->yes()) { continue; } - $originalType = $scope->getType($targetExpr); - $leftType = $leftFilteredScope->getType($targetExpr); - $rightType = $rightFilteredScope->getType($targetExpr); - - if ($leftType->equals($originalType) || !$originalType->isSuperTypeOf($leftType)->yes()) { - continue; - } - - if ($rightType->equals($originalType) || !$originalType->isSuperTypeOf($rightType)->yes()) { - continue; - } - - $unionType = TypeCombinator::union($leftType, $rightType); - if ($unionType->equals($originalType)) { - continue; - } + // the guards above pin the target as tracked on both filtered + // scopes - scope state answers without a walk + $candidates[] = [ + $targetExpr, + $leftScope->getType($targetExpr), + $rightScope->getType($targetExpr), + ]; + } - $types = $types->unionWith( - $this->typeSpecifier->create($targetExpr, $unionType, TypeSpecifierContext::createTrue(), $scope), - ); + if ($candidates === []) { + return null; } - return $types; + return new DisjunctionBranchUnionAugment($this->typeSpecifier, $candidates); } /** - * Combines several `processBooleanConditionalTypes()` results into one map. + * Captures the raw entries of a boolean-decomposition holder pair as a + * recipe; the state-dependent complement/target math runs against the + * applying scope when MutatingScope::filterBySpecifiedTypes() evaluates it. * - * A plain `array_merge()` would be keyed by the target expression string and - * therefore let a later result overwrite an earlier one targeting the same - * expression, silently dropping a holder. Holders for the same expression are - * unioned by their key instead so all of them survive. + * The condition side asserts that its sub-expression evaluates truthy. + * When that sub-expression is itself a compound boolean (e.g. `$a && $b`), + * the narrowings making it true are spread across both the sure and + * sureNot lists of its specification. All of them are conjuncts of the + * single "this side is true" condition, so they must be gathered together + * into one condition set. Picking only one list would drop a conjunct and + * let the resulting holder fire too eagerly. * - * @param list> $holderLists - * @return array + * @param MutatingScope|null $nonVariableTargetScope the operand-walk scope non-variable + * holder targets were tracked on; their types are pinned from it at compose + * time (null = read every target from the applying scope) */ - public function mergeConditionalHolders(array $holderLists): array + public function buildConditionalHolderRecipe(SpecifiedTypes $conditionSpecifiedTypes, SpecifiedTypes $holderSpecifiedTypes, bool $holdersFromSureTypes, bool $holderSideIsNegated, ?MutatingScope $nonVariableTargetScope, ?Expr $holderSideExpr = null): ?ConditionalExpressionHolderRecipe { - $result = []; - foreach ($holderLists as $holders) { - foreach ($holders as $exprString => $exprHolders) { - foreach ($exprHolders as $key => $holder) { - $result[$exprString][$key] = $holder; - } - } + // an alternative-form entry (a cross-kind either-branch merge) has no + // single condition type; dropping it from the condition set would let + // the holder fire too eagerly - build no holders from such a condition + if ($conditionSpecifiedTypes->getAlternativeTypes() !== []) { + return null; } - return $result; - } + // A holder side that is itself a compound boolean cannot always be split + // into independent per-expression holders. In the `BooleanAnd` false + // context the holder asserts its side is false: when that side is a + // conjunction (`$a && $b`), its negation is the disjunction `!$a || !$b`, + // which has no per-expression narrowing — narrowing each conjunct + // independently would drop a reachable value (e.g. `$a = false, $b = true`). + // Symmetrically, in the `BooleanOr` true context the holder asserts its + // side is true, and a disjunction side (`$a || $b`) is itself a disjunction. + // Such a side is left whole rather than split into over-narrowing holders. + if ($this->isUnsplittableCompoundHolderSide($holderSideExpr, $holderSideIsNegated)) { + return null; + } - /** - * @return array - */ - public function processBooleanConditionalTypes(Scope $scope, SpecifiedTypes $conditionSpecifiedTypes, SpecifiedTypes $holderSpecifiedTypes, bool $holdersFromSureTypes, bool $holderSideIsNegated, Scope $rightScope, ?Expr $holderSideExpr = null): array - { - // The condition side asserts that its sub-expression evaluates truthy. - // When that sub-expression is itself a compound boolean (e.g. `$a && $b`), - // the narrowings making it true are spread across both the sure and - // sureNot lists of its specification. All of them are conjuncts of the - // single "this side is true" condition, so they must be gathered together - // into one condition set. Picking only one list would drop a conjunct and - // let the resulting holder fire too eagerly. - $conditionExpressionTypes = []; - $droppedNoOpConditions = []; + $conditionEntries = []; foreach ($conditionSpecifiedTypes->getSureTypes() as $exprString => [$expr, $type]) { if (!$this->isTrackableExpression($expr)) { continue; } - $scopeType = $scope->getType($expr); - $conditionType = TypeCombinator::remove($scopeType, $type); - if ($scopeType->equals($conditionType)) { - $droppedNoOpConditions[$exprString] = true; - continue; - } - - $conditionExpressionTypes[$exprString] = ExpressionTypeHolder::createYes( - $expr, - $conditionType, - ); + $conditionEntries[] = [$exprString, $expr, true, $type]; } foreach ($conditionSpecifiedTypes->getSureNotTypes() as $exprString => [$expr, $type]) { if (!$this->isTrackableExpression($expr)) { continue; } - $scopeType = $scope->getType($expr); - $conditionType = TypeCombinator::intersect($scopeType, $type); - if ($scopeType->equals($conditionType)) { - $droppedNoOpConditions[$exprString] = true; - continue; - } - - $conditionExpressionTypes[$exprString] = ExpressionTypeHolder::createYes( - $expr, - $conditionType, - ); + $conditionEntries[] = [$exprString, $expr, false, $type]; } - if (count($conditionExpressionTypes) > 0) { - $holders = []; - $holderTypes = $holdersFromSureTypes ? $holderSpecifiedTypes->getSureTypes() : $holderSpecifiedTypes->getSureNotTypes(); + if ($conditionEntries === []) { + return null; + } - // A holder side that is itself a compound boolean cannot always be split - // into independent per-expression holders. In the `BooleanAnd` false - // context the holder asserts its side is false: when that side is a - // conjunction (`$a && $b`), its negation is the disjunction `!$a || !$b`, - // which has no per-expression narrowing — narrowing each conjunct - // independently would drop a reachable value (e.g. `$a = false, $b = true`). - // Symmetrically, in the `BooleanOr` true context the holder asserts its - // side is true, and a disjunction side (`$a || $b`) is itself a disjunction. - // Such a side is left whole rather than split into over-narrowing holders. - if ($this->isUnsplittableCompoundHolderSide($holderSideExpr, $holderSideIsNegated)) { - return []; + $holderEntries = []; + $holderTypes = $holdersFromSureTypes ? $holderSpecifiedTypes->getSureTypes() : $holderSpecifiedTypes->getSureNotTypes(); + foreach ($holderTypes as $exprString => [$expr, $type]) { + if (!$this->isTrackableExpression($expr)) { + continue; } - foreach ($holderTypes as $exprString => [$expr, $type]) { - if (!$this->isTrackableExpression($expr)) { - continue; - } - - // The target's only link to the antecedent was a no-op relation (e.g. - // `$a === $b`) that got dropped, so the antecedent no longer constrains - // it. Projecting a consequent onto it would fire unsoundly. Skip it. - if (array_key_exists($exprString, $droppedNoOpConditions)) { - continue; - } - - $conditions = $conditionExpressionTypes; - $droppedSelfCondition = null; - foreach ($conditions as $conditionExprString => $condition) { - if ($conditionExprString !== $exprString) { - continue; - } - $droppedSelfCondition = $condition; - unset($conditions[$conditionExprString]); - } - - if (count($conditions) === 0) { - continue; - } - - $targetScope = $expr instanceof Expr\Variable ? $scope : $rightScope; - $targetType = $targetScope->getType($expr); - $holderType = $holdersFromSureTypes - ? TypeCombinator::intersect($targetType, $type) - : TypeCombinator::remove($targetType, $type); - - // The dropped self-condition narrowed the target; without it the - // holder must allow the values it excluded, or it over-narrows when - // only the remaining conditions hold. So union back the complement. - if ($droppedSelfCondition !== null) { - $complement = TypeCombinator::remove($scope->getType($expr), $droppedSelfCondition->getType()); - if (!$complement instanceof NeverType) { - $holderType = TypeCombinator::union($holderType, $complement); - } - } - - // These boolean-decomposition holders only refine an expression's - // type in a future scope; they must never collapse it to never and - // thereby mark the whole scope unreachable. A never result is an - // artifact (e.g. removing a non-nullable property's full type after - // swapping isset() narrowing), not a real contradiction. - if ($holderType instanceof NeverType && !$targetType instanceof NeverType) { - continue; - } - $holder = new ConditionalExpressionHolder( - $conditions, - ExpressionTypeHolder::createYes($expr, $holderType), - ); - $holders[$exprString] ??= []; - $holders[$exprString][$holder->getKey()] = $holder; - } + $pinnedTargetType = !$expr instanceof Expr\Variable && $nonVariableTargetScope !== null + ? $nonVariableTargetScope->getType($expr) + : null; + $holderEntries[] = [$exprString, $expr, $type, $pinnedTargetType]; + } - return $holders; + if ($holderEntries === []) { + return null; } - return []; + return new ConditionalExpressionHolderRecipe($conditionEntries, $holderEntries, $holdersFromSureTypes); } /** @@ -301,33 +217,4 @@ private function isTrackableExpression(Expr $expr): bool || $expr instanceof Expr\StaticPropertyFetch; } - /** - * The eager form of the old SpecifiedTypes::normalize(): folds sure-not - * entries into sure entries by subtracting from the expression's type on - * the given scope. Only for consumers that need concrete sure types at - * composition time (conditional-holder building, decided operands) - - * merge paths use SpecifiedTypes::intersectWith() and evaluate at the - * application point instead. - */ - public function toSureTypes(SpecifiedTypes $types, Scope $scope): SpecifiedTypes - { - $sureTypes = $types->getSureTypes(); - - foreach ($types->getSureNotTypes() as $exprString => [$exprNode, $sureNotType]) { - if (!isset($sureTypes[$exprString])) { - $sureTypes[$exprString] = [$exprNode, TypeCombinator::remove($scope->getType($exprNode), $sureNotType)]; - continue; - } - - $sureTypes[$exprString][1] = TypeCombinator::remove($sureTypes[$exprString][1], $sureNotType); - } - - $result = new SpecifiedTypes($sureTypes, []); - if ($types->shouldOverwrite()) { - $result = $result->setAlwaysOverwriteTypes(); - } - - return $result->setRootExpr($types->getRootExpr()); - } - } diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 8dbbd7ae53..257ccc4de9 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -111,6 +111,7 @@ use function array_map; use function array_merge; use function array_pop; +use function array_shift; use function array_slice; use function array_unique; use function array_values; @@ -3338,6 +3339,22 @@ public function filterByFalseyValue(Expr $expr): self */ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self { + // deferred augments see this scope's pre-application state - the + // application point of the narrowing; their entries join this batch + $pendingAugments = $specifiedTypes->getDeferredAugments(); + while ($pendingAugments !== []) { + $augment = array_shift($pendingAugments); + $augmentTypes = $augment->evaluate($this); + if ($augmentTypes === null) { + continue; + } + + foreach ($augmentTypes->getDeferredAugments() as $nestedAugment) { + $pendingAugments[] = $nestedAugment; + } + $specifiedTypes = $specifiedTypes->unionWith($augmentTypes); + } + $typeSpecifications = ScopeOps::buildTypeSpecifications($specifiedTypes->getSureTypes(), $specifiedTypes->getSureNotTypes()); foreach ($specifiedTypes->getAlternativeTypes() as $exprString => [$alternativeExpr, $terms]) { @@ -3463,12 +3480,23 @@ private function applyFilteredConditions(self $scope, array $conditions, Specifi } } + $newConditionalExpressionHolders = $specifiedTypes->getNewConditionalExpressionHolders(); + foreach ($specifiedTypes->getConditionalExpressionHolderRecipes() as $recipe) { + // the recipes' state-dependent math runs here, against this scope's + // pre-application state - the application point of the narrowing + foreach ($recipe->evaluate($this) as $recipeExprString => $recipeHolders) { + foreach ($recipeHolders as $key => $holder) { + $newConditionalExpressionHolders[$recipeExprString][$key] = $holder; + } + } + } + /** @var static */ return ScopeOps::scopeWith( $scope, $scope->expressionTypes, $scope->nativeExpressionTypes, - $this->mergeConditionalExpressions($specifiedTypes->getNewConditionalExpressionHolders(), $scope->conditionalExpressions), + $this->mergeConditionalExpressions($newConditionalExpressionHolders, $scope->conditionalExpressions), $scope->currentlyAssignedExpressions, $scope->currentlyAllowedUndefinedExpressions, $scope->inFunctionCallsStack, diff --git a/src/Analyser/SpecifiedTypes.php b/src/Analyser/SpecifiedTypes.php index 222020de5c..e1cbc91586 100644 --- a/src/Analyser/SpecifiedTypes.php +++ b/src/Analyser/SpecifiedTypes.php @@ -17,6 +17,23 @@ final class SpecifiedTypes /** @var array */ private array $newConditionalExpressionHolders = []; + /** + * Deferred boolean-decomposition holders, evaluated against the applying + * scope by MutatingScope::filterBySpecifiedTypes(). + * + * @var list + */ + private array $conditionalExpressionHolderRecipes = []; + + /** + * State-dependent augmentations evaluated against the applying scope by + * MutatingScope::filterBySpecifiedTypes(); their entries join the applied + * batch. + * + * @var list + */ + private array $deferredAugments = []; + private ?Expr $rootExpr = null; /** @@ -63,11 +80,8 @@ public function __construct( */ public function setAlwaysOverwriteTypes(): self { - $self = new self($this->sureTypes, $this->sureNotTypes); - $self->alternativeTypes = $this->alternativeTypes; + $self = clone $this; $self->overwrite = true; - $self->newConditionalExpressionHolders = $this->newConditionalExpressionHolders; - $self->rootExpr = $this->rootExpr; return $self; } @@ -77,10 +91,7 @@ public function setAlwaysOverwriteTypes(): self */ public function setRootExpr(?Expr $rootExpr): self { - $self = new self($this->sureTypes, $this->sureNotTypes); - $self->alternativeTypes = $this->alternativeTypes; - $self->overwrite = $this->overwrite; - $self->newConditionalExpressionHolders = $this->newConditionalExpressionHolders; + $self = clone $this; $self->rootExpr = $rootExpr; return $self; @@ -91,15 +102,47 @@ public function setRootExpr(?Expr $rootExpr): self */ public function setNewConditionalExpressionHolders(array $newConditionalExpressionHolders): self { - $self = new self($this->sureTypes, $this->sureNotTypes); - $self->alternativeTypes = $this->alternativeTypes; - $self->overwrite = $this->overwrite; + $self = clone $this; $self->newConditionalExpressionHolders = $newConditionalExpressionHolders; - $self->rootExpr = $this->rootExpr; return $self; } + /** + * @param list $recipes + */ + public function setConditionalExpressionHolderRecipes(array $recipes): self + { + $self = clone $this; + $self->conditionalExpressionHolderRecipes = $recipes; + + return $self; + } + + /** + * @return list + */ + public function getConditionalExpressionHolderRecipes(): array + { + return $this->conditionalExpressionHolderRecipes; + } + + public function withDeferredAugment(DeferredSpecifiedTypesAugment $augment): self + { + $self = clone $this; + $self->deferredAugments = [...$this->deferredAugments, $augment]; + + return $self; + } + + /** + * @return list + */ + public function getDeferredAugments(): array + { + return $this->deferredAugments; + } + /** * @api * @return array @@ -126,6 +169,21 @@ public function getAlternativeTypes(): array return $this->alternativeTypes; } + /** + * A copy without conditional-expression holders and holder recipes - for + * the boolean-decomposition tails that replace them with freshly built + * recipes while keeping everything else (entries, alternatives, augments) + * intact. + */ + public function withoutConditionalExpressionHolders(): self + { + $self = clone $this; + $self->newConditionalExpressionHolders = []; + $self->conditionalExpressionHolderRecipes = []; + + return $self; + } + /** * A copy of this with the other's alternative-form entries - for the * composition tails that rebuild a SpecifiedTypes from the sure/sure-not @@ -162,18 +220,10 @@ public function getRootExpr(): ?Expr public function removeExpr(string $exprString): self { - $sureTypes = $this->sureTypes; - $sureNotTypes = $this->sureNotTypes; - $alternativeTypes = $this->alternativeTypes; - unset($sureTypes[$exprString]); - unset($sureNotTypes[$exprString]); - unset($alternativeTypes[$exprString]); - - $self = new self($sureTypes, $sureNotTypes); - $self->alternativeTypes = $alternativeTypes; - $self->overwrite = $this->overwrite; - $self->newConditionalExpressionHolders = $this->newConditionalExpressionHolders; - $self->rootExpr = $this->rootExpr; + $self = clone $this; + unset($self->sureTypes[$exprString]); + unset($self->sureNotTypes[$exprString]); + unset($self->alternativeTypes[$exprString]); return $self; } @@ -344,6 +394,8 @@ public function unionWith(SpecifiedTypes $other): self } } $result->newConditionalExpressionHolders = $conditionalExpressionHolders; + $result->conditionalExpressionHolderRecipes = array_merge($this->conditionalExpressionHolderRecipes, $other->conditionalExpressionHolderRecipes); + $result->deferredAugments = array_merge($this->deferredAugments, $other->deferredAugments); return $result->setRootExpr($rootExpr); } diff --git a/tests/PHPStan/Analyser/TypeSpecifierTest.php b/tests/PHPStan/Analyser/TypeSpecifierTest.php index 0d011b0b0b..8fedca4dfa 100644 --- a/tests/PHPStan/Analyser/TypeSpecifierTest.php +++ b/tests/PHPStan/Analyser/TypeSpecifierTest.php @@ -655,7 +655,7 @@ public static function dataCondition(): iterable [ new Expr\Empty_(new Variable('array')), [ - '$array' => 'array{}', + '$array' => '~mixed~(0|0.0|\'\'|\'0\'|array{}|false|null)', ], [ '$array' => '~0|0.0|\'\'|\'0\'|array{}|false|null', @@ -667,7 +667,7 @@ public static function dataCondition(): iterable '$array' => '~0|0.0|\'\'|\'0\'|array{}|false|null', ], [ - '$array' => 'array{}', + '$array' => '~mixed~(0|0.0|\'\'|\'0\'|array{}|false|null)', ], ], [ diff --git a/tests/PHPStan/Analyser/nsrt/bug-14966.php b/tests/PHPStan/Analyser/nsrt/bug-14966.php new file mode 100644 index 0000000000..0847440d0e --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14966.php @@ -0,0 +1,26 @@ + $haystack + */ +function resolve(?string $needle, array $haystack): string +{ + if ($needle !== null && in_array($needle, $haystack)) { + return $needle; + } elseif ($haystack !== []) { + // reaching this branch does not imply $needle is null - the && is + // also false for a non-null $needle that is not in the haystack + assertType('string|null', $needle); + if ($needle !== null) { + return 'other:' . $needle; + } + return 'null-branch'; + } + + return 'empty'; +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-9961.php b/tests/PHPStan/Analyser/nsrt/bug-9961.php index cd410c83e0..2ec01ea9ca 100644 --- a/tests/PHPStan/Analyser/nsrt/bug-9961.php +++ b/tests/PHPStan/Analyser/nsrt/bug-9961.php @@ -25,12 +25,12 @@ public function sayHello(Ia|Ic $a): mixed if ($a instanceof Ic && $a instanceof Id) { assertType('T of Bug9961\Ic&Bug9961\Id (method Bug9961\HelloWorld::sayHello(), argument)', $a); } elseif ($a instanceof A) { - assertType('Bug9961\A&T of T of Bug9961\Ia&Bug9961\Ib (method Bug9961\HelloWorld::sayHello(), argument) (method Bug9961\HelloWorld::sayHello(), argument)', $a); + assertType('Bug9961\A&T of Bug9961\Ia&Bug9961\Ib (method Bug9961\HelloWorld::sayHello(), argument)', $a); } else { throw new \Exception; } - assertType('(Bug9961\A&T of T of Bug9961\Ia&Bug9961\Ib (method Bug9961\HelloWorld::sayHello(), argument) (method Bug9961\HelloWorld::sayHello(), argument))|T of Bug9961\Ic&Bug9961\Id (method Bug9961\HelloWorld::sayHello(), argument)', $a); + assertType('(Bug9961\A&T of Bug9961\Ia&Bug9961\Ib (method Bug9961\HelloWorld::sayHello(), argument))|T of Bug9961\Ic&Bug9961\Id (method Bug9961\HelloWorld::sayHello(), argument)', $a); return $a; } diff --git a/tests/PHPStan/Analyser/nsrt/disjunction-holder-projection.php b/tests/PHPStan/Analyser/nsrt/disjunction-holder-projection.php new file mode 100644 index 0000000000..94ac718bba --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/disjunction-holder-projection.php @@ -0,0 +1,66 @@ += 8.0 + +namespace DisjunctionHolderProjection; + +use PHPStan\TrinaryLogic; +use function PHPStan\Testing\assertType; +use function PHPStan\Testing\assertVariableCertainty; + +class ClassA {} +class ClassB {} + +class Foo +{ + + public function maybeDefinedTargetStaysMaybe(bool $c, mixed $m): void + { + if ($c) { + $obj = $m; + } + + $isA = $obj instanceof ClassA; + $isB = $obj instanceof ClassB; + + assertVariableCertainty(TrinaryLogic::createMaybe(), $obj); + + if ($isA || $isB) { + // The projection of the stored-boolean holders must not fire for + // $obj: it is only Maybe-defined here, and a projected sure type + // would wrongly upgrade the certainty to Yes. + assertVariableCertainty(TrinaryLogic::createMaybe(), $obj); + assertType('mixed', $obj); + } + } + + public function reassignedTargetKeepsItsNewType(bool $c, mixed $m, mixed $m2): void + { + if ($c) { + $obj = $m; + } + + $isA = $obj instanceof ClassA; + $isB = $obj instanceof ClassB; + $cond = $isA || $isB; + + $obj = $m2; + + if ($cond) { + // The stored-boolean branch reads were captured while $obj was + // Maybe-defined; projecting them onto the reassigned $obj would + // resurrect the stale compose-time narrowing. + assertVariableCertainty(TrinaryLogic::createYes(), $obj); + assertType('mixed', $obj); + } + } + + public function definedTargetIsProjected(mixed $obj): void + { + $isA = $obj instanceof ClassA; + $isB = $obj instanceof ClassB; + + if ($isA || $isB) { + assertType('DisjunctionHolderProjection\ClassA|DisjunctionHolderProjection\ClassB', $obj); + } + } + +} diff --git a/tests/PHPStan/Analyser/nsrt/falsey-empty-certainty.php b/tests/PHPStan/Analyser/nsrt/falsey-empty-certainty.php index ba24b22730..7c6bd5fe97 100644 --- a/tests/PHPStan/Analyser/nsrt/falsey-empty-certainty.php +++ b/tests/PHPStan/Analyser/nsrt/falsey-empty-certainty.php @@ -64,7 +64,8 @@ function maybeEmpty(): void if (!empty($foo)) { assertVariableCertainty(TrinaryLogic::createYes(), $foo); } else { - assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); + // $foo is 1 when defined, so empty($foo) can only be true when it is undefined + assertVariableCertainty(TrinaryLogic::createNo(), $foo); } assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); } @@ -81,9 +82,10 @@ function maybeEmptyUnset(): void unset($foo); assertVariableCertainty(TrinaryLogic::createNo(), $foo); } else { - assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); + // $foo is 1 when defined, so empty($foo) can only be true when it is undefined + assertVariableCertainty(TrinaryLogic::createNo(), $foo); } - assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); + assertVariableCertainty(TrinaryLogic::createNo(), $foo); } diff --git a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php index 4bbb387ecb..cb51bb0268 100644 --- a/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeFunctionCallRuleTest.php @@ -1347,6 +1347,13 @@ public function testBug8980(): void $this->analyse([__DIR__ . '/data/bug-8980.php'], []); } + #[RequiresPhp('>= 8.1.0')] + public function testBug14908(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-14908.php'], []); + } + public function testBug6211(): void { $this->treatPhpDocTypesAsCertain = true; diff --git a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php index de35e7711e..c59730e3b9 100644 --- a/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php @@ -1266,6 +1266,17 @@ public function testBug14985(): void $this->analyse([__DIR__ . '/data/bug-14985.php'], []); } + #[RequiresPhp('>= 8.1.0')] + public function testBug14908(): void + { + $this->analyse([__DIR__ . '/data/bug-14908.php'], []); + } + + public function testBug14966(): void + { + $this->analyse([__DIR__ . '/data/bug-14966.php'], []); + } + public function testBug14847(): void { $this->analyse([__DIR__ . '/data/bug-14847.php'], [ diff --git a/tests/PHPStan/Rules/Comparison/data/bug-14908.php b/tests/PHPStan/Rules/Comparison/data/bug-14908.php new file mode 100644 index 0000000000..89b4fd7dd8 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-14908.php @@ -0,0 +1,44 @@ += 8.1 + +namespace Bug14908; + +use function in_array; + +enum Grade { case One; case Two; case Three; } +enum Kind { case K1; case K2; case K3; } + +class Flags { public bool $flagA = false; } + +function run(Kind $kind, Grade $grade, Flags $flags, bool $extra, bool $cond): void +{ + $forced = false; + if ( + $grade !== Grade::Three + && $cond + && in_array($kind, [Kind::K1, Kind::K2], true) + && $flags->flagA === true + ) { + $forced = true; + } + + // Intermediate `if` narrowing ANOTHER value (`$extra === false`) in a disjunction. + // This is the ingredient that defeats the #14807 fix. + if ( + $forced === false + && ( + ($grade === Grade::One && $extra === false) + || ($cond && $grade !== Grade::Three) + ) + ) { + throw new \Exception(); + } + + if ($grade !== Grade::Three) { + if ($flags->flagA === false) { + throw new \Exception(); + } + if (in_array($kind, [Kind::K1, Kind::K2], true)) { + echo "reachable"; + } + } +} diff --git a/tests/PHPStan/Rules/Comparison/data/bug-14966.php b/tests/PHPStan/Rules/Comparison/data/bug-14966.php new file mode 100644 index 0000000000..39a5209271 --- /dev/null +++ b/tests/PHPStan/Rules/Comparison/data/bug-14966.php @@ -0,0 +1,22 @@ + $haystack + */ +function resolve(?string $needle, array $haystack): string +{ + if ($needle !== null && in_array($needle, $haystack)) { + return $needle; + } elseif ($haystack !== []) { + if ($needle !== null) { + return 'other:' . $needle; + } + return 'null-branch'; + } + + return 'empty'; +}