From 9be81c3bfc3a81685520be19933d2468b7b3562e Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sun, 18 Oct 2020 15:28:15 +0200 Subject: [PATCH] Revert "Implement property name as an expression in TypesAssignedToPropertiesRule" This reverts commit fd66714ba06f9df099cb4bf2a182726fd1315c87. --- .../Properties/FoundPropertyReflection.php | 9 --- src/Rules/Properties/PropertyDescriptor.php | 9 --- .../Properties/PropertyReflectionFinder.php | 64 ------------------- .../TypesAssignedToPropertiesRule.php | 30 ++------- .../TypesAssignedToPropertiesRuleTest.php | 56 ---------------- 5 files changed, 4 insertions(+), 164 deletions(-) diff --git a/src/Rules/Properties/FoundPropertyReflection.php b/src/Rules/Properties/FoundPropertyReflection.php index 79d1c25caa..5f98640ac2 100644 --- a/src/Rules/Properties/FoundPropertyReflection.php +++ b/src/Rules/Properties/FoundPropertyReflection.php @@ -14,21 +14,17 @@ class FoundPropertyReflection implements PropertyReflection private PropertyReflection $originalPropertyReflection; - private string $propertyName; - private Type $readableType; private Type $writableType; public function __construct( PropertyReflection $originalPropertyReflection, - string $propertyName, Type $readableType, Type $writableType ) { $this->originalPropertyReflection = $originalPropertyReflection; - $this->propertyName = $propertyName; $this->readableType = $readableType; $this->writableType = $writableType; } @@ -38,11 +34,6 @@ public function getDeclaringClass(): ClassReflection return $this->originalPropertyReflection->getDeclaringClass(); } - public function getName(): string - { - return $this->propertyName; - } - public function isStatic(): bool { return $this->originalPropertyReflection->isStatic(); diff --git a/src/Rules/Properties/PropertyDescriptor.php b/src/Rules/Properties/PropertyDescriptor.php index e8e1a74a24..52ad46fe96 100644 --- a/src/Rules/Properties/PropertyDescriptor.php +++ b/src/Rules/Properties/PropertyDescriptor.php @@ -7,15 +7,6 @@ class PropertyDescriptor { - public function describePropertyByName(PropertyReflection $property, string $propertyName): string - { - if (!$property->isStatic()) { - return sprintf('Property %s::$%s', $property->getDeclaringClass()->getDisplayName(), $propertyName); - } - - return sprintf('Static property %s::$%s', $property->getDeclaringClass()->getDisplayName(), $propertyName); - } - /** * @param \PHPStan\Reflection\PropertyReflection $property * @param \PhpParser\Node\Expr\PropertyFetch|\PhpParser\Node\Expr\StaticPropertyFetch $propertyFetch diff --git a/src/Rules/Properties/PropertyReflectionFinder.php b/src/Rules/Properties/PropertyReflectionFinder.php index aa89bc0c1e..0a846766f1 100644 --- a/src/Rules/Properties/PropertyReflectionFinder.php +++ b/src/Rules/Properties/PropertyReflectionFinder.php @@ -2,79 +2,16 @@ namespace PHPStan\Rules\Properties; -use PhpParser\Node\VarLikeIdentifier; use PHPStan\Analyser\Scope; -use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\ObjectType; use PHPStan\Type\StaticType; use PHPStan\Type\ThisType; use PHPStan\Type\Type; use PHPStan\Type\TypeTraverser; -use PHPStan\Type\TypeUtils; class PropertyReflectionFinder { - /** - * @param \PhpParser\Node\Expr\PropertyFetch|\PhpParser\Node\Expr\StaticPropertyFetch $propertyFetch - * @param \PHPStan\Analyser\Scope $scope - * @return FoundPropertyReflection[] - */ - public function findPropertyReflectionsFromNode($propertyFetch, Scope $scope): array - { - if ($propertyFetch instanceof \PhpParser\Node\Expr\PropertyFetch) { - if ($propertyFetch->name instanceof \PhpParser\Node\Identifier) { - $names = [$propertyFetch->name->name]; - } else { - $names = array_map(static function (ConstantStringType $name): string { - return $name->getValue(); - }, TypeUtils::getConstantStrings($scope->getType($propertyFetch->name))); - } - - $reflections = []; - $propertyHolderType = $scope->getType($propertyFetch->var); - $fetchedOnThis = $propertyHolderType instanceof ThisType && $scope->isInClass(); - foreach ($names as $name) { - $reflection = $this->findPropertyReflection($propertyHolderType, $name, $scope, $fetchedOnThis); - if ($reflection === null) { - continue; - } - - $reflections[] = $reflection; - } - - return $reflections; - } - - if ($propertyFetch->class instanceof \PhpParser\Node\Name) { - $propertyHolderType = new ObjectType($scope->resolveName($propertyFetch->class)); - } else { - $propertyHolderType = $scope->getType($propertyFetch->class); - } - - $fetchedOnThis = $propertyHolderType instanceof ThisType && $scope->isInClass(); - - if ($propertyFetch->name instanceof VarLikeIdentifier) { - $names = [$propertyFetch->name->name]; - } else { - $names = array_map(static function (ConstantStringType $name): string { - return $name->getValue(); - }, TypeUtils::getConstantStrings($scope->getType($propertyFetch->name))); - } - - $reflections = []; - foreach ($names as $name) { - $reflection = $this->findPropertyReflection($propertyHolderType, $name, $scope, $fetchedOnThis); - if ($reflection === null) { - continue; - } - - $reflections[] = $reflection; - } - - return $reflections; - } - /** * @param \PhpParser\Node\Expr\PropertyFetch|\PhpParser\Node\Expr\StaticPropertyFetch $propertyFetch * @param \PHPStan\Analyser\Scope $scope @@ -131,7 +68,6 @@ private function findPropertyReflection(Type $propertyHolderType, string $proper return new FoundPropertyReflection( $originalProperty, - $propertyName, $readableType, $writableType ); diff --git a/src/Rules/Properties/TypesAssignedToPropertiesRule.php b/src/Rules/Properties/TypesAssignedToPropertiesRule.php index d38dd5d3a3..5c3b5cab06 100644 --- a/src/Rules/Properties/TypesAssignedToPropertiesRule.php +++ b/src/Rules/Properties/TypesAssignedToPropertiesRule.php @@ -4,7 +4,6 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; -use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Rules\RuleLevelHelper; use PHPStan\Type\VerbosityLevel; @@ -55,32 +54,11 @@ public function processNode(Node $node, Scope $scope): array /** @var \PhpParser\Node\Expr\PropertyFetch|\PhpParser\Node\Expr\StaticPropertyFetch $propertyFetch */ $propertyFetch = $node->var; - $propertyReflections = $this->propertyReflectionFinder->findPropertyReflectionsFromNode($propertyFetch, $scope); - - $errors = []; - foreach ($propertyReflections as $propertyReflection) { - $errors = array_merge($errors, $this->processSingleProperty( - $scope, - $propertyReflection, - $node - )); + $propertyReflection = $this->propertyReflectionFinder->findPropertyReflectionFromNode($propertyFetch, $scope); + if ($propertyReflection === null) { + return []; } - return $errors; - } - - /** - * @param Scope $scope - * @param FoundPropertyReflection $propertyReflection - * @param Node\Expr $node - * @return RuleError[] - */ - private function processSingleProperty( - Scope $scope, - FoundPropertyReflection $propertyReflection, - Node\Expr $node - ): array - { $propertyType = $propertyReflection->getWritableType(); if ($node instanceof Node\Expr\Assign) { @@ -89,7 +67,7 @@ private function processSingleProperty( $assignedValueType = $scope->getType($node); } if (!$this->ruleLevelHelper->accepts($propertyType, $assignedValueType, $scope->isDeclareStrictTypes())) { - $propertyDescription = $this->propertyDescriptor->describePropertyByName($propertyReflection, $propertyReflection->getName()); + $propertyDescription = $this->propertyDescriptor->describeProperty($propertyReflection, $propertyFetch); $verbosityLevel = VerbosityLevel::getRecommendedLevelByType($propertyType); return [ diff --git a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php index 3df83f3977..1c8fbea1ad 100644 --- a/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php @@ -91,60 +91,4 @@ public function testBug1216(): void ]); } - public function testTypesAssignedToPropertiesExpressionNames(): void - { - $this->analyse([__DIR__ . '/data/properties-from-array-into-object.php'], [ - [ - 'Property PropertiesFromArrayIntoObject\Foo::$lall (int) does not accept string.', - 42, - ], - [ - 'Property PropertiesFromArrayIntoObject\Foo::$lall (int) does not accept string.', - 54, - ], - [ - 'Property PropertiesFromArrayIntoObject\Foo::$test (int|null) does not accept stdClass.', - 66, - ], - [ - 'Property PropertiesFromArrayIntoObject\Foo::$lall (int) does not accept string.', - 69, - ], - [ - 'Property PropertiesFromArrayIntoObject\Foo::$foo (string) does not accept int.', - 73, - ], - [ - 'Property PropertiesFromArrayIntoObject\Foo::$foo (string) does not accept float.', - 83, - ], - [ - 'Property PropertiesFromArrayIntoObject\Foo::$lall (int) does not accept string.', - 110, - ], - [ - 'Property PropertiesFromArrayIntoObject\FooBar::$foo (string) does not accept float.', - 147, - ], - ]); - } - - public function testTypesAssignedToStaticPropertiesExpressionNames(): void - { - $this->analyse([__DIR__ . '/data/properties-from-array-into-static-object.php'], [ - [ - 'Static property PropertiesFromArrayIntoStaticObject\Foo::$lall (stdClass|null) does not accept string.', - 29, - ], - [ - 'Static property PropertiesFromArrayIntoStaticObject\Foo::$foo (string) does not accept float.', - 36, - ], - [ - 'Static property PropertiesFromArrayIntoStaticObject\FooBar::$foo (string) does not accept float.', - 72, - ], - ]); - } - }