Bleeding edge - cross-check generic interface implementations

conf/bleedingEdge.neon

@@ -22,5 +22,6 @@ parameters:
 		deepInspectTypes: true
 		neverInGenericReturnType: true
 		validateOverridingMethodsInStubs: true
+		crossCheckInterfaces: true
 	stubFiles:
 		- ../stubs/arrayFunctions.stub

conf/config.level2.neon

@@ -13,11 +13,9 @@ rules:
 	- PHPStan\Rules\Cast\PrintRule
 	- PHPStan\Rules\Comparison\UsageOfVoidMatchExpressionRule
 	- PHPStan\Rules\Functions\IncompatibleDefaultParameterTypeRule
-	- PHPStan\Rules\Generics\ClassAncestorsRule
 	- PHPStan\Rules\Generics\ClassTemplateTypeRule
 	- PHPStan\Rules\Generics\FunctionTemplateTypeRule
 	- PHPStan\Rules\Generics\FunctionSignatureVarianceRule
-	- PHPStan\Rules\Generics\InterfaceAncestorsRule
 	- PHPStan\Rules\Generics\InterfaceTemplateTypeRule
 	- PHPStan\Rules\Generics\MethodTemplateTypeRule
 	- PHPStan\Rules\Generics\MethodSignatureVarianceRule
@@ -46,6 +44,18 @@ services:
 			reportMaybes: %reportMaybes%
 		tags:
 			- phpstan.rules.rule
+	-
+		class: PHPStan\Rules\Generics\ClassAncestorsRule
+		arguments:
+			crossCheckInterfaces: %featureToggles.crossCheckInterfaces%
+		tags:
+			- phpstan.rules.rule
+	-
+		class: PHPStan\Rules\Generics\InterfaceAncestorsRule
+		arguments:
+			crossCheckInterfaces: %featureToggles.crossCheckInterfaces%
+		tags:
+			- phpstan.rules.rule
 	-
 		class: PHPStan\Rules\PhpDoc\InvalidPhpDocVarTagTypeRule
 		arguments:

conf/config.neon

@@ -45,6 +45,7 @@ parameters:
 		deepInspectTypes: false
 		neverInGenericReturnType: false
 		validateOverridingMethodsInStubs: false
+		crossCheckInterfaces: false
 	fileExtensions:
 		- php
 	checkAlwaysTrueCheckTypeFunctionCall: false
@@ -217,7 +218,8 @@ parametersSchema:
 		apiRules: bool(),
 		deepInspectTypes: bool(),
 		neverInGenericReturnType: bool(),
-		validateOverridingMethodsInStubs: bool()
+		validateOverridingMethodsInStubs: bool(),
+		crossCheckInterfaces: bool()
 	])
 	fileExtensions: listOf(string())
 	checkAlwaysTrueCheckTypeFunctionCall: bool()
@@ -422,6 +424,7 @@ services:
 		class: PHPStan\PhpDoc\StubValidator
 		arguments:
 			validateOverridingMethods: %featureToggles.validateOverridingMethodsInStubs%
+			crossCheckInterfaces: %featureToggles.crossCheckInterfaces%
 
 	-
 		class: PHPStan\Analyser\Analyser
@@ -814,6 +817,9 @@ services:
 	-
 		class: PHPStan\Rules\FunctionReturnTypeCheck
 
+	-
+		class: PHPStan\Rules\Generics\CrossCheckInterfacesHelper
+
 	-
 		class: PHPStan\Rules\Generics\GenericAncestorsCheck
 		arguments:

src/PhpDoc/StubValidator.php

@@ -22,6 +22,7 @@
 use PHPStan\Rules\Functions\MissingFunctionReturnTypehintRule;
 use PHPStan\Rules\Generics\ClassAncestorsRule;
 use PHPStan\Rules\Generics\ClassTemplateTypeRule;
+use PHPStan\Rules\Generics\CrossCheckInterfacesHelper;
 use PHPStan\Rules\Generics\FunctionSignatureVarianceRule;
 use PHPStan\Rules\Generics\FunctionTemplateTypeRule;
 use PHPStan\Rules\Generics\GenericAncestorsCheck;
@@ -57,13 +58,17 @@ class StubValidator
 
 	private bool $validateOverridingMethods;
 
