MethodSignatureRule - fixed error message

Author: ondrejmirtes

src/Rules/Methods/MethodSignatureRule.php

@@ -12,6 +12,7 @@
 use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\TrinaryLogic;
 use PHPStan\Type\MixedType;
+use PHPStan\Type\Type;
 use PHPStan\Type\TypehintHelper;
 use PHPStan\Type\VerbosityLevel;
 use PHPStan\Type\VoidType;
@@ -70,22 +71,22 @@ public function processNode(Node $node, Scope $scope): array
 				continue;
 			}
 
-			$returnTypeCompatibility = $this->checkReturnTypeCompatibility($parameters, $parentParameters);
+			[$returnTypeCompatibility, $returnType, $parentReturnType] = $this->checkReturnTypeCompatibility($parameters, $parentParameters);
 			if ($returnTypeCompatibility->no() || (!$returnTypeCompatibility->yes() && $this->reportMaybes)) {
 				$errors[] = RuleErrorBuilder::message(sprintf(
 					'Return type (%s) of method %s::%s() should be %s with return type (%s) of method %s::%s()',
-					$parameters->getPhpDocReturnType()->describe(VerbosityLevel::value()),
+					$returnType->describe(VerbosityLevel::value()),
 					$method->getDeclaringClass()->getDisplayName(),
 					$method->getName(),
 					$returnTypeCompatibility->no() ? 'compatible' : 'covariant',
-					$parentParameters->getPhpDocReturnType()->describe(VerbosityLevel::value()),
+					$parentReturnType->describe(VerbosityLevel::value()),
 					$parentMethod->getDeclaringClass()->getDisplayName(),
 					$parentMethod->getName()
 				))->build();
 			}
 
 			$parameterResults = $this->checkParameterTypeCompatibility($parameters->getParameters(), $parentParameters->getParameters());
-			foreach ($parameterResults as $parameterIndex => $parameterResult) {
+			foreach ($parameterResults as $parameterIndex => [$parameterResult, $parameterType, $parentParameterType]) {
 				if ($parameterResult->yes()) {
 					continue;
 				}
@@ -98,12 +99,12 @@ public function processNode(Node $node, Scope $scope): array
 					'Parameter #%d $%s (%s) of method %s::%s() should be %s with parameter $%s (%s) of method %s::%s()',
 					$parameterIndex + 1,
 					$parameter->getName(),
-					$parameter->getType()->describe(VerbosityLevel::value()),
+					$parameterType->describe(VerbosityLevel::value()),
 					$method->getDeclaringClass()->getDisplayName(),
 					$method->getName(),
 					$parameterResult->no() ? 'compatible' : 'contravariant',
 					$parentParameter->getName(),
-					$parentParameter->getType()->describe(VerbosityLevel::value()),
+					$parentParameterType->describe(VerbosityLevel::value()),
 					$parentMethod->getDeclaringClass()->getDisplayName(),
 					$parentMethod->getName()
 				))->build();
@@ -141,10 +142,15 @@ private function collectParentMethods(string $methodName, ClassReflection $class
 		return $parentMethods;
 	}
 
+	/**
+	 * @param ParametersAcceptorWithPhpDocs $currentVariant
+	 * @param ParametersAcceptorWithPhpDocs $parentVariant
+	 * @return array{TrinaryLogic, Type, Type}
+	 */
 	private function checkReturnTypeCompatibility(
 		ParametersAcceptorWithPhpDocs $currentVariant,
 		ParametersAcceptorWithPhpDocs $parentVariant
-	): TrinaryLogic
+	): array
 	{
 		$returnType = TypehintHelper::decideType(
 			$currentVariant->getNativeReturnType(),
@@ -156,21 +162,21 @@ private function checkReturnTypeCompatibility(
 		);
 		// Allow adding `void` return type hints when the parent defines no return type
 		if ($returnType instanceof VoidType && $parentReturnType instanceof MixedType) {
-			return TrinaryLogic::createYes();
+			return [TrinaryLogic::createYes(), $returnType, $parentReturnType];
 		}
 
 		// We can return anything
 		if ($parentReturnType instanceof VoidType) {
-			return TrinaryLogic::createYes();
+			return [TrinaryLogic::createYes(), $returnType, $parentReturnType];
 		}
 
-		return $parentReturnType->isSuperTypeOf($returnType);
+		return [$parentReturnType->isSuperTypeOf($returnType), $returnType, $parentReturnType];
 	}
 
 	/**
 	 * @param \PHPStan\Reflection\ParameterReflectionWithPhpDocs[] $parameters
 	 * @param \PHPStan\Reflection\ParameterReflectionWithPhpDocs[] $parentParameters
-	 * @return array<int, TrinaryLogic>
+	 * @return array<int, array{TrinaryLogic, Type, Type}>
 	 */
 	private function checkParameterTypeCompatibility(
 		array $parameters,
@@ -193,7 +199,7 @@ private function checkParameterTypeCompatibility(
 				$parentParameter->getPhpDocType()
 			);
 
-			$parameterResults[] = $parameterType->isSuperTypeOf($parentParameterType);
+			$parameterResults[] = [$parameterType->isSuperTypeOf($parentParameterType), $parameterType, $parentParameterType];
 		}
 
 		return $parameterResults;

tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php

@@ -216,4 +216,24 @@ public function testBug3997(): void
 		]);
 	}
 
+	public function testBug4003(): void
+	{
+		$this->reportMaybes = true;
+		$this->reportStatic = true;
+		$this->analyse([__DIR__ . '/data/bug-4003.php'], [
+			[
+				'Return type (string) of method Bug4003\Baz::foo() should be compatible with return type (int) of method Bug4003\Boo::foo()',
+				15,
+			],
+			[
+				'Parameter #1 $test (string) of method Bug4003\Ipsum::doFoo() should be compatible with parameter $test (int) of method Bug4003\Lorem::doFoo()',
+				38,
+			],
+			[
+				'Return type (void) of method Bug4003\Amet::bar() should be compatible with return type (*NEVER*) of method Bug4003\Dolor::bar()',
+				54,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Methods/data/bug-4003.php

@@ -0,0 +1,57 @@
+<?php
+
+namespace Bug4003;
+
+class Boo
+{
+	/** @return int */
+	public function foo()
+	{
+		return 1;
+	}
+}
+
+class Baz extends Boo {
+	public function foo(): string
+	{
+		return 'test';
+	}
+}
+
+
+class Lorem
+{
+
+	public function doFoo(int $test)
+	{
+
+	}
+
+}
+
+class Ipsum extends Lorem
+{
+
+	/**
+	 * @param string $test
+	 */
+	public function doFoo($test)
+	{
+
+	}
+
+}
+
+interface Dolor {
+	/**
+	 * @return void
+	 * @phpstan-return never
+	 */
+	public function bar();
+}
+
+class Amet implements Dolor {
+	public function bar(): void {
+		throw new \Exception();
+	}
+}