Skip to content

Commit 284af50

Browse files
ondrejmirtesondrejmirtes
committed
Bleeding edge - cross-check generic interface implementations
1 parent 1443c5e commit 284af50

12 files changed

+224
-14
lines changed

‎conf/bleedingEdge.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,6 @@ parameters:
2222
deepInspectTypes: true
2323
neverInGenericReturnType: true
2424
validateOverridingMethodsInStubs: true
25+
crossCheckInterfaces: true
2526
stubFiles:
2627
- ../stubs/arrayFunctions.stub

‎conf/config.level2.neon

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,9 @@ rules:
1313
- PHPStan\Rules\Cast\PrintRule
1414
- PHPStan\Rules\Comparison\UsageOfVoidMatchExpressionRule
1515
- PHPStan\Rules\Functions\IncompatibleDefaultParameterTypeRule
16-
- PHPStan\Rules\Generics\ClassAncestorsRule
1716
- PHPStan\Rules\Generics\ClassTemplateTypeRule
1817
- PHPStan\Rules\Generics\FunctionTemplateTypeRule
1918
- PHPStan\Rules\Generics\FunctionSignatureVarianceRule
20-
- PHPStan\Rules\Generics\InterfaceAncestorsRule
2119
- PHPStan\Rules\Generics\InterfaceTemplateTypeRule
2220
- PHPStan\Rules\Generics\MethodTemplateTypeRule
2321
- PHPStan\Rules\Generics\MethodSignatureVarianceRule
@@ -46,6 +44,18 @@ services:
4644
reportMaybes: %reportMaybes%
4745
tags:
4846
- phpstan.rules.rule
47+
-
48+
class: PHPStan\Rules\Generics\ClassAncestorsRule
49+
arguments:
50+
crossCheckInterfaces: %featureToggles.crossCheckInterfaces%
51+
tags:
52+
- phpstan.rules.rule
53+
-
54+
class: PHPStan\Rules\Generics\InterfaceAncestorsRule
55+
arguments:
56+
crossCheckInterfaces: %featureToggles.crossCheckInterfaces%
57+
tags:
58+
- phpstan.rules.rule
4959
-
5060
class: PHPStan\Rules\PhpDoc\InvalidPhpDocVarTagTypeRule
5161
arguments:

‎conf/config.neon

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ parameters:
4545
deepInspectTypes: false
4646
neverInGenericReturnType: false
4747
validateOverridingMethodsInStubs: false
48+
crossCheckInterfaces: false
4849
fileExtensions:
4950
- php
5051
checkAlwaysTrueCheckTypeFunctionCall: false
@@ -217,7 +218,8 @@ parametersSchema:
217218
apiRules: bool(),
218219
deepInspectTypes: bool(),
219220
neverInGenericReturnType: bool(),
220-
validateOverridingMethodsInStubs: bool()
221+
validateOverridingMethodsInStubs: bool(),
222+
crossCheckInterfaces: bool()
221223
])
222224
fileExtensions: listOf(string())
223225
checkAlwaysTrueCheckTypeFunctionCall: bool()
@@ -422,6 +424,7 @@ services:
422424
class: PHPStan\PhpDoc\StubValidator
423425
arguments:
424426
validateOverridingMethods: %featureToggles.validateOverridingMethodsInStubs%
427+
crossCheckInterfaces: %featureToggles.crossCheckInterfaces%
425428

426429
-
427430
class: PHPStan\Analyser\Analyser
@@ -814,6 +817,9 @@ services:
814817
-
815818
class: PHPStan\Rules\FunctionReturnTypeCheck
816819

820+
-
821+
class: PHPStan\Rules\Generics\CrossCheckInterfacesHelper
822+
817823
-
818824
class: PHPStan\Rules\Generics\GenericAncestorsCheck
819825
arguments:

