From dda66052b50dcdb6cc3dabfa96cfde8143c7e6b6 Mon Sep 17 00:00:00 2001
From: Tomas Votruba <tomas.vot@gmail.com>
Date: Sat, 10 Aug 2019 09:28:27 +0200
Subject: [PATCH] decopule NodeRemovingNodeVisitor
---
...eSetterOnlyPropertyAndMethodCallRector.php | 1 +
src/Application/ErrorAndDiffCollector.php | 21 ++-
src/Console/Output/ConsoleOutputFormatter.php | 52 +++++-
.../Node/Commander/NodeRemovingCommander.php | 151 +++++-------------
.../NodeVisitor/NodeRemovingNodeVisitor.php | 88 ++++++++++
.../NodeRemovingNodeVisitorFactory.php | 35 ++++
src/Reporting/RemovedNodesCollector.php | 23 ---
7 files changed, 226 insertions(+), 145 deletions(-)
create mode 100644 src/PhpParser/Node/NodeVisitor/NodeRemovingNodeVisitor.php
create mode 100644 src/PhpParser/Node/NodeVisitorFactory/NodeRemovingNodeVisitorFactory.php
delete mode 100644 src/Reporting/RemovedNodesCollector.php
diff --git a/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php b/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php
index e2f10f99865..d8b31220aa3 100644
--- a/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php
+++ b/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php
@@ -123,6 +123,7 @@ CODE_SAMPLE
$this->methodCallNamesToBeRemoved[] = $classMethodName;
$this->removeNode($node);
+
return;
}
diff --git a/src/Application/ErrorAndDiffCollector.php b/src/Application/ErrorAndDiffCollector.php
index da5acd355e0..525d78efab6 100644
--- a/src/Application/ErrorAndDiffCollector.php
+++ b/src/Application/ErrorAndDiffCollector.php
@@ -2,12 +2,13 @@
namespace Rector\Application;
+use PhpParser\Node;
use PHPStan\AnalysedCodeException;
use Rector\Application\FileSystem\RemovedAndAddedFilesCollector;
use Rector\ConsoleDiffer\DifferAndFormatter;
use Rector\Error\ExceptionCorrector;
+use Rector\PhpParser\Node\Commander\NodeRemovingCommander;
use Rector\Reporting\FileDiff;
-use Rector\Reporting\RemovedNodesCollector;
use Symplify\PackageBuilder\FileSystem\SmartFileInfo;
use Throwable;
@@ -44,22 +45,22 @@ final class ErrorAndDiffCollector
private $removedAndAddedFilesCollector;
/**
- * @var RemovedNodesCollector
+ * @var NodeRemovingCommander
*/
- private $removedNodesCollector;
+ private $nodeRemovingCommander;
public function __construct(
DifferAndFormatter $differAndFormatter,
AppliedRectorCollector $appliedRectorCollector,
ExceptionCorrector $exceptionCorrector,
RemovedAndAddedFilesCollector $removedAndAddedFilesCollector,
- RemovedNodesCollector $removedNodesCollector
+ NodeRemovingCommander $nodeRemovingCommander
) {
$this->differAndFormatter = $differAndFormatter;
$this->appliedRectorCollector = $appliedRectorCollector;
$this->exceptionCorrector = $exceptionCorrector;
$this->removedAndAddedFilesCollector = $removedAndAddedFilesCollector;
- $this->removedNodesCollector = $removedNodesCollector;
+ $this->nodeRemovingCommander = $nodeRemovingCommander;
}
public function addError(Error $error): void
@@ -82,7 +83,15 @@ final class ErrorAndDiffCollector
public function getRemovedNodeCount(): int
{
- return $this->removedNodesCollector->getCount();
+ return $this->nodeRemovingCommander->getCount();
+ }
+
+ /**
+ * @return Node[]
+ */
+ public function getRemovedNodes(): array
+ {
+ return $this->nodeRemovingCommander->getNodesToRemove();
}
public function addFileDiff(SmartFileInfo $smartFileInfo, string $newContent, string $oldContent): void
diff --git a/src/Console/Output/ConsoleOutputFormatter.php b/src/Console/Output/ConsoleOutputFormatter.php
index d7bc211b591..876ea356714 100644
--- a/src/Console/Output/ConsoleOutputFormatter.php
+++ b/src/Console/Output/ConsoleOutputFormatter.php
@@ -2,11 +2,15 @@
namespace Rector\Console\Output;
+use Nette\Utils\Strings;
use Rector\Application\Error;
use Rector\Application\ErrorAndDiffCollector;
use Rector\Contract\Console\Output\OutputFormatterInterface;
+use Rector\NodeTypeResolver\Node\AttributeKey;
+use Rector\PhpParser\Printer\BetterStandardPrinter;
use Rector\Reporting\FileDiff;
use Symfony\Component\Console\Style\SymfonyStyle;
+use Symplify\PackageBuilder\FileSystem\SmartFileInfo;
final class ConsoleOutputFormatter implements OutputFormatterInterface
{
@@ -20,9 +24,15 @@ final class ConsoleOutputFormatter implements OutputFormatterInterface
*/
private $symfonyStyle;
- public function __construct(SymfonyStyle $symfonyStyle)
+ /**
+ * @var BetterStandardPrinter
+ */
+ private $betterStandardPrinter;
+
+ public function __construct(SymfonyStyle $symfonyStyle, BetterStandardPrinter $betterStandardPrinter)
{
$this->symfonyStyle = $symfonyStyle;
+ $this->betterStandardPrinter = $betterStandardPrinter;
}
public function report(ErrorAndDiffCollector $errorAndDiffCollector): void
@@ -110,8 +120,44 @@ final class ConsoleOutputFormatter implements OutputFormatterInterface
);
}
- if ($errorAndDiffCollector->getRemovedNodeCount()) {
- $this->symfonyStyle->note(sprintf('%d nodes were removed', $errorAndDiffCollector->getRemovedNodeCount()));
+ $this->reportRemovedNodes($errorAndDiffCollector);
+ }
+
+ private function reportRemovedNodes(ErrorAndDiffCollector $errorAndDiffCollector): void
+ {
+ if ($errorAndDiffCollector->getRemovedNodeCount() === 0) {
+ return;
+ }
+
+ $this->symfonyStyle->warning(sprintf('%d nodes were removed', $errorAndDiffCollector->getRemovedNodeCount()));
+
+ if ($this->symfonyStyle->isVeryVerbose()) {
+ $i = 0;
+ foreach ($errorAndDiffCollector->getRemovedNodes() as $removedNode) {
+ /** @var SmartFileInfo $fileInfo */
+ $fileInfo = $removedNode->getAttribute(AttributeKey::FILE_INFO);
+
+ $this->symfonyStyle->writeln(sprintf(
+ '<options=bold>%d) %s:%d</>',
+ ++$i,
+ $fileInfo->getRelativeFilePath(),
+ $removedNode->getStartLine()
+ ));
+
+ $printedNode = $this->betterStandardPrinter->print($removedNode);
+
+ // color red + prefix with "-" to visually demonstrate removal
+ $printedNode = '-' . Strings::replace($printedNode, '#\n#', "\n-");
+ $printedNode = $this->colorTextToRed($printedNode);
+
+ $this->symfonyStyle->writeln($printedNode);
+ $this->symfonyStyle->newLine(1);
+ }
}
}
+
+ private function colorTextToRed(string $text): string
+ {
+ return '<fg=red>' . $text . '</fg=red>';
+ }
}
diff --git a/src/PhpParser/Node/Commander/NodeRemovingCommander.php b/src/PhpParser/Node/Commander/NodeRemovingCommander.php
index cf871fd2a26..6ae2864237a 100644
--- a/src/PhpParser/Node/Commander/NodeRemovingCommander.php
+++ b/src/PhpParser/Node/Commander/NodeRemovingCommander.php
@@ -3,19 +3,14 @@
namespace Rector\PhpParser\Node\Commander;
use PhpParser\Node;
-use PhpParser\Node\Expr;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Expression;
use PhpParser\NodeTraverser;
-use PhpParser\NodeVisitor;
-use PhpParser\NodeVisitorAbstract;
use Rector\Contract\PhpParser\Node\CommanderInterface;
use Rector\Exception\ShouldNotHappenException;
use Rector\NodeTypeResolver\Node\AttributeKey;
-use Rector\PhpParser\Node\NodeFactory;
-use Rector\PhpParser\Node\Resolver\NameResolver;
-use Rector\Reporting\RemovedNodesCollector;
+use Rector\PhpParser\Node\NodeVisitorFactory\NodeRemovingNodeVisitorFactory;
final class NodeRemovingCommander implements CommanderInterface
{
@@ -25,46 +20,24 @@ final class NodeRemovingCommander implements CommanderInterface
private $nodesToRemove = [];
/**
- * @var NodeFactory
+ * @var NodeRemovingNodeVisitorFactory
*/
- private $nodeFactory;
+ private $nodeRemovingNodeVisitorFactory;
- /**
- * @var NameResolver
- */
- private $nameResolver;
-
- /**
- * @var RemovedNodesCollector
- */
- private $removedNodesCollector;
-
- public function __construct(
- NodeFactory $nodeFactory,
- NameResolver $nameResolver,
- RemovedNodesCollector $removedNodesCollector
- ) {
- $this->nodeFactory = $nodeFactory;
- $this->nameResolver = $nameResolver;
- $this->removedNodesCollector = $removedNodesCollector;
+ public function __construct(NodeRemovingNodeVisitorFactory $nodeRemovingNodeVisitorFactory)
+ {
+ $this->nodeRemovingNodeVisitorFactory = $nodeRemovingNodeVisitorFactory;
}
public function addNode(Node $node): void
{
- $this->removedNodesCollector->collect($node);
-
// chain call: "->method()->another()"
- if ($node instanceof MethodCall && $node->var instanceof MethodCall) {
- throw new ShouldNotHappenException(
- 'Chain method calls cannot be removed this way. It would remove the whole tree of calls. Remove them manually by creating new parent node with no following method.'
- );
- }
+ $this->ensureIsNotPartOfChainMethodCall($node);
- if (! $node instanceof Expression && ($node->getAttribute(
- AttributeKey::PARENT_NODE
- ) instanceof Expression)) {
+ $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
+ if (! $node instanceof Expression && $parentNode instanceof Expression) {
// only expressions can be removed
- $node = $node->getAttribute(AttributeKey::PARENT_NODE);
+ $node = $parentNode;
}
/** @var Stmt $node */
@@ -78,96 +51,48 @@ final class NodeRemovingCommander implements CommanderInterface
public function traverseNodes(array $nodes): array
{
$nodeTraverser = new NodeTraverser();
- $nodeTraverser->addVisitor($this->createNodeVisitor());
+
+ $nodeRemovingNodeVisitor = $this->nodeRemovingNodeVisitorFactory->createFromNodesToRemove($this->nodesToRemove);
+ $nodeTraverser->addVisitor($nodeRemovingNodeVisitor);
return $nodeTraverser->traverse($nodes);
}
- public function isActive(): bool
- {
- return count($this->nodesToRemove) > 0;
- }
-
public function isNodeRemoved(Node $node): bool
{
return in_array($node, $this->nodesToRemove, true);
}
- private function createNodeVisitor(): NodeVisitor
+ public function isActive(): bool
{
- return new class($this->nodesToRemove, $this->nodeFactory, $this->nameResolver) extends NodeVisitorAbstract {
- /**
- * @var Stmt[]|Expr[]
- */
- private $nodesToRemove = [];
+ return $this->getCount() > 0;
+ }
- /**
- * @var NodeFactory
- */
- private $nodeFactory;
+ public function getCount(): int
+ {
+ return count($this->nodesToRemove);
+ }
- /**
- * @var NameResolver
- */
- private $nameResolver;
+ /**
+ * @return Node[]
+ */
+ public function getNodesToRemove(): array
+ {
+ return $this->nodesToRemove;
+ }
- /**
- * @param Stmt[] $nodesToRemove
- */
- public function __construct(array $nodesToRemove, NodeFactory $nodeFactory, NameResolver $nameResolver)
- {
- $this->nodesToRemove = $nodesToRemove;
- $this->nodeFactory = $nodeFactory;
- $this->nameResolver = $nameResolver;
- }
+ private function ensureIsNotPartOfChainMethodCall(Node $node): void
+ {
+ if (! $node instanceof MethodCall) {
+ return;
+ }
- /**
- * @return int|Node|null
- */
- public function enterNode(Node $node)
- {
- // special case for fluent methods
- foreach ($this->nodesToRemove as $key => $nodeToRemove) {
- if (! $nodeToRemove instanceof MethodCall) {
- continue;
- }
+ if (! $node->var instanceof MethodCall) {
+ return;
+ }
- if (! $node instanceof MethodCall || ! $node->var instanceof MethodCall) {
- continue;
- }
-
- if ($nodeToRemove !== $node->var) {
- continue;
- }
-
- $methodName = $this->nameResolver->getName($node->name);
- if ($methodName === null) {
- continue;
- }
-
- unset($this->nodesToRemove[$key]);
-
- return $this->nodeFactory->createMethodCall($node->var->var, $methodName, $node->args);
- }
-
- return null;
- }
-
- /**
- * @return int|Node|Node[]|null
- */
- public function leaveNode(Node $node)
- {
- foreach ($this->nodesToRemove as $key => $nodeToRemove) {
- if ($node === $nodeToRemove) {
- unset($this->nodesToRemove[$key]);
-
- return NodeTraverser::REMOVE_NODE;
- }
- }
-
- return $node;
- }
- };
+ throw new ShouldNotHappenException(
+ 'Chain method calls cannot be removed this way. It would remove the whole tree of calls. Remove them manually by creating new parent node with no following method.'
+ );
}
}
diff --git a/src/PhpParser/Node/NodeVisitor/NodeRemovingNodeVisitor.php b/src/PhpParser/Node/NodeVisitor/NodeRemovingNodeVisitor.php
new file mode 100644
index 00000000000..561464dee4e
--- /dev/null
+++ b/src/PhpParser/Node/NodeVisitor/NodeRemovingNodeVisitor.php
@@ -0,0 +1,88 @@
+<?php declare(strict_types=1);
+
+namespace Rector\PhpParser\Node\NodeVisitor;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Stmt;
+use PhpParser\NodeTraverser;
+use PhpParser\NodeVisitorAbstract;
+use Rector\PhpParser\Node\NodeFactory;
+use Rector\PhpParser\Node\Resolver\NameResolver;
+
+final class NodeRemovingNodeVisitor extends NodeVisitorAbstract
+{
+ /**
+ * @var Stmt[]|Expr[]
+ */
+ private $nodesToRemove = [];
+
+ /**
+ * @var NodeFactory
+ */
+ private $nodeFactory;
+
+ /**
+ * @var NameResolver
+ */
+ private $nameResolver;
+
+ /**
+ * @param Stmt[] $nodesToRemove
+ */
+ public function __construct(array $nodesToRemove, NodeFactory $nodeFactory, NameResolver $nameResolver)
+ {
+ $this->nodesToRemove = $nodesToRemove;
+ $this->nodeFactory = $nodeFactory;
+ $this->nameResolver = $nameResolver;
+ }
+
+ /**
+ * @return int|Node|null
+ */
+ public function enterNode(Node $node)
+ {
+ // special case for fluent methods
+ foreach ($this->nodesToRemove as $key => $nodeToRemove) {
+ if (! $nodeToRemove instanceof MethodCall) {
+ continue;
+ }
+
+ if (! $node instanceof MethodCall || ! $node->var instanceof MethodCall) {
+ continue;
+ }
+
+ if ($nodeToRemove !== $node->var) {
+ continue;
+ }
+
+ $methodName = $this->nameResolver->getName($node->name);
+ if ($methodName === null) {
+ continue;
+ }
+
+ unset($this->nodesToRemove[$key]);
+
+ return $this->nodeFactory->createMethodCall($node->var->var, $methodName, $node->args);
+ }
+
+ return null;
+ }
+
+ /**
+ * @return int|Node|Node[]|null
+ */
+ public function leaveNode(Node $node)
+ {
+ foreach ($this->nodesToRemove as $key => $nodeToRemove) {
+ if ($node === $nodeToRemove) {
+ unset($this->nodesToRemove[$key]);
+
+ return NodeTraverser::REMOVE_NODE;
+ }
+ }
+
+ return $node;
+ }
+}
diff --git a/src/PhpParser/Node/NodeVisitorFactory/NodeRemovingNodeVisitorFactory.php b/src/PhpParser/Node/NodeVisitorFactory/NodeRemovingNodeVisitorFactory.php
new file mode 100644
index 00000000000..8114a4ce837
--- /dev/null
+++ b/src/PhpParser/Node/NodeVisitorFactory/NodeRemovingNodeVisitorFactory.php
@@ -0,0 +1,35 @@
+<?php declare(strict_types=1);
+
+namespace Rector\PhpParser\Node\NodeVisitorFactory;
+
+use PhpParser\Node;
+use Rector\PhpParser\Node\NodeFactory;
+use Rector\PhpParser\Node\NodeVisitor\NodeRemovingNodeVisitor;
+use Rector\PhpParser\Node\Resolver\NameResolver;
+
+final class NodeRemovingNodeVisitorFactory
+{
+ /**
+ * @var NodeFactory
+ */
+ private $nodeFactory;
+
+ /**
+ * @var NameResolver
+ */
+ private $nameResolver;
+
+ public function __construct(NodeFactory $nodeFactory, NameResolver $nameResolver)
+ {
+ $this->nodeFactory = $nodeFactory;
+ $this->nameResolver = $nameResolver;
+ }
+
+ /**
+ * @param Node[] $nodesToRemove
+ */
+ public function createFromNodesToRemove(array $nodesToRemove): NodeRemovingNodeVisitor
+ {
+ return new NodeRemovingNodeVisitor($nodesToRemove, $this->nodeFactory, $this->nameResolver);
+ }
+}
diff --git a/src/Reporting/RemovedNodesCollector.php b/src/Reporting/RemovedNodesCollector.php
deleted file mode 100644
index ad128691347..00000000000
--- a/src/Reporting/RemovedNodesCollector.php
+++ /dev/null
@@ -1,23 +0,0 @@
-<?php declare(strict_types=1);
-
-namespace Rector\Reporting;
-
-use PhpParser\Node;
-
-final class RemovedNodesCollector
-{
- /**
- * @var Node[]
- */
- private $removedNodes = [];
-
- public function collect(Node $node): void
- {
- $this->removedNodes[] = $node;
- }
-
- public function getCount(): int
- {
- return count($this->removedNodes);
- }
-}