diff --git a/src/Analyser/ConditionalThrowTypeResolver.php b/src/Analyser/ConditionalThrowTypeResolver.php new file mode 100644 index 00000000000..475fce096ee --- /dev/null +++ b/src/Analyser/ConditionalThrowTypeResolver.php @@ -0,0 +1,176 @@ +hasTemplateOrLateResolvableType()) { + return $throwType; + } + + // ConditionalTypeForParameter (e.g. ($x is 0 ? Exception : void)) is resolved + // against the argument types passed at the call site. + $throwType = self::mapConditionalTypesForParameter($throwType, self::getPassedArgs($parametersAcceptor, $args, $scope)); + + // ConditionalType whose subject is a template type (e.g. (TKey is int ? void : Exception)) + // is resolved against the template types inferred from the call site. + if ($parametersAcceptor instanceof ExtendedParametersAcceptor) { + $throwType = TemplateTypeHelper::resolveTemplateTypes( + $throwType, + $parametersAcceptor->getResolvedTemplateTypeMap(), + $parametersAcceptor->getCallSiteVarianceMap(), + TemplateTypeVariance::createCovariant(), + ); + } + + return TypeUtils::resolveLateResolvableTypes($throwType, false); + } + + public static function resolveForScope(Type $throwType, Scope $scope): Type + { + if (!$throwType->hasTemplateOrLateResolvableType()) { + return $throwType; + } + + $passedArgs = []; + foreach (self::collectParameterNames($throwType) as $parameterName) { + $variableName = substr($parameterName, 1); + if (!$scope->hasVariableType($variableName)->yes()) { + continue; + } + + $passedArgs[$parameterName] = $scope->getType(new Variable($variableName)); + } + + $throwType = self::mapConditionalTypesForParameter($throwType, $passedArgs); + + // A ConditionalType whose subject is a template type cannot be resolved to a single + // branch inside the function body (the template is not bound to a concrete type there), + // so it is conservatively collapsed to the union of its branches — the broadest set of + // exceptions the declaration permits — rather than left as a Maybe-certain conditional. + return TypeUtils::resolveLateResolvableTypes($throwType, true); + } + + /** + * @param array $passedArgs + */ + private static function mapConditionalTypesForParameter(Type $throwType, array $passedArgs): Type + { + if ($passedArgs === []) { + return $throwType; + } + + return TypeTraverser::map($throwType, static function (Type $type, callable $traverse) use ($passedArgs): Type { + if ($type instanceof ConditionalTypeForParameter && array_key_exists($type->getParameterName(), $passedArgs)) { + $type = $traverse($type); + if ($type instanceof ConditionalTypeForParameter) { + return $type->toConditional($passedArgs[$type->getParameterName()]); + } + + return $type; + } + + return $traverse($type); + }); + } + + /** + * @return list + */ + private static function collectParameterNames(Type $throwType): array + { + $names = []; + TypeTraverser::map($throwType, static function (Type $type, callable $traverse) use (&$names): Type { + if ($type instanceof ConditionalTypeForParameter) { + $names[] = $type->getParameterName(); + } + + return $traverse($type); + }); + + return $names; + } + + /** + * @param Arg[] $args + * @return array + */ + private static function getPassedArgs(ParametersAcceptor $parametersAcceptor, array $args, Scope $scope): array + { + $parameters = $parametersAcceptor->getParameters(); + + $namedArgTypes = []; + $i = 0; + foreach ($args as $arg) { + if ($arg->unpack) { + // unpacked arguments cannot be reliably mapped to a single parameter + $i++; + continue; + } + + if ($arg->name !== null) { + $namedArgTypes[$arg->name->toString()] = $scope->getType($arg->value); + continue; + } + + if (isset($parameters[$i])) { + $namedArgTypes[$parameters[$i]->getName()] = $scope->getType($arg->value); + } elseif (count($parameters) > 0) { + $lastParameter = array_last($parameters); + if ($lastParameter->isVariadic()) { + $namedArgTypes[$lastParameter->getName()] = $scope->getType($arg->value); + } + } + + $i++; + } + + $passedArgs = []; + foreach ($parameters as $parameter) { + if (array_key_exists($parameter->getName(), $namedArgTypes)) { + $passedArgs['$' . $parameter->getName()] = $namedArgTypes[$parameter->getName()]; + } elseif ($parameter->getDefaultValue() !== null) { + $passedArgs['$' . $parameter->getName()] = $parameter->getDefaultValue(); + } + } + + return $passedArgs; + } + +} diff --git a/src/Analyser/ExprHandler/FuncCallHandler.php b/src/Analyser/ExprHandler/FuncCallHandler.php index d28a3225dee..11fa7098c9a 100644 --- a/src/Analyser/ExprHandler/FuncCallHandler.php +++ b/src/Analyser/ExprHandler/FuncCallHandler.php @@ -14,6 +14,7 @@ use PhpParser\Node\Scalar\String_; use PhpParser\Node\Stmt; use PHPStan\Analyser\ArgumentsNormalizer; +use PHPStan\Analyser\ConditionalThrowTypeResolver; use PHPStan\Analyser\ExpressionContext; use PHPStan\Analyser\ExpressionResult; use PHPStan\Analyser\ExpressionResultStorage; @@ -604,6 +605,9 @@ private function getFunctionThrowPoint( } $throwType = $functionReflection->getThrowType(); + if ($throwType !== null && $parametersAcceptor !== null) { + $throwType = ConditionalThrowTypeResolver::resolveForCall($throwType, $parametersAcceptor, $normalizedFuncCall->getArgs(), $scope); + } if ($throwType === null) { $returnType = $scope->getType($normalizedFuncCall); if ($returnType instanceof NeverType && $returnType->isExplicit()) { diff --git a/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php b/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php index 08ff8735587..838aaefe556 100644 --- a/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php +++ b/src/Analyser/ExprHandler/Helper/MethodThrowPointHelper.php @@ -4,6 +4,7 @@ use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\StaticCall; +use PHPStan\Analyser\ConditionalThrowTypeResolver; use PHPStan\Analyser\ExpressionContext; use PHPStan\Analyser\InternalThrowPoint; use PHPStan\Analyser\MutatingScope; @@ -76,6 +77,9 @@ public function getThrowPoint( } $throwType = $methodReflection->getThrowType(); + if ($throwType !== null) { + $throwType = ConditionalThrowTypeResolver::resolveForCall($throwType, $parametersAcceptor, $normalizedMethodCall->getArgs(), $scope); + } if ($throwType === null) { $returnType = $scope->getType($normalizedMethodCall); if ($returnType instanceof NeverType && $returnType->isExplicit()) { diff --git a/src/Analyser/ExprHandler/NewHandler.php b/src/Analyser/ExprHandler/NewHandler.php index 2360ccd8076..eed726bd2bf 100644 --- a/src/Analyser/ExprHandler/NewHandler.php +++ b/src/Analyser/ExprHandler/NewHandler.php @@ -9,6 +9,7 @@ use PhpParser\Node\Name; use PhpParser\Node\Stmt; use PHPStan\Analyser\ArgumentsNormalizer; +use PHPStan\Analyser\ConditionalThrowTypeResolver; use PHPStan\Analyser\ExpressionContext; use PHPStan\Analyser\ExpressionResult; use PHPStan\Analyser\ExpressionResultStorage; @@ -302,7 +303,7 @@ private function getConstructorThrowPoint(MethodReflection $constructorReflectio } if ($constructorReflection->getThrowType() !== null) { - $throwType = $constructorReflection->getThrowType(); + $throwType = ConditionalThrowTypeResolver::resolveForCall($constructorReflection->getThrowType(), $parametersAcceptor, $args, $scope); if (!$throwType->isVoid()->yes()) { return InternalThrowPoint::createExplicit($scope, $throwType, $new, true); } diff --git a/src/PhpDoc/ResolvedPhpDocBlock.php b/src/PhpDoc/ResolvedPhpDocBlock.php index be6da4fc46d..4192b115f24 100644 --- a/src/PhpDoc/ResolvedPhpDocBlock.php +++ b/src/PhpDoc/ResolvedPhpDocBlock.php @@ -278,7 +278,7 @@ public function merge(ResolvedPhpDocBlock $parent, InheritedPhpDocParameterMappi $result->paramsImmediatelyInvokedCallable = self::mergeParamsImmediatelyInvokedCallable($this->getParamsImmediatelyInvokedCallable(), $parent, $parameterMapping); $result->paramClosureThisTags = self::mergeParamClosureThisTags($this->getParamClosureThisTags(), $parent, $parameterMapping, $parentClass); $result->returnTag = self::mergeReturnTags($this->getReturnTag(), $declaringClass, $parent, $parameterMapping, $parentClass); - $result->throwsTag = self::mergeThrowsTags($this->getThrowsTag(), $parent); + $result->throwsTag = self::mergeThrowsTags($this->getThrowsTag(), $parent, $parameterMapping); $result->mixinTags = $this->getMixinTags(); $result->requireExtendsTags = $this->getRequireExtendsTags(); $result->requireImplementsTags = $this->getRequireImplementsTags(); @@ -1016,13 +1016,20 @@ private static function mergeDeprecatedTags(?DeprecatedTag $deprecatedTag, bool return $result; } - private static function mergeThrowsTags(?ThrowsTag $throwsTag, self $parent): ?ThrowsTag + private static function mergeThrowsTags(?ThrowsTag $throwsTag, self $parent, InheritedPhpDocParameterMapping $parameterMapping): ?ThrowsTag { if ($throwsTag !== null) { return $throwsTag; } - return $parent->getThrowsTag(); + $parentThrowsTag = $parent->getThrowsTag(); + if ($parentThrowsTag === null) { + return null; + } + + // Conditional @throws types like ($x is 0 ? Exception : void) reference parameter + // names that may differ in the overriding method, so remap them just like @return. + return new ThrowsTag($parameterMapping->transformConditionalReturnTypeWithParameterNameMapping($parentThrowsTag->getType())); } /** diff --git a/src/Rules/Exceptions/MissingCheckedExceptionInThrowsCheck.php b/src/Rules/Exceptions/MissingCheckedExceptionInThrowsCheck.php index 62de7f9e2ad..9f3717d5f99 100644 --- a/src/Rules/Exceptions/MissingCheckedExceptionInThrowsCheck.php +++ b/src/Rules/Exceptions/MissingCheckedExceptionInThrowsCheck.php @@ -3,6 +3,7 @@ namespace PHPStan\Rules\Exceptions; use PhpParser\Node; +use PHPStan\Analyser\ConditionalThrowTypeResolver; use PHPStan\Analyser\ThrowPoint; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; @@ -41,11 +42,15 @@ public function check(?Type $throwType, array $throwPoints): array continue; } + // Conditional @throws types like ($x is 0 ? Exception : void) are resolved + // against the parameter variables narrowed in the scope of the throw point. + $resolvedThrowType = ConditionalThrowTypeResolver::resolveForScope($throwType, $throwPoint->getScope()); + foreach (TypeUtils::flattenTypes($throwPoint->getType()) as $throwPointType) { if ($throwPointType->isSuperTypeOf(new ObjectType(Throwable::class))->yes()) { continue; } - if ($throwType->isSuperTypeOf($throwPointType)->yes()) { + if ($resolvedThrowType->isSuperTypeOf($throwPointType)->yes()) { continue; } diff --git a/src/Rules/PhpDoc/InvalidThrowsPhpDocValueRule.php b/src/Rules/PhpDoc/InvalidThrowsPhpDocValueRule.php index 9a89cfce380..1b3f2ae26b5 100644 --- a/src/Rules/PhpDoc/InvalidThrowsPhpDocValueRule.php +++ b/src/Rules/PhpDoc/InvalidThrowsPhpDocValueRule.php @@ -10,6 +10,8 @@ use PHPStan\Node\InPropertyHookNode; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\Type\ConditionalType; +use PHPStan\Type\ConditionalTypeForParameter; use PHPStan\Type\FileTypeMapper; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; @@ -69,10 +71,6 @@ public function processNode(Node $node, Scope $scope): array } $phpDocThrowsType = $resolvedPhpDoc->getThrowsTag()->getType(); - if ($phpDocThrowsType->isVoid()->yes()) { - return []; - } - if ($this->isThrowsValid($phpDocThrowsType)) { return []; } @@ -87,10 +85,29 @@ public function processNode(Node $node, Scope $scope): array private function isThrowsValid(Type $phpDocThrowsType): bool { + // `void` standalone means "does not throw" and is a valid @throws type (it is + // likewise allowed as a branch of a conditional throws type). As a union member + // such as Throwable|void it is rejected in the UnionType handling below. + if ($phpDocThrowsType->isVoid()->yes()) { + return true; + } + + // Conditional @throws types like ($x is 0 ? Exception : void) are valid as long + // as both branches are valid throws types (a Throwable subtype or void). + if ($phpDocThrowsType instanceof ConditionalType) { + return $this->isThrowsValid($phpDocThrowsType->getIf()) + && $this->isThrowsValid($phpDocThrowsType->getElse()); + } + + if ($phpDocThrowsType instanceof ConditionalTypeForParameter) { + return $this->isThrowsValid($phpDocThrowsType->getIf()) + && $this->isThrowsValid($phpDocThrowsType->getElse()); + } + $throwType = new ObjectType(Throwable::class); if ($phpDocThrowsType instanceof UnionType) { foreach ($phpDocThrowsType->getTypes() as $innerType) { - if (!$this->isThrowsValid($innerType)) { + if ($innerType->isVoid()->yes() || !$this->isThrowsValid($innerType)) { return false; } } diff --git a/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInFunctionThrowsRuleTest.php b/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInFunctionThrowsRuleTest.php index 5c0d007b848..a6a3b6ae275 100644 --- a/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInFunctionThrowsRuleTest.php +++ b/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInFunctionThrowsRuleTest.php @@ -52,4 +52,27 @@ public function testRule(): void ]); } + public function testConditionalThrows(): void + { + require_once __DIR__ . '/data/conditional-throws-function.php'; + $this->analyse([__DIR__ . '/data/conditional-throws-function.php'], [ + [ + 'Function ConditionalThrowsFunction\callsZero() throws checked exception Exception but it\'s missing from the PHPDoc @throws tag.', + 23, + ], + [ + 'Function ConditionalThrowsFunction\callsUnknown() throws checked exception Exception but it\'s missing from the PHPDoc @throws tag.', + 35, + ], + [ + 'Function ConditionalThrowsFunction\lookupString() throws checked exception Exception but it\'s missing from the PHPDoc @throws tag.', + 68, + ], + [ + 'Function ConditionalThrowsFunction\lookupUnknown() throws checked exception Exception but it\'s missing from the PHPDoc @throws tag.', + 77, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInMethodThrowsRuleTest.php b/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInMethodThrowsRuleTest.php index 73bb2894d8b..ab4652a5aa1 100644 --- a/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInMethodThrowsRuleTest.php +++ b/tests/PHPStan/Rules/Exceptions/MissingCheckedExceptionInMethodThrowsRuleTest.php @@ -110,4 +110,30 @@ public function testBug13792(): void ]); } + public function testConditionalThrows(): void + { + $this->analyse([__DIR__ . '/data/conditional-throws-method.php'], [ + [ + 'Method ConditionalThrowsMethod\Caller::methodCallZero() throws checked exception Exception but it\'s missing from the PHPDoc @throws tag.', + 81, + ], + [ + 'Method ConditionalThrowsMethod\Caller::staticCallZero() throws checked exception Exception but it\'s missing from the PHPDoc @throws tag.', + 93, + ], + [ + 'Method ConditionalThrowsMethod\Caller::constructorZero() throws checked exception Exception but it\'s missing from the PHPDoc @throws tag.', + 105, + ], + [ + 'Method ConditionalThrowsMethod\Caller::lookupString() throws checked exception Exception but it\'s missing from the PHPDoc @throws tag.', + 123, + ], + [ + 'Method ConditionalThrowsMethod\Caller::inheritedMethodCallZero() throws checked exception Exception but it\'s missing from the PHPDoc @throws tag.', + 129, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Exceptions/data/conditional-throws-function.php b/tests/PHPStan/Rules/Exceptions/data/conditional-throws-function.php new file mode 100644 index 00000000000..b120acd7d90 --- /dev/null +++ b/tests/PHPStan/Rules/Exceptions/data/conditional-throws-function.php @@ -0,0 +1,78 @@ + $x + * @throws void + */ +function callsRange(int $x): void +{ + inverse($x); +} + +/** + * @template TKey of int|string + * @param TKey $key + * @throws (TKey is int ? void : Exception) + */ +function lookup($key): void +{ + if (is_string($key)) { + throw new Exception('String keys are not supported.'); + } +} + +/** @throws void */ +function lookupInt(): void +{ + lookup(1); +} + +/** @throws void */ +function lookupString(): void +{ + lookup('foo'); +} + +/** + * @param int|string $key + * @throws void + */ +function lookupUnknown($key): void +{ + lookup($key); +} diff --git a/tests/PHPStan/Rules/Exceptions/data/conditional-throws-method.php b/tests/PHPStan/Rules/Exceptions/data/conditional-throws-method.php new file mode 100644 index 00000000000..580cf88d5ea --- /dev/null +++ b/tests/PHPStan/Rules/Exceptions/data/conditional-throws-method.php @@ -0,0 +1,138 @@ +inverse(0); + } + + /** @throws void */ + public function methodCallNonZero(Service $service): void + { + $service->inverse(7); + } + + /** @throws void */ + public function staticCallZero(): void + { + Service::staticInverse(0); + } + + /** @throws void */ + public function staticCallNonZero(): void + { + Service::staticInverse(7); + } + + /** @throws void */ + public function constructorZero(): void + { + new Service(0); + } + + /** @throws void */ + public function constructorNonZero(): void + { + new Service(7); + } + + /** @throws void */ + public function lookupInt(Service $service): void + { + $service->lookup(1); + } + + /** @throws void */ + public function lookupString(Service $service): void + { + $service->lookup('foo'); + } + + /** @throws void */ + public function inheritedMethodCallZero(Service2 $service): void + { + $service->inverse(0); + } + + /** @throws void */ + public function inheritedMethodCallNonZero(Service2 $service): void + { + $service->inverse(7); + } + +} diff --git a/tests/PHPStan/Rules/PhpDoc/InvalidThrowsPhpDocValueRuleTest.php b/tests/PHPStan/Rules/PhpDoc/InvalidThrowsPhpDocValueRuleTest.php index 01edaab08c6..8c31e1e3741 100644 --- a/tests/PHPStan/Rules/PhpDoc/InvalidThrowsPhpDocValueRuleTest.php +++ b/tests/PHPStan/Rules/PhpDoc/InvalidThrowsPhpDocValueRuleTest.php @@ -58,6 +58,14 @@ public function testRule(): void 'PHPDoc tag @throws with type stdClass is not subtype of Throwable', 118, ], + [ + 'PHPDoc tag @throws with type ($x is int ? stdClass : void) is not subtype of Throwable', + 141, + ], + [ + 'PHPDoc tag @throws with type (TKey of int|string is int ? void : stdClass) is not subtype of Throwable', + 159, + ], ]); } diff --git a/tests/PHPStan/Rules/PhpDoc/data/incompatible-throws.php b/tests/PHPStan/Rules/PhpDoc/data/incompatible-throws.php index 7ce3088fc3f..e349be40e31 100644 --- a/tests/PHPStan/Rules/PhpDoc/data/incompatible-throws.php +++ b/tests/PHPStan/Rules/PhpDoc/data/incompatible-throws.php @@ -117,3 +117,45 @@ function inlineThrows() /** @throws \stdClass */ $i = 1; } + +/** + * @param int $x + * @throws ($x is 0 ? \Exception : void) + */ +function conditionalThrows($x) +{ +} + +/** + * @param int $x + * @throws ($x is 0 ? \Exception : \RuntimeException) + */ +function conditionalThrowsBothBranches($x) +{ +} + +/** + * @param int $x + * @throws ($x is 0 ? \stdClass : void) + */ +function conditionalThrowsInvalidBranch($x) +{ +} + +/** + * @template TKey of int|string + * @param TKey $key + * @throws (TKey is int ? void : \Exception) + */ +function conditionalThrowsForTemplate($key) +{ +} + +/** + * @template TKey of int|string + * @param TKey $key + * @throws (TKey is int ? void : \stdClass) + */ +function conditionalThrowsForTemplateInvalidBranch($key) +{ +}