‎src/PhpDoc/StubValidator.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use PHPStan\Rules\Functions\MissingFunctionReturnTypehintRule;
2323
use PHPStan\Rules\Generics\ClassAncestorsRule;
2424
use PHPStan\Rules\Generics\ClassTemplateTypeRule;
25+
use PHPStan\Rules\Generics\CrossCheckInterfacesHelper;
2526
use PHPStan\Rules\Generics\FunctionSignatureVarianceRule;
2627
use PHPStan\Rules\Generics\FunctionTemplateTypeRule;
2728
use PHPStan\Rules\Generics\GenericAncestorsCheck;
@@ -57,13 +58,17 @@ class StubValidator
5758

5859
private bool $validateOverridingMethods;
5960

61+
private bool $crossCheckInterfaces;
62+
6063
public function __construct(
6164
DerivativeContainerFactory $derivativeContainerFactory,
62-
bool $validateOverridingMethods
65+
bool $validateOverridingMethods,
66+
bool $crossCheckInterfaces
6367
)
6468
{
6569
$this->derivativeContainerFactory = $derivativeContainerFactory;
6670
$this->validateOverridingMethods = $validateOverridingMethods;
71+
$this->crossCheckInterfaces = $crossCheckInterfaces;
6772
}
6873

6974
/**
@@ -133,6 +138,7 @@ private function getRuleRegistry(Container $container): Registry
133138
$functionDefinitionCheck = $container->getByType(FunctionDefinitionCheck::class);
134139
$missingTypehintCheck = $container->getByType(MissingTypehintCheck::class);
135140
$unresolvableTypeHelper = $container->getByType(UnresolvableTypeHelper::class);
141+
$crossCheckInterfacesHelper = $container->getByType(CrossCheckInterfacesHelper::class);
136142

137143
$rules = [
138144
// level 0
@@ -145,11 +151,11 @@ private function getRuleRegistry(Container $container): Registry
145151
new ExistingClassesInPropertiesRule($reflectionProvider, $classCaseSensitivityCheck, true, false),
146152

147153
// level 2
148-
new ClassAncestorsRule($fileTypeMapper, $genericAncestorsCheck),
154+
new ClassAncestorsRule($fileTypeMapper, $genericAncestorsCheck, $crossCheckInterfacesHelper, $this->crossCheckInterfaces),
149155
new ClassTemplateTypeRule($templateTypeCheck),
150156
new FunctionTemplateTypeRule($fileTypeMapper, $templateTypeCheck),
151157
new FunctionSignatureVarianceRule($varianceCheck),
152-
new InterfaceAncestorsRule($fileTypeMapper, $genericAncestorsCheck),
158+
new InterfaceAncestorsRule($fileTypeMapper, $genericAncestorsCheck, $crossCheckInterfacesHelper, $this->crossCheckInterfaces),
153159
new InterfaceTemplateTypeRule($fileTypeMapper, $templateTypeCheck),
154160
new MethodTemplateTypeRule($fileTypeMapper, $templateTypeCheck),
155161
new MethodSignatureVarianceRule($varianceCheck),

‎src/Reflection/ClassReflection.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,8 +1068,8 @@ private function getFirstExtendsTag(): ?ExtendsTag
10681068
return null;
10691069
}
10701070

1071-
/** @return ExtendsTag[] */
1072-
private function getExtendsTags(): array
1071+
/** @return array<string, ExtendsTag> */
1072+
public function getExtendsTags(): array
10731073
{
10741074
$resolvedPhpDoc = $this->getResolvedPhpDoc();
10751075
if ($resolvedPhpDoc === null) {
@@ -1079,8 +1079,8 @@ private function getExtendsTags(): array
10791079
return $resolvedPhpDoc->getExtendsTags();
10801080
}
10811081

1082-
/** @return ImplementsTag[] */
1083-
private function getImplementsTags(): array
1082+
/** @return array<string, ImplementsTag> */
1083+
public function getImplementsTags(): array
10841084
{
10851085
$resolvedPhpDoc = $this->getResolvedPhpDoc();
10861086
if ($resolvedPhpDoc === null) {

‎src/Rules/Generics/ClassAncestorsRule.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,21 @@ class ClassAncestorsRule implements Rule
2121

2222
private \PHPStan\Rules\Generics\GenericAncestorsCheck $genericAncestorsCheck;
2323

24+
private CrossCheckInterfacesHelper $crossCheckInterfacesHelper;
25+
26+
private bool $crossCheckInterfaces;
27+
2428
public function __construct(
2529
FileTypeMapper $fileTypeMapper,
26-
GenericAncestorsCheck $genericAncestorsCheck
30+
GenericAncestorsCheck $genericAncestorsCheck,
31+
CrossCheckInterfacesHelper $crossCheckInterfacesHelper,
32+
bool $crossCheckInterfaces = false
2733
)
2834
{
2935
$this->fileTypeMapper = $fileTypeMapper;
3036
$this->genericAncestorsCheck = $genericAncestorsCheck;
37+
$this->crossCheckInterfacesHelper = $crossCheckInterfacesHelper;
38+
$this->crossCheckInterfaces = $crossCheckInterfaces;
3139
}
3240

3341
public function getNodeType(): string
@@ -99,6 +107,12 @@ public function processNode(Node $node, Scope $scope): array
99107
sprintf('in implemented type %%s of class %s', $className)
100108
);
101109

110+
if ($this->crossCheckInterfaces) {
111+
foreach ($this->crossCheckInterfacesHelper->check($classReflection) as $error) {
112+
$implementsErrors[] = $error;
113+
}
114+
}
115+
102116
return array_merge($extendsErrors, $implementsErrors);
103117
}
104118

‎src/Rules/Generics/CrossCheckInterfacesHelper.php

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Generics;
4+
5+
use PHPStan\Reflection\ClassReflection;
6+
use PHPStan\Rules\RuleError;
7+
use PHPStan\Rules\RuleErrorBuilder;
8+
use PHPStan\Type\VerbosityLevel;
9+
10+
class CrossCheckInterfacesHelper
11+
{
12+
13+
/**
14+
* @return RuleError[]
15+
*/
16+
public function check(ClassReflection $classReflection): array
17+
{
18+
$interfaceTemplateTypeMaps = [];
19+
$errors = [];
20+
$check = static function (ClassReflection $classReflection) use (&$interfaceTemplateTypeMaps, &$check, &$errors): void {
21+
foreach ($classReflection->getInterfaces() as $interface) {
22+
if (!$interface->isGeneric()) {
23+
continue;
24+
}
25+
26+
if (array_key_exists($interface->getName(), $interfaceTemplateTypeMaps)) {
27+
$otherMap = $interfaceTemplateTypeMaps[$interface->getName()];
28+
foreach ($interface->getActiveTemplateTypeMap()->getTypes() as $name => $type) {
29+
$otherType = $otherMap->getType($name);
30+
if ($otherType === null) {
31+
continue;
32+
}
33+
34+
if ($type->equals($otherType)) {
35+
continue;
36+
}
37+
38+
$errors[] = RuleErrorBuilder::message(sprintf(
39+
'%s specifies template type %s of interface %s as %s but it\'s already specified as %s.',
40+
$classReflection->isInterface() ? sprintf('Interface %s', $classReflection->getName()) : sprintf('Class %s', $classReflection->getName()),
41+
$name,
42+
$interface->getName(),
43+
$type->describe(VerbosityLevel::value()),
44+
$otherType->describe(VerbosityLevel::value())
45+
))->build();
46+
}
47+
continue;
48+
}
49+
50+
$interfaceTemplateTypeMaps[$interface->getName()] = $interface->getActiveTemplateTypeMap();
51+
}
52+
53+
$parent = $classReflection->getParentClass();
54+
while ($parent !== false) {
55+
$check($parent);
56+
$parent = $parent->getParentClass();
57+
}
58+
59+
foreach ($classReflection->getInterfaces() as $interface) {
60+
$check($interface);
61+
}
62+
};
63+
64+
$check($classReflection);
65+
66+
return $errors;
67+
}
68+
69+
}

‎src/Rules/Generics/InterfaceAncestorsRule.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,21 @@ class InterfaceAncestorsRule implements Rule
2121

2222
private \PHPStan\Rules\Generics\GenericAncestorsCheck $genericAncestorsCheck;
2323

24+
private CrossCheckInterfacesHelper $crossCheckInterfacesHelper;
25+
26+
private bool $crossCheckInterfaces;
27+
2428
public function __construct(
2529
FileTypeMapper $fileTypeMapper,
26-
GenericAncestorsCheck $genericAncestorsCheck
30+
GenericAncestorsCheck $genericAncestorsCheck,
31+
CrossCheckInterfacesHelper $crossCheckInterfacesHelper,
32+
bool $crossCheckInterfaces = false
2733
)
2834
{
2935
$this->fileTypeMapper = $fileTypeMapper;
3036
$this->genericAncestorsCheck = $genericAncestorsCheck;
37+
$this->crossCheckInterfacesHelper = $crossCheckInterfacesHelper;
38+
$this->crossCheckInterfaces = $crossCheckInterfaces;
3139
}
3240

3341
public function getNodeType(): string
@@ -96,6 +104,12 @@ public function processNode(Node $node, Scope $scope): array
96104
''
97105
);
98106

