OverridingMethodRule - check return type covariance

phpstan · Jun 5, 2020 · ada27eb · ada27eb
1 parent f478b91
commit ada27eb
Show file tree

Hide file tree

Showing 5 changed files with 173 additions and 1 deletion.
diff --git a/src/Php/PhpVersion.php b/src/Php/PhpVersion.php
@@ -22,4 +22,9 @@ public function supportsParameterContravariance(): bool
 		return $this->versionId >= 70400;
 	}
 
+	public function supportsReturnCovariance(): bool
+	{
+		return $this->versionId >= 70400;
+	}
+
 }
diff --git a/src/Reflection/Php/PhpMethodFromParserNodeReflection.php b/src/Reflection/Php/PhpMethodFromParserNodeReflection.php
@@ -13,6 +13,7 @@
 use PHPStan\Type\ObjectWithoutClassType;
 use PHPStan\Type\StringType;
 use PHPStan\Type\Type;
+use PHPStan\Type\TypeCombinator;
 use PHPStan\Type\VoidType;
 
 class PhpMethodFromParserNodeReflection extends PhpFunctionFromParserNodeReflection implements MethodReflection
@@ -78,7 +79,7 @@ public function __construct(
 		}
 		if ($name === '__set_state') {
 			$realReturnTypePresent = true;
-			$realReturnType = new ObjectWithoutClassType();
+			$realReturnType = TypeCombinator::intersect(new ObjectWithoutClassType(), $realReturnType);
 		}
 
 		parent::__construct(

diff --git a/src/Rules/Methods/OverridingMethodRule.php b/src/Rules/Methods/OverridingMethodRule.php
@@ -6,6 +6,7 @@
 use PHPStan\Analyser\Scope;
 use PHPStan\Node\InClassMethodNode;
 use PHPStan\Php\PhpVersion;
+use PHPStan\Reflection\FunctionVariantWithPhpDocs;
 use PHPStan\Reflection\MethodPrototypeReflection;
 use PHPStan\Reflection\Native\NativeParameterReflection;
 use PHPStan\Reflection\ParametersAcceptorSelector;
@@ -276,6 +277,38 @@ public function processNode(Node $node, Scope $scope): array
 			))->nonIgnorable()->build();
 		}
 
+		$methodReturnType = $methodVariant->getNativeReturnType();
+
+		if ($prototypeVariant instanceof FunctionVariantWithPhpDocs) {
+			$prototypeReturnType = $prototypeVariant->getNativeReturnType();
+		} else {
+			$prototypeReturnType = $prototypeVariant->getReturnType();
+		}
+
+		if (!$this->isTypeCompatible($prototypeReturnType, $methodReturnType, $this->phpVersion->supportsReturnCovariance())) {
+			if ($this->phpVersion->supportsReturnCovariance()) {
+				$messages[] = RuleErrorBuilder::message(sprintf(
+					'Return type %s of method %s::%s() is not covariant with return type %s of method %s::%s().',
+					$methodReturnType->describe(VerbosityLevel::typeOnly()),
+					$method->getDeclaringClass()->getName(),
+					$method->getName(),
+					$prototypeReturnType->describe(VerbosityLevel::typeOnly()),
+					$prototype->getDeclaringClass()->getName(),
+					$prototype->getName()
+				))->nonIgnorable()->build();
+			} else {
+				$messages[] = RuleErrorBuilder::message(sprintf(
+					'Return type %s of method %s::%s() is not compatible with return type %s of method %s::%s().',
+					$methodReturnType->describe(VerbosityLevel::typeOnly()),
+					$method->getDeclaringClass()->getName(),
+					$method->getName(),
+					$prototypeReturnType->describe(VerbosityLevel::typeOnly()),
+					$prototype->getDeclaringClass()->getName(),
+					$prototype->getName()
+				))->nonIgnorable()->build();
+			}
+		}
+
 		return $messages;
 	}
 

diff --git a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php
@@ -216,4 +216,66 @@ public function testParameterContravariance(
 		$this->analyse([$file], $expectedErrors);
 	}
 
+	public function dataReturnTypeCovariance(): array
+	{
+		return [
+			[
+				70300,
+				[
+					[
+						'Return type iterable of method ReturnTypeCovariance\Bar::doBar() is not compatible with return type array of method ReturnTypeCovariance\Foo::doBar().',
+						38,
+					],
+					[
+						'Return type InvalidArgumentException of method ReturnTypeCovariance\Bar::doBaz() is not compatible with return type Exception of method ReturnTypeCovariance\Foo::doBaz().',
+						43,
+					],
+					[
+						'Return type Exception of method ReturnTypeCovariance\Bar::doLorem() is not compatible with return type InvalidArgumentException of method ReturnTypeCovariance\Foo::doLorem().',
+						48,
+					],
+					[
+						'Return type mixed of method ReturnTypeCovariance\B::foo() is not compatible with return type stdClass|null of method ReturnTypeCovariance\A::foo().',
+						66,
+					],
+				],
+			],
+			[
+				70400,
+				[
+					[
+						'Return type iterable of method ReturnTypeCovariance\Bar::doBar() is not covariant with return type array of method ReturnTypeCovariance\Foo::doBar().',
+						38,
+					],
+					[
+						'Return type Exception of method ReturnTypeCovariance\Bar::doLorem() is not covariant with return type InvalidArgumentException of method ReturnTypeCovariance\Foo::doLorem().',
+						48,
+					],
+					[
+						'Return type mixed of method ReturnTypeCovariance\B::foo() is not covariant with return type stdClass|null of method ReturnTypeCovariance\A::foo().',
+						66,
+					],
+				],
+			],
+		];
+	}
+
+	/**
+	 * @dataProvider dataReturnTypeCovariance
+	 * @param int $phpVersion
+	 * @param mixed[] $expectedErrors
+	 */
+	public function testReturnTypeCovariance(
+		int $phpVersion,
+		array $expectedErrors
+	): void
+	{
+		if (!self::$useStaticReflectionProvider) {
+			$this->markTestSkipped('Test requires static reflection.');
+		}
+
+		$this->phpVersionId = $phpVersion;
+		$this->analyse([__DIR__ . '/data/return-type-covariance.php'], $expectedErrors);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Methods/data/return-type-covariance.php b/tests/PHPStan/Rules/Methods/data/return-type-covariance.php
@@ -0,0 +1,71 @@
+<?php
+
+namespace ReturnTypeCovariance;
+
+class Foo
+{
+
+	public function doFoo(): iterable
+	{
+
+	}
+
+	public function doBar(): array
+	{
+
+	}
+
+	public function doBaz(): \Exception
+	{
+
+	}
+
+	public function doLorem(): \InvalidArgumentException
+	{
+
+	}
+
+}
+
+class Bar extends Foo
+{
+
+	public function doFoo(): array
+	{
+
+	}
+
+	public function doBar(): iterable
+	{
+
+	}
+
+	public function doBaz(): \InvalidArgumentException
+	{
+
+	}
+
+	public function doLorem(): \Exception
+	{
+
+	}
+
+}
+
+class A
+{
+
+	public function foo(string $s): ?\stdClass
+	{
+
+	}
+}
+class B extends A
+{
+
+	public function foo($s)
+	{
+		return rand(0, 1) ? new stdClass : null;
+	}
+
+}