From d97ddee4d53fa82962452f2b9fe60c334b9149f9 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 24 Feb 2021 18:21:34 +0100 Subject: [PATCH] Type description verbosity - be more verbose when invariant template type is involved --- .../Arrays/AppendedArrayItemTypeRule.php | 2 +- src/Rules/FunctionCallParametersCheck.php | 2 +- src/Rules/FunctionReturnTypeCheck.php | 3 +- .../IncompatibleDefaultParameterTypeRule.php | 2 +- src/Rules/Generators/YieldFromTypeRule.php | 4 +- src/Rules/Generators/YieldTypeRule.php | 4 +- .../IncompatibleDefaultParameterTypeRule.php | 2 +- ...aultValueTypesAssignedToPropertiesRule.php | 2 +- .../TypesAssignedToPropertiesRule.php | 2 +- src/Type/VerbosityLevel.php | 51 ++++++++++++- .../Rules/Methods/ReturnTypeRuleTest.php | 18 +++++ tests/PHPStan/Rules/Methods/data/bug-4590.php | 75 +++++++++++++++++++ 12 files changed, 153 insertions(+), 14 deletions(-) create mode 100644 tests/PHPStan/Rules/Methods/data/bug-4590.php diff --git a/src/Rules/Arrays/AppendedArrayItemTypeRule.php b/src/Rules/Arrays/AppendedArrayItemTypeRule.php index 04a387e7ef..246a31b25c 100644 --- a/src/Rules/Arrays/AppendedArrayItemTypeRule.php +++ b/src/Rules/Arrays/AppendedArrayItemTypeRule.php @@ -76,7 +76,7 @@ public function processNode(\PhpParser\Node $node, Scope $scope): array $itemType = $assignedToType->getItemType(); if (!$this->ruleLevelHelper->accepts($itemType, $assignedValueType, $scope->isDeclareStrictTypes())) { - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($itemType); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($itemType, $assignedValueType); return [ RuleErrorBuilder::message(sprintf( 'Array (%s) does not accept %s.', diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index 3ca0cacda8..042ee038a9 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -254,7 +254,7 @@ static function (Type $type): bool { && !$parameter->passedByReference()->createsNewVariable() && !$this->ruleLevelHelper->accepts($parameterType, $argumentValueType, $scope->isDeclareStrictTypes()) ) { - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($parameterType); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($parameterType, $argumentValueType); $parameterDescription = sprintf('%s$%s', $parameter->isVariadic() ? '...' : '', $parameter->getName()); $errors[] = RuleErrorBuilder::message(sprintf( $messages[6], diff --git a/src/Rules/FunctionReturnTypeCheck.php b/src/Rules/FunctionReturnTypeCheck.php index 694222ac01..f3ee8d568b 100644 --- a/src/Rules/FunctionReturnTypeCheck.php +++ b/src/Rules/FunctionReturnTypeCheck.php @@ -68,7 +68,7 @@ public function checkReturnType( } $isVoidSuperType = (new VoidType())->isSuperTypeOf($returnType); - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType, null); if ($returnValue === null) { if (!$isVoidSuperType->no()) { return []; @@ -83,6 +83,7 @@ public function checkReturnType( } $returnValueType = $scope->getType($returnValue); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType, $returnValueType); if ($isVoidSuperType->yes()) { return [ diff --git a/src/Rules/Functions/IncompatibleDefaultParameterTypeRule.php b/src/Rules/Functions/IncompatibleDefaultParameterTypeRule.php index 713fb0a759..031a4b9c31 100644 --- a/src/Rules/Functions/IncompatibleDefaultParameterTypeRule.php +++ b/src/Rules/Functions/IncompatibleDefaultParameterTypeRule.php @@ -51,7 +51,7 @@ public function processNode(Node $node, Scope $scope): array continue; } - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($parameterType); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($parameterType, $defaultValueType); $errors[] = RuleErrorBuilder::message(sprintf( 'Default value of the parameter #%d $%s (%s) of function %s() is incompatible with type %s.', diff --git a/src/Rules/Generators/YieldFromTypeRule.php b/src/Rules/Generators/YieldFromTypeRule.php index 137c75d981..8e6980eae2 100644 --- a/src/Rules/Generators/YieldFromTypeRule.php +++ b/src/Rules/Generators/YieldFromTypeRule.php @@ -80,7 +80,7 @@ public function processNode(Node $node, Scope $scope): array $messages = []; if (!$this->ruleLevelHelper->accepts($returnType->getIterableKeyType(), $exprType->getIterableKeyType(), $scope->isDeclareStrictTypes())) { - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType->getIterableKeyType()); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType->getIterableKeyType(), $exprType->getIterableKeyType()); $messages[] = RuleErrorBuilder::message(sprintf( 'Generator expects key type %s, %s given.', $returnType->getIterableKeyType()->describe($verbosityLevel), @@ -88,7 +88,7 @@ public function processNode(Node $node, Scope $scope): array ))->line($node->expr->getLine())->build(); } if (!$this->ruleLevelHelper->accepts($returnType->getIterableValueType(), $exprType->getIterableValueType(), $scope->isDeclareStrictTypes())) { - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType->getIterableValueType()); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType->getIterableValueType(), $exprType->getIterableValueType()); $messages[] = RuleErrorBuilder::message(sprintf( 'Generator expects value type %s, %s given.', $returnType->getIterableValueType()->describe($verbosityLevel), diff --git a/src/Rules/Generators/YieldTypeRule.php b/src/Rules/Generators/YieldTypeRule.php index 67a189a476..f0b04ba578 100644 --- a/src/Rules/Generators/YieldTypeRule.php +++ b/src/Rules/Generators/YieldTypeRule.php @@ -64,7 +64,7 @@ public function processNode(Node $node, Scope $scope): array $messages = []; if (!$this->ruleLevelHelper->accepts($returnType->getIterableKeyType(), $keyType, $scope->isDeclareStrictTypes())) { - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType->getIterableKeyType()); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType->getIterableKeyType(), $keyType); $messages[] = RuleErrorBuilder::message(sprintf( 'Generator expects key type %s, %s given.', $returnType->getIterableKeyType()->describe($verbosityLevel), @@ -72,7 +72,7 @@ public function processNode(Node $node, Scope $scope): array ))->build(); } if (!$this->ruleLevelHelper->accepts($returnType->getIterableValueType(), $valueType, $scope->isDeclareStrictTypes())) { - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType->getIterableValueType()); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($returnType->getIterableValueType(), $valueType); $messages[] = RuleErrorBuilder::message(sprintf( 'Generator expects value type %s, %s given.', $returnType->getIterableValueType()->describe($verbosityLevel), diff --git a/src/Rules/Methods/IncompatibleDefaultParameterTypeRule.php b/src/Rules/Methods/IncompatibleDefaultParameterTypeRule.php index 097150c3cc..172b0170ee 100644 --- a/src/Rules/Methods/IncompatibleDefaultParameterTypeRule.php +++ b/src/Rules/Methods/IncompatibleDefaultParameterTypeRule.php @@ -52,7 +52,7 @@ public function processNode(Node $node, Scope $scope): array continue; } - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($parameterType); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($parameterType, $defaultValueType); $errors[] = RuleErrorBuilder::message(sprintf( 'Default value of the parameter #%d $%s (%s) of method %s::%s() is incompatible with type %s.', diff --git a/src/Rules/Properties/DefaultValueTypesAssignedToPropertiesRule.php b/src/Rules/Properties/DefaultValueTypesAssignedToPropertiesRule.php index c7fe39d6d3..24d929355e 100644 --- a/src/Rules/Properties/DefaultValueTypesAssignedToPropertiesRule.php +++ b/src/Rules/Properties/DefaultValueTypesAssignedToPropertiesRule.php @@ -52,7 +52,7 @@ public function processNode(Node $node, Scope $scope): array return []; } - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($propertyType); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($propertyType, $defaultValueType); return [ RuleErrorBuilder::message(sprintf( diff --git a/src/Rules/Properties/TypesAssignedToPropertiesRule.php b/src/Rules/Properties/TypesAssignedToPropertiesRule.php index e3fd9b4dab..1a123ffdea 100644 --- a/src/Rules/Properties/TypesAssignedToPropertiesRule.php +++ b/src/Rules/Properties/TypesAssignedToPropertiesRule.php @@ -89,7 +89,7 @@ private function processSingleProperty( } if (!$this->ruleLevelHelper->accepts($propertyType, $assignedValueType, $scope->isDeclareStrictTypes())) { $propertyDescription = $this->propertyDescriptor->describePropertyByName($propertyReflection, $propertyReflection->getName()); - $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($propertyType); + $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($propertyType, $assignedValueType); return [ RuleErrorBuilder::message(sprintf( diff --git a/src/Type/VerbosityLevel.php b/src/Type/VerbosityLevel.php index e6b02d0e09..8cd82fdd28 100644 --- a/src/Type/VerbosityLevel.php +++ b/src/Type/VerbosityLevel.php @@ -4,6 +4,8 @@ use PHPStan\Type\Accessory\AccessoryNumericStringType; use PHPStan\Type\Accessory\NonEmptyArrayType; +use PHPStan\Type\Generic\GenericObjectType; +use PHPStan\Type\Generic\TemplateType; class VerbosityLevel { @@ -49,10 +51,9 @@ public static function cache(): self return self::create(self::CACHE); } - public static function getRecommendedLevelByType(Type $type): self + public static function getRecommendedLevelByType(Type $acceptingType, ?Type $acceptedType = null): self { - $moreVerbose = false; - TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$moreVerbose): Type { + $moreVerboseCallback = static function (Type $type, callable $traverse) use (&$moreVerbose): Type { if ($type->isCallable()->yes()) { $moreVerbose = true; return $type; @@ -69,9 +70,53 @@ public static function getRecommendedLevelByType(Type $type): self $moreVerbose = true; return $type; } + return $traverse($type); + }; + + /** @var bool $moreVerbose */ + $moreVerbose = false; + TypeTraverser::map($acceptingType, $moreVerboseCallback); + + if ($moreVerbose) { + return self::value(); + } + + if ($acceptedType === null) { + return self::typeOnly(); + } + + $containsInvariantTemplateType = false; + TypeTraverser::map($acceptingType, static function (Type $type, callable $traverse) use (&$containsInvariantTemplateType): Type { + if ($type instanceof GenericObjectType) { + $reflection = $type->getClassReflection(); + if ($reflection !== null) { + $templateTypeMap = $reflection->getTemplateTypeMap(); + foreach ($templateTypeMap->getTypes() as $templateType) { + if (!$templateType instanceof TemplateType) { + continue; + } + + if (!$templateType->getVariance()->invariant()) { + continue; + } + + $containsInvariantTemplateType = true; + return $type; + } + } + } + return $traverse($type); }); + if (!$containsInvariantTemplateType) { + return self::typeOnly(); + } + + /** @var bool $moreVerbose */ + $moreVerbose = false; + TypeTraverser::map($acceptedType, $moreVerboseCallback); + return $moreVerbose ? self::value() : self::typeOnly(); } diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index 7bf82cc5ba..0f0d07b8ab 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -426,4 +426,22 @@ public function testInferArrayKey(): void $this->analyse([__DIR__ . '/data/infer-array-key.php'], []); } + public function testBug4590(): void + { + $this->analyse([__DIR__ . '/data/bug-4590.php'], [ + [ + 'Method Bug4590\Controller::test1() should return Bug4590\OkResponse> but returns Bug4590\OkResponse string)>.', + 39, + ], + [ + 'Method Bug4590\Controller::test2() should return Bug4590\OkResponse> but returns Bug4590\OkResponse.', + 47, + ], + [ + 'Method Bug4590\Controller::test3() should return Bug4590\OkResponse> but returns Bug4590\OkResponse.', + 55, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-4590.php b/tests/PHPStan/Rules/Methods/data/bug-4590.php new file mode 100644 index 0000000000..a3db4a3445 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-4590.php @@ -0,0 +1,75 @@ +body = $body; + } + + /** + * @phpstan-return T + */ + public function getBody() + { + return $this->body; + } +} + +class Controller +{ + /** + * @return OkResponse> + */ + public function test1(): OkResponse + { + return new OkResponse(["ok" => "hello"]); + } + + /** + * @return OkResponse> + */ + public function test2(): OkResponse + { + return new OkResponse([0 => "hello"]); + } + + /** + * @return OkResponse + */ + public function test3(): OkResponse + { + return new OkResponse(["hello"]); + } + + /** + * @return OkResponse + */ + public function test4(): OkResponse + { + return new OkResponse("hello"); + } + + /** + * @param array $a + * @return OkResponse> + */ + public function test5(array $a): OkResponse + { + return new OkResponse($a); + } + +}