octoleo/rector
1
1
Fork 0
mirror of https://github.com/rectorphp/rector.git synced 2024-06-08 04:10:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

[DeadCode] Add RemoveJustPropertyFetchRector (#2433)

Browse Source
This commit is contained in:
Tomas Votruba 2022-06-04 19:31:24 +02:00 committed by GitHub
parent 70261b71b7
commit b0a6173550
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
27 changed files with 525 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
packages/BetterPhpDocParser/PhpDocInfo/PhpDocInfo.php
View File

@ -337,13 +337,12 @@ final class PhpDocInfo
$paramTypesByName = [];
foreach ($this->phpDocNode->getParamTagValues() as $paramTagValueNode) {
$parameterName = $paramTagValueNode->parameterName;
$parameterType = $this->staticTypeMapper->mapPHPStanPhpDocTypeToPHPStanType(
$paramTagValueNode,
$this->node
);
$paramTypesByName[$parameterName] = $parameterType;
$paramTypesByName[$paramTagValueNode->parameterName] = $parameterType;
}
return $paramTypesByName;

1
packages/BetterPhpDocParser/PhpDocInfo/PhpDocInfoFactory.php
View File

@ -114,7 +114,6 @@ final class PhpDocInfoFactory
$phpDocChildNodes = $phpDocNode->children;
$phpDocChildNode = array_pop($phpDocChildNodes);
$startAndEnd = $phpDocChildNode->getAttribute(PhpDocAttributeKey::START_AND_END);
if ($startAndEnd instanceof StartAndEnd) {

2
packages/NodeCollector/BinaryOpConditionsCollector.php
View File

@ -38,7 +38,7 @@ final class BinaryOpConditionsCollector
$conditions[] = $expr->right;
$expr = $expr->left;
if ($expr::class !== $binaryOpClass) {
if ($binaryOpClass !== $expr::class) {
$conditions[] = $expr;
break;
}

3
packages/NodeCollector/NodeAnalyzer/ArrayCallableMethodMatcher.php
View File

@ -48,8 +48,7 @@ final class ArrayCallableMethodMatcher
*/
public function match(Array_ $array): null | ArrayCallableDynamicMethod | ArrayCallable
{
$arrayItems = $array->items;
if (count($arrayItems) !== 2) {
if (count($array->items) !== 2) {
return null;
}

3
packages/NodeTypeResolver/PhpDoc/NodeAnalyzer/DocBlockNamespaceRenamer.php
View File

@ -44,10 +44,9 @@ final class DocBlockNamespaceRenamer
return null;
}
$name = $docNode->name;
$trimmedName = ltrim($docNode->name, '\\');
if ($name === $trimmedName) {
if ($docNode->name === $trimmedName) {
return null;
}

7
packages/PhpAttribute/AttributeArrayNameInliner.php
View File

@ -39,9 +39,10 @@ final class AttributeArrayNameInliner
continue;
}
$key = $arrayItem->key;
if ($key instanceof String_) {
$args[] = new Arg($arrayItem->value, false, false, [], new Identifier($key->value));
if ($arrayItem->key instanceof String_) {
$string = $arrayItem->key;
$argumentName = new Identifier($string->value);
$args[] = new Arg($arrayItem->value, false, false, [], $argumentName);
} else {
$args[] = new Arg($arrayItem->value);
}

3
packages/StaticTypeMapper/PhpDocParser/IdentifierTypeMapper.php
View File

@ -84,8 +84,7 @@ final class IdentifierTypeMapper implements PhpDocTypeMapperInterface
}
if (str_starts_with($typeNode->name, '\\')) {
$type = $typeNode->name;
$typeWithoutPreslash = Strings::substring($type, 1);
$typeWithoutPreslash = Strings::substring($typeNode->name, 1);
$objectType = new FullyQualifiedObjectType($typeWithoutPreslash);
} else {
if ($typeNode->name === 'scalar') {

5
packages/StaticTypeMapper/PhpDocParser/NullableTypeMapper.php
View File

@ -36,9 +36,8 @@ final class NullableTypeMapper implements PhpDocTypeMapperInterface
*/
public function mapToPHPStanType(TypeNode $typeNode, Node $node, NameScope $nameScope): Type
{
$type = $typeNode->type;
if ($type instanceof IdentifierTypeNode) {
$type = $this->identifierTypeMapper->mapToPHPStanType($type, $node, $nameScope);
if ($typeNode->type instanceof IdentifierTypeNode) {
$type = $this->identifierTypeMapper->mapToPHPStanType($typeNode->type, $node, $nameScope);
return new UnionType([new NullType(), $type]);
}

17
rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_conditional_assign.php.inc Normal file
View File

@ -0,0 +1,17 @@
<?php
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Fixture;
final class SkipConditionalAssign
{
private $id;
public function run(?int $id)
{
if ($id === null) {
$id = $this->id;
}
return $id;
}
}

23
rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_double_property_fetch.php.inc Normal file
View File

@ -0,0 +1,23 @@
<?php
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Fixture;
use PHPStan\PhpDocParser\Ast\PhpDoc\GenericTagValueNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode;
final class SkipDoublePropertyFetch
{
public function run(PhpDocTagNode $phpDocTagNode)
{
if (! $phpDocTagNode->value instanceof GenericTagValueNode) {
return null;
}
$description = $phpDocTagNode->value->value;
if ($description === '') {
return 'no';
}
return $description;
}
}

24
rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_in_while_assign.php.inc Normal file
View File

@ -0,0 +1,24 @@
<?php
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Fixture;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp;
final class SkipInWhileAssign
{
public function run(Expr $expr, string $binaryOpClass)
{
$conditions = [];
while ($expr instanceof BinaryOp) {
$conditions[] = $expr->right;
$expr = $expr->left;
if ($binaryOpClass !== $expr::class) {
$conditions[] = $expr;
break;
}
}
}
}

17
rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_nested_property_fetch.php.inc Normal file
View File

@ -0,0 +1,17 @@
<?php
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Fixture;
use PhpParser\Node\Expr\Assign;
final class SkipNestedPropertyFetch
{
public function run(Assign $assign)
{
if ($assign->var instanceof Assign) {
$nestedAssign = $assign->var;
echo $nestedAssign->expr;
}
}
}

14
rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_property_write.php.inc Normal file
View File

@ -0,0 +1,14 @@
<?php
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Fixture;
use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocNode;
final class SkipPropertyWrite
{
public function run(PhpDocNode $phpDocNode)
{
$phpDocChildNodes = $phpDocNode->children;
$phpDocChildNode = array_pop($phpDocChildNodes);
}
}

19
rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_var_annotated_assign.php.inc Normal file
View File

@ -0,0 +1,19 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Fixture;
use PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode;
use PHPUnit\Framework\TestCase;
final class SkipVarAnnotatedAssign extends TestCase
{
public function go()
{
/** @var PhpDocTagNode $childNode */
$propertyTagValueNode = $childNode->value;
$this->assertInstanceOf(ParamTagValueNode::class, $propertyTagValueNode);
}
}

33
rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/some_class.php.inc Normal file
View File

@ -0,0 +1,33 @@
<?php
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Fixture;
final class SomeClass
{
private $name;
public function run()
{
$name = $this->name;
return $name;
}
}
?>
-----
<?php
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Fixture;
final class SomeClass
{
private $name;
public function run()
{
return $this->name;
}
}
?>

47
rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/with_simple_method_call.php.inc Normal file
View File

@ -0,0 +1,47 @@
<?php
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Fixture;
use Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Source\AnotherCaller;
final class WithSimleMethodCall
{
private AnotherCaller $anotherCaller;
public function __construct(AnotherCaller $anotherCaller)
{
$this->anotherCaller = $anotherCaller;
}
public function run()
{
$anotherCaller = $this->anotherCaller;
$result = $anotherCaller->callMe();
}
}
?>
-----
<?php
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Fixture;
use Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Source\AnotherCaller;
final class WithSimleMethodCall
{
private AnotherCaller $anotherCaller;
public function __construct(AnotherCaller $anotherCaller)
{
$this->anotherCaller = $anotherCaller;
}
public function run()
{
$result = $this->anotherCaller->callMe();
}
}
?>

33
rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/RemoveJustPropertyFetchRectorTest.php Normal file
View File

@ -0,0 +1,33 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector;
use Iterator;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
use Symplify\SmartFileSystem\SmartFileInfo;
final class RemoveJustPropertyFetchRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideData()
*/
public function test(SmartFileInfo $fileInfo): void
{
$this->doTestFileInfo($fileInfo);
}
/**
* @return Iterator<SmartFileInfo>
*/
public function provideData(): Iterator
{
return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture');
}
public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule.php';
}
}

12
rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Source/AnotherCaller.php Normal file
View File

@ -0,0 +1,12 @@
<?php
declare(strict_types=1);
namespace Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\Source;
final class AnotherCaller
{
public function callMe()
{
}
}

10
rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/config/configured_rule.php Normal file
View File

@ -0,0 +1,10 @@
<?php
declare(strict_types=1);
use Rector\Config\RectorConfig;
use Rector\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector;
return static function (RectorConfig $rectorConfig): void {
$rectorConfig->rule(RemoveJustPropertyFetchRector::class);
};

220
rules/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector.php Normal file
View File

@ -0,0 +1,220 @@
<?php
declare(strict_types=1);
namespace Rector\DeadCode\Rector\StmtsAwareInterface;
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\While_;
use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
use Rector\Core\Rector\AbstractRector;
use Rector\DeadCode\ValueObject\PropertyFetchToVariableAssign;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\ReadWrite\NodeFinder\NodeUsageFinder;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
/**
* @see \Rector\Tests\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector\RemoveJustPropertyFetchRectorTest
*/
final class RemoveJustPropertyFetchRector extends AbstractRector
{
public function __construct(
private readonly NodeUsageFinder $nodeUsageFinder
) {
}
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Inline property fetch assign to a variable, that has no added value', [
new CodeSample(
<<<'CODE_SAMPLE'
final class SomeClass
{
private $name;
public function run()
{
$name = $this->name;
return $name;
}
}
CODE_SAMPLE
,
<<<'CODE_SAMPLE'
final class SomeClass
{
private $name;
public function run()
{
return $this->name;
}
}
CODE_SAMPLE
),
]);
}
/**
* @return array<class-string<Node>>
*/
public function getNodeTypes(): array
{
return [StmtsAwareInterface::class];
}
/**
* @param StmtsAwareInterface $node
*/
public function refactor(Node $node): ?Node
{
$stmts = (array) $node->stmts;
if ($stmts === []) {
return null;
}
$variableUsages = [];
$currentStmtKey = null;
$variableToPropertyAssign = null;
foreach ($stmts as $key => $stmt) {
$variableToPropertyAssign = $this->matchVariableToPropertyAssign($stmt);
if (! $variableToPropertyAssign instanceof PropertyFetchToVariableAssign) {
continue;
}
$assignPhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($stmt);
// there is a @var tag on purpose, keep the assign
if ($assignPhpDocInfo->getVarTagValueNode() instanceof VarTagValueNode) {
continue;
}
$followingStmts = array_slice($stmts, $key + 1);
$variableUsages = $this->nodeUsageFinder->findVariableUsages(
$followingStmts,
$variableToPropertyAssign->getVariable()
);
$currentStmtKey = $key;
// @todo validate the variable is not used in some place where property fetch cannot be used
break;
}
// filter out variable usages that are part of nested property fetch, or change variable
$variableUsages = $this->filterOutReferencedVariableUsages($variableUsages);
if (! $variableToPropertyAssign instanceof PropertyFetchToVariableAssign) {
return null;
}
if ($variableUsages === []) {
return null;
}
/** @var int $currentStmtKey */
return $this->replaceVariablesWithPropertyFetch(
$node,
$currentStmtKey,
$variableUsages,
$variableToPropertyAssign->getPropertyFetch()
);
}
/**
* @param Variable[] $variableUsages
*/
private function replaceVariablesWithPropertyFetch(
StmtsAwareInterface $stmtsAware,
int $currentStmtsKey,
array $variableUsages,
PropertyFetch $propertyFetch
): StmtsAwareInterface {
// remove assign node
unset($stmtsAware->stmts[$currentStmtsKey]);
$this->traverseNodesWithCallable(
$stmtsAware,
function (Node $node) use ($variableUsages, $propertyFetch): ?PropertyFetch {
if (! in_array($node, $variableUsages, true)) {
return null;
}
return $propertyFetch;
}
);
return $stmtsAware;
}
/**
* @param Variable[] $variableUsages
* @return Variable[]
*/
private function filterOutReferencedVariableUsages(array $variableUsages): array
{
return array_filter($variableUsages, function (Variable $variable): bool {
$variableUsageParent = $variable->getAttribute(AttributeKey::PARENT_NODE);
if ($variableUsageParent instanceof Arg) {
$variableUsageParent = $variableUsageParent->getAttribute(AttributeKey::PARENT_NODE);
}
// skip nested property fetch, the assign is for purpose of named variable
if ($variableUsageParent instanceof PropertyFetch) {
return false;
}
// skip, as assign can be used in a loop
$parentWhile = $this->betterNodeFinder->findParentType($variable, While_::class);
if ($parentWhile instanceof While_) {
return false;
}
if (! $variableUsageParent instanceof FuncCall) {
return true;
}
return ! $this->isName($variableUsageParent, 'array_pop');
});
}
private function matchVariableToPropertyAssign(Stmt $stmt): ?PropertyFetchToVariableAssign
{
if (! $stmt instanceof Expression) {
return null;
}
if (! $stmt->expr instanceof Assign) {
return null;
}
$assign = $stmt->expr;
if (! $assign->expr instanceof PropertyFetch) {
return null;
}
// keep property fetch nesting
if ($assign->expr->var instanceof PropertyFetch) {
return null;
}
if (! $assign->var instanceof Variable) {
return null;
}
return new PropertyFetchToVariableAssign($assign->var, $assign->expr);
}
}

27
rules/DeadCode/ValueObject/PropertyFetchToVariableAssign.php Normal file
View File

@ -0,0 +1,27 @@
<?php
declare(strict_types=1);
namespace Rector\DeadCode\ValueObject;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\Variable;
final class PropertyFetchToVariableAssign
{
public function __construct(
private readonly Variable $variable,
private readonly PropertyFetch $propertyFetch,
) {
}
public function getVariable(): Variable
{
return $this->variable;
}
public function getPropertyFetch(): PropertyFetch
{
return $this->propertyFetch;
}
}

5
src/NodeAnalyzer/CompactFuncCallAnalyzer.php
View File

@ -26,12 +26,11 @@ final class CompactFuncCallAnalyzer
return false;
}
$variableName = $variable->name;
if (! is_string($variableName)) {
if (! is_string($variable->name)) {
return false;
}
return $this->isInArgOrArrayItemNodes($funcCall->args, $variableName);
return $this->isInArgOrArrayItemNodes($funcCall->args, $variable->name);
}
/**

4
src/NodeManipulator/ArrayManipulator.php
View File

@ -35,9 +35,7 @@ final class ArrayManipulator
continue;
}
$key = $item->key;
if (! $this->isAllowedArrayKey($key)) {
if (! $this->isAllowedArrayKey($item->key)) {
return true;
}

6
src/NodeManipulator/ForeachManipulator.php
View File

@ -15,13 +15,11 @@ final class ForeachManipulator
*/
public function matchOnlyStmt(Foreach_ $foreach, callable $callable): ?Node
{
$stmts = $foreach->stmts;
if (count($stmts) !== 1) {
if (count($foreach->stmts) !== 1) {
return null;
}
$innerNode = $stmts[0];
$innerNode = $foreach->stmts[0];
$innerNode = $innerNode instanceof Expression ? $innerNode->expr : $innerNode;
return $callable($innerNode, $foreach);

5
src/NodeManipulator/IfManipulator.php
View File

@ -43,12 +43,11 @@ final class IfManipulator
*/
public function matchIfNotNullReturnValue(If_ $if): ?Expr
{
$stmts = $if->stmts;
if (count($stmts) !== 1) {
if (count($if->stmts) !== 1) {
return null;
}
$insideIfNode = $stmts[0];
$insideIfNode = $if->stmts[0];
if (! $insideIfNode instanceof Return_) {
return null;
}

3
src/NodeManipulator/VariableManipulator.php
View File

@ -156,8 +156,7 @@ final class VariableManipulator
return false;
}
$variable = $assign->var;
$variableUsages = $this->collectVariableUsages($classMethod, $variable, $assign);
$variableUsages = $this->collectVariableUsages($classMethod, $assign->var, $assign);
foreach ($variableUsages as $variableUsage) {
$parent = $variableUsage->getAttribute(AttributeKey::PARENT_NODE);

17
src/PhpParser/Comparing/ConditionSearcher.php
View File

@ -23,12 +23,12 @@ final class ConditionSearcher
public function hasIfAndElseForVariableRedeclaration(Assign $assign, If_ $if): bool
{
$elseNode = $if->else;
if (! $elseNode instanceof Else_) {
if (! $if->else instanceof Else_) {
return false;
}
$ifElse = $if->else;
/** @var Variable $varNode */
$varNode = $assign->var;
@ -51,7 +51,7 @@ final class ConditionSearcher
return false;
}
return $this->hasVariableRedeclaration($varNode, $elseNode->stmts);
return $this->hasVariableRedeclaration($varNode, $ifElse->stmts);
}
/**
@ -96,11 +96,14 @@ final class ConditionSearcher
return false;
}
$assignVar = $stmt->expr->var;
if (! $assignVar instanceof Variable) {
$assign = $stmt->expr;
if (! $assign->var instanceof Variable) {
return false;
}
return $variable->name === $assignVar->name;
$assignedVariable = $assign->var;
return $variable->name === $assignedVariable->name;
}
}