From da6e74b5dc2cc6e69c34fb8d24a8173802a403e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pudil?= Date: Sat, 10 Dec 2022 19:00:48 +0100 Subject: [PATCH] Check generic variance rules in properties --- conf/bleedingEdge.neon | 1 + conf/config.level2.neon | 4 + conf/config.neon | 2 + src/Rules/Generics/PropertyVarianceRule.php | 50 ++++++ src/Rules/Generics/VarianceCheck.php | 39 +++++ .../Generics/PropertyVarianceRuleTest.php | 147 ++++++++++++++++++ .../data/property-variance-promoted.php | 93 +++++++++++ .../data/property-variance-readonly.php | 89 +++++++++++ .../data/property-variance-static.php | 14 ++ .../Rules/Generics/data/property-variance.php | 102 ++++++++++++ 10 files changed, 541 insertions(+) create mode 100644 src/Rules/Generics/PropertyVarianceRule.php create mode 100644 tests/PHPStan/Rules/Generics/PropertyVarianceRuleTest.php create mode 100644 tests/PHPStan/Rules/Generics/data/property-variance-promoted.php create mode 100644 tests/PHPStan/Rules/Generics/data/property-variance-readonly.php create mode 100644 tests/PHPStan/Rules/Generics/data/property-variance-static.php create mode 100644 tests/PHPStan/Rules/Generics/data/property-variance.php diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 9481e3f1592..7305c3a32e5 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 c30d85a2442..d68d0b943c1 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 bd508026595..98017afc03c 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/PropertyVarianceRule.php b/src/Rules/Generics/PropertyVarianceRule.php new file mode 100644 index 00000000000..b7d323569ee --- /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 314fb8647f2..360c7c7ce21 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; @@ -77,6 +78,44 @@ public function checkParametersAcceptor( return $errors; } + /** @return RuleError[] */ + 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, diff --git a/tests/PHPStan/Rules/Generics/PropertyVarianceRuleTest.php b/tests/PHPStan/Rules/Generics/PropertyVarianceRuleTest.php new file mode 100644 index 00000000000..ddd3772b7d2 --- /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/property-variance-promoted.php b/tests/PHPStan/Rules/Generics/data/property-variance-promoted.php new file mode 100644 index 00000000000..87c515cc655 --- /dev/null +++ b/tests/PHPStan/Rules/Generics/data/property-variance-promoted.php @@ -0,0 +1,93 @@ + $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 00000000000..9ddb64fb8e3 --- /dev/null +++ b/tests/PHPStan/Rules/Generics/data/property-variance-readonly.php @@ -0,0 +1,89 @@ + */ + 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 00000000000..6aed001412d --- /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; +}