From 059a6254d23670727480330f9953b3fc870c9040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pudil?= Date: Fri, 31 Mar 2023 14:42:23 +0200 Subject: [PATCH 1/3] fix position variance of static method parameters --- .../FunctionSignatureVarianceRule.php | 1 - .../Generics/MethodSignatureVarianceRule.php | 1 - src/Rules/Generics/VarianceCheck.php | 5 +- tests/PHPStan/Generics/data/variance-2.json | 10 +++ .../MethodSignatureVarianceRuleTest.php | 27 +++++++ .../data/method-signature-variance-static.php | 72 +++++++++++++++++++ 6 files changed, 110 insertions(+), 6 deletions(-) create mode 100644 tests/PHPStan/Rules/Generics/data/method-signature-variance-static.php diff --git a/src/Rules/Generics/FunctionSignatureVarianceRule.php b/src/Rules/Generics/FunctionSignatureVarianceRule.php index 9e5bea17053..7885aadc0aa 100644 --- a/src/Rules/Generics/FunctionSignatureVarianceRule.php +++ b/src/Rules/Generics/FunctionSignatureVarianceRule.php @@ -37,7 +37,6 @@ public function processNode(Node $node, Scope $scope): array sprintf('in return type of function %s()', $functionName), sprintf('in function %s()', $functionName), false, - false, ); } diff --git a/src/Rules/Generics/MethodSignatureVarianceRule.php b/src/Rules/Generics/MethodSignatureVarianceRule.php index 02caaff9c1e..9b789755594 100644 --- a/src/Rules/Generics/MethodSignatureVarianceRule.php +++ b/src/Rules/Generics/MethodSignatureVarianceRule.php @@ -35,7 +35,6 @@ public function processNode(Node $node, Scope $scope): array sprintf('in param-out type of parameter %%s of method %s::%s()', SprintfHelper::escapeFormatString($method->getDeclaringClass()->getDisplayName()), SprintfHelper::escapeFormatString($method->getName())), sprintf('in return type of method %s::%s()', $method->getDeclaringClass()->getDisplayName(), $method->getName()), sprintf('in method %s::%s()', $method->getDeclaringClass()->getDisplayName(), $method->getName()), - $method->isStatic(), $method->isPrivate() || $method->getName() === '__construct', ); } diff --git a/src/Rules/Generics/VarianceCheck.php b/src/Rules/Generics/VarianceCheck.php index f754feacba9..f79e9ab7ec9 100644 --- a/src/Rules/Generics/VarianceCheck.php +++ b/src/Rules/Generics/VarianceCheck.php @@ -26,7 +26,6 @@ public function checkParametersAcceptor( string $parameterOutTypeMessage, string $returnTypeMessage, string $generalMessage, - bool $isStatic, bool $isPrivate, ): array { @@ -52,9 +51,7 @@ public function checkParametersAcceptor( } $covariant = TemplateTypeVariance::createCovariant(); - $parameterVariance = $isStatic - ? TemplateTypeVariance::createStatic() - : TemplateTypeVariance::createContravariant(); + $parameterVariance = TemplateTypeVariance::createContravariant(); foreach ($parametersAcceptor->getParameters() as $parameterReflection) { $type = $parameterReflection->getType(); diff --git a/tests/PHPStan/Generics/data/variance-2.json b/tests/PHPStan/Generics/data/variance-2.json index d1474e29e5a..40e142c2fab 100644 --- a/tests/PHPStan/Generics/data/variance-2.json +++ b/tests/PHPStan/Generics/data/variance-2.json @@ -59,6 +59,16 @@ "line": 124, "ignorable": true }, + { + "message": "Template type T is declared as covariant, but occurs in contravariant position in parameter t of method PHPStan\\Generics\\Variance\\ConstructorAndStatic::create().", + "line": 153, + "ignorable": true + }, + { + "message": "Template type T is declared as covariant, but occurs in contravariant position in parameter w of method PHPStan\\Generics\\Variance\\ConstructorAndStatic::create().", + "line": 153, + "ignorable": true + }, { "message": "Template type T is declared as covariant, but occurs in invariant position in parameter v of method PHPStan\\Generics\\Variance\\ConstructorAndStatic::create().", "line": 153, diff --git a/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php b/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php index 80d09a5c360..e8eb6c429be 100644 --- a/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php +++ b/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php @@ -180,6 +180,33 @@ public function testRule(): void ]); $this->analyse([__DIR__ . '/data/method-signature-variance-constructor.php'], []); + + $this->analyse([__DIR__ . '/data/method-signature-variance-static.php'], [ + [ + 'Template type X is declared as covariant, but occurs in contravariant position in parameter a of method MethodSignatureVariance\Static\B::a().', + 43, + ], + [ + 'Template type X is declared as covariant, but occurs in contravariant position in parameter c of method MethodSignatureVariance\Static\B::a().', + 43, + ], + [ + 'Template type X is declared as covariant, but occurs in contravariant position in return type of method MethodSignatureVariance\Static\B::c().', + 49, + ], + [ + 'Template type X is declared as contravariant, but occurs in covariant position in parameter b of method MethodSignatureVariance\Static\C::a().', + 62, + ], + [ + 'Template type X is declared as contravariant, but occurs in covariant position in return type of method MethodSignatureVariance\Static\C::b().', + 65, + ], + [ + 'Template type X is declared as contravariant, but occurs in covariant position in return type of method MethodSignatureVariance\Static\C::d().', + 71, + ], + ]); } public function testBug8880(): void diff --git a/tests/PHPStan/Rules/Generics/data/method-signature-variance-static.php b/tests/PHPStan/Rules/Generics/data/method-signature-variance-static.php new file mode 100644 index 00000000000..02611a83f63 --- /dev/null +++ b/tests/PHPStan/Rules/Generics/data/method-signature-variance-static.php @@ -0,0 +1,72 @@ + $b + * @param Out $c + */ + static function a($a, $b, $c) {} + + /** @return X */ + static function b() {} + + /** @return In */ + static function c() {} + + /** @return Out */ + static function d() {} +} + +/** @template-covariant X */ +class B { + /** + * @param X $a + * @param In $b + * @param Out $c + */ + static function a($a, $b, $c) {} + + /** @return X */ + static function b() {} + + /** @return In */ + static function c() {} + + /** @return Out */ + static function d() {} +} + +/** @template-contravariant X */ +class C { + /** + * @param X $a + * @param In $b + * @param Out $c + */ + static function a($a, $b, $c) {} + + /** @return X */ + static function b() {} + + /** @return In */ + static function c() {} + + /** @return Out */ + static function d() {} +} From 85bfdc73775650eeeb39574f6a04d2044275f5e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pudil?= Date: Sat, 1 Apr 2023 17:39:40 +0200 Subject: [PATCH 2/3] fix NS in tests --- .../Generics/MethodSignatureVarianceRuleTest.php | 12 ++++++------ .../data/method-signature-variance-static.php | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php b/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php index e8eb6c429be..e03442155af 100644 --- a/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php +++ b/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php @@ -183,27 +183,27 @@ public function testRule(): void $this->analyse([__DIR__ . '/data/method-signature-variance-static.php'], [ [ - 'Template type X is declared as covariant, but occurs in contravariant position in parameter a of method MethodSignatureVariance\Static\B::a().', + 'Template type X is declared as covariant, but occurs in contravariant position in parameter a of method MethodSignatureVariance\StaticMethod\B::a().', 43, ], [ - 'Template type X is declared as covariant, but occurs in contravariant position in parameter c of method MethodSignatureVariance\Static\B::a().', + 'Template type X is declared as covariant, but occurs in contravariant position in parameter c of method MethodSignatureVariance\StaticMethod\B::a().', 43, ], [ - 'Template type X is declared as covariant, but occurs in contravariant position in return type of method MethodSignatureVariance\Static\B::c().', + 'Template type X is declared as covariant, but occurs in contravariant position in return type of method MethodSignatureVariance\StaticMethod\B::c().', 49, ], [ - 'Template type X is declared as contravariant, but occurs in covariant position in parameter b of method MethodSignatureVariance\Static\C::a().', + 'Template type X is declared as contravariant, but occurs in covariant position in parameter b of method MethodSignatureVariance\StaticMethod\C::a().', 62, ], [ - 'Template type X is declared as contravariant, but occurs in covariant position in return type of method MethodSignatureVariance\Static\C::b().', + 'Template type X is declared as contravariant, but occurs in covariant position in return type of method MethodSignatureVariance\StaticMethod\C::b().', 65, ], [ - 'Template type X is declared as contravariant, but occurs in covariant position in return type of method MethodSignatureVariance\Static\C::d().', + 'Template type X is declared as contravariant, but occurs in covariant position in return type of method MethodSignatureVariance\StaticMethod\C::d().', 71, ], ]); diff --git a/tests/PHPStan/Rules/Generics/data/method-signature-variance-static.php b/tests/PHPStan/Rules/Generics/data/method-signature-variance-static.php index 02611a83f63..692f6672f25 100644 --- a/tests/PHPStan/Rules/Generics/data/method-signature-variance-static.php +++ b/tests/PHPStan/Rules/Generics/data/method-signature-variance-static.php @@ -1,6 +1,6 @@ Date: Sun, 2 Apr 2023 16:36:17 +0200 Subject: [PATCH 3/3] hide fixed static parameter variance behind a feature toggle --- conf/bleedingEdge.neon | 1 + conf/config.neon | 3 +++ src/Rules/Generics/FunctionSignatureVarianceRule.php | 1 + src/Rules/Generics/MethodSignatureVarianceRule.php | 1 + src/Rules/Generics/VarianceCheck.php | 6 +++++- tests/PHPStan/Rules/Generics/ClassAncestorsRuleTest.php | 2 +- tests/PHPStan/Rules/Generics/EnumAncestorsRuleTest.php | 2 +- tests/PHPStan/Rules/Generics/InterfaceAncestorsRuleTest.php | 2 +- tests/PHPStan/Rules/Generics/UsedTraitsRuleTest.php | 2 +- 9 files changed, 15 insertions(+), 5 deletions(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 2a26a15f2a0..820aadd5943 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -33,3 +33,4 @@ parameters: instanceofType: true paramOutVariance: true allInvalidPhpDocs: true + strictStaticMethodTemplateTypeVariance: true diff --git a/conf/config.neon b/conf/config.neon index ce812461786..eecbe3a58bc 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -63,6 +63,7 @@ parameters: instanceofType: false paramOutVariance: false allInvalidPhpDocs: false + strictStaticMethodTemplateTypeVariance: false fileExtensions: - php checkAdvancedIsset: false @@ -296,6 +297,7 @@ parametersSchema: instanceofType: bool() paramOutVariance: bool() allInvalidPhpDocs: bool() + strictStaticMethodTemplateTypeVariance: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() @@ -1050,6 +1052,7 @@ services: class: PHPStan\Rules\Generics\VarianceCheck arguments: checkParamOutVariance: %featureToggles.paramOutVariance% + strictStaticVariance: %featureToggles.strictStaticMethodTemplateTypeVariance% - class: PHPStan\Rules\IssetCheck diff --git a/src/Rules/Generics/FunctionSignatureVarianceRule.php b/src/Rules/Generics/FunctionSignatureVarianceRule.php index 7885aadc0aa..9e5bea17053 100644 --- a/src/Rules/Generics/FunctionSignatureVarianceRule.php +++ b/src/Rules/Generics/FunctionSignatureVarianceRule.php @@ -37,6 +37,7 @@ public function processNode(Node $node, Scope $scope): array sprintf('in return type of function %s()', $functionName), sprintf('in function %s()', $functionName), false, + false, ); } diff --git a/src/Rules/Generics/MethodSignatureVarianceRule.php b/src/Rules/Generics/MethodSignatureVarianceRule.php index 9b789755594..02caaff9c1e 100644 --- a/src/Rules/Generics/MethodSignatureVarianceRule.php +++ b/src/Rules/Generics/MethodSignatureVarianceRule.php @@ -35,6 +35,7 @@ public function processNode(Node $node, Scope $scope): array sprintf('in param-out type of parameter %%s of method %s::%s()', SprintfHelper::escapeFormatString($method->getDeclaringClass()->getDisplayName()), SprintfHelper::escapeFormatString($method->getName())), sprintf('in return type of method %s::%s()', $method->getDeclaringClass()->getDisplayName(), $method->getName()), sprintf('in method %s::%s()', $method->getDeclaringClass()->getDisplayName(), $method->getName()), + $method->isStatic(), $method->isPrivate() || $method->getName() === '__construct', ); } diff --git a/src/Rules/Generics/VarianceCheck.php b/src/Rules/Generics/VarianceCheck.php index f79e9ab7ec9..432a47af4ac 100644 --- a/src/Rules/Generics/VarianceCheck.php +++ b/src/Rules/Generics/VarianceCheck.php @@ -15,6 +15,7 @@ class VarianceCheck public function __construct( private bool $checkParamOutVariance, + private bool $strictStaticVariance, ) { } @@ -26,6 +27,7 @@ public function checkParametersAcceptor( string $parameterOutTypeMessage, string $returnTypeMessage, string $generalMessage, + bool $isStatic, bool $isPrivate, ): array { @@ -51,7 +53,9 @@ public function checkParametersAcceptor( } $covariant = TemplateTypeVariance::createCovariant(); - $parameterVariance = TemplateTypeVariance::createContravariant(); + $parameterVariance = $isStatic && !$this->strictStaticVariance + ? TemplateTypeVariance::createStatic() + : TemplateTypeVariance::createContravariant(); foreach ($parametersAcceptor->getParameters() as $parameterReflection) { $type = $parameterReflection->getType(); diff --git a/tests/PHPStan/Rules/Generics/ClassAncestorsRuleTest.php b/tests/PHPStan/Rules/Generics/ClassAncestorsRuleTest.php index f3b74573b6c..f2cda5c0dc1 100644 --- a/tests/PHPStan/Rules/Generics/ClassAncestorsRuleTest.php +++ b/tests/PHPStan/Rules/Generics/ClassAncestorsRuleTest.php @@ -17,7 +17,7 @@ protected function getRule(): Rule new GenericAncestorsCheck( $this->createReflectionProvider(), new GenericObjectTypeCheck(), - new VarianceCheck(true), + new VarianceCheck(true, true), true, [], ), diff --git a/tests/PHPStan/Rules/Generics/EnumAncestorsRuleTest.php b/tests/PHPStan/Rules/Generics/EnumAncestorsRuleTest.php index 76e6c1b13ce..55657313520 100644 --- a/tests/PHPStan/Rules/Generics/EnumAncestorsRuleTest.php +++ b/tests/PHPStan/Rules/Generics/EnumAncestorsRuleTest.php @@ -18,7 +18,7 @@ protected function getRule(): Rule new GenericAncestorsCheck( $this->createReflectionProvider(), new GenericObjectTypeCheck(), - new VarianceCheck(true), + new VarianceCheck(true, true), true, [], ), diff --git a/tests/PHPStan/Rules/Generics/InterfaceAncestorsRuleTest.php b/tests/PHPStan/Rules/Generics/InterfaceAncestorsRuleTest.php index 0b7c5f6055c..5ed0203d942 100644 --- a/tests/PHPStan/Rules/Generics/InterfaceAncestorsRuleTest.php +++ b/tests/PHPStan/Rules/Generics/InterfaceAncestorsRuleTest.php @@ -17,7 +17,7 @@ protected function getRule(): Rule new GenericAncestorsCheck( $this->createReflectionProvider(), new GenericObjectTypeCheck(), - new VarianceCheck(true), + new VarianceCheck(true, true), true, [], ), diff --git a/tests/PHPStan/Rules/Generics/UsedTraitsRuleTest.php b/tests/PHPStan/Rules/Generics/UsedTraitsRuleTest.php index f4552c5ead9..95ba70b56ea 100644 --- a/tests/PHPStan/Rules/Generics/UsedTraitsRuleTest.php +++ b/tests/PHPStan/Rules/Generics/UsedTraitsRuleTest.php @@ -19,7 +19,7 @@ protected function getRule(): Rule new GenericAncestorsCheck( $this->createReflectionProvider(), new GenericObjectTypeCheck(), - new VarianceCheck(true), + new VarianceCheck(true, true), true, [], ),