From 6874e6fc2e8aad09e7e09fabe9549a3d1b032ea1 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 19 Jul 2023 21:00:06 +0700 Subject: [PATCH] [Performance] [Php74] Reduce ClassReflection lookup from property on PropertyTypeChangeGuard --- rules/Php74/Guard/MakePropertyTypedGuard.php | 5 +++-- rules/Php74/Guard/PropertyTypeChangeGuard.php | 17 +++++++-------- .../Guard/MakePropertyPromotionGuard.php | 12 ++++++++--- ...ertyAssignToConstructorPromotionRector.php | 21 +++++++++++++++++-- .../Guard/PropertyTypeOverrideGuard.php | 4 ++-- ...opertyTypeFromStrictSetterGetterRector.php | 15 +++++++++++-- .../TypedPropertyFromAssignsRector.php | 8 +++---- 7 files changed, 57 insertions(+), 25 deletions(-) diff --git a/rules/Php74/Guard/MakePropertyTypedGuard.php b/rules/Php74/Guard/MakePropertyTypedGuard.php index e008ae5afdc..89412f39f21 100644 --- a/rules/Php74/Guard/MakePropertyTypedGuard.php +++ b/rules/Php74/Guard/MakePropertyTypedGuard.php @@ -5,6 +5,7 @@ namespace Rector\Php74\Guard; use PhpParser\Node\Stmt\Property; +use PHPStan\Reflection\ClassReflection; final class MakePropertyTypedGuard { @@ -13,12 +14,12 @@ public function __construct( ) { } - public function isLegal(Property $property, bool $inlinePublic = true): bool + public function isLegal(Property $property, ClassReflection $classReflection, bool $inlinePublic = true): bool { if ($property->type !== null) { return false; } - return $this->propertyTypeChangeGuard->isLegal($property, $inlinePublic); + return $this->propertyTypeChangeGuard->isLegal($property, $classReflection, $inlinePublic); } } diff --git a/rules/Php74/Guard/PropertyTypeChangeGuard.php b/rules/Php74/Guard/PropertyTypeChangeGuard.php index e4b31f02315..2d4083b9818 100644 --- a/rules/Php74/Guard/PropertyTypeChangeGuard.php +++ b/rules/Php74/Guard/PropertyTypeChangeGuard.php @@ -8,7 +8,6 @@ use PHPStan\Reflection\ClassReflection; use Rector\Core\NodeAnalyzer\PropertyAnalyzer; use Rector\Core\NodeManipulator\PropertyManipulator; -use Rector\Core\Reflection\ReflectionResolver; use Rector\NodeNameResolver\NodeNameResolver; use Rector\Privatization\Guard\ParentPropertyLookupGuard; @@ -18,22 +17,20 @@ public function __construct( private readonly NodeNameResolver $nodeNameResolver, private readonly PropertyAnalyzer $propertyAnalyzer, private readonly PropertyManipulator $propertyManipulator, - private readonly ParentPropertyLookupGuard $parentPropertyLookupGuard, - private readonly ReflectionResolver $reflectionResolver + private readonly ParentPropertyLookupGuard $parentPropertyLookupGuard ) { } - public function isLegal(Property $property, bool $inlinePublic = true, bool $isConstructorPromotion = false): bool - { + public function isLegal( + Property $property, + ClassReflection $classReflection, + bool $inlinePublic = true, + bool $isConstructorPromotion = false + ): bool { if (count($property->props) > 1) { return false; } - $classReflection = $this->reflectionResolver->resolveClassReflection($property); - if (! $classReflection instanceof ClassReflection) { - return false; - } - /** * - trait properties are unpredictable based on class context they appear in * - on interface properties as well, as interface not allowed to have property diff --git a/rules/Php80/Guard/MakePropertyPromotionGuard.php b/rules/Php80/Guard/MakePropertyPromotionGuard.php index ea65435e0d0..e263733b9ee 100644 --- a/rules/Php80/Guard/MakePropertyPromotionGuard.php +++ b/rules/Php80/Guard/MakePropertyPromotionGuard.php @@ -8,6 +8,7 @@ use PhpParser\Node\Param; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\Property; +use PHPStan\Reflection\ClassReflection; use Rector\Php74\Guard\PropertyTypeChangeGuard; final class MakePropertyPromotionGuard @@ -17,9 +18,14 @@ public function __construct( ) { } - public function isLegal(Class_ $class, Property $property, Param $param, bool $inlinePublic = true): bool - { - if (! $this->propertyTypeChangeGuard->isLegal($property, $inlinePublic, true)) { + public function isLegal( + Class_ $class, + ClassReflection $classReflection, + Property $property, + Param $param, + bool $inlinePublic = true + ): bool { + if (! $this->propertyTypeChangeGuard->isLegal($property, $classReflection, $inlinePublic, true)) { return false; } diff --git a/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php b/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php index d46a079072d..a5c153a3b5a 100644 --- a/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php +++ b/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php @@ -23,6 +23,7 @@ use Rector\Core\Contract\Rector\ConfigurableRectorInterface; use Rector\Core\NodeAnalyzer\ParamAnalyzer; use Rector\Core\Rector\AbstractRector; +use Rector\Core\Reflection\ReflectionResolver; use Rector\Core\ValueObject\MethodName; use Rector\Core\ValueObject\PhpVersionFeature; use Rector\DeadCode\PhpDoc\TagRemover\VarTagRemover; @@ -66,7 +67,8 @@ public function __construct( private readonly ParamAnalyzer $paramAnalyzer, private readonly PhpDocTypeChanger $phpDocTypeChanger, private readonly MakePropertyPromotionGuard $makePropertyPromotionGuard, - private readonly TypeComparator $typeComparator + private readonly TypeComparator $typeComparator, + private readonly ReflectionResolver $reflectionResolver ) { } @@ -136,6 +138,7 @@ public function refactor(Node $node): ?Node } $classMethodPhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($constructClassMethod); + $classReflection = null; foreach ($promotionCandidates as $promotionCandidate) { // does property have some useful annotations? @@ -146,7 +149,21 @@ public function refactor(Node $node): ?Node continue; } - if (! $this->makePropertyPromotionGuard->isLegal($node, $property, $param, $this->inlinePublic)) { + if ($classReflection === null) { + $classReflection = $this->reflectionResolver->resolveClassReflection($node); + } + + if ($classReflection === null) { + return null; + } + + if (! $this->makePropertyPromotionGuard->isLegal( + $node, + $classReflection, + $property, + $param, + $this->inlinePublic + )) { continue; } diff --git a/rules/TypeDeclaration/Guard/PropertyTypeOverrideGuard.php b/rules/TypeDeclaration/Guard/PropertyTypeOverrideGuard.php index b1f5074dc06..844521bb134 100644 --- a/rules/TypeDeclaration/Guard/PropertyTypeOverrideGuard.php +++ b/rules/TypeDeclaration/Guard/PropertyTypeOverrideGuard.php @@ -19,11 +19,11 @@ public function __construct( public function isLegal(Property $property, ClassReflection $classReflection): bool { - $propertyName = $this->nodeNameResolver->getName($property); - if (! $this->makePropertyTypedGuard->isLegal($property)) { + if (! $this->makePropertyTypedGuard->isLegal($property, $classReflection)) { return false; } + $propertyName = $this->nodeNameResolver->getName($property); foreach ($classReflection->getParents() as $parentClassReflection) { $nativeReflectionClass = $parentClassReflection->getNativeReflection(); diff --git a/rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php b/rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php index 50706a1643f..dfe79f65c5c 100644 --- a/rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php +++ b/rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php @@ -14,6 +14,7 @@ use PHPStan\Type\TypeCombinator; use PHPStan\Type\UnionType; use Rector\Core\Rector\AbstractRector; +use Rector\Core\Reflection\ReflectionResolver; use Rector\Core\ValueObject\PhpVersionFeature; use Rector\Php74\Guard\MakePropertyTypedGuard; use Rector\PHPStanStaticTypeMapper\Enum\TypeKind; @@ -31,7 +32,8 @@ final class PropertyTypeFromStrictSetterGetterRector extends AbstractRector impl public function __construct( private readonly GetterTypeDeclarationPropertyTypeInferer $getterTypeDeclarationPropertyTypeInferer, private readonly SetterTypeDeclarationPropertyTypeInferer $setterTypeDeclarationPropertyTypeInferer, - private readonly MakePropertyTypedGuard $makePropertyTypedGuard + private readonly MakePropertyTypedGuard $makePropertyTypedGuard, + private readonly ReflectionResolver $reflectionResolver ) { } @@ -91,6 +93,7 @@ public function getNodeTypes(): array public function refactor(Node $node): ?Node { $hasChanged = false; + $classReflection = null; foreach ($node->getProperties() as $property) { if ($property->type instanceof Node) { @@ -110,7 +113,15 @@ public function refactor(Node $node): ?Node continue; } - if (! $this->makePropertyTypedGuard->isLegal($property, false)) { + if ($classReflection === null) { + $classReflection = $this->reflectionResolver->resolveClassReflection($node); + } + + if ($classReflection === null) { + return null; + } + + if (! $this->makePropertyTypedGuard->isLegal($property, $classReflection, false)) { continue; } diff --git a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromAssignsRector.php b/rules/TypeDeclaration/Rector/Property/TypedPropertyFromAssignsRector.php index e59645cab5b..f13ff752714 100644 --- a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromAssignsRector.php +++ b/rules/TypeDeclaration/Rector/Property/TypedPropertyFromAssignsRector.php @@ -119,10 +119,6 @@ public function refactor(Node $node): ?Node $classReflection = null; foreach ($node->getProperties() as $property) { - if (! $this->makePropertyTypedGuard->isLegal($property, $this->inlinePublic)) { - continue; - } - // non-private property can be anything with not inline public configured if (! $property->isPrivate() && ! $this->inlinePublic) { continue; @@ -136,6 +132,10 @@ public function refactor(Node $node): ?Node return null; } + if (! $this->makePropertyTypedGuard->isLegal($property, $classReflection, $this->inlinePublic)) { + continue; + } + $inferredType = $this->allAssignNodePropertyTypeInferer->inferProperty($property, $classReflection); if (! $inferredType instanceof Type) { continue;