[Privatization] Handle empty construct change property to local variable on ChangeLocalPropertyToVariableRector (#479)

* fixing 6559

* Fixed 🎉

* different name  to show it only change the property fetch

* Fixed 🎉

* fixture name

* skip no typed property never changing, means it is null

* eol

* phpstan

* skip never assigned, means null

* skip never assigned, means null
This commit is contained in:
Abdul Malik Ikhsan 2021-07-21 20:42:20 +07:00 committed by GitHub
parent 24a32dfefb
commit dc10595b7c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 132 additions and 7 deletions

View File

@ -0,0 +1,39 @@
<?php
namespace Rector\Tests\Privatization\Rector\Class_\ChangeLocalPropertyToVariableRector\Fixture;
final class NoAssignInConstruct
{
private string $name;
public function __construct()
{
}
public function sayHello(string $aName)
{
$this->name = $aName;
return "Hello {$this->name}";
}
}
?>
-----
<?php
namespace Rector\Tests\Privatization\Rector\Class_\ChangeLocalPropertyToVariableRector\Fixture;
final class NoAssignInConstruct
{
public function __construct()
{
}
public function sayHello(string $aName)
{
$name = $aName;
return "Hello {$name}";
}
}
?>

View File

@ -0,0 +1,11 @@
<?php
final class SkipNeverAssigned
{
private $name;
public function sayHello()
{
return "Hello {$this->name}";
}
}

View File

@ -0,0 +1,17 @@
<?php
namespace Rector\Tests\Privatization\Rector\Class_\ChangeLocalPropertyToVariableRector\Fixture;
final class SkipNoAssignInConstructHasDefaultValue
{
private string $name = 'test';
public function __construct()
{
}
public function sayHello()
{
return "Hello {$this->name}";
}
}

View File

@ -0,0 +1,17 @@
<?php
namespace Rector\Tests\Privatization\Rector\Class_\ChangeLocalPropertyToVariableRector\Fixture;
final class SkipPropertyPromotion
{
public function __construct(private string $name)
{
}
public function sayHello()
{
return "Hello {$this->name}";
}
}
?>

View File

@ -13,6 +13,7 @@ use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Do_;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Property;
use PhpParser\Node\Stmt\While_;
use PhpParser\NodeTraverser;
use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
@ -44,17 +45,24 @@ final class PropertyFetchByMethodAnalyzer
$propertyUsageByMethods = [];
foreach ($propertyNames as $propertyName) {
if ($this->isPropertyHasDefaultValue($class, $propertyName)) {
continue;
}
foreach ($class->getMethods() as $classMethod) {
// assigned in constructor injection → skip
if ($this->nodeNameResolver->isName($classMethod, MethodName::CONSTRUCT)) {
if ($this->isInConstructWithPropertyChanging($classMethod, $propertyName)) {
return [];
}
if (! $this->propertyFetchAnalyzer->containsLocalPropertyFetchName($classMethod, $propertyName)) {
if ($this->isContainsLocalPropertyFetchNameOrPropertyChangingInMultipleMethodCalls(
$classMethod,
$propertyName
)) {
continue;
}
if ($this->isPropertyChangingInMultipleMethodCalls($classMethod, $propertyName)) {
if (! $this->isPropertyChanging($classMethod, $propertyName)) {
continue;
}
@ -66,6 +74,32 @@ final class PropertyFetchByMethodAnalyzer
return $propertyUsageByMethods;
}
private function isContainsLocalPropertyFetchNameOrPropertyChangingInMultipleMethodCalls(
ClassMethod $classMethod,
string $propertyName
): bool
{
if (! $this->propertyFetchAnalyzer->containsLocalPropertyFetchName($classMethod, $propertyName)) {
return true;
}
return $this->isPropertyChangingInMultipleMethodCalls($classMethod, $propertyName);
}
private function isPropertyHasDefaultValue(Class_ $class, string $propertyName): bool
{
$property = $class->getProperty($propertyName);
return $property instanceof Property && $property->props[0]->default;
}
private function isInConstructWithPropertyChanging(ClassMethod $classMethod, string $propertyName): bool
{
if (! $this->nodeNameResolver->isName($classMethod, MethodName::CONSTRUCT)) {
return false;
}
return $this->isPropertyChanging($classMethod, $propertyName);
}
/**
* Covers https://github.com/rectorphp/rector/pull/2558#discussion_r363036110
*/
@ -98,11 +132,9 @@ final class PropertyFetchByMethodAnalyzer
return null;
}
if ($node instanceof If_) {
$isPropertyReadInIf = $this->refactorIf($node, $propertyName);
}
$isPropertyReadInIf = $this->verifyPropertyReadInIf($isPropertyReadInIf, $node, $propertyName);
$isPropertyChanging = $this->isPropertyChanging($node, $propertyName);
if (! $isPropertyChanging) {
return null;
}
@ -114,6 +146,15 @@ final class PropertyFetchByMethodAnalyzer
return $isPropertyChanging || $isIfFollowedByAssign || $isPropertyReadInIf;
}
private function verifyPropertyReadInIf(?bool $isPropertyReadInIf, Node $node, string $propertyName): ?bool
{
if ($node instanceof If_) {
$isPropertyReadInIf = $this->refactorIf($node, $propertyName);
}
return $isPropertyReadInIf;
}
private function isScopeChangingNode(Node $node): bool
{
foreach (self::SCOPE_CHANGING_NODE_TYPES as $type) {