Fix mess in ClassReflection::getInterfaces()

phpstan · Jul 10, 2021 · cd02bf8 · cd02bf8
1 parent 0f8ead7
commit cd02bf8
Show file tree

Hide file tree

Showing 12 changed files with 172 additions and 87 deletions.
diff --git a/src/PhpDoc/PhpDocNodeResolver.php b/src/PhpDoc/PhpDocNodeResolver.php
@@ -191,7 +191,7 @@ public function resolveExtendsTags(PhpDocNode $phpDocNode, NameScope $nameScope)
 
 		foreach (['@extends', '@template-extends', '@phpstan-extends'] as $tagName) {
 			foreach ($phpDocNode->getExtendsTagValues($tagName) as $tagValue) {
-				$resolved[$tagValue->type->type->name] = new ExtendsTag(
+				$resolved[$nameScope->resolveStringName($tagValue->type->type->name)] = new ExtendsTag(
 					$this->typeNodeResolver->resolve($tagValue->type, $nameScope)
 				);
 			}
@@ -209,7 +209,7 @@ public function resolveImplementsTags(PhpDocNode $phpDocNode, NameScope $nameSco
 
 		foreach (['@implements', '@template-implements', '@phpstan-implements'] as $tagName) {
 			foreach ($phpDocNode->getImplementsTagValues($tagName) as $tagValue) {
-				$resolved[$tagValue->type->type->name] = new ImplementsTag(
+				$resolved[$nameScope->resolveStringName($tagValue->type->type->name)] = new ImplementsTag(
 					$this->typeNodeResolver->resolve($tagValue->type, $nameScope)
 				);
 			}
@@ -227,7 +227,7 @@ public function resolveUsesTags(PhpDocNode $phpDocNode, NameScope $nameScope): a
 
 		foreach (['@use', '@template-use', '@phpstan-use'] as $tagName) {
 			foreach ($phpDocNode->getUsesTagValues($tagName) as $tagValue) {
-				$resolved[$tagValue->type->type->name] = new UsesTag(
+				$resolved[$nameScope->resolveStringName($tagValue->type->type->name)] = new UsesTag(
 					$this->typeNodeResolver->resolve($tagValue->type, $nameScope)
 				);
 			}

diff --git a/src/Reflection/ClassReflection.php b/src/Reflection/ClassReflection.php
@@ -596,77 +596,112 @@ public function getInterfaces(): array
 			return $this->cachedInterfaces;
 		}
 
-		$interfaces = [];
-
+		$interfaces = $this->getImmediateInterfaces();
+		$immediateInterfaces = $interfaces;
 		$parent = $this->getParentClass();
-		if ($parent !== false) {
-			foreach ($parent->getInterfaces() as $interface) {
-				$interfaces[$interface->getName()] = $interface;
+		while ($parent !== false) {
+			foreach ($parent->getImmediateInterfaces() as $parentInterface) {
+				$interfaces[$parentInterface->getName()] = $parentInterface;
+				foreach ($this->collectInterfaces($parentInterface) as $parentInterfaceInterface) {
+					$interfaces[$parentInterfaceInterface->getName()] = $parentInterfaceInterface;
+				}
 			}
-		}
 
-		if ($this->reflection->isInterface()) {
-			$implementsTags = $this->getExtendsTags();
-		} else {
-			$implementsTags = $this->getImplementsTags();
+			$parent = $parent->getParentClass();
 		}
 
-		$interfaceNames = $this->reflection->getInterfaceNames();
-		$genericInterfaces = [];
+		foreach ($immediateInterfaces as $immediateInterface) {
+			foreach ($this->collectInterfaces($immediateInterface) as $interfaceInterface) {
+				$interfaces[$interfaceInterface->getName()] = $interfaceInterface;
+			}
+		}
 
-		foreach ($implementsTags as $implementsTag) {
-			$implementedType = $implementsTag->getType();
+		$this->cachedInterfaces = $interfaces;
 
-			if (!$this->isValidAncestorType($implementedType, $interfaceNames)) {
-				continue;
-			}
+		return $interfaces;
+	}
 
-			if ($this->isGeneric()) {
-				$implementedType = TemplateTypeHelper::resolveTemplateTypes(
-					$implementedType,
-					$this->getActiveTemplateTypeMap()
-				);
+	/**
+	 * @return \PHPStan\Reflection\ClassReflection[]
+	 */
+	private function collectInterfaces(ClassReflection $interface): array
+	{
+		$interfaces = [];
+		foreach ($interface->getImmediateInterfaces() as $immediateInterface) {
+			$interfaces[$immediateInterface->getName()] = $immediateInterface;
+			foreach ($this->collectInterfaces($immediateInterface) as $immediateInterfaceInterface) {
+				$interfaces[$immediateInterfaceInterface->getName()] = $immediateInterfaceInterface;
 			}
+		}
 
-			if (!$implementedType instanceof GenericObjectType) {
-				continue;
-			}
+		return $interfaces;
+	}
 
-			$reflectionIface = $implementedType->getClassReflection();
-			if ($reflectionIface === null) {
-				continue;
+	/**
+	 * @return \PHPStan\Reflection\ClassReflection[]
+	 */
+	private function getImmediateInterfaces(): array
+	{
+		$indirectInterfaceNames = [];
+		$parent = $this->getParentClass();
+		while ($parent !== false) {
+			foreach ($parent->getNativeReflection()->getInterfaceNames() as $parentInterfaceName) {
+				$indirectInterfaceNames[] = $parentInterfaceName;
 			}
 
-			$genericInterfaces[] = $reflectionIface;
+			$parent = $parent->getParentClass();
 		}
 
-		foreach ($genericInterfaces as $genericInterface) {
-			$interfaces = array_merge($interfaces, $genericInterface->getInterfaces());
+		foreach ($this->getNativeReflection()->getInterfaces() as $interfaceInterface) {
+			foreach ($interfaceInterface->getInterfaceNames() as $interfaceInterfaceName) {
+				$indirectInterfaceNames[] = $interfaceInterfaceName;
+			}
 		}
 
-		foreach ($genericInterfaces as $genericInterface) {
-			$interfaces[$genericInterface->getName()] = $genericInterface;
+		if ($this->reflection->isInterface()) {
+			$implementsTags = $this->getExtendsTags();
+		} else {
+			$implementsTags = $this->getImplementsTags();
 		}
 
-		foreach ($interfaceNames as $interfaceName) {
-			if (isset($interfaces[$interfaceName])) {
+		$immediateInterfaceNames = array_diff($this->getNativeReflection()->getInterfaceNames(), $indirectInterfaceNames);
+		$immediateInterfaces = [];
+		foreach ($immediateInterfaceNames as $immediateInterfaceName) {
+			if (!$this->reflectionProvider->hasClass($immediateInterfaceName)) {
 				continue;
 			}
 
-			$interfaceReflection = $this->reflectionProvider->getClass($interfaceName);
-			if (!$interfaceReflection->isGeneric()) {
-				$interfaces[$interfaceName] = $interfaceReflection;
+			$immediateInterface = $this->reflectionProvider->getClass($immediateInterfaceName);
+			if (array_key_exists($immediateInterface->getName(), $implementsTags)) {
+				$implementsTag = $implementsTags[$immediateInterface->getName()];
+				$implementedType = $implementsTag->getType();
+				if ($this->isGeneric()) {
+					$implementedType = TemplateTypeHelper::resolveTemplateTypes(
+						$implementedType,
+						$this->getActiveTemplateTypeMap()
+					);
+				}
+
+				if (
+					$implementedType instanceof GenericObjectType
+					&& $implementedType->getClassReflection() !== null
+				) {
+					$immediateInterfaces[$immediateInterface->getName()] = $implementedType->getClassReflection();
+					continue;
+				}
+			}
+
+			if ($immediateInterface->isGeneric()) {
+				$immediateInterfaces[$immediateInterface->getName()] = $immediateInterface->withTypes(
+					array_values($immediateInterface->getTemplateTypeMap()->resolveToBounds()->getTypes())
+				);
 				continue;
 			}
 
-			$interfaces[$interfaceName] = $interfaceReflection->withTypes(
-				array_values($interfaceReflection->getTemplateTypeMap()->resolveToBounds()->getTypes())
-			);
+			$immediateInterfaces[$immediateInterface->getName()] = $immediateInterface;
 		}
 
-		$this->cachedInterfaces = $interfaces;
-
-		return $interfaces;
+		return $immediateInterfaces;
 	}
 
 	/**
@@ -1069,7 +1104,7 @@ private function getFirstExtendsTag(): ?ExtendsTag
 	}
 
 	/** @return array<string, ExtendsTag> */
-	private function getExtendsTags(): array
+	public function getExtendsTags(): array
 	{
 		$resolvedPhpDoc = $this->getResolvedPhpDoc();
 		if ($resolvedPhpDoc === null) {
@@ -1080,7 +1115,7 @@ private function getExtendsTags(): array
 	}
 
 	/** @return array<string, ImplementsTag> */
-	private function getImplementsTags(): array
+	public function getImplementsTags(): array
 	{
 		$resolvedPhpDoc = $this->getResolvedPhpDoc();
 		if ($resolvedPhpDoc === null) {

diff --git a/src/Rules/Generics/CrossCheckInterfacesHelper.php b/src/Rules/Generics/CrossCheckInterfacesHelper.php
@@ -17,7 +17,7 @@ public function check(ClassReflection $classReflection): array
 	{
 		$interfaceTemplateTypeMaps = [];
 		$errors = [];
-		$check = static function (ClassReflection $classReflection) use (&$interfaceTemplateTypeMaps, &$check, &$errors): void {
+		$check = static function (ClassReflection $classReflection, bool $first) use (&$interfaceTemplateTypeMaps, &$check, &$errors): void {
 			foreach ($classReflection->getInterfaces() as $interface) {
 				if (!$interface->isGeneric()) {
 					continue;
@@ -51,17 +51,40 @@ public function check(ClassReflection $classReflection): array
 			}
 
 			$parent = $classReflection->getParentClass();
-			while ($parent !== false) {
-				$check($parent);
-				$parent = $parent->getParentClass();
+			$checkParents = true;
+			if ($first && $parent !== false) {
+				$extendsTags = $classReflection->getExtendsTags();
+				if (!array_key_exists($parent->getName(), $extendsTags)) {
+					$checkParents = false;
+				}
+			}
+
+			if ($checkParents) {
+				while ($parent !== false) {
+					$check($parent, false);
+					$parent = $parent->getParentClass();
+				}
 			}
 
+			$interfaceTags = [];
+			if ($first) {
+				if ($classReflection->isInterface()) {
+					$interfaceTags = $classReflection->getExtendsTags();
+				} else {
+					$interfaceTags = $classReflection->getImplementsTags();
+				}
+			}
 			foreach ($classReflection->getInterfaces() as $interface) {
-				$check($interface);
+				if ($first) {
+					if (!array_key_exists($interface->getName(), $interfaceTags)) {
+						continue;
+					}
+				}
+				$check($interface, false);
 			}
 		};
 
-		$check($classReflection);
+		$check($classReflection, true);
 
 		return $errors;
 	}

diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php
@@ -438,6 +438,8 @@ public function dataFileAsserts(): iterable
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/sscanf.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/generic-offset-get.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/generic-object-lower-bound.php');
+		yield from $this->gatherAssertTypes(__DIR__ . '/data/class-reflection-interfaces.php');
+		yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Methods/data/bug-4415.php');
 	}
 
 	/**

diff --git a/tests/PHPStan/Analyser/data/class-reflection-interfaces.php b/tests/PHPStan/Analyser/data/class-reflection-interfaces.php
@@ -0,0 +1,26 @@
+<?php
+
+namespace ClassReflectionInterfaces;
+
+use function PHPStan\Testing\assertType;
+
+/**
+ * @extends \Traversable<int, mixed>
+ */
+interface ResultStatement extends \Traversable
+{
+
+}
+
+interface Statement extends ResultStatement
+{
+
+}
+
+function (Statement $s): void
+{
+	foreach ($s as $k => $v) {
+		assertType('int', $k);
+		assertType('mixed', $v);
+	}
+};
diff --git a/tests/PHPStan/Reflection/ClassReflectionTest.php b/tests/PHPStan/Reflection/ClassReflectionTest.php
@@ -138,24 +138,9 @@ public function testGenericInheritance(): void
 		$this->assertSame('GenericInheritance\\C0<DateTime>', $parent->getDisplayName());
 
 		$this->assertSame([
-			'GenericInheritance\\I0<DateTime>',
-			'GenericInheritance\\I1<int>',
 			'GenericInheritance\\I<DateTime>',
-		], array_map(static function (ClassReflection $r): string {
-			return $r->getDisplayName();
-		}, array_values($reflection->getInterfaces())));
-	}
-
-	public function testGenericInheritanceOverride(): void
-	{
-		/** @var Broker $broker */
-		$broker = self::getContainer()->getService('broker');
-		$reflection = $broker->getClass(\GenericInheritance\Override::class);
-
-		$this->assertSame([
-			'GenericInheritance\\I0<DateTimeInterface>',
+			'GenericInheritance\\I0<DateTime>',
 			'GenericInheritance\\I1<int>',
-			'GenericInheritance\\I<DateTimeInterface>',
 		], array_map(static function (ClassReflection $r): string {
 			return $r->getDisplayName();
 		}, array_values($reflection->getInterfaces())));

diff --git a/tests/PHPStan/Reflection/data/GenericInheritance.php b/tests/PHPStan/Reflection/data/GenericInheritance.php
@@ -46,10 +46,3 @@ class C0 implements I {
  */
 class C extends C0 {
 }
-
-
-/**
- * @implements I<\DateTimeInterface>
- */
-class Override extends C {
-}
diff --git a/tests/PHPStan/Rules/Generics/ClassAncestorsRuleTest.php b/tests/PHPStan/Rules/Generics/ClassAncestorsRuleTest.php
@@ -210,7 +210,7 @@ public function testCrossCheckInterfaces(): void
 	{
 		$this->analyse([__DIR__ . '/data/cross-check-interfaces.php'], [
 			[
-				'Interface CrossCheckInterfaces\ItemListInterface specifies template type TValue of interface Traversable as CrossCheckInterfaces\Item but it\'s already specified as string.',
+				'Interface IteratorAggregate specifies template type TValue of interface Traversable as string but it\'s already specified as CrossCheckInterfaces\Item.',
 				19,
 			],
 		]);

diff --git a/tests/PHPStan/Rules/Generics/InterfaceAncestorsRuleTest.php b/tests/PHPStan/Rules/Generics/InterfaceAncestorsRuleTest.php
@@ -203,7 +203,7 @@ public function testCrossCheckInterfaces(): void
 	{
 		$this->analyse([__DIR__ . '/data/cross-check-interfaces-interfaces.php'], [
 			[
-				'Interface CrossCheckInterfacesInInterfaces\ItemListInterface specifies template type TValue of interface Traversable as CrossCheckInterfacesInInterfaces\Item but it\'s already specified as string.',
+				'Interface IteratorAggregate specifies template type TValue of interface Traversable as string but it\'s already specified as CrossCheckInterfacesInInterfaces\Item.',
 				19,
 			],
 		]);

diff --git a/tests/PHPStan/Rules/Generics/data/cross-check-interfaces-interfaces.php b/tests/PHPStan/Rules/Generics/data/cross-check-interfaces-interfaces.php
@@ -28,3 +28,21 @@ interface ItemList2 extends  \IteratorAggregate, ItemListInterface
 {
 
 }
+
+interface ItemList3 extends ItemList // do not report
+{
+
+}
+
+/**
+ * @extends \Traversable<int, mixed>
+ */
+interface ResultStatement extends \Traversable
+{
+
+}
+
+interface Statement extends ResultStatement
+{
+
+}
diff --git a/tests/PHPStan/Rules/Methods/MissingMethodReturnTypehintRuleTest.php b/tests/PHPStan/Rules/Methods/MissingMethodReturnTypehintRuleTest.php
@@ -69,13 +69,7 @@ public function testArrayTypehintWithoutNullInPhpDoc(): void
 
 	public function testBug4415(): void
 	{
-		$this->analyse([__DIR__ . '/data/bug-4415.php'], [
-			[
-				'Method Bug4415Rule\CategoryCollection::getIterator() return type has no value type specified in iterable type Iterator.',
-				76,
-				MissingTypehintCheck::TURN_OFF_MISSING_ITERABLE_VALUE_TYPE_TIP,
-			],
-		]);
+		$this->analyse([__DIR__ . '/data/bug-4415.php'], []);
 	}
 
 	public function testBug5089(): void
-Original file line number
+Diff line change
@@ Expand Up / @@ -210,7 +210,7 @@ public function testCrossCheckInterfaces(): void @@
     	{
     		$this->analyse([__DIR__ . '/data/cross-check-interfaces.php'], [
     			[
-    				'Interface CrossCheckInterfaces\ItemListInterface specifies template type TValue of interface Traversable as CrossCheckInterfaces\Item but it\'s already specified as string.',
+    				'Interface IteratorAggregate specifies template type TValue of interface Traversable as string but it\'s already specified as CrossCheckInterfaces\Item.',
 ,
     			],
     		]);
@@ Expand Down @@