diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 9481e3f159..7305c3a32e 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -25,3 +25,4 @@ parameters: unescapeStrings: true duplicateStubs: true invarianceComposition: true + checkPropertyVariance: true diff --git a/conf/config.level2.neon b/conf/config.level2.neon index c30d85a244..d68d0b943c 100644 --- a/conf/config.level2.neon +++ b/conf/config.level2.neon @@ -50,6 +50,8 @@ conditionalTags: phpstan.rules.rule: %featureToggles.illegalConstructorMethodCall% PHPStan\Rules\Methods\IllegalConstructorStaticCallRule: phpstan.rules.rule: %featureToggles.illegalConstructorMethodCall% + PHPStan\Rules\Generics\PropertyVarianceRule: + phpstan.rules.rule: %featureToggles.checkPropertyVariance% services: - @@ -75,3 +77,5 @@ services: checkMissingVarTagTypehint: %checkMissingVarTagTypehint% tags: - phpstan.rules.rule + - + class: PHPStan\Rules\Generics\PropertyVarianceRule diff --git a/conf/config.neon b/conf/config.neon index bd50802659..98017afc03 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -55,6 +55,7 @@ parameters: unescapeStrings: false duplicateStubs: false invarianceComposition: false + checkPropertyVariance: false fileExtensions: - php checkAdvancedIsset: false @@ -274,6 +275,7 @@ parametersSchema: unescapeStrings: bool() duplicateStubs: bool() invarianceComposition: bool() + checkPropertyVariance: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Rules/Generics/FunctionSignatureVarianceRule.php b/src/Rules/Generics/FunctionSignatureVarianceRule.php index 374c40ff40..c8a45e6846 100644 --- a/src/Rules/Generics/FunctionSignatureVarianceRule.php +++ b/src/Rules/Generics/FunctionSignatureVarianceRule.php @@ -41,6 +41,7 @@ public function processNode(Node $node, Scope $scope): array sprintf('in function %s()', $functionName), false, false, + false, ); } diff --git a/src/Rules/Generics/GenericAncestorsCheck.php b/src/Rules/Generics/GenericAncestorsCheck.php index 7660f86cb2..ae7bfbd578 100644 --- a/src/Rules/Generics/GenericAncestorsCheck.php +++ b/src/Rules/Generics/GenericAncestorsCheck.php @@ -103,7 +103,7 @@ public function check( $invalidVarianceMessage, $ancestorType->describe(VerbosityLevel::typeOnly()), ); - foreach ($this->varianceCheck->check($variance, $ancestorType, $messageContext) as $message) { + foreach ($this->varianceCheck->check($variance, $ancestorType, $messageContext, false, false) as $message) { $messages[] = $message; } } diff --git a/src/Rules/Generics/MethodSignatureVarianceRule.php b/src/Rules/Generics/MethodSignatureVarianceRule.php index a7f56d585a..4f2e6ae20c 100644 --- a/src/Rules/Generics/MethodSignatureVarianceRule.php +++ b/src/Rules/Generics/MethodSignatureVarianceRule.php @@ -38,7 +38,8 @@ public function processNode(Node $node, Scope $scope): array sprintf('in 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->getName() === '__construct' || $method->isStatic(), + $method->getName() === '__construct', + $method->isStatic(), $method->isPrivate(), ); } diff --git a/src/Rules/Generics/PropertyVarianceRule.php b/src/Rules/Generics/PropertyVarianceRule.php new file mode 100644 index 0000000000..b7d323569e --- /dev/null +++ b/src/Rules/Generics/PropertyVarianceRule.php @@ -0,0 +1,50 @@ + + */ +class PropertyVarianceRule implements Rule +{ + + public function __construct(private VarianceCheck $varianceCheck) + { + } + + public function getNodeType(): string + { + return ClassPropertyNode::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $classReflection = $scope->getClassReflection(); + if (!$classReflection instanceof ClassReflection) { + return []; + } + + if (!$classReflection->hasNativeProperty($node->getName())) { + return []; + } + + $propertyReflection = $classReflection->getNativeProperty($node->getName()); + + return $this->varianceCheck->checkProperty( + $propertyReflection, + sprintf('in property %s::$%s', SprintfHelper::escapeFormatString($classReflection->getDisplayName()), SprintfHelper::escapeFormatString($node->getName())), + $propertyReflection->isStatic(), + $propertyReflection->isPrivate(), + $propertyReflection->isReadOnly(), + ); + } + +} diff --git a/src/Rules/Generics/VarianceCheck.php b/src/Rules/Generics/VarianceCheck.php index 3d1670a3c8..360c7c7ce2 100644 --- a/src/Rules/Generics/VarianceCheck.php +++ b/src/Rules/Generics/VarianceCheck.php @@ -3,6 +3,7 @@ namespace PHPStan\Rules\Generics; use PHPStan\Reflection\ParametersAcceptor; +use PHPStan\Reflection\PropertyReflection; use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\Generic\TemplateType; @@ -19,6 +20,7 @@ public function checkParametersAcceptor( string $parameterTypeMessage, string $returnTypeMessage, string $generalMessage, + bool $isConstructor, bool $isStatic, bool $isPrivate, ): array @@ -26,38 +28,50 @@ public function checkParametersAcceptor( $errors = []; foreach ($parametersAcceptor->getTemplateTypeMap()->getTypes() as $templateType) { - if (!$templateType instanceof TemplateType - || $templateType->getScope()->getFunctionName() === null + if (!$templateType instanceof TemplateType) { + continue; + } + + if ($templateType->getScope()->getClassName() !== null + && $templateType->getScope()->getFunctionName() === '__construct' + ) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'Constructor is not allowed to define type parameters, but template type %s is defined %s.', + $templateType->getName(), + $generalMessage, + ))->build(); + continue; + } + + if ($templateType->getScope()->getFunctionName() === null || $templateType->getVariance()->invariant() ) { continue; } $errors[] = RuleErrorBuilder::message(sprintf( - 'Variance annotation is only allowed for type parameters of classes and interfaces, but occurs in template type %s in %s.', + 'Variance annotation is only allowed for type parameters of classes and interfaces, but occurs in template type %s %s.', $templateType->getName(), $generalMessage, ))->build(); } - if ($isPrivate) { + if ($isConstructor) { return $errors; } foreach ($parametersAcceptor->getParameters() as $parameterReflection) { - $variance = $isStatic - ? TemplateTypeVariance::createStatic() - : TemplateTypeVariance::createContravariant(); + $variance = TemplateTypeVariance::createContravariant(); $type = $parameterReflection->getType(); $message = sprintf($parameterTypeMessage, $parameterReflection->getName()); - foreach ($this->check($variance, $type, $message) as $error) { + foreach ($this->check($variance, $type, $message, $isStatic, $isPrivate) as $error) { $errors[] = $error; } } $variance = TemplateTypeVariance::createCovariant(); $type = $parametersAcceptor->getReturnType(); - foreach ($this->check($variance, $type, $returnTypeMessage) as $error) { + foreach ($this->check($variance, $type, $returnTypeMessage, $isStatic, $isPrivate) as $error) { $errors[] = $error; } @@ -65,12 +79,70 @@ public function checkParametersAcceptor( } /** @return RuleError[] */ - public function check(TemplateTypeVariance $positionVariance, Type $type, string $messageContext): array + public function checkProperty( + PropertyReflection $propertyReflection, + string $message, + bool $isStatic, + bool $isPrivate, + bool $isReadOnly, + ): array + { + $readableType = $propertyReflection->getReadableType(); + $writableType = $propertyReflection->getWritableType(); + + if ($readableType->equals($writableType)) { + $variance = $isReadOnly + ? TemplateTypeVariance::createCovariant() + : TemplateTypeVariance::createInvariant(); + + return $this->check($variance, $readableType, $message, $isStatic, $isPrivate); + } + + $errors = []; + + if ($propertyReflection->isReadable()) { + foreach ($this->check(TemplateTypeVariance::createCovariant(), $readableType, $message, $isStatic, $isPrivate) as $error) { + $errors[] = $error; + } + } + + if ($propertyReflection->isWritable()) { + $checkStatic = $isStatic && !$propertyReflection->isReadable(); + foreach ($this->check(TemplateTypeVariance::createContravariant(), $writableType, $message, $checkStatic, $isPrivate) as $error) { + $errors[] = $error; + } + } + + return $errors; + } + + /** @return RuleError[] */ + public function check( + TemplateTypeVariance $positionVariance, + Type $type, + string $messageContext, + bool $isStatic, + bool $isPrivate, + ): array { $errors = []; foreach ($type->getReferencedTemplateTypes($positionVariance) as $reference) { $referredType = $reference->getType(); + + if ($isStatic && $referredType->getScope()->getFunctionName() === null) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'Class template type %s cannot be referenced in a static member %s.', + $referredType->getName(), + $messageContext, + ))->build(); + continue; + } + + if ($isPrivate) { + continue; + } + if (($referredType->getScope()->getFunctionName() !== null && !$referredType->getVariance()->invariant()) || $this->isTemplateTypeVarianceValid($reference->getPositionVariance(), $referredType)) { continue; diff --git a/tests/PHPStan/Rules/Generics/FunctionSignatureVarianceRuleTest.php b/tests/PHPStan/Rules/Generics/FunctionSignatureVarianceRuleTest.php index f80773321a..27a57a128e 100644 --- a/tests/PHPStan/Rules/Generics/FunctionSignatureVarianceRuleTest.php +++ b/tests/PHPStan/Rules/Generics/FunctionSignatureVarianceRuleTest.php @@ -22,7 +22,7 @@ public function testRule(): void { $this->analyse([__DIR__ . '/data/function-signature-variance.php'], [ [ - 'Variance annotation is only allowed for type parameters of classes and interfaces, but occurs in template type T in in function FunctionSignatureVariance\f().', + 'Variance annotation is only allowed for type parameters of classes and interfaces, but occurs in template type T in function FunctionSignatureVariance\f().', 20, ], ]); diff --git a/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php b/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php index 07d3c237b8..2c5fd990dc 100644 --- a/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php +++ b/tests/PHPStan/Rules/Generics/MethodSignatureVarianceRuleTest.php @@ -22,11 +22,11 @@ public function testRule(): void { $this->analyse([__DIR__ . '/data/method-signature-variance.php'], [ [ - 'Variance annotation is only allowed for type parameters of classes and interfaces, but occurs in template type U in in method MethodSignatureVariance\C::b().', + 'Variance annotation is only allowed for type parameters of classes and interfaces, but occurs in template type U in method MethodSignatureVariance\C::b().', 16, ], [ - 'Variance annotation is only allowed for type parameters of classes and interfaces, but occurs in template type U in in method MethodSignatureVariance\C::c().', + 'Variance annotation is only allowed for type parameters of classes and interfaces, but occurs in template type U in method MethodSignatureVariance\C::c().', 22, ], ]); @@ -174,6 +174,24 @@ public function testRule(): void 71, ], ]); + + $this->analyse([__DIR__ . '/data/method-signature-variance-static.php'], [ + [ + 'Class template type X cannot be referenced in a static member in return type of method MethodSignatureVariance\Static\C::b().', + 18, + ], + [ + 'Class template type X cannot be referenced in a static member in return type of method MethodSignatureVariance\Static\C::c().', + 23, + ], + ]); + + $this->analyse([__DIR__ . '/data/method-signature-variance-constructor.php'], [ + [ + 'Constructor is not allowed to define type parameters, but template type Y is defined in method MethodSignatureVariance\Constructor\D::__construct().', + 79, + ], + ]); } } diff --git a/tests/PHPStan/Rules/Generics/PropertyVarianceRuleTest.php b/tests/PHPStan/Rules/Generics/PropertyVarianceRuleTest.php new file mode 100644 index 0000000000..ddd3772b7d --- /dev/null +++ b/tests/PHPStan/Rules/Generics/PropertyVarianceRuleTest.php @@ -0,0 +1,147 @@ + + */ +class PropertyVarianceRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new PropertyVarianceRule( + self::getContainer()->getByType(VarianceCheck::class), + ); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/property-variance.php'], [ + [ + 'Template type X is declared as covariant, but occurs in invariant position in property PropertyVariance\B::$a.', + 51, + ], + [ + 'Template type X is declared as covariant, but occurs in invariant position in property PropertyVariance\B::$b.', + 54, + ], + [ + 'Template type X is declared as covariant, but occurs in invariant position in property PropertyVariance\B::$c.', + 57, + ], + [ + 'Template type X is declared as covariant, but occurs in invariant position in property PropertyVariance\B::$d.', + 60, + ], + [ + 'Template type X is declared as contravariant, but occurs in invariant position in property PropertyVariance\C::$a.', + 80, + ], + [ + 'Template type X is declared as contravariant, but occurs in invariant position in property PropertyVariance\C::$b.', + 83, + ], + [ + 'Template type X is declared as contravariant, but occurs in invariant position in property PropertyVariance\C::$c.', + 86, + ], + [ + 'Template type X is declared as contravariant, but occurs in invariant position in property PropertyVariance\C::$d.', + 89, + ], + ]); + + $this->analyse([__DIR__ . '/data/property-variance-static.php'], [ + [ + 'Class template type X cannot be referenced in a static member in property PropertyVariance\Static\A::$a.', + 10, + ], + [ + 'Class template type X cannot be referenced in a static member in property PropertyVariance\Static\A::$b.', + 13, + ], + ]); + } + + public function testPromoted(): void + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Test requires PHP 8.0.'); + } + + $this->analyse([__DIR__ . '/data/property-variance-promoted.php'], [ + [ + 'Template type X is declared as covariant, but occurs in invariant position in property PropertyVariance\Promoted\B::$a.', + 58, + ], + [ + 'Template type X is declared as covariant, but occurs in invariant position in property PropertyVariance\Promoted\B::$b.', + 59, + ], + [ + 'Template type X is declared as covariant, but occurs in invariant position in property PropertyVariance\Promoted\B::$c.', + 60, + ], + [ + 'Template type X is declared as covariant, but occurs in invariant position in property PropertyVariance\Promoted\B::$d.', + 61, + ], + [ + 'Template type X is declared as contravariant, but occurs in invariant position in property PropertyVariance\Promoted\C::$a.', + 84, + ], + [ + 'Template type X is declared as contravariant, but occurs in invariant position in property PropertyVariance\Promoted\C::$b.', + 85, + ], + [ + 'Template type X is declared as contravariant, but occurs in invariant position in property PropertyVariance\Promoted\C::$c.', + 86, + ], + [ + 'Template type X is declared as contravariant, but occurs in invariant position in property PropertyVariance\Promoted\C::$d.', + 87, + ], + ]); + } + + public function testReadOnly(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Test requires PHP 8.1.'); + } + + $this->analyse([__DIR__ . '/data/property-variance-readonly.php'], [ + [ + 'Template type X is declared as covariant, but occurs in contravariant position in property PropertyVariance\ReadOnly\B::$b.', + 45, + ], + [ + 'Template type X is declared as covariant, but occurs in invariant position in property PropertyVariance\ReadOnly\B::$d.', + 51, + ], + [ + 'Template type X is declared as contravariant, but occurs in covariant position in property PropertyVariance\ReadOnly\C::$a.', + 62, + ], + [ + 'Template type X is declared as contravariant, but occurs in covariant position in property PropertyVariance\ReadOnly\C::$c.', + 68, + ], + [ + 'Template type X is declared as contravariant, but occurs in invariant position in property PropertyVariance\ReadOnly\C::$d.', + 71, + ], + [ + 'Template type X is declared as contravariant, but occurs in covariant position in property PropertyVariance\ReadOnly\D::$a.', + 86, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Generics/data/method-signature-variance-constructor.php b/tests/PHPStan/Rules/Generics/data/method-signature-variance-constructor.php new file mode 100644 index 0000000000..fe99a57e36 --- /dev/null +++ b/tests/PHPStan/Rules/Generics/data/method-signature-variance-constructor.php @@ -0,0 +1,80 @@ + $b + * @param In> $c + * @param In> $d + * @param In> $e + * @param Out $f + * @param Out> $g + * @param Out> $h + * @param Out> $i + * @param Invariant $j + * @param Invariant> $k + * @param Invariant> $l + */ + function __construct($a, $b, $c, $d, $e, $f, $g, $h, $i, $j, $k, $l) {} +} + +/** @template-covariant X */ +class B { + /** + * @param X $a + * @param In $b + * @param In> $c + * @param In> $d + * @param In> $e + * @param Out $f + * @param Out> $g + * @param Out> $h + * @param Out> $i + * @param Invariant $j + * @param Invariant> $k + * @param Invariant> $l + */ + function __construct($a, $b, $c, $d, $e, $f, $g, $h, $i, $j, $k, $l) {} +} + +/** @template-contravariant X */ +class C { + /** + * @param X $a + * @param In $b + * @param In> $c + * @param In> $d + * @param In> $e + * @param Out $f + * @param Out> $g + * @param Out> $h + * @param Out> $i + * @param Invariant $j + * @param Invariant> $k + * @param Invariant> $l + */ + function __construct($a, $b, $c, $d, $e, $f, $g, $h, $i, $j, $k, $l) {} +} + +/** @template X */ +class D { + /** + * @template Y of X + */ + function __construct() {} +} 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 0000000000..d2add07785 --- /dev/null +++ b/tests/PHPStan/Rules/Generics/data/method-signature-variance-static.php @@ -0,0 +1,24 @@ += 8.0 + +namespace PropertyVariance\Promoted; + +/** @template-contravariant T */ +interface In { +} + +/** @template-covariant T */ +interface Out { +} + +/** @template T */ +interface Invariant { +} + +/** + * @template X + */ +class A { + /** + * @param X $a + * @param In $b + * @param Out $c + * @param Invariant $d + * @param X $e + * @param In $f + * @param Out $g + * @param Invariant $h + */ + public function __construct( + public $a, + public $b, + public $c, + public $d, + private $e, + private $f, + private $g, + private $h, + ) {} +} + +/** + * @template-covariant X + */ +class B { + /** + * @param X $a + * @param In $b + * @param Out $c + * @param Invariant $d + * @param X $e + * @param In $f + * @param Out $g + * @param Invariant $h + */ + public function __construct( + public $a, + public $b, + public $c, + public $d, + private $e, + private $f, + private $g, + private $h, + ) {} +} + +/** + * @template-contravariant X + */ +class C { + /** + * @param X $a + * @param In $b + * @param Out $c + * @param Invariant $d + * @param X $e + * @param In $f + * @param Out $g + * @param Invariant $h + */ + public function __construct( + public $a, + public $b, + public $c, + public $d, + private $e, + private $f, + private $g, + private $h, + ) {} +} diff --git a/tests/PHPStan/Rules/Generics/data/property-variance-readonly.php b/tests/PHPStan/Rules/Generics/data/property-variance-readonly.php new file mode 100644 index 0000000000..faefc2bd8b --- /dev/null +++ b/tests/PHPStan/Rules/Generics/data/property-variance-readonly.php @@ -0,0 +1,89 @@ += 8.1 + +namespace PropertyVariance\ReadOnly; + +/** @template-contravariant T */ +interface In { +} + +/** @template-covariant T */ +interface Out { +} + +/** @template T */ +interface Invariant { +} + +/** + * @template X + */ +class A { + /** @var X */ + public readonly mixed $a; + + /** @var In */ + public readonly mixed $b; + + /** @var Out */ + public readonly mixed $c; + + /** @var Invariant */ + public readonly mixed $d; + + /** @var Invariant */ + private readonly mixed $e; +} + +/** + * @template-covariant X + */ +class B { + /** @var X */ + public readonly mixed $a; + + /** @var In */ + public readonly mixed $b; + + /** @var Out */ + public readonly mixed $c; + + /** @var Invariant */ + public readonly mixed $d; + + /** @var Invariant */ + private readonly mixed $e; +} + +/** + * @template-contravariant X + */ +class C { + /** @var X */ + public readonly mixed $a; + + /** @var In */ + public readonly mixed $b; + + /** @var Out */ + public readonly mixed $c; + + /** @var Invariant */ + public readonly mixed $d; + + /** @var Invariant */ + private readonly mixed $e; +} + +/** + * @template-contravariant X + */ +class D { + /** + * @param X $a + * @param X $b + */ + public function __construct( + public readonly mixed $a, + private readonly mixed $b, + ) {} +} diff --git a/tests/PHPStan/Rules/Generics/data/property-variance-static.php b/tests/PHPStan/Rules/Generics/data/property-variance-static.php new file mode 100644 index 0000000000..6aed001412 --- /dev/null +++ b/tests/PHPStan/Rules/Generics/data/property-variance-static.php @@ -0,0 +1,14 @@ + */ + public $b; + + /** @var Out */ + public $c; + + /** @var Invariant */ + public $d; + + /** @var X */ + private $e; + + /** @var In */ + private $f; + + /** @var Out */ + private $g; + + /** @var Invariant */ + private $h; +} + +/** + * @template-covariant X + */ +class B { + /** @var X */ + public $a; + + /** @var In */ + public $b; + + /** @var Out */ + public $c; + + /** @var Invariant */ + public $d; + + /** @var X */ + private $e; + + /** @var In */ + private $f; + + /** @var Out */ + private $g; + + /** @var Invariant */ + private $h; +} + +/** + * @template-contravariant X + */ +class C { + /** @var X */ + public $a; + + /** @var In */ + public $b; + + /** @var Out */ + public $c; + + /** @var Invariant */ + public $d; + + /** @var X */ + private $e; + + /** @var In */ + private $f; + + /** @var Out */ + private $g; + + /** @var Invariant */ + private $h; +}