Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .phpstorm.meta.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
\Rector\NodeTypeResolver\Node\AttributeKey::PARENT_CLASS_NAME,
\Rector\NodeTypeResolver\Node\AttributeKey::NEXT_NODE,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_NODE,
\Rector\NodeTypeResolver\Node\AttributeKey::CURRENT_EXPRESSION,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_EXPRESSION,
\Rector\NodeTypeResolver\Node\AttributeKey::CURRENT_STATEMENT,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_STATEMENT,
\Rector\NodeTypeResolver\Node\AttributeKey::USE_NODES,
\Rector\NodeTypeResolver\Node\AttributeKey::START_TOKEN_POSITION,
\Rector\NodeTypeResolver\Node\AttributeKey::ORIGINAL_NODE,
Expand All @@ -55,8 +55,8 @@
\Rector\NodeTypeResolver\Node\AttributeKey::PARENT_CLASS_NAME,
\Rector\NodeTypeResolver\Node\AttributeKey::NEXT_NODE,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_NODE,
\Rector\NodeTypeResolver\Node\AttributeKey::CURRENT_EXPRESSION,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_EXPRESSION,
\Rector\NodeTypeResolver\Node\AttributeKey::CURRENT_STATEMENT,
\Rector\NodeTypeResolver\Node\AttributeKey::PREVIOUS_STATEMENT,
\Rector\NodeTypeResolver\Node\AttributeKey::USE_NODES,
\Rector\NodeTypeResolver\Node\AttributeKey::START_TOKEN_POSITION,
\Rector\NodeTypeResolver\Node\AttributeKey::ORIGINAL_NODE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private function shouldSkip(Return_ $returnNode): bool
*/
private function isPreviousExpressionVisuallySimilar(Expression $previousExpression, Node $previousNode): bool
{
$prePreviousExpression = $previousExpression->getAttribute(AttributeKey::PREVIOUS_EXPRESSION);
$prePreviousExpression = $previousExpression->getAttribute(AttributeKey::PREVIOUS_STATEMENT);
if ($prePreviousExpression instanceof Expression && $prePreviousExpression->expr instanceof AssignOp) {
if ($this->areNodesEqual($prePreviousExpression->expr->var, $previousNode->var)) {
return true;
Expand Down
11 changes: 6 additions & 5 deletions packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use PhpParser\Node\Stmt\Do_;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\ElseIf_;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Foreach_;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\While_;
Expand Down Expand Up @@ -75,24 +76,24 @@ public function refactor(Node $node): ?Node
return null;
}

$previousExpression = $node->getAttribute(AttributeKey::PREVIOUS_EXPRESSION);
if ($previousExpression === null) {
$previousStatement = $node->getAttribute(AttributeKey::PREVIOUS_STATEMENT);
if (! $previousStatement instanceof Expression) {
return null;
}

if (! $previousExpression->expr instanceof Assign) {
if (! $previousStatement->expr instanceof Assign) {
return null;
}

if (! $this->areNodesEqual($previousExpression->expr, $node)) {
if (! $this->areNodesEqual($previousStatement->expr, $node)) {
return null;
}

if ($node->expr instanceof FuncCall || $node->expr instanceof StaticCall || $node->expr instanceof MethodCall) {
return null;
}

if ($this->shouldSkipForDifferentScope($node, $previousExpression)) {
if ($this->shouldSkipForDifferentScope($node, $previousStatement)) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,19 +72,23 @@ public function refactor(Node $node): ?Node
return null;
}

if (! $this->isUnreachable($node)) {
return null;
// might be PHPStan false positive, better skip
$previousStatement = $node->getAttribute(AttributeKey::PREVIOUS_STATEMENT);
if ($previousStatement instanceof If_) {
$node->setAttribute(
AttributeKey::IS_UNREACHABLE,
$previousStatement->getAttribute(AttributeKey::IS_UNREACHABLE)
);
}

// 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 ($previousStatement instanceof While_) {
$node->setAttribute(
AttributeKey::IS_UNREACHABLE,
$previousStatement->getAttribute(AttributeKey::IS_UNREACHABLE)
);
}

if ($previousNode instanceof While_) {
$node->setAttribute(AttributeKey::IS_UNREACHABLE, false);
if (! $this->isUnreachable($node)) {
return null;
}

Expand All @@ -106,15 +110,15 @@ private function isUnreachable(Node $node): bool
}

// traverse up for unreachable node in the same scope
$previousNode = $node->getAttribute(AttributeKey::PREVIOUS_NODE);
$previousNode = $node->getAttribute(AttributeKey::PREVIOUS_STATEMENT);

while ($previousNode instanceof Node && ! $this->isBreakingScopeNode($node)) {
while ($previousNode instanceof Node && ! $this->isBreakingScopeNode($previousNode)) {
$isUnreachable = $previousNode->getAttribute(AttributeKey::IS_UNREACHABLE);
if ($isUnreachable === true) {
return true;
}

$previousNode = $previousNode->getAttribute(AttributeKey::PREVIOUS_NODE);
$previousNode = $previousNode->getAttribute(AttributeKey::PREVIOUS_STATEMENT);
}

return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

namespace Rector\DeadCode\Tests\Rector\Stmt\RemoveUnreachableStatementRector\KeepFalsePositiveWhile;

class SomeFilter
{
public function filter($in, $out, &$consumed, $closing)
{
while ($res = stream_bucket_make_writeable($in)) {
stream_bucket_append($out, $res);
$consumed += $res->datalen;
}

return PSFS_PASS_ON;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public function provideDataForTest(): Iterator
yield [__DIR__ . '/Fixture/skip_marked_skipped_test_file.php.inc'];
yield [__DIR__ . '/Fixture/keep_comment.php.inc'];
yield [__DIR__ . '/Fixture/if_else.php.inc'];
yield [__DIR__ . '/Fixture/keep_false_positive_while.php.inc'];
}

protected function getRectorClass(): string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,12 @@ private function processMysqlFetchField(Assign $assign, FuncCall $funcCall): Ass
'stmts' => [new Expression($funcCall)],
]);

$previousExpression = $assign->getAttribute(AttributeKey::PREVIOUS_EXPRESSION);
if ($previousExpression === null) {
$previousStatement = $assign->getAttribute(AttributeKey::PREVIOUS_STATEMENT);
if ($previousStatement === null) {
throw new ShouldNotHappenException();
}

$this->addNodeAfterNode($forNode, $previousExpression);
$this->addNodeAfterNode($forNode, $previousStatement);

return $assign;
}
Expand Down
14 changes: 12 additions & 2 deletions packages/NodeTypeResolver/src/Node/AttributeKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,12 @@ final class AttributeKey
/**
* @var string
*/
public const PREVIOUS_EXPRESSION = 'previousExpression';
public const PREVIOUS_STATEMENT = 'previousExpression';

/**
* @var string
*/
public const CURRENT_EXPRESSION = 'currentExpression';
public const CURRENT_STATEMENT = 'currentExpression';

/**
* @var string
Expand All @@ -113,4 +113,14 @@ final class AttributeKey
* @var string
*/
public const IS_UNREACHABLE = 'is_unreachable';

/**
* @var string
*/
public const FUNCTION_NAME = 'function_name';

/**
* @var string
*/
public const FUNCTION_NODE = 'function_node';
}
28 changes: 14 additions & 14 deletions packages/NodeTypeResolver/src/NodeScopeAndMetadataDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
use PhpParser\NodeVisitor\CloningVisitor;
use PhpParser\NodeVisitor\NameResolver;
use Rector\Configuration\Configuration;
use Rector\NodeTypeResolver\NodeVisitor\ClassAndMethodNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\ExpressionNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\FileInfoNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\FunctionMethodAndClassNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\NamespaceNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\NodeCollectorNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\ParentAndNextNodeVisitor;
use Rector\NodeTypeResolver\NodeVisitor\StatementNodeVisitor;
use Rector\NodeTypeResolver\PHPStan\Scope\NodeScopeResolver;

final class NodeScopeAndMetadataDecorator
Expand All @@ -35,19 +35,19 @@ final class NodeScopeAndMetadataDecorator
private $parentAndNextNodeVisitor;

/**
* @var ClassAndMethodNodeVisitor
* @var FunctionMethodAndClassNodeVisitor
*/
private $classAndMethodNodeVisitor;
private $functionMethodAndClassNodeVisitor;

/**
* @var NamespaceNodeVisitor
*/
private $namespaceNodeVisitor;

/**
* @var ExpressionNodeVisitor
* @var StatementNodeVisitor
*/
private $expressionNodeVisitor;
private $statementNodeVisitor;

/**
* @var FileInfoNodeVisitor
Expand All @@ -68,19 +68,19 @@ public function __construct(
NodeScopeResolver $nodeScopeResolver,
ParentAndNextNodeVisitor $parentAndNextNodeVisitor,
CloningVisitor $cloningVisitor,
ClassAndMethodNodeVisitor $classAndMethodNodeVisitor,
FunctionMethodAndClassNodeVisitor $functionMethodAndClassNodeVisitor,
NamespaceNodeVisitor $namespaceNodeVisitor,
ExpressionNodeVisitor $expressionNodeVisitor,
StatementNodeVisitor $statementNodeVisitor,
FileInfoNodeVisitor $fileInfoNodeVisitor,
NodeCollectorNodeVisitor $nodeCollectorNodeVisitor,
Configuration $configuration
) {
$this->nodeScopeResolver = $nodeScopeResolver;
$this->parentAndNextNodeVisitor = $parentAndNextNodeVisitor;
$this->cloningVisitor = $cloningVisitor;
$this->classAndMethodNodeVisitor = $classAndMethodNodeVisitor;
$this->functionMethodAndClassNodeVisitor = $functionMethodAndClassNodeVisitor;
$this->namespaceNodeVisitor = $namespaceNodeVisitor;
$this->expressionNodeVisitor = $expressionNodeVisitor;
$this->statementNodeVisitor = $statementNodeVisitor;
$this->fileInfoNodeVisitor = $fileInfoNodeVisitor;
$this->nodeCollectorNodeVisitor = $nodeCollectorNodeVisitor;
$this->configuration = $configuration;
Expand Down Expand Up @@ -115,14 +115,14 @@ public function decorateNodesFromFile(array $nodes, string $filePath, bool $need
$nodeTraverser = new NodeTraverser();
$nodeTraverser->addVisitor($this->cloningVisitor); // needed also for format preserving printing
$nodeTraverser->addVisitor($this->parentAndNextNodeVisitor);
$nodeTraverser->addVisitor($this->classAndMethodNodeVisitor);
$nodeTraverser->addVisitor($this->functionMethodAndClassNodeVisitor);
$nodeTraverser->addVisitor($this->namespaceNodeVisitor);

$nodes = $nodeTraverser->traverse($nodes);

// this split is needed, so nodes have names, classes and namespaces
$nodeTraverser = new NodeTraverser();
$nodeTraverser->addVisitor($this->expressionNodeVisitor);
$nodeTraverser->addVisitor($this->statementNodeVisitor);
$nodeTraverser->addVisitor($this->fileInfoNodeVisitor);
$nodeTraverser->addVisitor($this->nodeCollectorNodeVisitor);

Expand All @@ -137,8 +137,8 @@ public function decorateNodesFromString(array $nodes): array
{
$nodeTraverser = new NodeTraverser();
$nodeTraverser->addVisitor($this->parentAndNextNodeVisitor);
$nodeTraverser->addVisitor($this->classAndMethodNodeVisitor);
$nodeTraverser->addVisitor($this->expressionNodeVisitor);
$nodeTraverser->addVisitor($this->functionMethodAndClassNodeVisitor);
$nodeTraverser->addVisitor($this->statementNodeVisitor);

return $nodeTraverser->traverse($nodes);
}
Expand Down
Loading