107+
if ($this->crossCheckInterfaces) {
108+
foreach ($this->crossCheckInterfacesHelper->check($classReflection) as $error) {
109+
$implementsErrors[] = $error;
110+
}
111+
}
112+
99113
return array_merge($extendsErrors, $implementsErrors);
100114
}
101115

‎tests/PHPStan/Rules/Generics/ClassAncestorsRuleTest.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ protected function getRule(): Rule
2121
new GenericObjectTypeCheck(),
2222
new VarianceCheck(),
2323
true
24-
)
24+
),
25+
new CrossCheckInterfacesHelper(),
26+
true
2527
);
2628
}
2729

@@ -204,4 +206,14 @@ public function testBug3922Reversed(): void
204206
]);
205207
}
206208

209+
public function testCrossCheckInterfaces(): void
210+
{
211+
$this->analyse([__DIR__ . '/data/cross-check-interfaces.php'], [
212+
[
213+
'Interface CrossCheckInterfaces\ItemListInterface specifies template type TValue of interface Traversable as CrossCheckInterfaces\Item but it\'s already specified as string.',
214+
19,
215+
],
216+
]);
217+
}
218+
207219
}

‎tests/PHPStan/Rules/Generics/InterfaceAncestorsRuleTest.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ protected function getRule(): Rule
2121
new GenericObjectTypeCheck(),
2222
new VarianceCheck(),
2323
true
24-
)
24+
),
25+
new CrossCheckInterfacesHelper(),
26+
true
2527
);
2628
}
2729

@@ -197,4 +199,14 @@ public function testRuleExtends(): void
197199
]);
198200
}
199201

202+
public function testCrossCheckInterfaces(): void
203+
{
204+
$this->analyse([__DIR__ . '/data/cross-check-interfaces-interfaces.php'], [
205+
[
206+
'Interface CrossCheckInterfacesInInterfaces\ItemListInterface specifies template type TValue of interface Traversable as CrossCheckInterfacesInInterfaces\Item but it\'s already specified as string.',
207+
19,
208+
],
209+
]);
210+
}
211+
200212
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
namespace CrossCheckInterfacesInInterfaces;
4+
5+
final class Item
6+
{
7+
}
8+
9+
/**
10+
* @extends \Traversable<int, Item>
11+
*/
12+
interface ItemListInterface extends \Traversable
13+
{
14+
}
15+
16+
/**
17+
* @extends \IteratorAggregate<int, string>
18+
*/
19+
interface ItemList extends \IteratorAggregate, ItemListInterface
20+
{
21+
22+
}
23+
24+
/**
25+
* @extends \IteratorAggregate<int, Item>
26+
*/
27+
interface ItemList2 extends \IteratorAggregate, ItemListInterface
28+
{
29+
30+
}

0 commit comments

Comments
 (0)