A bundle of generic variance enhancements

conf/bleedingEdge.neon

@@ -25,3 +25,4 @@ parameters:
 		unescapeStrings: true
 		duplicateStubs: true
 		invarianceComposition: true
+		checkPropertyVariance: true

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

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()

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,
 		);
 	}
 

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;
 			}
 		}

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(),
 		);
 	}

src/Rules/Generics/PropertyVarianceRule.php

@@ -0,0 +1,50 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Generics;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Internal\SprintfHelper;
+use PHPStan\Node\ClassPropertyNode;
+use PHPStan\Reflection\ClassReflection;
+use PHPStan\Rules\Rule;
+use function sprintf;
+
+/**
+ * @implements Rule<ClassPropertyNode>
+ */
+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(),
+		);
+	}
+
+}

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,58 +20,129 @@ public function checkParametersAcceptor(
 		string $parameterTypeMessage,
 		string $returnTypeMessage,
 		string $generalMessage,
+		bool $isConstructor,
 		bool $isStatic,
 		bool $isPrivate,
 	): array
 	{
 		$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;
 		}
 
 		return $errors;
 	}
 
 	/** @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;

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,
 			],
 		]);

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,
+			],
+		]);
 	}
 
 }