Skip to content

Commit

Permalink
Bleeding edge - OverridingMethodRule - include template types in prot…
Browse files Browse the repository at this point in the history
…otype declaring class
  • Loading branch information
ondrejmirtes committed Apr 4, 2023
1 parent 5ff0a2e commit ca2c66c
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 33 deletions.
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@ parameters:
allInvalidPhpDocs: true
strictStaticMethodTemplateTypeVariance: true
propertyVariance: true
genericPrototypeMessage: true
1 change: 1 addition & 0 deletions conf/config.level0.neon
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ services:
class: PHPStan\Rules\Methods\OverridingMethodRule
arguments:
checkPhpDocMethodSignatures: %checkPhpDocMethodSignatures%
genericPrototypeMessage: %featureToggles.genericPrototypeMessage%
tags:
- phpstan.rules.rule

Expand Down
4 changes: 4 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ parameters:
allInvalidPhpDocs: false
strictStaticMethodTemplateTypeVariance: false
propertyVariance: false
genericPrototypeMessage: false
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down Expand Up @@ -300,6 +301,7 @@ parametersSchema:
allInvalidPhpDocs: bool()
strictStaticMethodTemplateTypeVariance: bool()
propertyVariance: bool()
genericPrototypeMessage: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down Expand Up @@ -1085,6 +1087,8 @@ services:

-
class: PHPStan\Rules\Methods\MethodParameterComparisonHelper
arguments:
genericPrototypeMessage: %featureToggles.genericPrototypeMessage%

-
class: PHPStan\Rules\MissingTypehintCheck
Expand Down
2 changes: 1 addition & 1 deletion src/PhpDoc/StubValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ private function getRuleRegistry(Container $container): RuleRegistry
new ExistingClassesInTypehintsRule($functionDefinitionCheck),
new \PHPStan\Rules\Functions\ExistingClassesInTypehintsRule($functionDefinitionCheck),
new ExistingClassesInPropertiesRule($reflectionProvider, $classCaseSensitivityCheck, $unresolvableTypeHelper, $phpVersion, true, false),
new OverridingMethodRule($phpVersion, new MethodSignatureRule(true, true), true, new MethodParameterComparisonHelper($phpVersion)),
new OverridingMethodRule($phpVersion, new MethodSignatureRule(true, true), true, new MethodParameterComparisonHelper($phpVersion, $container->getParameter('featureToggles')['genericPrototypeMessage']), $container->getParameter('featureToggles')['genericPrototypeMessage']),
new DuplicateDeclarationRule(),

