[DeadCode] Add RemoveUnreachableStatementRector

This commit is contained in:
Tomas Votruba 2019-10-02 22:13:37 +02:00
parent 8efea04952
commit 6a56bf2a8d
14 changed files with 373 additions and 9 deletions

View File

@ -33,7 +33,8 @@ expectedArguments(
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_EXPRESSION,
\Rector\NodeTypeResolver\Node\AttributeKey::USE_NODES,
\Rector\NodeTypeResolver\Node\AttributeKey::START_TOKEN_POSITION,
\Rector\NodeTypeResolver\Node\AttributeKey::ORIGINAL_NODE
\Rector\NodeTypeResolver\Node\AttributeKey::ORIGINAL_NODE,
\Rector\NodeTypeResolver\Node\AttributeKey::IS_UNREACHABLE,
);
expectedArguments(
@ -53,5 +54,7 @@ expectedArguments(
\Rector\NodeTypeResolver\Node\AttributeKey::CURRENT_EXPRESSION,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_EXPRESSION,
\Rector\NodeTypeResolver\Node\AttributeKey::USE_NODES,
\Rector\NodeTypeResolver\Node\AttributeKey::START_TOKEN_POSITION
\Rector\NodeTypeResolver\Node\AttributeKey::START_TOKEN_POSITION,
\Rector\NodeTypeResolver\Node\AttributeKey::ORIGINAL_NODE,
\Rector\NodeTypeResolver\Node\AttributeKey::IS_UNREACHABLE,
);

View File

@ -28,3 +28,4 @@ services:
Rector\DeadCode\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector: ~
Rector\DeadCode\Rector\Property\RemoveNullPropertyInitializationRector: ~
Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector: ~

View File

@ -0,0 +1,147 @@
<?php declare(strict_types=1);
namespace Rector\DeadCode\Rector\Stmt;
use PhpParser\Node;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Namespace_;
use PhpParser\Node\Stmt\Nop;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
/**
* @see https://github.com/phpstan/phpstan/blob/83078fe308a383c618b8c1caec299e5765d9ac82/src/Node/UnreachableStatementNode.php
*
* @see \Rector\DeadCode\Tests\Rector\Stmt\RemoveUnreachableStatementRector\RemoveUnreachableStatementRectorTest
*/
final class RemoveUnreachableStatementRector extends AbstractRector
{
public function getDefinition(): RectorDefinition
{
return new RectorDefinition('Remove unreachable statements', [
new CodeSample(
<<<'PHP'
class SomeClass
{
public function run()
{
return 5;
$removeMe = 10;
}
}
PHP
,
<<<'PHP'
class SomeClass
{
public function run()
{
return 5;
}
}
PHP
),
]);
}
/**
* @return string[]
*/
public function getNodeTypes(): array
{
return [Stmt::class];
}
/**
* @param Stmt $node
*/
public function refactor(Node $node): ?Node
{
if ($node instanceof Nop) {
return null;
}
if (! $this->isUnreachable($node)) {
return null;
}
// might be PHPStan false positive, better skip
$previousNode = $node->getAttribute(AttributeKey::PREVIOUS_NODE);
if ($previousNode instanceof If_) {
$node->setAttribute(AttributeKey::IS_UNREACHABLE, false);
return null;
}
if ($this->isAfterMarkTestSkippedMethodCall($node)) {
$node->setAttribute(AttributeKey::IS_UNREACHABLE, false);
return null;
}
$this->removeNode($node);
return null;
}
private function isUnreachable(Node $node): bool
{
$isUnreachable = $node->getAttribute(AttributeKey::IS_UNREACHABLE);
if ($isUnreachable === true) {
return true;
}
// traverse up for unreachable node in the same scope
$previousNode = $node->getAttribute(AttributeKey::PREVIOUS_NODE);
while ($previousNode instanceof Node && ! $this->isBreakingScopeNode($node)) {
$isUnreachable = $previousNode->getAttribute(AttributeKey::IS_UNREACHABLE);
if ($isUnreachable === true) {
return true;
}
$previousNode = $previousNode->getAttribute(AttributeKey::PREVIOUS_NODE);
}
return false;
}
/**
* Keep content after markTestSkipped(), intentional temporary
*/
private function isAfterMarkTestSkippedMethodCall(Node $node): bool
{
return (bool) $this->betterNodeFinder->findFirstPrevious($node, function (Node $node): bool {
if (! $node instanceof MethodCall) {
return false;
}
return $this->isName($node->name, 'markTestSkipped');
});
}
/**
* Check nodes that breaks scope while traversing up
*/
private function isBreakingScopeNode(Node $node): bool
{
if ($node instanceof ClassLike) {
return true;
}
if ($node instanceof ClassMethod) {
return true;
}
if ($node instanceof Namespace_) {
return true;
}
return false;
}
}

View File

@ -0,0 +1,29 @@
<?php
namespace Rector\DeadCode\Tests\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
class AfterReturn
{
public function run()
{
return 5;
$removeMe = 10;
}
}
?>
-----
<?php
namespace Rector\DeadCode\Tests\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
class AfterReturn
{
public function run()
{
return 5;
}
}
?>

View File

@ -0,0 +1,58 @@
<?php
namespace Rector\DeadCode\Tests\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
use Exception;
class AfterThrows
{
public function run()
{
throw new Exception();
$removeMe = 10;
$removeMe = 10;
$removeMe = 10;
$removeMe = 10;
$removeMe = 10;
$removeMe = 10;
$removeMe = 10;
$removeMe = 10;
$removeMe = 10;
$removeMe = 10;
}
public function another()
{
$keepMe = 10;
throw new Exception();
$removeMe = 10;
}
}
?>
-----
<?php
namespace Rector\DeadCode\Tests\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
use Exception;
class AfterThrows
{
public function run()
{
throw new Exception();
}
public function another()
{
$keepMe = 10;
throw new Exception();
}
}
?>

View File

@ -0,0 +1,13 @@
<?php
namespace Rector\DeadCode\Tests\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
class KeepComment
{
public function run()
{
throw new Exception();
// keep me
}
}

View File

@ -0,0 +1,18 @@
<?php
namespace Rector\DeadCode\Tests\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
class KeepFalsePositiveAlwaysTrueIf
{
public function run(int $value)
{
$value = 'one';
if (is_int($value)) {
return 5;
}
$removeMe = 10;
return $removeMe;
}
}

View File

@ -0,0 +1,29 @@
<?php declare(strict_types=1);
namespace Rector\DeadCode\Tests\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
final class SkipConditionBefore
{
/**
* @var bool
*/
private $shouldUpdate = false;
public function run()
{
$this->shouldUpdate = false;
$this->someMethod();
if ($this->shouldUpdate === false) {
return 5;
}
$value = 5;
$value2 = 1515;
}
private function someMethod(): void
{
$this->shouldUpdate = true;
}
}

View File

@ -0,0 +1,17 @@
<?php declare(strict_types=1);
namespace Rector\DeadCode\Tests\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
use PHPUnit\Framework\TestCase;
final class SkipMarkedSkippedTestFile extends TestCase
{
public function testMultipleArguments(): void
{
$this->markTestSkipped('Conflicting with previous test() for unknown reason. Works well separately');
$this->assertTrue('...');
$this->assertFalse('...');
$this->assertSame('...', '...');
}
}

View File

@ -0,0 +1,33 @@
<?php declare(strict_types=1);
namespace Rector\DeadCode\Tests\Rector\Stmt\RemoveUnreachableStatementRector;
use Iterator;
use Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
final class RemoveUnreachableStatementRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideDataForTest()
*/
public function test(string $file): void
{
$this->doTestFile($file);
}
public function provideDataForTest(): Iterator
{
yield [__DIR__ . '/Fixture/after_return.php.inc'];
yield [__DIR__ . '/Fixture/after_throws.php.inc'];
yield [__DIR__ . '/Fixture/keep_false_positive_always_true_if.php.inc'];
yield [__DIR__ . '/Fixture/skip_condition_before.php.inc'];
yield [__DIR__ . '/Fixture/skip_marked_skipped_test_file.php.inc'];
yield [__DIR__ . '/Fixture/keep_comment.php.inc'];
}
protected function getRectorClass(): string
{
return RemoveUnreachableStatementRector::class;
}
}

