From 1593d006138230202e20bd209da375ca1e171876 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 10 Sep 2023 22:24:21 +0200 Subject: [PATCH] [Internals] Decouple output from AbstractRector, narrow debugging output to file path (#4976) --- .../PHPStanServicesFactory.php | 4 +- .../PHPUnit/AbstractRectorTestCase.php | 4 +- src/Application/ApplicationFileProcessor.php | 16 ++++- src/Application/FileProcessor.php | 12 ---- src/Configuration/ConfigurationFactory.php | 4 +- src/Console/Command/WorkerCommand.php | 32 +++++++-- .../LazyContainerFactory.php | 2 - src/Logging/RectorOutput.php | 65 ------------------- src/Rector/AbstractRector.php | 19 ------ src/ValueObject/Configuration.php | 8 ++- 10 files changed, 55 insertions(+), 111 deletions(-) delete mode 100644 src/Logging/RectorOutput.php diff --git a/packages/NodeTypeResolver/DependencyInjection/PHPStanServicesFactory.php b/packages/NodeTypeResolver/DependencyInjection/PHPStanServicesFactory.php index bc15a82a120..d5af83da3c3 100644 --- a/packages/NodeTypeResolver/DependencyInjection/PHPStanServicesFactory.php +++ b/packages/NodeTypeResolver/DependencyInjection/PHPStanServicesFactory.php @@ -27,8 +27,8 @@ final class PHPStanServicesFactory { private readonly Container $container; - public function __construct( - ) { + public function __construct() + { $containerFactory = new ContainerFactory(getcwd()); $additionalConfigFiles = $this->resolveAdditionalConfigFiles(); diff --git a/packages/Testing/PHPUnit/AbstractRectorTestCase.php b/packages/Testing/PHPUnit/AbstractRectorTestCase.php index 74a474a5e21..f24fbbacc08 100644 --- a/packages/Testing/PHPUnit/AbstractRectorTestCase.php +++ b/packages/Testing/PHPUnit/AbstractRectorTestCase.php @@ -256,8 +256,8 @@ private function processFilePath(string $filePath): string $this->applicationFileProcessor->processFiles([$filePath], $configuration); - return $this->currentFileProvider->getFile() - ->getFileContent(); + $currentFile = $this->currentFileProvider->getFile(); + return $currentFile->getFileContent(); } private function createInputFilePath(string $fixtureFilePath): string diff --git a/src/Application/ApplicationFileProcessor.php b/src/Application/ApplicationFileProcessor.php index 91356f67370..71c587fe068 100644 --- a/src/Application/ApplicationFileProcessor.php +++ b/src/Application/ApplicationFileProcessor.php @@ -80,10 +80,18 @@ public function run(Configuration $configuration, InputInterface $input): Proces }; } + if ($configuration->isDebug()) { + $preFileCallback = function (string $filePath): void { + $this->symfonyStyle->writeln('[file] ' . $filePath); + }; + } else { + $preFileCallback = null; + } + if ($configuration->isParallel()) { $processResult = $this->runParallel($filePaths, $input, $postFileCallback); } else { - $processResult = $this->processFiles($filePaths, $configuration, $postFileCallback); + $processResult = $this->processFiles($filePaths, $configuration, $preFileCallback, $postFileCallback); } $processResult->addSystemErrors($this->systemErrors); @@ -95,11 +103,13 @@ public function run(Configuration $configuration, InputInterface $input): Proces /** * @param string[] $filePaths + * @param callable(string $file): void|null $preFileCallback * @param callable(int $fileCount): void|null $postFileCallback */ public function processFiles( array $filePaths, Configuration $configuration, + ?callable $preFileCallback = null, ?callable $postFileCallback = null ): ProcessResult { /** @var SystemError[] $systemErrors */ @@ -112,6 +122,10 @@ public function processFiles( $collectedData = []; foreach ($filePaths as $filePath) { + if ($preFileCallback !== null) { + $preFileCallback($filePath); + } + $file = new File($filePath, UtilsFileSystem::read($filePath)); try { diff --git a/src/Application/FileProcessor.php b/src/Application/FileProcessor.php index 7875e29ef46..6ff4ac9a729 100644 --- a/src/Application/FileProcessor.php +++ b/src/Application/FileProcessor.php @@ -106,8 +106,6 @@ public function processFile(File $file, Configuration $configuration): FileProce private function parseFileAndDecorateNodes(File $file): ?SystemError { - $this->notifyFile($file); - try { $this->parseFileNodes($file); } catch (ShouldNotHappenException $shouldNotHappenException) { @@ -175,16 +173,6 @@ private function printFile(File $file, Configuration $configuration): void $file->changeFileContent($newContent); } - private function notifyFile(File $file): void - { - if (! $this->symfonyStyle->isVerbose()) { - return; - } - - $relativeFilePath = $this->filePathHelper->relativePath($file->getFilePath()); - $this->symfonyStyle->writeln($relativeFilePath); - } - private function parseFileNodes(File $file): void { // store tokens by absolute path, so we don't have to print them right now diff --git a/src/Configuration/ConfigurationFactory.php b/src/Configuration/ConfigurationFactory.php index 2a1eca958bc..1db594e79aa 100644 --- a/src/Configuration/ConfigurationFactory.php +++ b/src/Configuration/ConfigurationFactory.php @@ -48,6 +48,7 @@ public function createFromInput(InputInterface $input): Configuration $isParallel = SimpleParameterProvider::provideBoolParameter(Option::PARALLEL); $parallelPort = (string) $input->getOption(Option::PARALLEL_PORT); $parallelIdentifier = (string) $input->getOption(Option::PARALLEL_IDENTIFIER); + $isDebug = (bool) $input->getOption(Option::DEBUG); $memoryLimit = $this->resolveMemoryLimit($input); @@ -62,7 +63,8 @@ public function createFromInput(InputInterface $input): Configuration $parallelPort, $parallelIdentifier, $isParallel, - $memoryLimit + $memoryLimit, + $isDebug ); } diff --git a/src/Console/Command/WorkerCommand.php b/src/Console/Command/WorkerCommand.php index ce19396c27e..c0b5b6d8d25 100644 --- a/src/Console/Command/WorkerCommand.php +++ b/src/Console/Command/WorkerCommand.php @@ -69,7 +69,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int $tcpConnector = new TcpConnector($streamSelectLoop); $promise = $tcpConnector->connect('127.0.0.1:' . $configuration->getParallelPort()); - $promise->then(function (ConnectionInterface $connection) use ($parallelIdentifier, $configuration): void { + $promise->then(function (ConnectionInterface $connection) use ( + $parallelIdentifier, + $configuration, + $output + ): void { $inDecoder = new Decoder($connection, true, 512, JSON_INVALID_UTF8_IGNORE); $outEncoder = new Encoder($connection, JSON_INVALID_UTF8_IGNORE); @@ -78,7 +82,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int ReactCommand::IDENTIFIER => $parallelIdentifier, ]); - $this->runWorker($outEncoder, $inDecoder, $configuration); + $this->runWorker($outEncoder, $inDecoder, $configuration, $output); }); $streamSelectLoop->run(); @@ -86,10 +90,22 @@ protected function execute(InputInterface $input, OutputInterface $output): int return self::SUCCESS; } - private function runWorker(Encoder $encoder, Decoder $decoder, Configuration $configuration): void - { + private function runWorker( + Encoder $encoder, + Decoder $decoder, + Configuration $configuration, + OutputInterface $output + ): void { $this->dynamicSourceLocatorDecorator->addPaths($configuration->getPaths()); + if ($configuration->isDebug()) { + $preFileCallback = static function (string $filePath) use ($output): void { + $output->writeln($filePath); + }; + } else { + $preFileCallback = null; + } + // 1. handle system error $handleErrorCallback = static function (Throwable $throwable) use ($encoder): void { $systemError = new SystemError($throwable->getMessage(), $throwable->getFile(), $throwable->getLine()); @@ -108,7 +124,7 @@ private function runWorker(Encoder $encoder, Decoder $decoder, Configuration $co $encoder->on(ReactEvent::ERROR, $handleErrorCallback); // 2. collect diffs + errors from file processor - $decoder->on(ReactEvent::DATA, function (array $json) use ($encoder, $configuration): void { + $decoder->on(ReactEvent::DATA, function (array $json) use ($preFileCallback, $encoder, $configuration): void { $action = $json[ReactCommand::ACTION]; if ($action !== Action::MAIN) { return; @@ -117,7 +133,11 @@ private function runWorker(Encoder $encoder, Decoder $decoder, Configuration $co /** @var string[] $filePaths */ $filePaths = $json[Bridge::FILES] ?? []; - $processResult = $this->applicationFileProcessor->processFiles($filePaths, $configuration); + $processResult = $this->applicationFileProcessor->processFiles( + $filePaths, + $configuration, + $preFileCallback + ); /** * this invokes all listeners listening $decoder->on(...) @see \Symplify\EasyParallel\Enum\ReactEvent::DATA diff --git a/src/DependencyInjection/LazyContainerFactory.php b/src/DependencyInjection/LazyContainerFactory.php index f79897ccabb..178f9a46577 100644 --- a/src/DependencyInjection/LazyContainerFactory.php +++ b/src/DependencyInjection/LazyContainerFactory.php @@ -61,7 +61,6 @@ use Rector\Core\Contract\DependencyInjection\ResetableInterface; use Rector\Core\Contract\Rector\RectorInterface; use Rector\Core\Logging\CurrentRectorProvider; -use Rector\Core\Logging\RectorOutput; use Rector\Core\NodeDecorator\CreatedByRuleDecorator; use Rector\Core\PhpParser\Comparing\NodeComparator; use Rector\Core\PhpParser\Node\BetterNodeFinder; @@ -515,7 +514,6 @@ static function (AbstractRector $rector, Container $container): void { $container->make(CurrentFileProvider::class), $container->make(CreatedByRuleDecorator::class), $container->make(ChangedNodeScopeRefresher::class), - $container->make(RectorOutput::class), ); } ); diff --git a/src/Logging/RectorOutput.php b/src/Logging/RectorOutput.php deleted file mode 100644 index eefb5167287..00000000000 --- a/src/Logging/RectorOutput.php +++ /dev/null @@ -1,65 +0,0 @@ -symfonyStyle->isDebug(); - } - - /** - * @param class-string $rectorClass - */ - public function printCurrentFileAndRule(string $filePath, string $rectorClass): void - { - $relativeFilePath = $this->filePathHelper->relativePath($filePath); - - $this->symfonyStyle->writeln('[file] ' . $relativeFilePath); - $this->symfonyStyle->writeln('[rule] ' . $rectorClass); - } - - public function startConsumptions(): void - { - $this->startTime = microtime(true); - $this->previousMemory = memory_get_peak_usage(true); - } - - public function printConsumptions(): void - { - if ($this->startTime === null || $this->previousMemory === null) { - return; - } - - $elapsedTime = microtime(true) - $this->startTime; - $currentTotalMemory = memory_get_peak_usage(true); - - $previousMemory = $this->previousMemory; - $consumed = sprintf( - '--- consumed %s, total %s, took %.2f s', - BytesHelper::bytes($currentTotalMemory - $previousMemory), - BytesHelper::bytes($currentTotalMemory), - $elapsedTime - ); - $this->symfonyStyle->writeln($consumed); - $this->symfonyStyle->newLine(1); - } -} diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 8052a249b9a..7e384e6f42e 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -19,7 +19,6 @@ use Rector\Core\Contract\Rector\RectorInterface; use Rector\Core\Exception\ShouldNotHappenException; use Rector\Core\Logging\CurrentRectorProvider; -use Rector\Core\Logging\RectorOutput; use Rector\Core\NodeDecorator\CreatedByRuleDecorator; use Rector\Core\PhpParser\Comparing\NodeComparator; use Rector\Core\PhpParser\Node\BetterNodeFinder; @@ -88,8 +87,6 @@ abstract class AbstractRector extends NodeVisitorAbstract implements RectorInter private CreatedByRuleDecorator $createdByRuleDecorator; - private RectorOutput $rectorOutput; - private ?int $toBeRemovedNodeId = null; public function autowire( @@ -108,7 +105,6 @@ public function autowire( CurrentFileProvider $currentFileProvider, CreatedByRuleDecorator $createdByRuleDecorator, ChangedNodeScopeRefresher $changedNodeScopeRefresher, - RectorOutput $rectorOutput ): void { $this->nodeNameResolver = $nodeNameResolver; $this->nodeTypeResolver = $nodeTypeResolver; @@ -125,7 +121,6 @@ public function autowire( $this->currentFileProvider = $currentFileProvider; $this->createdByRuleDecorator = $createdByRuleDecorator; $this->changedNodeScopeRefresher = $changedNodeScopeRefresher; - $this->rectorOutput = $rectorOutput; } /** @@ -157,30 +152,16 @@ final public function enterNode(Node $node): int|Node|null return null; } - $isDebug = $this->rectorOutput->isDebug(); - $this->currentRectorProvider->changeCurrentRector($this); // for PHP doc info factory and change notifier $this->currentNodeProvider->setNode($node); - if ($isDebug) { - $this->rectorOutput->printCurrentFileAndRule($filePath, static::class); - } - $this->changedNodeScopeRefresher->reIndexNodeAttributes($node); - if ($isDebug) { - $this->rectorOutput->startConsumptions(); - } - // ensure origNode pulled before refactor to avoid changed during refactor, ref https://3v4l.org/YMEGN $originalNode = $node->getAttribute(AttributeKey::ORIGINAL_NODE) ?? $node; $refactoredNode = $this->refactor($node); - if ($isDebug) { - $this->rectorOutput->printConsumptions(); - } - // @see NodeTraverser::* codes, e.g. removal of node of stopping the traversing if ($refactoredNode === NodeTraverser::REMOVE_NODE) { $this->toBeRemovedNodeId = spl_object_id($originalNode); diff --git a/src/ValueObject/Configuration.php b/src/ValueObject/Configuration.php index 00be88d5e34..55703442ea9 100644 --- a/src/ValueObject/Configuration.php +++ b/src/ValueObject/Configuration.php @@ -24,7 +24,8 @@ public function __construct( private readonly string | null $parallelPort = null, private readonly string | null $parallelIdentifier = null, private readonly bool $isParallel = false, - private readonly string|null $memoryLimit = null + private readonly string|null $memoryLimit = null, + private readonly bool $isDebug = false ) { } @@ -89,4 +90,9 @@ public function getMemoryLimit(): ?string { return $this->memoryLimit; } + + public function isDebug(): bool + { + return $this->isDebug; + } }