// level 2
Expand Down
22 changes: 11 additions & 11 deletions src/Rules/Methods/MethodParameterComparisonHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
class MethodParameterComparisonHelper
{

public function __construct(private PhpVersion $phpVersion)
public function __construct(private PhpVersion $phpVersion, private bool $genericPrototypeMessage)
{
}

Expand All @@ -47,7 +47,7 @@ public function compare(MethodPrototypeReflection $prototype, PhpMethodFromParse
'Method %s::%s() overrides method %s::%s() but misses parameter #%d $%s.',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
$i + 1,
$prototypeParameter->getName(),
Expand All @@ -73,7 +73,7 @@ public function compare(MethodPrototypeReflection $prototype, PhpMethodFromParse
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand All @@ -92,7 +92,7 @@ public function compare(MethodPrototypeReflection $prototype, PhpMethodFromParse
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand Down Expand Up @@ -133,7 +133,7 @@ public function compare(MethodPrototypeReflection $prototype, PhpMethodFromParse
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand Down Expand Up @@ -181,7 +181,7 @@ public function compare(MethodPrototypeReflection $prototype, PhpMethodFromParse
$i + $j + 1,
$remainingPrototypeParameter->getName(),
$remainingPrototypeParameter->getNativeType()->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand All @@ -201,7 +201,7 @@ public function compare(MethodPrototypeReflection $prototype, PhpMethodFromParse
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand All @@ -223,7 +223,7 @@ public function compare(MethodPrototypeReflection $prototype, PhpMethodFromParse
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand Down Expand Up @@ -253,7 +253,7 @@ public function compare(MethodPrototypeReflection $prototype, PhpMethodFromParse
$i + 1,
$prototypeParameter->getName(),
$prototypeParameterType->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand Down Expand Up @@ -281,7 +281,7 @@ public function compare(MethodPrototypeReflection $prototype, PhpMethodFromParse
$i + 1,
$prototypeParameter->getName(),
$prototypeParameterType->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand All @@ -301,7 +301,7 @@ public function compare(MethodPrototypeReflection $prototype, PhpMethodFromParse
$i + 1,
$prototypeParameter->getName(),
$prototypeParameterType->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand Down
19 changes: 10 additions & 9 deletions src/Rules/Methods/OverridingMethodRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public function __construct(
private MethodSignatureRule $methodSignatureRule,
private bool $checkPhpDocMethodSignatures,
private MethodParameterComparisonHelper $methodParameterComparisonHelper,
private bool $genericPrototypeMessage,
)
{
}
Expand All @@ -53,7 +54,7 @@ public function processNode(Node $node, Scope $scope): array
'Method %s::%s() overrides final method %s::%s().',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$parent->getDisplayName(false),
$parent->getDisplayName($this->genericPrototypeMessage),
$parentConstructor->getName(),
))->nonIgnorable()->build(),
], $node, $scope);
Expand All @@ -74,7 +75,7 @@ public function processNode(Node $node, Scope $scope): array
'Method %s::%s() overrides final method %s::%s().',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
}
Expand All @@ -85,7 +86,7 @@ public function processNode(Node $node, Scope $scope): array
'Non-static method %s::%s() overrides static method %s::%s().',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
}
Expand All @@ -94,7 +95,7 @@ public function processNode(Node $node, Scope $scope): array
'Static method %s::%s() overrides non-static method %s::%s().',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
}
Expand All @@ -106,7 +107,7 @@ public function processNode(Node $node, Scope $scope): array
$method->isPrivate() ? 'Private' : 'Protected',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
}
Expand All @@ -115,7 +116,7 @@ public function processNode(Node $node, Scope $scope): array
'Private method %s::%s() overriding protected method %s::%s() should be protected or public.',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
}
Expand Down Expand Up @@ -143,7 +144,7 @@ public function processNode(Node $node, Scope $scope): array
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getTentativeReturnType()->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->tip('Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.')->nonIgnorable()->build();
}
Expand All @@ -165,7 +166,7 @@ public function processNode(Node $node, Scope $scope): array
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototypeReturnType->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
} else {
Expand All @@ -175,7 +176,7 @@ public function processNode(Node $node, Scope $scope): array
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototypeReturnType->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName(false),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
}
Expand Down
5 changes: 3 additions & 2 deletions tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ protected function getRule(): Rule
$phpVersion,
new MethodSignatureRule($this->reportMaybes, $this->reportStatic),
true,
new MethodParameterComparisonHelper($phpVersion),
new MethodParameterComparisonHelper($phpVersion, true),
true,
);
}

Expand Down Expand Up @@ -384,7 +385,7 @@ public function testBug7652(): void
$this->reportStatic = true;
$this->analyse([__DIR__ . '/data/bug-7652.php'], [
[
'Return type mixed of method Bug7652\Options::offsetGet() is not covariant with tentative return type mixed of method ArrayAccess::offsetGet().',
'Return type mixed of method Bug7652\Options::offsetGet() is not covariant with tentative return type mixed of method ArrayAccess<key-of<TArray of array>,value-of<TArray of array>>::offsetGet().',
23,
'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.',
],
Expand Down
21 changes: 11 additions & 10 deletions tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ protected function getRule(): Rule
$phpVersion,
new MethodSignatureRule(true, true),
false,
new MethodParameterComparisonHelper($phpVersion),
new MethodParameterComparisonHelper($phpVersion, true),
true,
);
}

