Skip to content

Commit

Permalink
[Privatization] Add ParentPropertyLookupGuard service (#2152)
Browse files Browse the repository at this point in the history
* [Privatization] Add ParentPropertyLookupGuard service

* no extends is allowed

* handle parent class not autoloaded

* handle parent class not autoloaded
  • Loading branch information
samsonasik committed Apr 24, 2022
1 parent 366c994 commit c3adc6b
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 107 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

namespace Rector\Tests\Privatization\Rector\Property\PrivatizeFinalClassPropertyRector\Fixture;

final class KeepParentNotAutoloaded extends NotAutoloadedParent
{
protected $value = 100;
}
2 changes: 1 addition & 1 deletion rules/Php81/Rector/ClassMethod/NewInInitializerRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public function provideMinPhpVersion(): int

private function isOverrideAbstractMethod(ClassMethod $classMethod): bool
{
$classReflection = $this->reflectionResolver->resolveClassReflectionFromClassMethod($classMethod);
$classReflection = $this->reflectionResolver->resolveClassReflection($classMethod);
$methodName = $this->nodeNameResolver->getName($classMethod);

return $classReflection instanceof ClassReflection && $this->classChildAnalyzer->hasAbstractParentClassMethod(
Expand Down
130 changes: 130 additions & 0 deletions rules/Privatization/Guard/ParentPropertyLookupGuard.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
<?php

declare(strict_types=1);

namespace Rector\Privatization\Guard;

use PhpParser\Node;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\Property;
use PHPStan\Reflection\ClassReflection;
use Rector\Core\Enum\ObjectReference;
use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
use Rector\Core\PhpParser\AstResolver;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\Core\Reflection\ReflectionResolver;
use Rector\NodeNameResolver\NodeNameResolver;

final class ParentPropertyLookupGuard
{
public function __construct(
private readonly BetterNodeFinder $betterNodeFinder,
private readonly ReflectionResolver $reflectionResolver,
private readonly NodeNameResolver $nodeNameResolver,
private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer,
private readonly AstResolver $astResolver
) {
}

public function isLegal(Property $property): bool
{
$class = $this->betterNodeFinder->findParentType($property, Class_::class);

if (! $class instanceof Class_) {
return false;
}

if ($class->extends === null) {
return true;
}

$classReflection = $this->reflectionResolver->resolveClassReflection($property);
if (! $classReflection instanceof ClassReflection) {
return false;
}

$propertyName = $this->nodeNameResolver->getName($property);
$className = $classReflection->getName();

$parents = $classReflection->getParents();

// parent class not autoloaded
if ($parents === []) {
return false;
}

foreach ($parents as $parent) {
if ($parent->hasProperty($propertyName)) {
return false;
}

if ($this->isFoundInParentClassMethods($parent, $propertyName, $className)) {
return false;
}
}

return true;
}

private function isFoundInParentClassMethods(
ClassReflection $parentClassReflection,
string $propertyName,
string $className
): bool {
$classLike = $this->astResolver->resolveClassFromName($parentClassReflection->getName());
if (! $classLike instanceof Class_) {
return false;
}

$methods = $classLike->getMethods();
foreach ($methods as $method) {
$isFound = $this->isFoundInMethodStmts((array) $method->stmts, $propertyName, $className);
if ($isFound) {
return true;
}
}

return false;
}

/**
* @param Stmt[] $stmts
*/
private function isFoundInMethodStmts(array $stmts, string $propertyName, string $className): bool
{
return (bool) $this->betterNodeFinder->findFirst($stmts, function (Node $subNode) use (
$propertyName,
$className
): bool {
if (! $this->propertyFetchAnalyzer->isPropertyFetch($subNode)) {
return false;
}

/** @var PropertyFetch|StaticPropertyFetch $subNode */
if ($subNode instanceof PropertyFetch) {
if (! $subNode->var instanceof Variable) {
return false;
}

if (! $this->nodeNameResolver->isName($subNode->var, 'this')) {
return false;
}

return $this->nodeNameResolver->isName($subNode, $propertyName);
}

if (! $this->nodeNameResolver->isNames(
$subNode->class,
[ObjectReference::SELF()->getValue(), ObjectReference::STATIC()->getValue(), $className]
)) {
return false;
}

return $this->nodeNameResolver->isName($subNode->name, $propertyName);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,10 @@
namespace Rector\Privatization\Rector\Property;

use PhpParser\Node;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\Property;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ClassReflection;
use Rector\Core\Enum\ObjectReference;
use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
use Rector\Core\PhpParser\AstResolver;
use Rector\Core\Rector\AbstractRector;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\Privatization\Guard\ParentPropertyLookupGuard;
use Rector\Privatization\NodeManipulator\VisibilityManipulator;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
Expand All @@ -30,8 +20,7 @@ final class PrivatizeFinalClassPropertyRector extends AbstractRector
{
public function __construct(
private readonly VisibilityManipulator $visibilityManipulator,
private readonly AstResolver $astResolver,
private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer
private readonly ParentPropertyLookupGuard $parentPropertyLookupGuard
) {
}

Expand Down Expand Up @@ -82,12 +71,7 @@ public function refactor(Node $node): ?Node
return null;
}

if ($classLike->extends === null) {
$this->visibilityManipulator->makePrivate($node);
return $node;
}

if ($this->isPropertyVisibilityGuardedByParent($node, $classLike)) {
if (! $this->parentPropertyLookupGuard->isLegal($node)) {
return null;
}

Expand All @@ -104,90 +88,4 @@ private function shouldSkipProperty(Property $property): bool

return ! $property->isProtected();
}

private function isPropertyVisibilityGuardedByParent(Property $property, Class_ $class): bool
{
if ($class->extends === null) {
return false;
}

/** @var Scope $scope */
$scope = $property->getAttribute(AttributeKey::SCOPE);

/** @var ClassReflection $classReflection */
$classReflection = $scope->getClassReflection();

$propertyName = $this->getName($property);
$className = (string) $this->nodeNameResolver->getName($class);

foreach ($classReflection->getParents() as $parentClassReflection) {
if ($parentClassReflection->hasProperty($propertyName)) {
return true;
}

if ($this->isFoundInParentClassMethods($parentClassReflection, $propertyName, $className)) {
return true;
}
}

return false;
}

private function isFoundInParentClassMethods(
ClassReflection $parentClassReflection,
string $propertyName,
string $className
): bool {
$classLike = $this->astResolver->resolveClassFromName($parentClassReflection->getName());
if (! $classLike instanceof ClassLike) {
return false;
}

$methods = $classLike->getMethods();
foreach ($methods as $method) {
$isFound = $this->isFoundInMethodStmts((array) $method->stmts, $propertyName, $className);
if ($isFound) {
return true;
}
}

return false;
}

/**
* @param Stmt[] $stmts
*/
private function isFoundInMethodStmts(array $stmts, string $propertyName, string $className): bool
{
return (bool) $this->betterNodeFinder->findFirst($stmts, function (Node $subNode) use (
$propertyName,
$className
): bool {
if (! $this->propertyFetchAnalyzer->isPropertyFetch($subNode)) {
return false;
}

/** @var PropertyFetch|StaticPropertyFetch $subNode */
if ($subNode instanceof PropertyFetch) {
if (! $subNode->var instanceof Variable) {
return false;
}

if (! $this->nodeNameResolver->isName($subNode->var, 'this')) {
return false;
}

return $this->nodeNameResolver->isName($subNode, $propertyName);
}

if (! $this->nodeNameResolver->isNames(
$subNode->class,
[ObjectReference::SELF()->getValue(), ObjectReference::STATIC()->getValue(), $className]
)) {
return false;
}

return $this->nodeNameResolver->isName($subNode->name, $propertyName);
});
}
}
3 changes: 2 additions & 1 deletion src/Reflection/ReflectionResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Property;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\FunctionReflection;
Expand Down Expand Up @@ -55,7 +56,7 @@ public function resolveClassAndAnonymousClass(ClassLike $classLike): ClassReflec
return $this->reflectionProvider->getClass($className);
}

public function resolveClassReflectionFromClassMethod(ClassMethod $classMethod): ?ClassReflection
public function resolveClassReflection(ClassMethod|Property $classMethod): ?ClassReflection
{
$scope = $classMethod->getAttribute(AttributeKey::SCOPE);

Expand Down

0 comments on commit c3adc6b

Please sign in to comment.