From ca2c66cc4dff59ba44d52b82cb9e0aa3256240f3 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 4 Apr 2023 19:36:11 +0200 Subject: [PATCH] Bleeding edge - OverridingMethodRule - include template types in prototype declaring class --- conf/bleedingEdge.neon | 1 + conf/config.level0.neon | 1 + conf/config.neon | 4 ++++ src/PhpDoc/StubValidator.php | 2 +- .../MethodParameterComparisonHelper.php | 22 +++++++++---------- src/Rules/Methods/OverridingMethodRule.php | 19 ++++++++-------- .../Rules/Methods/MethodSignatureRuleTest.php | 5 +++-- .../Methods/OverridingMethodRuleTest.php | 21 +++++++++--------- 8 files changed, 42 insertions(+), 33 deletions(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 85de2394f7..173734d56a 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -35,3 +35,4 @@ parameters: allInvalidPhpDocs: true strictStaticMethodTemplateTypeVariance: true propertyVariance: true + genericPrototypeMessage: true diff --git a/conf/config.level0.neon b/conf/config.level0.neon index 7a16e8bbbb..c64d8b354a 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -145,6 +145,7 @@ services: class: PHPStan\Rules\Methods\OverridingMethodRule arguments: checkPhpDocMethodSignatures: %checkPhpDocMethodSignatures% + genericPrototypeMessage: %featureToggles.genericPrototypeMessage% tags: - phpstan.rules.rule diff --git a/conf/config.neon b/conf/config.neon index d8455e517b..cc15584c2a 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -65,6 +65,7 @@ parameters: allInvalidPhpDocs: false strictStaticMethodTemplateTypeVariance: false propertyVariance: false + genericPrototypeMessage: false fileExtensions: - php checkAdvancedIsset: false @@ -300,6 +301,7 @@ parametersSchema: allInvalidPhpDocs: bool() strictStaticMethodTemplateTypeVariance: bool() propertyVariance: bool() + genericPrototypeMessage: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() @@ -1085,6 +1087,8 @@ services: - class: PHPStan\Rules\Methods\MethodParameterComparisonHelper + arguments: + genericPrototypeMessage: %featureToggles.genericPrototypeMessage% - class: PHPStan\Rules\MissingTypehintCheck diff --git a/src/PhpDoc/StubValidator.php b/src/PhpDoc/StubValidator.php index 27582a614c..e26d44d1df 100644 --- a/src/PhpDoc/StubValidator.php +++ b/src/PhpDoc/StubValidator.php @@ -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 diff --git a/src/Rules/Methods/MethodParameterComparisonHelper.php b/src/Rules/Methods/MethodParameterComparisonHelper.php index 6d4505d58d..d81f85e206 100644 --- a/src/Rules/Methods/MethodParameterComparisonHelper.php +++ b/src/Rules/Methods/MethodParameterComparisonHelper.php @@ -24,7 +24,7 @@ class MethodParameterComparisonHelper { - public function __construct(private PhpVersion $phpVersion) + public function __construct(private PhpVersion $phpVersion, private bool $genericPrototypeMessage) { } @@ -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(), @@ -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(), )); @@ -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(), )); @@ -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(), )); @@ -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(), )); @@ -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(), )); @@ -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(), )); @@ -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(), )); @@ -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(), )); @@ -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(), )); diff --git a/src/Rules/Methods/OverridingMethodRule.php b/src/Rules/Methods/OverridingMethodRule.php index ef98c12a31..6ac892de28 100644 --- a/src/Rules/Methods/OverridingMethodRule.php +++ b/src/Rules/Methods/OverridingMethodRule.php @@ -29,6 +29,7 @@ public function __construct( private MethodSignatureRule $methodSignatureRule, private bool $checkPhpDocMethodSignatures, private MethodParameterComparisonHelper $methodParameterComparisonHelper, + private bool $genericPrototypeMessage, ) { } @@ -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); @@ -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(); } @@ -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(); } @@ -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(); } @@ -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(); } @@ -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(); } @@ -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(); } @@ -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 { @@ -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(); } diff --git a/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php b/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php index c2e1a1682c..df18fc3b3c 100644 --- a/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php +++ b/tests/PHPStan/Rules/Methods/MethodSignatureRuleTest.php @@ -25,7 +25,8 @@ protected function getRule(): Rule $phpVersion, new MethodSignatureRule($this->reportMaybes, $this->reportStatic), true, - new MethodParameterComparisonHelper($phpVersion), + new MethodParameterComparisonHelper($phpVersion, true), + true, ); } @@ -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,value-of>::offsetGet().', 23, 'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.', ], diff --git a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php index 4ca3e19dc7..c328859596 100644 --- a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php +++ b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php @@ -25,7 +25,8 @@ protected function getRule(): Rule $phpVersion, new MethodSignatureRule(true, true), false, - new MethodParameterComparisonHelper($phpVersion), + new MethodParameterComparisonHelper($phpVersion, true), + true, ); } @@ -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::setSize().', 125, ], [ @@ -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::offsetExists().', 313, ], ]; @@ -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::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::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::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::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::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::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::rewind().', 91, 'Make it covariant, or use the #[\ReturnTypeWillChange] attribute to temporarily suppress the error.', ],