From 14b9bca6cb29206d92b41910e3d14ba66950868a Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 30 Nov 2022 07:13:20 +0700 Subject: [PATCH 1/5] [TypeDeclaration] Use existing MakePropertyTypedGuard on TypedPropertyFromStrictConstructorRector --- .../Guard/PropertyTypeOverrideGuard.php | 10 ++++- ...pedPropertyFromStrictConstructorRector.php | 41 ++----------------- src/NodeManipulator/PropertyManipulator.php | 4 ++ 3 files changed, 15 insertions(+), 40 deletions(-) diff --git a/rules/TypeDeclaration/Guard/PropertyTypeOverrideGuard.php b/rules/TypeDeclaration/Guard/PropertyTypeOverrideGuard.php index d6a66910e7e..1c26890eb31 100644 --- a/rules/TypeDeclaration/Guard/PropertyTypeOverrideGuard.php +++ b/rules/TypeDeclaration/Guard/PropertyTypeOverrideGuard.php @@ -8,12 +8,14 @@ use PHPStan\Reflection\ClassReflection; use Rector\Core\Reflection\ReflectionResolver; use Rector\NodeNameResolver\NodeNameResolver; +use Rector\Php74\Guard\MakePropertyTypedGuard; final class PropertyTypeOverrideGuard { public function __construct( private readonly NodeNameResolver $nodeNameResolver, - private readonly ReflectionResolver $reflectionResolver + private readonly ReflectionResolver $reflectionResolver, + private readonly MakePropertyTypedGuard $makePropertyTypedGuard ) { } @@ -22,10 +24,14 @@ public function isLegal(Property $property): bool $classReflection = $this->reflectionResolver->resolveClassReflection($property); if (! $classReflection instanceof ClassReflection) { - return true; + return false; } $propertyName = $this->nodeNameResolver->getName($property); + if (! $this->makePropertyTypedGuard->isLegal($property)) { + return false; + } + foreach ($classReflection->getParents() as $parentClassReflection) { $nativeReflectionClass = $parentClassReflection->getNativeReflection(); diff --git a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php b/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php index 32a27a5614a..f0c2dab298f 100644 --- a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php +++ b/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php @@ -8,7 +8,6 @@ use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Property; -use PHPStan\Reflection\ReflectionProvider; use PHPStan\Type\MixedType; use PHPStan\Type\ObjectType; use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger; @@ -17,6 +16,7 @@ use Rector\Core\ValueObject\MethodName; use Rector\Core\ValueObject\PhpVersionFeature; use Rector\DeadCode\PhpDoc\TagRemover\VarTagRemover; +use Rector\Php74\Guard\MakePropertyTypedGuard; use Rector\PHPStanStaticTypeMapper\Enum\TypeKind; use Rector\TypeDeclaration\AlreadyAssignDetector\ConstructorAssignDetector; use Rector\TypeDeclaration\Guard\PropertyTypeOverrideGuard; @@ -37,7 +37,7 @@ public function __construct( private readonly ConstructorAssignDetector $constructorAssignDetector, private readonly PhpVersionProvider $phpVersionProvider, private readonly PropertyTypeOverrideGuard $propertyTypeOverrideGuard, - private readonly ReflectionProvider $reflectionProvider, + private readonly MakePropertyTypedGuard $makePropertyTypedGuard ) { } @@ -94,7 +94,7 @@ public function refactor(Node $node): ?Node } foreach ($node->getProperties() as $property) { - if ($this->shouldSkipProperty($property, $node)) { + if (! $this->makePropertyTypedGuard->isLegal($property)) { continue; } @@ -159,41 +159,6 @@ public function provideMinPhpVersion(): int return PhpVersionFeature::TYPED_PROPERTIES; } - /** - * @return string[] - */ - private function resolveTraitPropertyNames(Class_ $class): array - { - $traitPropertyNames = []; - - foreach ($class->getTraitUses() as $traitUse) { - foreach ($traitUse->traits as $traitName) { - $traitNameString = $this->getName($traitName); - if (! $this->reflectionProvider->hasClass($traitNameString)) { - continue; - } - - $traitClassReflection = $this->reflectionProvider->getClass($traitNameString); - $nativeReflection = $traitClassReflection->getNativeReflection(); - foreach ($nativeReflection->getProperties() as $property) { - $traitPropertyNames[] = $property->getName(); - } - } - } - - return $traitPropertyNames; - } - - private function shouldSkipProperty(Property $property, Class_ $class): bool - { - if ($property->type !== null) { - return true; - } - - $traitPropertyNames = $this->resolveTraitPropertyNames($class); - return $this->isNames($property, $traitPropertyNames); - } - private function isVarDocPreffered(Property $property): bool { if ($property->isPublic()) { diff --git a/src/NodeManipulator/PropertyManipulator.php b/src/NodeManipulator/PropertyManipulator.php index 9d006ef3346..ec6d23f6974 100644 --- a/src/NodeManipulator/PropertyManipulator.php +++ b/src/NodeManipulator/PropertyManipulator.php @@ -242,6 +242,10 @@ public function isUsedByTrait(ClassReflection $classReflection, string $property continue; } + if ($trait->getProperty($propertyName) instanceof Property) { + return true; + } + if ($this->propertyFetchAnalyzer->containsLocalPropertyFetchName($trait, $propertyName)) { return true; } From ccd671111789fe4656f8666663d8dafdfa27b254 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 30 Nov 2022 07:17:09 +0700 Subject: [PATCH 2/5] clean up --- .../Property/TypedPropertyFromStrictConstructorRector.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php b/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php index f0c2dab298f..fe6694aac97 100644 --- a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php +++ b/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php @@ -36,8 +36,7 @@ public function __construct( private readonly PhpDocTypeChanger $phpDocTypeChanger, private readonly ConstructorAssignDetector $constructorAssignDetector, private readonly PhpVersionProvider $phpVersionProvider, - private readonly PropertyTypeOverrideGuard $propertyTypeOverrideGuard, - private readonly MakePropertyTypedGuard $makePropertyTypedGuard + private readonly PropertyTypeOverrideGuard $propertyTypeOverrideGuard ) { } @@ -94,10 +93,6 @@ public function refactor(Node $node): ?Node } foreach ($node->getProperties() as $property) { - if (! $this->makePropertyTypedGuard->isLegal($property)) { - continue; - } - $propertyType = $this->trustedClassMethodPropertyTypeInferer->inferProperty( $property, $constructClassMethod From 4321f7c2e3fc3071a110391cd12f76d719fe357e Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Wed, 30 Nov 2022 00:18:53 +0000 Subject: [PATCH 3/5] [ci-review] Rector Rectify --- .../Rector/Property/TypedPropertyFromStrictConstructorRector.php | 1 - 1 file changed, 1 deletion(-) diff --git a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php b/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php index fe6694aac97..a1dfb5a9b8c 100644 --- a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php +++ b/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php @@ -16,7 +16,6 @@ use Rector\Core\ValueObject\MethodName; use Rector\Core\ValueObject\PhpVersionFeature; use Rector\DeadCode\PhpDoc\TagRemover\VarTagRemover; -use Rector\Php74\Guard\MakePropertyTypedGuard; use Rector\PHPStanStaticTypeMapper\Enum\TypeKind; use Rector\TypeDeclaration\AlreadyAssignDetector\ConstructorAssignDetector; use Rector\TypeDeclaration\Guard\PropertyTypeOverrideGuard; From e7c74910ad649a9c2ef0c14a311917ff38db286a Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 30 Nov 2022 07:50:21 +0700 Subject: [PATCH 4/5] phpstan --- src/NodeAnalyzer/PropertyFetchAnalyzer.php | 5 +++++ src/NodeManipulator/PropertyManipulator.php | 4 ---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/NodeAnalyzer/PropertyFetchAnalyzer.php b/src/NodeAnalyzer/PropertyFetchAnalyzer.php index 3772dc01d60..b3eb9367c0f 100644 --- a/src/NodeAnalyzer/PropertyFetchAnalyzer.php +++ b/src/NodeAnalyzer/PropertyFetchAnalyzer.php @@ -17,6 +17,7 @@ use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Expression; +use PhpParser\Node\Stmt\Property; use PhpParser\Node\Stmt\Trait_; use PHPStan\Type\ObjectType; use Rector\Core\Enum\ObjectReference; @@ -107,6 +108,10 @@ public function countLocalPropertyFetchName(Class_ $class, string $propertyName) public function containsLocalPropertyFetchName(Trait_ $trait, string $propertyName): bool { + if ($trait->getProperty($propertyName) instanceof Property) { + return true; + } + return (bool) $this->betterNodeFinder->findFirst( $trait, fn (Node $node): bool => $this->isLocalPropertyFetchName($node, $propertyName) diff --git a/src/NodeManipulator/PropertyManipulator.php b/src/NodeManipulator/PropertyManipulator.php index ec6d23f6974..9d006ef3346 100644 --- a/src/NodeManipulator/PropertyManipulator.php +++ b/src/NodeManipulator/PropertyManipulator.php @@ -242,10 +242,6 @@ public function isUsedByTrait(ClassReflection $classReflection, string $property continue; } - if ($trait->getProperty($propertyName) instanceof Property) { - return true; - } - if ($this->propertyFetchAnalyzer->containsLocalPropertyFetchName($trait, $propertyName)) { return true; } From 361feb7711a7ffdae4a09e4c2206228bf0191f7a Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 30 Nov 2022 07:54:02 +0700 Subject: [PATCH 5/5] phpstan --- phpstan.neon | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan.neon b/phpstan.neon index 4d0b2325da8..9ecf467bf16 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -779,4 +779,4 @@ parameters: # on purpose, as rule it about to be removed - '#Register "Rector\\Php74\\Rector\\Property\\TypedPropertyRector" service to "php74\.php" config set#' - - '#Cognitive complexity for "Rector\\TypeDeclaration\\Rector\\Property\\TypedPropertyFromStrictConstructorRector\:\:refactor\(\)" is 13, keep it under 10#' + - '#Cognitive complexity for "Rector\\TypeDeclaration\\Rector\\Property\\TypedPropertyFromStrictConstructorRector\:\:refactor\(\)" is 12, keep it under 10#'