+	private bool $crossCheckInterfaces;
+
 	public function __construct(
 		DerivativeContainerFactory $derivativeContainerFactory,
-		bool $validateOverridingMethods
+		bool $validateOverridingMethods,
+		bool $crossCheckInterfaces
 	)
 	{
 		$this->derivativeContainerFactory = $derivativeContainerFactory;
 		$this->validateOverridingMethods = $validateOverridingMethods;
+		$this->crossCheckInterfaces = $crossCheckInterfaces;
 	}
 
 	/**
@@ -133,6 +138,7 @@ private function getRuleRegistry(Container $container): Registry
 		$functionDefinitionCheck = $container->getByType(FunctionDefinitionCheck::class);
 		$missingTypehintCheck = $container->getByType(MissingTypehintCheck::class);
 		$unresolvableTypeHelper = $container->getByType(UnresolvableTypeHelper::class);
+		$crossCheckInterfacesHelper = $container->getByType(CrossCheckInterfacesHelper::class);
 
 		$rules = [
 			// level 0
@@ -145,11 +151,11 @@ private function getRuleRegistry(Container $container): Registry
 			new ExistingClassesInPropertiesRule($reflectionProvider, $classCaseSensitivityCheck, true, false),
 
 			// level 2
-			new ClassAncestorsRule($fileTypeMapper, $genericAncestorsCheck),
+			new ClassAncestorsRule($fileTypeMapper, $genericAncestorsCheck, $crossCheckInterfacesHelper, $this->crossCheckInterfaces),
 			new ClassTemplateTypeRule($templateTypeCheck),
 			new FunctionTemplateTypeRule($fileTypeMapper, $templateTypeCheck),
 			new FunctionSignatureVarianceRule($varianceCheck),
-			new InterfaceAncestorsRule($fileTypeMapper, $genericAncestorsCheck),
+			new InterfaceAncestorsRule($fileTypeMapper, $genericAncestorsCheck, $crossCheckInterfacesHelper, $this->crossCheckInterfaces),
 			new InterfaceTemplateTypeRule($fileTypeMapper, $templateTypeCheck),
 			new MethodTemplateTypeRule($fileTypeMapper, $templateTypeCheck),
 			new MethodSignatureVarianceRule($varianceCheck),

src/Reflection/ClassReflection.php

@@ -1068,8 +1068,8 @@ private function getFirstExtendsTag(): ?ExtendsTag
 		return null;
 	}
 
-	/** @return ExtendsTag[] */
-	private function getExtendsTags(): array
+	/** @return array<string, ExtendsTag> */
+	public function getExtendsTags(): array
 	{
 		$resolvedPhpDoc = $this->getResolvedPhpDoc();
 		if ($resolvedPhpDoc === null) {
@@ -1079,8 +1079,8 @@ private function getExtendsTags(): array
 		return $resolvedPhpDoc->getExtendsTags();
 	}
 
-	/** @return ImplementsTag[] */
-	private function getImplementsTags(): array
+	/** @return array<string, ImplementsTag> */
+	public function getImplementsTags(): array
 	{
 		$resolvedPhpDoc = $this->getResolvedPhpDoc();
 		if ($resolvedPhpDoc === null) {

src/Rules/Generics/ClassAncestorsRule.php

@@ -21,13 +21,21 @@ class ClassAncestorsRule implements Rule
 
 	private \PHPStan\Rules\Generics\GenericAncestorsCheck $genericAncestorsCheck;
 
+	private CrossCheckInterfacesHelper $crossCheckInterfacesHelper;
+
+	private bool $crossCheckInterfaces;
+
 	public function __construct(
 		FileTypeMapper $fileTypeMapper,
-		GenericAncestorsCheck $genericAncestorsCheck
+		GenericAncestorsCheck $genericAncestorsCheck,
+		CrossCheckInterfacesHelper $crossCheckInterfacesHelper,
+		bool $crossCheckInterfaces = false
 	)
 	{
 		$this->fileTypeMapper = $fileTypeMapper;
 		$this->genericAncestorsCheck = $genericAncestorsCheck;
+		$this->crossCheckInterfacesHelper = $crossCheckInterfacesHelper;
+		$this->crossCheckInterfaces = $crossCheckInterfaces;
 	}
 
 	public function getNodeType(): string
@@ -99,6 +107,12 @@ public function processNode(Node $node, Scope $scope): array
 			sprintf('in implemented type %%s of class %s', $className)
 		);
 
+		if ($this->crossCheckInterfaces) {
+			foreach ($this->crossCheckInterfacesHelper->check($classReflection) as $error) {
+				$implementsErrors[] = $error;
+			}
+		}
+
 		return array_merge($extendsErrors, $implementsErrors);
 	}
 

src/Rules/Generics/CrossCheckInterfacesHelper.php

@@ -0,0 +1,69 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Generics;
+
+use PHPStan\Reflection\ClassReflection;
+use PHPStan\Rules\RuleError;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Type\VerbosityLevel;
+
+class CrossCheckInterfacesHelper
+{
+
+	/**
+	 * @return RuleError[]
+	 */
+	public function check(ClassReflection $classReflection): array
+	{
+		$interfaceTemplateTypeMaps = [];
+		$errors = [];
+		$check = static function (ClassReflection $classReflection) use (&$interfaceTemplateTypeMaps, &$check, &$errors): void {
+			foreach ($classReflection->getInterfaces() as $interface) {
+				if (!$interface->isGeneric()) {
+					continue;
+				}
+
+				if (array_key_exists($interface->getName(), $interfaceTemplateTypeMaps)) {
+					$otherMap = $interfaceTemplateTypeMaps[$interface->getName()];
+					foreach ($interface->getActiveTemplateTypeMap()->getTypes() as $name => $type) {
+						$otherType = $otherMap->getType($name);
+						if ($otherType === null) {
+							continue;
+						}
+
+						if ($type->equals($otherType)) {
+							continue;
+						}
+
+						$errors[] = RuleErrorBuilder::message(sprintf(
+							'%s specifies template type %s of interface %s as %s but it\'s already specified as %s.',
+							$classReflection->isInterface() ? sprintf('Interface %s', $classReflection->getName()) : sprintf('Class %s', $classReflection->getName()),
+							$name,
+							$interface->getName(),
+							$type->describe(VerbosityLevel::value()),
+							$otherType->describe(VerbosityLevel::value())
+						))->build();
+					}
+					continue;
+				}
+
+				$interfaceTemplateTypeMaps[$interface->getName()] = $interface->getActiveTemplateTypeMap();
+			}
+
+			$parent = $classReflection->getParentClass();
+			while ($parent !== false) {
+				$check($parent);
+				$parent = $parent->getParentClass();
+			}
+
+			foreach ($classReflection->getInterfaces() as $interface) {
+				$check($interface);
+			}
+		};
+
+		$check($classReflection);
+
+		return $errors;
+	}
+
+}

src/Rules/Generics/InterfaceAncestorsRule.php

@@ -21,13 +21,21 @@ class InterfaceAncestorsRule implements Rule
 
 	private \PHPStan\Rules\Generics\GenericAncestorsCheck $genericAncestorsCheck;
 
+	private CrossCheckInterfacesHelper $crossCheckInterfacesHelper;
+
+	private bool $crossCheckInterfaces;
+
 	public function __construct(
 		FileTypeMapper $fileTypeMapper,
-		GenericAncestorsCheck $genericAncestorsCheck
+		GenericAncestorsCheck $genericAncestorsCheck,
+		CrossCheckInterfacesHelper $crossCheckInterfacesHelper,
+		bool $crossCheckInterfaces = false
 	)
 	{
 		$this->fileTypeMapper = $fileTypeMapper;
 		$this->genericAncestorsCheck = $genericAncestorsCheck;
+		$this->crossCheckInterfacesHelper = $crossCheckInterfacesHelper;
+		$this->crossCheckInterfaces = $crossCheckInterfaces;
 	}
 
 	public function getNodeType(): string
@@ -96,6 +104,12 @@ public function processNode(Node $node, Scope $scope): array
 			''
 		);
 