View File

@ -106,4 +106,9 @@ final class AttributeKey
* Use often in php-parser
*/
public const KIND = 'kind';
/**
* @var string
*/
public const IS_UNREACHABLE = 'is_unreachable';
}

View File

@ -11,6 +11,7 @@ use PhpParser\NodeTraverser;
use PHPStan\Analyser\NodeScopeResolver as PHPStanNodeScopeResolver;
use PHPStan\Analyser\Scope;
use PHPStan\Broker\Broker;
use PHPStan\Node\UnreachableStatementNode;
use Rector\Exception\ShouldNotHappenException;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\NodeTypeResolver\PHPStan\Collector\TraitNodeScopeCollector;
@ -87,7 +88,14 @@ final class NodeScopeResolver
return;
}
$node->setAttribute(AttributeKey::SCOPE, $scope);
// special case for unreachable nodes
if ($node instanceof UnreachableStatementNode) {
$originalNode = $node->getOriginalStatement();
$originalNode->setAttribute(AttributeKey::IS_UNREACHABLE, true);
$originalNode->setAttribute(AttributeKey::SCOPE, $scope);
} else {
$node->setAttribute(AttributeKey::SCOPE, $scope);
}
};
$this->phpStanNodeScopeResolver->processNodes($nodes, $scope, $nodeCallback);

