Type description verbosity - be more verbose when invariant template …

…type is involved
phpstan · Feb 25, 2021 · d97ddee · d97ddee
1 parent 5efd078
commit d97ddee
Show file tree

Hide file tree

Showing 12 changed files with 153 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -80,15 +80,15 @@ 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),
 				$exprType->getIterableKeyType()->describe($verbosityLevel)
 			))->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
@@ -64,15 +64,15 @@ 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),
 				$keyType->describe($verbosityLevel)
 			))->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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<array<string, string>> but returns Bug4590\OkResponse<array(\'ok\' => string)>.',
+				39,
+			],
+			[
+				'Method Bug4590\Controller::test2() should return Bug4590\OkResponse<array<int, string>> but returns Bug4590\OkResponse<array(string)>.',
+				47,
+			],
+			[
+				'Method Bug4590\Controller::test3() should return Bug4590\OkResponse<array<string>> but returns Bug4590\OkResponse<array(string)>.',
+				55,
+			],
+		]);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Methods/data/bug-4590.php b/tests/PHPStan/Rules/Methods/data/bug-4590.php
@@ -0,0 +1,75 @@
+<?php
+
+namespace Bug4590;
+
+/**
+ * @template T
+ */
+class OkResponse
+{
+	/**
+	 * @phpstan-var T
+	 */
+	private $body;
+
+	/**
+	 * @phpstan-param T $body
+	 */
+	public function __construct($body)
+	{
+		$this->body = $body;
+	}
+
+	/**
+	 * @phpstan-return T
+	 */
+	public function getBody()
+	{
+		return $this->body;
+	}
+}
+
+class Controller
+{
+	/**
+	 * @return OkResponse<array<string, string>>
+	 */
+	public function test1(): OkResponse
+	{
+		return new OkResponse(["ok" => "hello"]);
+	}
+
+	/**
+	 * @return OkResponse<array<int, string>>
+	 */
+	public function test2(): OkResponse
+	{
+		return new OkResponse([0 => "hello"]);
+	}
+
+	/**
+	 * @return OkResponse<string[]>
+	 */
+	public function test3(): OkResponse
+	{
+		return new OkResponse(["hello"]);
+	}
+
+	/**
+	 * @return OkResponse<string>
+	 */
+	public function test4(): OkResponse
+	{
+		return new OkResponse("hello");
+	}
+
+	/**
+	 * @param array<int, string> $a
+	 * @return OkResponse<array<int, string>>
+	 */
+	public function test5(array $a): OkResponse
+	{
+		return new OkResponse($a);
+	}
+
+}