Skip to content

Commit

Permalink
[Performance] [Php74] Reduce ClassReflection lookup from property on …
Browse files Browse the repository at this point in the history
…PropertyTypeChangeGuard (#4545)
  • Loading branch information
samsonasik authored Jul 20, 2023
1 parent a2c12ab commit 141a94a
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 25 deletions.
5 changes: 3 additions & 2 deletions rules/Php74/Guard/MakePropertyTypedGuard.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Rector\Php74\Guard;

use PhpParser\Node\Stmt\Property;
use PHPStan\Reflection\ClassReflection;

final class MakePropertyTypedGuard
{
Expand All @@ -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);
}
}
17 changes: 7 additions & 10 deletions rules/Php74/Guard/PropertyTypeChangeGuard.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down
12 changes: 9 additions & 3 deletions rules/Php80/Guard/MakePropertyPromotionGuard.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
) {
}

Expand Down Expand Up @@ -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?
Expand All @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions rules/TypeDeclaration/Guard/PropertyTypeOverrideGuard.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
) {
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit 141a94a

Please sign in to comment.