[Php81] Skip used as ArrayDimFetch on Arg on side effect FuncCall on ReadOnlyPropertyRector (#1821)

* [Php81] Skip used in CallLike arg on ReadOnlyPropertyRector

* rename fixture

* use VisibilityManipulator->hasVisibility for Private detection

* different method for refactor property

* more fixture for Property

* [ci-review] Rector Rectify

* Fixed 🎉

* [ci-review] Rector Rectify

* [ci-review] Rector Rectify

Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
Abdul Malik Ikhsan 2022-02-17 03:59:24 +07:00 committed by GitHub
parent a560205c75
commit 4cf6522e77
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 105 additions and 25 deletions

View File

@ -7,6 +7,7 @@ namespace Rector\ReadWrite\NodeAnalyzer;
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\Isset_;
use PhpParser\Node\Expr\PostDec;
use PhpParser\Node\Expr\PostInc;
@ -18,6 +19,7 @@ use PhpParser\Node\Stmt\Unset_;
use Rector\Core\Exception\ShouldNotHappenException;
use Rector\Core\NodeManipulator\AssignManipulator;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\DeadCode\SideEffect\PureFunctionDetector;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\ReadWrite\Guard\VariableToConstantGuard;
@ -28,6 +30,7 @@ final class ReadWritePropertyAnalyzer
private readonly AssignManipulator $assignManipulator,
private readonly ReadExprAnalyzer $readExprAnalyzer,
private readonly BetterNodeFinder $betterNodeFinder,
private readonly PureFunctionDetector $pureFunctionDetector
) {
}
@ -51,7 +54,32 @@ final class ReadWritePropertyAnalyzer
return $this->isArrayDimFetchRead($parent);
}
return ! $this->assignManipulator->isLeftPartOfAssign($node);
if (! $parent instanceof ArrayDimFetch) {
return ! $this->assignManipulator->isLeftPartOfAssign($node);
}
if (! $this->isArrayDimFetchInImpureFunction($parent, $node)) {
return ! $this->assignManipulator->isLeftPartOfAssign($node);
}
return false;
}
private function isArrayDimFetchInImpureFunction(ArrayDimFetch $arrayDimFetch, Node $node): bool
{
if ($arrayDimFetch->var === $node) {
$arg = $this->betterNodeFinder->findParentType($arrayDimFetch, Arg::class);
if ($arg instanceof Arg) {
$parentArg = $arg->getAttribute(AttributeKey::PARENT_NODE);
if (! $parentArg instanceof FuncCall) {
return false;
}
return ! $this->pureFunctionDetector->detect($parentArg);
}
}
return false;
}
private function unwrapPostPreIncDec(Node $node): Node

View File

@ -0,0 +1,19 @@
<?php
namespace Rector\Tests\Php81\Rector\Property\ReadOnlyPropertyRector\Fixture;
final class SkipUsedAsArrayDimFetchInSideEffectCallArg
{
/**
* @param array<string, array<string>> $items
*/
public function __construct(private array $items)
{}
public function popFooItem(): void
{
if (!empty($this->items) && array_key_exists('foo', $this->items)) {
array_pop($this->items['foo']);
}
}
}

View File

@ -0,0 +1,23 @@
<?php
namespace Rector\Tests\Php81\Rector\Property\ReadOnlyPropertyRector\Fixture;
final class SkipUsedAsArrayDimFetchInSideEffectCallArg2
{
private array $items;
/**
* @param array<string, array<string>> $items
*/
public function __construct(array $items)
{
$this->items = $items;
}
public function popFooItem(): void
{
if (!empty($this->items) && array_key_exists('foo', $this->items)) {
array_pop($this->items['foo']);
}
}
}

View File

@ -85,29 +85,7 @@ CODE_SAMPLE
return $this->refactorParam($node);
}
// 1. is property read-only?
if ($this->propertyManipulator->isPropertyChangeableExceptConstructor($node)) {
return null;
}
if ($node->isReadonly()) {
return null;
}
if ($node->props[0]->default instanceof Expr) {
return null;
}
if ($node->type === null) {
return null;
}
if ($node->flags !== Visibility::PRIVATE) {
return null;
}
$this->visibilityManipulator->makeReadonly($node);
return $node;
return $this->refactorProperty($node);
}
public function provideMinPhpVersion(): int
@ -115,9 +93,36 @@ CODE_SAMPLE
return PhpVersionFeature::READONLY_PROPERTY;
}
private function refactorProperty(Property $property): ?Property
{
// 1. is property read-only?
if ($this->propertyManipulator->isPropertyChangeableExceptConstructor($property)) {
return null;
}
if ($property->isReadonly()) {
return null;
}
if ($property->props[0]->default instanceof Expr) {
return null;
}
if ($property->type === null) {
return null;
}
if (! $this->visibilityManipulator->hasVisibility($property, Visibility::PRIVATE)) {
return null;
}
$this->visibilityManipulator->makeReadonly($property);
return $property;
}
private function refactorParam(Param $param): Param | null
{
if ($param->flags !== Visibility::PRIVATE) {
if (! $this->visibilityManipulator->hasVisibility($param, Visibility::PRIVATE)) {
return null;
}

View File

@ -9,6 +9,7 @@ use Doctrine\ORM\Mapping\Table;
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\ArrayDimFetch;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\PostDec;
use PhpParser\Node\Expr\PostInc;
@ -212,6 +213,10 @@ final class PropertyManipulator
}
}
if ($parent instanceof ArrayDimFetch) {
return ! $this->readWritePropertyAnalyzer->isRead($propertyFetch);
}
return false;
}