[Privatization] Add constant support on ChangeReadOnlyVariableWithDefaultValueToConstantRector (#1890)

* [Privatization] Add constant support on ChangeReadOnlyVariableWithDefaultValueToConstantRector

* implemented

* [ci-review] Rector Rectify

* add missing Use ConstFetch

* [ci-review] Rector Rectify

* rename method

* phpstan

* [ci-review] Rector Rectify

* flip check

* dependency circular between ExprAnalyzer and ArrayManipulator for check dynamic value

* [ci-review] Rector Rectify

* [ci-review] Rector Rectify

* clean up

* phpstan

* [ci-review] Rector Rectify

* final touch: clean up

* [ci-review] Rector Rectify

Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
Abdul Malik Ikhsan 2022-03-02 18:37:04 +07:00 committed by GitHub
parent 0cac723079
commit 926a7e02bc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 118 additions and 99 deletions

View File

@ -77,9 +77,7 @@ final class ContextAnalyzer
public function isHasAssignWithIndirectReturn(Node $node, If_ $if): bool
{
$loopNodes = self::LOOP_NODES;
foreach ($loopNodes as $loopNode) {
foreach (self::LOOP_NODES as $loopNode) {
$loopObjectType = new ObjectType($loopNode);
$parentType = $this->nodeTypeResolver->getType($node);
$superType = $parentType->isSuperTypeOf($loopObjectType);

View File

@ -0,0 +1,35 @@
<?php
namespace Rector\Tests\Privatization\Rector\Class_\ChangeReadOnlyVariableWithDefaultValueToConstantRector\Fixture;
class SomeConstant
{
public function run()
{
$replacements = \PHPUnit\Framework\TestCase\Notice::class;
if ($replacements === 'asdf') {
}
}
}
?>
-----
<?php
namespace Rector\Tests\Privatization\Rector\Class_\ChangeReadOnlyVariableWithDefaultValueToConstantRector\Fixture;
class SomeConstant
{
/**
* @var string
*/
private const REPLACEMENTS = \PHPUnit\Framework\TestCase\Notice::class;
public function run()
{
if (self::REPLACEMENTS === 'asdf') {
}
}
}
?>

View File

@ -7,18 +7,15 @@ namespace Rector\Php81\NodeAnalyzer;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\ArrayItem;
use PhpParser\Node\Expr\ClassConstFetch;
use PhpParser\Node\Expr\ConstFetch;
use PhpParser\Node\Expr\New_;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\Name\FullyQualified;
use PhpParser\Node\Scalar;
use Rector\Core\NodeAnalyzer\ExprAnalyzer;
use Rector\Core\NodeManipulator\ArrayManipulator;
final class ComplexNewAnalyzer
{
public function __construct(
private readonly ArrayManipulator $arrayManipulator,
private readonly ExprAnalyzer $exprAnalyzer
) {
}
@ -38,15 +35,12 @@ final class ComplexNewAnalyzer
continue;
}
// new inside array is allowed for New in initializer
if ($value instanceof Array_ && $this->isAllowedArray($value)) {
continue;
}
if ($value instanceof Scalar) {
continue;
}
if ($this->isAllowedConstFetchOrClassConstFeth($value)) {
if (! $this->exprAnalyzer->isDynamicValue($value)) {
continue;
}
@ -56,19 +50,6 @@ final class ComplexNewAnalyzer
return false;
}
private function isAllowedConstFetchOrClassConstFeth(Expr $expr): bool
{
if (! in_array($expr::class, [ConstFetch::class, ClassConstFetch::class], true)) {
return false;
}
if ($expr instanceof ClassConstFetch) {
return $expr->class instanceof Name && $expr->name instanceof Identifier;
}
return true;
}
private function isAllowedNew(Expr $expr): bool
{
if ($expr instanceof New_) {
@ -80,7 +61,7 @@ final class ComplexNewAnalyzer
private function isAllowedArray(Array_ $array): bool
{
if (! $this->exprAnalyzer->isDynamicArray($array)) {
if (! $this->arrayManipulator->isDynamicArray($array)) {
return true;
}

View File

@ -25,13 +25,17 @@ final class ConsoleApplication extends Application
*/
private const NAME = 'Rector';
/**
* @var string
*/
private const VERSION = VersionResolver::PACKAGE_VERSION;
/**
* @param Command[] $commands
*/
public function __construct(CommandNaming $commandNaming, array $commands = [])
{
$version = VersionResolver::PACKAGE_VERSION;
parent::__construct(self::NAME, $version);
parent::__construct(self::NAME, self::VERSION);
foreach ($commands as $command) {
$commandName = $commandNaming->resolveFromCommand($command);

View File

@ -6,14 +6,16 @@ namespace Rector\Core\NodeAnalyzer;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\ArrayItem;
use PhpParser\Node\Expr\ClassConstFetch;
use PhpParser\Node\Expr\ConstFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\FunctionLike;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\Scalar;
use PhpParser\Node\Scalar\LNumber;
use PhpParser\Node\Scalar\String_;
use PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
use Rector\Core\NodeManipulator\ArrayManipulator;
use Rector\Core\PhpParser\Comparing\NodeComparator;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\NodeNameResolver\NodeNameResolver;
@ -24,7 +26,8 @@ final class ExprAnalyzer
private readonly NodeComparator $nodeComparator,
private readonly BetterNodeFinder $betterNodeFinder,
private readonly PhpDocInfoFactory $phpDocInfoFactory,
private readonly NodeNameResolver $nodeNameResolver
private readonly NodeNameResolver $nodeNameResolver,
private readonly ArrayManipulator $arrayManipulator
) {
}
@ -61,52 +64,29 @@ final class ExprAnalyzer
return false;
}
public function isDynamicArray(Array_ $array): bool
{
foreach ($array->items as $item) {
if (! $item instanceof ArrayItem) {
continue;
}
$key = $item->key;
if (! $this->isAllowedArrayKey($key)) {
return true;
}
$value = $item->value;
if (! $this->isAllowedArrayValue($value)) {
return true;
}
}
return false;
}
private function isAllowedArrayKey(?Expr $expr): bool
{
if (! $expr instanceof Expr) {
return true;
}
return in_array($expr::class, [String_::class, LNumber::class], true);
}
private function isAllowedArrayValue(Expr $expr): bool
{
if ($expr instanceof Array_) {
return true;
}
return $this->isAllowedArrayOrScalar($expr);
}
private function isAllowedArrayOrScalar(Expr $expr): bool
public function isDynamicValue(Expr $expr): bool
{
if (! $expr instanceof Array_) {
return $expr instanceof Scalar;
if ($expr instanceof Scalar) {
return false;
}
return ! $this->isAllowedConstFetchOrClassConstFeth($expr);
}
return ! $this->isDynamicArray($expr);
return $this->arrayManipulator->isDynamicArray($expr);
}
private function isAllowedConstFetchOrClassConstFeth(Expr $expr): bool
{
if (! in_array($expr::class, [ConstFetch::class, ClassConstFetch::class], true)) {
return false;
}
if ($expr instanceof ClassConstFetch) {
return $expr->class instanceof Name && $expr->name instanceof Identifier;
}
return true;
}
}

View File

@ -4,42 +4,50 @@ declare(strict_types=1);
namespace Rector\Core\NodeManipulator;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\ArrayItem;
use PhpParser\Node\Scalar;
use PhpParser\Node\Scalar\LNumber;
use PhpParser\Node\Scalar\String_;
use Rector\ChangesReporting\Collector\RectorChangeCollector;
use Rector\Core\NodeAnalyzer\ExprAnalyzer;
use Symfony\Contracts\Service\Attribute\Required;
final class ArrayManipulator
{
private ExprAnalyzer $exprAnalyzer;
public function __construct(
private readonly RectorChangeCollector $rectorChangeCollector
) {
}
public function isArrayOnlyScalarValues(Array_ $array): bool
#[Required]
public function autowire(ExprAnalyzer $exprAnalyzer): void
{
foreach ($array->items as $arrayItem) {
if (! $arrayItem instanceof ArrayItem) {
$this->exprAnalyzer = $exprAnalyzer;
}
public function isDynamicArray(Array_ $array): bool
{
foreach ($array->items as $item) {
if (! $item instanceof ArrayItem) {
continue;
}
if ($arrayItem->value instanceof Array_) {
if (! $this->isArrayOnlyScalarValues($arrayItem->value)) {
return false;
}
$key = $item->key;
continue;
if (! $this->isAllowedArrayKey($key)) {
return true;
}
if ($arrayItem->value instanceof Scalar) {
continue;
$value = $item->value;
if (! $this->isAllowedArrayValue($value)) {
return true;
}
return false;
}
return true;
return false;
}
public function addItemToArrayUnderKey(Array_ $array, ArrayItem $newArrayItem, string $key): void
@ -97,4 +105,22 @@ final class ArrayManipulator
return $arrayItem->key->value === $name;
}
private function isAllowedArrayKey(?Expr $expr): bool
{
if (! $expr instanceof Expr) {
return true;
}
return in_array($expr::class, [String_::class, LNumber::class], true);
}
private function isAllowedArrayValue(Expr $expr): bool
{
if ($expr instanceof Array_) {
return ! $this->isDynamicArray($expr);
}
return ! $this->exprAnalyzer->isDynamicValue($expr);
}
}

View File

@ -7,14 +7,13 @@ namespace Rector\Core\NodeManipulator;
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Scalar;
use PhpParser\Node\Scalar\Encapsed;
use PhpParser\Node\Scalar\EncapsedStringPart;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\ClassMethod;
use Rector\Core\NodeAnalyzer\ExprAnalyzer;
use Rector\Core\PhpParser\Comparing\NodeComparator;
use Rector\Core\PhpParser\Node\BetterNodeFinder;
use Rector\NodeNameResolver\NodeNameResolver;
@ -25,13 +24,13 @@ use Symplify\Astral\NodeTraverser\SimpleCallableNodeTraverser;
final class VariableManipulator
{
public function __construct(
private readonly ArrayManipulator $arrayManipulator,
private readonly AssignManipulator $assignManipulator,
private readonly BetterNodeFinder $betterNodeFinder,
private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
private readonly NodeNameResolver $nodeNameResolver,
private readonly VariableToConstantGuard $variableToConstantGuard,
private readonly NodeComparator $nodeComparator
private readonly NodeComparator $nodeComparator,
private readonly ExprAnalyzer $exprAnalyzer
) {
}
@ -53,7 +52,7 @@ final class VariableManipulator
return null;
}
if (! $node->expr instanceof Array_ && ! $node->expr instanceof Scalar) {
if ($this->exprAnalyzer->isDynamicValue($node->expr)) {
return null;
}
@ -61,10 +60,6 @@ final class VariableManipulator
return null;
}
if ($node->expr instanceof Array_ && ! $this->arrayManipulator->isArrayOnlyScalarValues($node->expr)) {
return null;
}
if ($this->isTestCaseExpectedVariable($node->var)) {
return null;
}