+		if ($this->crossCheckInterfaces) {
+			foreach ($this->crossCheckInterfacesHelper->check($classReflection) as $error) {
+				$implementsErrors[] = $error;
+			}
+		}
+
 		return array_merge($extendsErrors, $implementsErrors);
 	}
 

tests/PHPStan/Rules/Generics/ClassAncestorsRuleTest.php

@@ -21,7 +21,9 @@ protected function getRule(): Rule
 				new GenericObjectTypeCheck(),
 				new VarianceCheck(),
 				true
-			)
+			),
+			new CrossCheckInterfacesHelper(),
+			true
 		);
 	}
 
@@ -204,4 +206,14 @@ public function testBug3922Reversed(): void
 		]);
 	}
 
+	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.',
+				19,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Generics/InterfaceAncestorsRuleTest.php

@@ -21,7 +21,9 @@ protected function getRule(): Rule
 				new GenericObjectTypeCheck(),
 				new VarianceCheck(),
 				true
-			)
+			),
+			new CrossCheckInterfacesHelper(),
+			true
 		);
 	}
 
@@ -197,4 +199,14 @@ public function testRuleExtends(): void
 		]);
 	}
 
+	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.',
+				19,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/Generics/data/cross-check-interfaces-interfaces.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace CrossCheckInterfacesInInterfaces;
+
+final class Item
+{
+}
+
+/**
+ * @extends \Traversable<int, Item>
+ */
+interface ItemListInterface extends \Traversable
+{
+}
+
+/**
+ * @extends \IteratorAggregate<int, string>
+ */
+interface ItemList extends \IteratorAggregate, ItemListInterface
+{
+
+}
+
+/**
+ * @extends \IteratorAggregate<int, Item>
+ */
+interface ItemList2 extends  \IteratorAggregate, ItemListInterface
+{
+
+}