View File

@ -3,8 +3,8 @@ parameters:
auto_import_names: true
exclude_paths:
# - "/Fixture/"
# - "/Fixtures/"
- "/Fixture/"
- "/Fixtures/"
- "/Expected/"
- "/Source/"
- "packages/Symfony/src/Bridge/DefaultAnalyzedSymfonyApplicationContainer.php"
@ -16,3 +16,6 @@ parameters:
# so Rector code is still PHP 7.1 compatible
php_version_features: '7.1'
services:
Rector\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector: ~

View File

@ -50,8 +50,8 @@ final class PhpDocClassRenamer
$this->shouldUpdate = false;
$phpDocInfo = $this->phpDocInfoFactory->createFromNode($node);
$this->procesAssertChoiceTagValueNode($oldToNewClasses, $phpDocInfo);
$this->procesDoctrineRelationTagValueNode($oldToNewClasses, $phpDocInfo);
$this->processAssertChoiceTagValueNode($oldToNewClasses, $phpDocInfo);
$this->processDoctrineRelationTagValueNode($oldToNewClasses, $phpDocInfo);
$this->processSerializerTypeTagValueNode($oldToNewClasses, $phpDocInfo);
if ($this->shouldUpdate === false) {
@ -65,7 +65,7 @@ final class PhpDocClassRenamer
/**
* @param string[] $oldToNewClasses
*/
private function procesAssertChoiceTagValueNode(array $oldToNewClasses, PhpDocInfo $phpDocInfo): void
private function processAssertChoiceTagValueNode(array $oldToNewClasses, PhpDocInfo $phpDocInfo): void
{
$choiceTagValueNode = $phpDocInfo->getByType(AssertChoiceTagValueNode::class);
if (! $choiceTagValueNode instanceof AssertChoiceTagValueNode) {
@ -86,7 +86,7 @@ final class PhpDocClassRenamer
/**
* @param string[] $oldToNewClasses
*/
private function procesDoctrineRelationTagValueNode(array $oldToNewClasses, PhpDocInfo $phpDocInfo): void
private function processDoctrineRelationTagValueNode(array $oldToNewClasses, PhpDocInfo $phpDocInfo): void
{
$relationTagValueNode = $phpDocInfo->getByType(DoctrineRelationTagValueNodeInterface::class);
if (! $relationTagValueNode instanceof DoctrineRelationTagValueNodeInterface) {