diff --git a/src/Analyser/ExprHandler/ArrayDimFetchHandler.php b/src/Analyser/ExprHandler/ArrayDimFetchHandler.php index f5e26d83d89..4bfa98c7e52 100644 --- a/src/Analyser/ExprHandler/ArrayDimFetchHandler.php +++ b/src/Analyser/ExprHandler/ArrayDimFetchHandler.php @@ -104,6 +104,13 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, $impurePoints = array_merge($dimResult->getImpurePoints(), $varResult->getImpurePoints()); $varType = $varResult->getType(); + // an offset read that is a link in a nullsafe chain may never run - see + // MethodCallHandler::processExpr(). The dimension is walked BEFORE the + // receiver here, so the short-circuited world is the pre-dimension scope. + $mayShortCircuit = $varResult->containsNullsafe() && TypeCombinator::containsNull($varType); + if ($mayShortCircuit) { + $scope = $scope->mergeWith($beforeScope); + } $offsetGetCall = null; if (!$varType->isArray()->yes() && !(new ObjectType(ArrayAccess::class))->isSuperTypeOf($varType)->no()) { $throwPoints = array_merge($throwPoints, $this->methodThrowPointHelper->getThrowPointsForCallOnType( @@ -123,9 +130,17 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, $scope, beforeScope: $beforeScope, expr: $expr, - variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), $dimResult->getVariableFlow(), self::offsetRead($expr, $dimResult, $context), VariableFlowBuilder::throws($expr, $throwPoints)), + variableFlow: VariableFlow::sequence( + $varResult->getVariableFlow(), + // the dimension was not evaluated in the short-circuited world + $mayShortCircuit + ? VariableFlow::choice($dimResult->getVariableFlow(), null) + : $dimResult->getVariableFlow(), + self::offsetRead($expr, $dimResult, $context), + VariableFlowBuilder::throws($expr, $throwPoints), + ), hasYield: $dimResult->hasYield() || $varResult->hasYield(), - isAlwaysTerminating: $dimResult->isAlwaysTerminating() || $varResult->isAlwaysTerminating(), + isAlwaysTerminating: (!$mayShortCircuit && $dimResult->isAlwaysTerminating()) || $varResult->isAlwaysTerminating(), throwPoints: $throwPoints, impurePoints: $impurePoints, containsNullsafe: $varResult->containsNullsafe(), diff --git a/src/Analyser/ExprHandler/Helper/DefaultNarrowingHelper.php b/src/Analyser/ExprHandler/Helper/DefaultNarrowingHelper.php index ec0be10b988..06a03c39583 100644 --- a/src/Analyser/ExprHandler/Helper/DefaultNarrowingHelper.php +++ b/src/Analyser/ExprHandler/Helper/DefaultNarrowingHelper.php @@ -347,6 +347,23 @@ public function createNullsafeReceiverOnlyTypes(MutatingScope $s, Expr $subject, return $this->createFirstNullsafeReceiverTypes($s, $subject) ?? new SpecifiedTypes([], []); } + /** + * Whether a call on a `?->` chain may have been skipped in the branch + * $context describes, so nothing the callee declares narrows there. + */ + public function callMayHaveBeenSkipped(?ExpressionResult $receiverResult, Type $receiverType, TypeSpecifierContext $context): bool + { + if ($receiverResult === null || !$receiverResult->containsNullsafe()) { + return false; + } + + if (!$context->null() && !$context->falseyButNotFalse()) { + return false; + } + + return TypeCombinator::containsNull($receiverType); + } + /** * Whether the constraint (or the subject's own type) rules the nullsafe * short-circuit null out, so the chain's receivers can narrow not-null. diff --git a/src/Analyser/ExprHandler/MethodCallHandler.php b/src/Analyser/ExprHandler/MethodCallHandler.php index 7ee09182825..519efa7867f 100644 --- a/src/Analyser/ExprHandler/MethodCallHandler.php +++ b/src/Analyser/ExprHandler/MethodCallHandler.php @@ -125,10 +125,16 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex // the var was processed above as the receiver; read its already-computed // result instead of re-walking via Scope::getType(). $calledOnType = $varResult->getType(); + // A plain call that is a link in a nullsafe chain may never run: the chain + // short-circuits to null before the arguments are evaluated and before any + // of the call's effects happen. NullsafeMethodCallHandler does the same for + // the `?->` it owns; here the `?->` sits below a plain `->`. + $mayShortCircuit = $varResult->containsNullsafe() && TypeCombinator::containsNull($calledOnType); // A call configured as early-terminating never returns: give it an explicit // never so the statement's exit point follows from the result type, instead of // NodeScopeResolver re-deriving it via Scope::getType(). - $isEarlyTerminating = $expr->name instanceof Identifier + $isEarlyTerminating = !$mayShortCircuit + && $expr->name instanceof Identifier && $this->earlyTerminatingHelper->isEarlyTerminatingMethodCall($expr->name->name, $calledOnType); $isAlwaysTerminating = $isAlwaysTerminating || $isEarlyTerminating; if ($expr->name instanceof Identifier) { @@ -155,7 +161,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex if ($parametersAcceptor !== null) { $normalizedExpr = ArgumentsNormalizer::reorderMethodArguments($parametersAcceptor, $expr) ?? $expr; $returnType = $parametersAcceptor->getReturnType(); - $isAlwaysTerminating = $isAlwaysTerminating || ($returnType instanceof NeverType && $returnType->isExplicit()); + $isAlwaysTerminating = $isAlwaysTerminating || (!$mayShortCircuit && $returnType instanceof NeverType && $returnType->isExplicit()); } $scopeBeforeArgs = $scope; @@ -286,7 +292,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex // return type; a conditional-return never (e.g. `($x is Foo ? never : // string)`) only resolves to never once the actual argument types are // folded in by the type-driven resolved acceptor. - if ($resolvedParametersAcceptor !== null) { + if ($resolvedParametersAcceptor !== null && !$mayShortCircuit) { $resolvedReturnType = $resolvedParametersAcceptor->getReturnType(); $isAlwaysTerminating = $isAlwaysTerminating || ($resolvedReturnType instanceof NeverType && $resolvedReturnType->isExplicit()); } @@ -355,16 +361,24 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $hasYield = $hasYield || $argsResult->hasYield(); $throwPoints = array_merge($throwPoints, $argsResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $argsResult->getImpurePoints()); - $isAlwaysTerminating = $isAlwaysTerminating || $argsResult->isAlwaysTerminating(); + $isAlwaysTerminating = $isAlwaysTerminating || (!$mayShortCircuit && $argsResult->isAlwaysTerminating()); + $argumentsFlow = VariableFlowBuilder::arguments($expr, $argsResult, $storage); $variableFlow = VariableFlow::sequence( $varResult->getVariableFlow(), $nameResult !== null ? $nameResult->getVariableFlow() : null, - VariableFlowBuilder::arguments($expr, $argsResult, $storage), + // the short-circuited world evaluates none of the arguments + $mayShortCircuit ? VariableFlow::choice($argumentsFlow, null) : $argumentsFlow, VariableFlowBuilder::throws($expr, $throwPoints), $isAlwaysTerminating ? VariableFlow::exit(VariableFlow::STOP) : null, ); + // the call's scope effects (@param-out, @phpstan-self-out, invalidations) + // only happened in the world where the chain did not short-circuit + if ($mayShortCircuit) { + $scope = $scope->mergeWith($scopeBeforeArgs); + } + $result = $preliminaryResult->finalize($scope, $hasYield, $isAlwaysTerminating, $throwPoints, $impurePoints, $variableFlow); // the var was processed above as the receiver; read its already-computed @@ -492,7 +506,10 @@ private function specifyTypes(MutatingScope $scope, Expr $expr, Expr $normalized // result instead of re-walking via Scope::getType(). $methodCalledOnType = $varResult->getTypeOnScope($scope, $scope->nativeTypesPromoted); $methodReflection = $scope->getMethodReflection($methodCalledOnType, $expr->name->name); - if ($methodReflection !== null) { + // a call on a nullsafe chain may never have run - the branches that still + // admit the short-circuit's null get no callee-derived narrowing at all + $mayHaveBeenSkipped = $this->defaultNarrowingHelper->callMayHaveBeenSkipped($varResult, $methodCalledOnType, $context); + if ($methodReflection !== null && !$mayHaveBeenSkipped) { $args = $expr->getArgs(); $referencedClasses = $methodCalledOnType->getObjectClassNames(); diff --git a/src/Analyser/ExprHandler/PropertyFetchHandler.php b/src/Analyser/ExprHandler/PropertyFetchHandler.php index e49ad9eab6c..3d5a3ddfb82 100644 --- a/src/Analyser/ExprHandler/PropertyFetchHandler.php +++ b/src/Analyser/ExprHandler/PropertyFetchHandler.php @@ -82,6 +82,7 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, PropertyFetc $impurePoints = $varResult->getImpurePoints(); $isAlwaysTerminating = $varResult->isAlwaysTerminating(); $scope = $varResult->getScope(); + $mayShortCircuit = false; if ($expr->name instanceof Identifier) { if ($this->phpVersion->supportsPropertyHooks()) { $propertyName = $expr->name->toString(); @@ -97,11 +98,20 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, PropertyFetc } } } elseif ($nameResult !== null) { + // a fetch that is a link in a nullsafe chain may never run - see + // MethodCallHandler::processExpr(). Only the dynamic name is skipped + // with it, so an Identifier name never has to resolve the receiver type. + $mayShortCircuit = $varResult->containsNullsafe() && TypeCombinator::containsNull($varResult->getType()); $hasYield = $hasYield || $nameResult->hasYield(); $throwPoints = array_merge($throwPoints, $nameResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $nameResult->getImpurePoints()); - $isAlwaysTerminating = $isAlwaysTerminating || $nameResult->isAlwaysTerminating(); + $isAlwaysTerminating = $isAlwaysTerminating || (!$mayShortCircuit && $nameResult->isAlwaysTerminating()); $scope = $nameResult->getScope(); + if ($mayShortCircuit) { + // the dynamic name expression was not evaluated in the + // short-circuited world + $scope = $scope->mergeWith($varResult->getScope()); + } if ($this->phpVersion->supportsPropertyHooks()) { $throwPoints[] = InternalThrowPoint::createImplicit($scope, $expr); } @@ -111,7 +121,13 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, PropertyFetc $scope, beforeScope: $beforeScope, expr: $expr, - variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), $nameResult !== null ? $nameResult->getVariableFlow() : null, VariableFlowBuilder::throws($expr, $throwPoints)), + variableFlow: VariableFlow::sequence( + $varResult->getVariableFlow(), + $nameResult !== null && $mayShortCircuit + ? VariableFlow::choice($nameResult->getVariableFlow(), null) + : ($nameResult !== null ? $nameResult->getVariableFlow() : null), + VariableFlowBuilder::throws($expr, $throwPoints), + ), hasYield: $hasYield, isAlwaysTerminating: $isAlwaysTerminating, throwPoints: $throwPoints, diff --git a/src/Analyser/ExprHandler/StaticCallHandler.php b/src/Analyser/ExprHandler/StaticCallHandler.php index 378204c0de4..36ba8e5062c 100644 --- a/src/Analyser/ExprHandler/StaticCallHandler.php +++ b/src/Analyser/ExprHandler/StaticCallHandler.php @@ -110,11 +110,17 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $containsNullsafe = $classResult->containsNullsafe(); } + // `$a?->b::c()` is a link in a nullsafe chain and may never run: the chain + // short-circuits to null before the arguments are evaluated and before any + // of the call's effects happen. See MethodCallHandler::processExpr(). + $mayShortCircuit = $classResult !== null + && $containsNullsafe + && TypeCombinator::containsNull($classResult->getType()); // A static call configured as early-terminating never returns: give it an // explicit never so the statement's exit point follows from the result type, // instead of NodeScopeResolver re-deriving it via Scope::getType(). $isEarlyTerminating = false; - if ($expr->name instanceof Identifier) { + if ($expr->name instanceof Identifier && !$mayShortCircuit) { $earlyTerminatingClassType = $expr->class instanceof Name ? $scope->resolveTypeByName($expr->class) : $classResult->getType(); @@ -247,7 +253,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex if ($parametersAcceptor !== null) { $normalizedExpr = ArgumentsNormalizer::reorderStaticCallArguments($parametersAcceptor, $expr) ?? $expr; $returnType = $parametersAcceptor->getReturnType(); - $isAlwaysTerminating = $isAlwaysTerminating || ($returnType instanceof NeverType && $returnType->isExplicit()); + $isAlwaysTerminating = $isAlwaysTerminating || (!$mayShortCircuit && $returnType instanceof NeverType && $returnType->isExplicit()); } $scopeBeforeArgs = $scope; if ($parametersAcceptor !== null && $context->getInAssignRightSideExpr() === $expr) { @@ -281,7 +287,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex // type; a conditional-return never (e.g. `($x is Foo ? never : string)`) // only resolves to never once the actual argument types are folded in by the // type-driven resolved acceptor. - if ($resolvedParametersAcceptor !== null) { + if ($resolvedParametersAcceptor !== null && !$mayShortCircuit) { $resolvedReturnType = $resolvedParametersAcceptor->getReturnType(); $isAlwaysTerminating = $isAlwaysTerminating || ($resolvedReturnType instanceof NeverType && $resolvedReturnType->isExplicit()); } @@ -429,16 +435,24 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $hasYield = $hasYield || $argsResult->hasYield(); $throwPoints = array_merge($throwPoints, $argsResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $argsResult->getImpurePoints()); - $isAlwaysTerminating = $isAlwaysTerminating || $argsResult->isAlwaysTerminating(); + $isAlwaysTerminating = $isAlwaysTerminating || (!$mayShortCircuit && $argsResult->isAlwaysTerminating()); + $argumentsFlow = VariableFlowBuilder::arguments($expr, $argsResult, $storage); $variableFlow = VariableFlow::sequence( $classResult !== null ? $classResult->getVariableFlow() : null, $nameResult !== null ? $nameResult->getVariableFlow() : null, - VariableFlowBuilder::arguments($expr, $argsResult, $storage), + // the short-circuited world evaluates none of the arguments + $mayShortCircuit ? VariableFlow::choice($argumentsFlow, null) : $argumentsFlow, VariableFlowBuilder::throws($expr, $throwPoints), $isAlwaysTerminating ? VariableFlow::exit(VariableFlow::STOP) : null, ); + // the call's scope effects (@param-out, invalidations) only happened in the + // world where the chain did not short-circuit + if ($mayShortCircuit) { + $scope = $scope->mergeWith($scopeBeforeArgs); + } + return $preliminaryResult->finalize($scope, $hasYield, $isAlwaysTerminating, $throwPoints, $impurePoints, $variableFlow); } @@ -571,7 +585,10 @@ private function specifyTypes(MutatingScope $scope, Expr $expr, Expr $normalized } $staticMethodReflection = $scope->getMethodReflection($calleeType, $expr->name->name); - if ($staticMethodReflection !== null) { + // see MethodCallHandler::specifyTypes() - `$a?->b::c()` short-circuits too, + // so the branches admitting its null get no callee-derived narrowing + $mayHaveBeenSkipped = $this->defaultNarrowingHelper->callMayHaveBeenSkipped($classResult, $calleeType, $context); + if ($staticMethodReflection !== null && !$mayHaveBeenSkipped) { $args = $expr->getArgs(); $referencedClasses = $calleeType->getObjectClassNames(); diff --git a/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php b/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php index b66c90d1676..ef3b685b5b2 100644 --- a/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php +++ b/src/Analyser/ExprHandler/StaticPropertyFetchHandler.php @@ -98,19 +98,37 @@ public function composeResult(StaticPropertyFetch $expr, ?ExpressionResult $clas $isAlwaysTerminating = $classResult->isAlwaysTerminating(); $scope = $classResult->getScope(); } + $mayShortCircuit = false; if ($nameResult !== null) { + // `$a?->b::$$name` is a link in a nullsafe chain and may never run - see + // MethodCallHandler::processExpr(). Only the dynamic name is skipped with + // it, so a plain `::$name` never has to resolve the class expression type. + $mayShortCircuit = $classResult !== null + && $classResult->containsNullsafe() + && TypeCombinator::containsNull($classResult->getType()); $hasYield = $hasYield || $nameResult->hasYield(); $throwPoints = array_merge($throwPoints, $nameResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $nameResult->getImpurePoints()); - $isAlwaysTerminating = $isAlwaysTerminating || $nameResult->isAlwaysTerminating(); + $isAlwaysTerminating = $isAlwaysTerminating || (!$mayShortCircuit && $nameResult->isAlwaysTerminating()); $scope = $nameResult->getScope(); + if ($mayShortCircuit) { + // the dynamic name expression was not evaluated in the + // short-circuited world + $scope = $scope->mergeWith($classResult->getScope()); + } } return $this->expressionResultFactory->create( $scope, beforeScope: $beforeScope, expr: $expr, - variableFlow: VariableFlow::sequence($classResult !== null ? $classResult->getVariableFlow() : null, $nameResult !== null ? $nameResult->getVariableFlow() : null, VariableFlowBuilder::throws($expr, $throwPoints)), + variableFlow: VariableFlow::sequence( + $classResult !== null ? $classResult->getVariableFlow() : null, + $nameResult !== null && $mayShortCircuit + ? VariableFlow::choice($nameResult->getVariableFlow(), null) + : ($nameResult !== null ? $nameResult->getVariableFlow() : null), + VariableFlowBuilder::throws($expr, $throwPoints), + ), hasYield: $hasYield, isAlwaysTerminating: $isAlwaysTerminating, throwPoints: $throwPoints, diff --git a/src/Analyser/TypeSpecifierContext.php b/src/Analyser/TypeSpecifierContext.php index d7379c26ac5..07d616f3edf 100644 --- a/src/Analyser/TypeSpecifierContext.php +++ b/src/Analyser/TypeSpecifierContext.php @@ -85,6 +85,12 @@ public function falsey(): bool return $this->value !== null && (bool) ($this->value & self::CONTEXT_FALSEY); } + /** Whether the branch admits falsey values other than `false`, e.g. `null`. */ + public function falseyButNotFalse(): bool + { + return $this->value !== null && (bool) ($this->value & self::CONTEXT_FALSEY_BUT_NOT_FALSE); + } + public function null(): bool { return $this->value === null; diff --git a/tests/PHPStan/Analyser/nsrt/bug-12925.php b/tests/PHPStan/Analyser/nsrt/bug-12925.php new file mode 100644 index 00000000000..e7a05d8a0fb --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-12925.php @@ -0,0 +1,25 @@ += 8.0 + +namespace Bug12925; + +use function PHPStan\Testing\assertType; + +/** + * @template TIsZero of bool = bool + */ +final class Decimal +{ + /** @param numeric-string $value */ + public function __construct(private string $value) {} + /** + * @phpstan-assert-if-true self $this + * @phpstan-assert-if-false self $this + */ + public function isZero(): bool { return bccomp($this->value, '0', 2) === 0; } +} +class C { public function __construct(public Decimal $p) {} } +$c = rand() ? new C(new Decimal((string)rand())) : null; + +assertType('Bug12925\C|null', $c); +echo $c?->p->isZero() ? 'Free' : 'Buying'; +assertType('Bug12925\C|null', $c); diff --git a/tests/PHPStan/Analyser/nsrt/bug-15016.php b/tests/PHPStan/Analyser/nsrt/bug-15016.php new file mode 100644 index 00000000000..5c4f6032dba --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15016.php @@ -0,0 +1,140 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug15016; + +use function PHPStan\Testing\assertType; + +/** @template-covariant T of bool = bool */ +final class Decimal +{ + + /** @phpstan-assert-if-false self $this */ + public function isZero(): bool + { + return true; + } + + /** @phpstan-assert-if-true self $this */ + public function isNotZero(): bool + { + return true; + } + + /** @phpstan-assert self $this */ + public function assertZero(): void + { + } + + /** @phpstan-assert-if-false self $other */ + public function isZeroOf(Decimal $other): bool + { + return true; + } + + /** @phpstan-assert-if-false self $other */ + public static function isZeroOfStatic(Decimal $other): bool + { + return true; + } + + /** + * @param mixed $x + * @return ($x is int ? true : false) + */ + public function isInt($x): bool + { + return is_int($x); + } + +} + +final class Subscription +{ + + public Decimal $price; + +} + +/** @return array */ +function test(?Subscription $lastPeriod): array +{ + return [ + 'a' => $lastPeriod?->price->isZero() ? 'free' : 'paid', + 'b' => $lastPeriod?->price, + ]; +} + +function assertIfFalse(?Subscription $lastPeriod): void +{ + if ($lastPeriod?->price->isZero()) { + assertType('Bug15016\Subscription', $lastPeriod); + assertType('Bug15016\\Decimal~Bug15016\\Decimal', $lastPeriod?->price); + } else { + assertType('Bug15016\Subscription|null', $lastPeriod); + assertType('Bug15016\\Decimal|null', $lastPeriod?->price); + } +} + +function assertIfFalseStrictlyFalse(?Subscription $lastPeriod): void +{ + if ($lastPeriod?->price->isZero() === false) { + assertType('Bug15016\Subscription', $lastPeriod); + assertType('Bug15016\Decimal', $lastPeriod?->price); + } +} + +function assertIfTrue(?Subscription $lastPeriod): void +{ + if ($lastPeriod?->price->isNotZero()) { + assertType('Bug15016\Subscription', $lastPeriod); + assertType('Bug15016\Decimal', $lastPeriod?->price); + } else { + assertType('Bug15016\Subscription|null', $lastPeriod); + assertType('Bug15016\\Decimal|null', $lastPeriod?->price); + } +} + +function assertUnconditionally(?Subscription $lastPeriod): void +{ + $lastPeriod?->price->assertZero(); + assertType('Bug15016\Subscription|null', $lastPeriod); + assertType('Bug15016\\Decimal|null', $lastPeriod?->price); +} + +function assertOnParameter(?Subscription $lastPeriod, Decimal $other): void +{ + if ($lastPeriod?->price->isZeroOf($other)) { + assertType('Bug15016\\Decimal~Bug15016\\Decimal', $other); + } else { + assertType('Bug15016\\Decimal', $other); + } +} + +function assertOnParameterOfStaticCall(?Subscription $lastPeriod, Decimal $other): void +{ + if ($lastPeriod?->price::isZeroOfStatic($other)) { + assertType('Bug15016\\Decimal~Bug15016\\Decimal', $other); + } else { + assertType('Bug15016\\Decimal', $other); + } +} + +function conditionalReturnType(?Subscription $lastPeriod, mixed $x): void +{ + if ($lastPeriod?->price->isInt($x)) { + assertType('int', $x); + } else { + assertType('mixed', $x); + } +} + +function nonNullableReceiverStillNarrows(Subscription $lastPeriod, Decimal $other): void +{ + if ($lastPeriod?->price->isZeroOf($other)) { + assertType('Bug15016\\Decimal~Bug15016\\Decimal', $other); + } else { + assertType('Bug15016\Decimal', $other); + } +} diff --git a/tests/PHPStan/Analyser/nsrt/nullsafe-short-circuit-effects.php b/tests/PHPStan/Analyser/nsrt/nullsafe-short-circuit-effects.php new file mode 100644 index 00000000000..8e9e65a35b4 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/nullsafe-short-circuit-effects.php @@ -0,0 +1,90 @@ += 8.0 + +declare(strict_types = 1); + +namespace NullsafeShortCircuitEffects; + +use function PHPStan\Testing\assertType; + +class Decimal +{ + + /** @param-out int $x */ + public function fill(int|string &$x): void + { + } + + /** @param-out int $x */ + public static function fillStatic(int|string &$x): void + { + } + + /** @phpstan-self-out Zero */ + public function makeZero(): void + { + } + + /** @return never */ + public function fail(): void + { + exit(1); + } + + /** @return never */ + public static function failStatic(): void + { + exit(1); + } + +} + +class Zero extends Decimal +{ + +} + +final class Holder +{ + + public Decimal $price; + +} + +function paramOut(?Holder $h): void +{ + $x = 'str'; + $h?->price->fill($x); + assertType("'str'|int", $x); +} + +function paramOutOfStaticCall(?Holder $h): void +{ + $x = 'str'; + $h?->price::fillStatic($x); + assertType("'str'|int", $x); +} + +function paramOutNonNullableReceiver(Holder $h): void +{ + $x = 'str'; + $h->price->fill($x); + assertType('int', $x); +} + +function selfOut(?Holder $h): void +{ + $h?->price->makeZero(); + assertType('NullsafeShortCircuitEffects\Decimal|null', $h?->price); +} + +function selfOutNonNullableReceiver(Holder $h): void +{ + $h->price->makeZero(); + assertType('NullsafeShortCircuitEffects\Zero', $h->price); +} + +function neverReturningCall(?Holder $h): void +{ + assertType('null', $h?->price->fail()); + assertType('null', $h?->price::failStatic()); +} diff --git a/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php b/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php index acce1cb9e04..b437f2abd87 100644 --- a/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php @@ -414,4 +414,20 @@ public function testFirstClassCallableTerminatingVar(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testNullsafeShortCircuitTerminating(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/nullsafe-short-circuit-terminating.php'], [ + [ + 'Unreachable statement - code above always terminates.', + 49, + ], + [ + 'Unreachable statement - code above always terminates.', + 56, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/DeadCode/data/nullsafe-short-circuit-terminating.php b/tests/PHPStan/Rules/DeadCode/data/nullsafe-short-circuit-terminating.php new file mode 100644 index 00000000000..d4a298ca3b2 --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/nullsafe-short-circuit-terminating.php @@ -0,0 +1,57 @@ += 8.0 + +declare(strict_types = 1); + +namespace NullsafeShortCircuitTerminating; + +class Decimal +{ + + /** @return never */ + public function fail(): void + { + exit(1); + } + + /** @return never */ + public static function failStatic(): void + { + exit(1); + } + +} + +final class Holder +{ + + public Decimal $price; + +} + +function nullableReceiver(?Holder $h): int +{ + $h?->price->fail(); + + return 1; +} + +function nullableReceiverStaticCall(?Holder $h): int +{ + $h?->price::failStatic(); + + return 1; +} + +function nonNullableReceiver(Holder $h): int +{ + $h->price->fail(); + + return 1; +} + +function nonNullableNullsafeReceiver(Holder $h): int +{ + $h?->price->fail(); + + return 1; +} diff --git a/tests/PHPStan/Rules/Properties/NullsafePropertyFetchRuleTest.php b/tests/PHPStan/Rules/Properties/NullsafePropertyFetchRuleTest.php index 4d0fa6e3f2e..9d1b794f146 100644 --- a/tests/PHPStan/Rules/Properties/NullsafePropertyFetchRuleTest.php +++ b/tests/PHPStan/Rules/Properties/NullsafePropertyFetchRuleTest.php @@ -112,4 +112,11 @@ public function testBug6922(): void $this->analyse([__DIR__ . '/data/bug-6922.php'], []); } + #[RequiresPhp('>= 8.0.0')] + public function testBug15016(): void + { + $this->treatPhpDocTypesAsCertain = true; + $this->analyse([__DIR__ . '/data/bug-15016.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/bug-15016.php b/tests/PHPStan/Rules/Properties/data/bug-15016.php new file mode 100644 index 00000000000..93620493342 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/bug-15016.php @@ -0,0 +1,33 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug15016NullsafeProperty; + +/** @template-covariant T of bool = bool */ +final class Decimal +{ + + /** @phpstan-assert-if-false self $this */ + public function isZero(): bool + { + return true; + } + +} + +final class Subscription +{ + + public Decimal $price; + +} + +/** @return array */ +function test(?Subscription $lastPeriod): array +{ + return [ + 'a' => $lastPeriod?->price->isZero() ? 'free' : 'paid', + 'b' => $lastPeriod?->price, + ]; +} diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php index aaa061cbe1e..a9dfd601ec8 100644 --- a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php +++ b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php @@ -466,6 +466,35 @@ public function testBug9392(): void $this->analyse([__DIR__ . '/data/bug-9392.php'], []); } + #[RequiresPhp('>= 8.0.0')] + public function testNullsafeShortCircuitArgs(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->checkMaybeUndefinedVariables = true; + $this->analyse([__DIR__ . '/data/nullsafe-short-circuit-args.php'], [ + [ + 'Variable $a might not be defined.', + 37, + ], + [ + 'Variable $b might not be defined.', + 43, + ], + [ + 'Variable $c might not be defined.', + 49, + ], + [ + 'Variable $d might not be defined.', + 55, + ], + [ + 'Variable $e might not be defined.', + 61, + ], + ]); + } + public function testBug14418(): void { $this->cliArgumentsVariablesRegistered = true; diff --git a/tests/PHPStan/Rules/Variables/data/nullsafe-short-circuit-args.php b/tests/PHPStan/Rules/Variables/data/nullsafe-short-circuit-args.php new file mode 100644 index 00000000000..cdc9d4b8e8a --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/nullsafe-short-circuit-args.php @@ -0,0 +1,72 @@ += 8.0 + +declare(strict_types = 1); + +namespace NullsafeShortCircuitArgs; + +class Decimal +{ + + /** @var array */ + public array $arr = []; + + public int $foo = 1; + + public static int $staticFoo = 1; + + public function pass(mixed $x): void + { + } + + public static function passStatic(mixed $x): void + { + } + +} + +final class Holder +{ + + public Decimal $price; + +} + +function methodCallArgument(?Holder $h): void +{ + $h?->price->pass($a = 5); + echo $a; +} + +function staticCallArgument(?Holder $h): void +{ + $h?->price::passStatic($b = 5); + echo $b; +} + +function dynamicPropertyName(?Holder $h): void +{ + echo $h?->price->{$c = 'foo'}; + echo $c; +} + +function offsetDimension(?Holder $h): void +{ + echo $h?->price->arr[$d = 'x']; + echo $d; +} + +function dynamicStaticPropertyName(?Holder $h): void +{ + echo $h?->price::${$e = 'staticFoo'}; + echo $e; +} + +function nonNullableReceiver(Holder $h): void +{ + $h->price->pass($f = 5); + echo $f; + echo $h->price->{$g = 'foo'}; + echo $g; + echo $h->price->arr[$i = 'x']; + echo $i; +}