Expand Down Expand Up @@ -84,7 +85,7 @@ public function testOverridingFinalMethod(int $phpVersion, string $contravariant
115,
],
[
'Parameter #1 $size (int) of method OverridingFinalMethod\FixedArray::setSize() is not ' . $contravariantMessage . ' with parameter #1 $size (mixed) of method SplFixedArray::setSize().',
'Parameter #1 $size (int) of method OverridingFinalMethod\FixedArray::setSize() is not ' . $contravariantMessage . ' with parameter #1 $size (mixed) of method SplFixedArray<mixed>::setSize().',
125,
],
[
Expand Down Expand Up @@ -120,7 +121,7 @@ public function testOverridingFinalMethod(int $phpVersion, string $contravariant
280,
],
[
'Parameter #1 $index (int) of method OverridingFinalMethod\FixedArrayOffsetExists::offsetExists() is not ' . $contravariantMessage . ' with parameter #1 $offset (mixed) of method ArrayAccess::offsetExists().',
'Parameter #1 $index (int) of method OverridingFinalMethod\FixedArrayOffsetExists::offsetExists() is not ' . $contravariantMessage . ' with parameter #1 $offset (mixed) of method ArrayAccess<int,mixed>::offsetExists().',
313,
],
];
Expand Down Expand Up @@ -472,37 +473,37 @@ public function dataTentativeReturnTypes(): array
80100,
[
[
'Return type mixed of method TentativeReturnTypes\Foo::getIterator() is not covariant with tentative return type Traversable of method IteratorAggregate::getIterator().',
'Return type mixed of method TentativeReturnTypes\Foo::getIterator() is not covariant with tentative return type Traversable of method IteratorAggregate<mixed,mixed>::getIterator().',
8,
'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.',
],
[
'Return type string of method TentativeReturnTypes\Lorem::getIterator() is not covariant with tentative return type Traversable of method IteratorAggregate::getIterator().',
'Return type string of method TentativeReturnTypes\Lorem::getIterator() is not covariant with tentative return type Traversable of method IteratorAggregate<mixed,mixed>::getIterator().',
40,
'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.',
],
[
'Return type mixed of method TentativeReturnTypes\UntypedIterator::current() is not covariant with tentative return type mixed of method Iterator::current().',
'Return type mixed of method TentativeReturnTypes\UntypedIterator::current() is not covariant with tentative return type mixed of method Iterator<mixed,mixed>::current().',
75,
'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.',
],
[
'Return type mixed of method TentativeReturnTypes\UntypedIterator::next() is not covariant with tentative return type void of method Iterator::next().',
'Return type mixed of method TentativeReturnTypes\UntypedIterator::next() is not covariant with tentative return type void of method Iterator<mixed,mixed>::next().',
79,
'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.',
],
[
'Return type mixed of method TentativeReturnTypes\UntypedIterator::key() is not covariant with tentative return type mixed of method Iterator::key().',
'Return type mixed of method TentativeReturnTypes\UntypedIterator::key() is not covariant with tentative return type mixed of method Iterator<mixed,mixed>::key().',
83,
'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.',
],
[
'Return type mixed of method TentativeReturnTypes\UntypedIterator::valid() is not covariant with tentative return type bool of method Iterator::valid().',
'Return type mixed of method TentativeReturnTypes\UntypedIterator::valid() is not covariant with tentative return type bool of method Iterator<mixed,mixed>::valid().',
87,
'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.',
],
[
'Return type mixed of method TentativeReturnTypes\UntypedIterator::rewind() is not covariant with tentative return type void of method Iterator::rewind().',
'Return type mixed of method TentativeReturnTypes\UntypedIterator::rewind() is not covariant with tentative return type void of method Iterator<mixed,mixed>::rewind().',
91,
'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.',
],
Expand Down

0 comments on commit ca2c66c

Please sign in to comment.