Skip to content

Commit

Permalink
[Php81] Skip multiple assign on __construct on ReadOnlyPropertyRector (
Browse files Browse the repository at this point in the history
…#2088)

* [Php81] Skip multiple assign on __construct on ReadOnlyPropertyRector

* Fixed 🎉

* final touch: check multi assign on last check

* final touch: eol

* final touch: stop traversal when count already = 2

* final touch: check only in __construct

* [ci-review] Rector Rectify

* [ci-review] Rector Rectify

* really final touch: ensure parent ClassLike is equals with the property ClassLike

* really final touch: ensure parent ClassLike is equals with the property ClassLike

Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
samsonasik and actions-user committed Apr 17, 2022
1 parent 996caa3 commit c7ff218
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

namespace Rector\Tests\Php81\Rector\Property\ReadOnlyPropertyRector\Fixture;

final class SkipMultipleAssign
{
private string $name;

public function __construct(string $name, bool $flag = true)
{
$this->name = $name;

if ($flag) {
$this->name = 'change';
}

if ($flag) {
$this->name = 'change again';
}
}
}
6 changes: 6 additions & 0 deletions rules/Php81/Rector/Property/ReadOnlyPropertyRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use PhpParser\Node\Expr;
use PhpParser\Node\Param;
use PhpParser\Node\Stmt\Property;
use Rector\Core\NodeManipulator\PropertyFetchAssignManipulator;
use Rector\Core\NodeManipulator\PropertyManipulator;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\ValueObject\PhpVersionFeature;
Expand All @@ -27,6 +28,7 @@ final class ReadOnlyPropertyRector extends AbstractRector implements MinPhpVersi
{
public function __construct(
private readonly PropertyManipulator $propertyManipulator,
private readonly PropertyFetchAssignManipulator $propertyFetchAssignManipulator,
private readonly VisibilityManipulator $visibilityManipulator,
) {
}
Expand Down Expand Up @@ -121,6 +123,10 @@ private function refactorProperty(Property $property): ?Property
return null;
}

if ($this->propertyFetchAssignManipulator->isAssignedMultipleTimesInConstructor($property)) {
return null;
}

$this->visibilityManipulator->makeReadonly($property);

$attributeGroups = $property->attrGroups;
Expand Down
55 changes: 54 additions & 1 deletion src/NodeManipulator/PropertyFetchAssignManipulator.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,70 @@
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Property;
use PhpParser\NodeTraverser;
use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\Core\ValueObject\MethodName;
use Rector\NodeNameResolver\NodeNameResolver;
use Symplify\Astral\NodeTraverser\SimpleCallableNodeTraverser;

final class PropertyFetchAssignManipulator
{
public function __construct(
private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
private readonly NodeNameResolver $nodeNameResolver
private readonly NodeNameResolver $nodeNameResolver,
private readonly BetterNodeFinder $betterNodeFinder,
private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer
) {
}

public function isAssignedMultipleTimesInConstructor(Property $property): bool
{
$classLike = $this->betterNodeFinder->findParentType($property, ClassLike::class);
if (! $classLike instanceof ClassLike) {
return false;
}

$classMethod = $classLike->getMethod(MethodName::CONSTRUCT);
if (! $classMethod instanceof ClassMethod) {
return false;
}

$count = 0;
$propertyName = $this->nodeNameResolver->getName($property);

$this->simpleCallableNodeTraverser->traverseNodesWithCallable(
(array) $classMethod->getStmts(),
function (Node $node) use ($propertyName, $classLike, &$count): ?int {
if (! $node instanceof Assign) {
return null;
}

if (! $this->propertyFetchAnalyzer->isLocalPropertyFetchName($node->var, $propertyName)) {
return null;
}

$parentClassLike = $this->betterNodeFinder->findParentType($node, ClassLike::class);
if ($parentClassLike !== $classLike) {
return null;
}

++$count;

if ($count === 2) {
return NodeTraverser::STOP_TRAVERSAL;
}

return null;
}
);

return $count === 2;
}

/**
* @return string[]
*/
Expand Down

0 comments on commit c7ff218

Please sign in to comment.