Skip to content

Commit

Permalink
[Internals] Decouple output from AbstractRector, narrow debugging out…
Browse files Browse the repository at this point in the history
…put to file path (#4976)
  • Loading branch information
TomasVotruba committed Sep 10, 2023
1 parent 3fcbb53 commit 1593d00
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ final class PHPStanServicesFactory
{
private readonly Container $container;

public function __construct(
) {
public function __construct()
{
$containerFactory = new ContainerFactory(getcwd());
$additionalConfigFiles = $this->resolveAdditionalConfigFiles();

Expand Down
4 changes: 2 additions & 2 deletions packages/Testing/PHPUnit/AbstractRectorTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 15 additions & 1 deletion src/Application/ApplicationFileProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 */
Expand All @@ -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 {
Expand Down
12 changes: 0 additions & 12 deletions src/Application/FileProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion src/Configuration/ConfigurationFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -62,7 +63,8 @@ public function createFromInput(InputInterface $input): Configuration
$parallelPort,
$parallelIdentifier,
$isParallel,
$memoryLimit
$memoryLimit,
$isDebug
);
}

Expand Down
32 changes: 26 additions & 6 deletions src/Console/Command/WorkerCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -78,18 +82,30 @@ protected function execute(InputInterface $input, OutputInterface $output): int
ReactCommand::IDENTIFIER => $parallelIdentifier,
]);

$this->runWorker($outEncoder, $inDecoder, $configuration);
$this->runWorker($outEncoder, $inDecoder, $configuration, $output);
});

$streamSelectLoop->run();

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());
Expand All @@ -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;
Expand All @@ -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
Expand Down
2 changes: 0 additions & 2 deletions src/DependencyInjection/LazyContainerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
);
}
);
Expand Down
65 changes: 0 additions & 65 deletions src/Logging/RectorOutput.php

This file was deleted.

19 changes: 0 additions & 19 deletions src/Rector/AbstractRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -88,8 +87,6 @@ abstract class AbstractRector extends NodeVisitorAbstract implements RectorInter

private CreatedByRuleDecorator $createdByRuleDecorator;

private RectorOutput $rectorOutput;

private ?int $toBeRemovedNodeId = null;

public function autowire(
Expand All @@ -108,7 +105,6 @@ public function autowire(
CurrentFileProvider $currentFileProvider,
CreatedByRuleDecorator $createdByRuleDecorator,
ChangedNodeScopeRefresher $changedNodeScopeRefresher,
RectorOutput $rectorOutput
): void {
$this->nodeNameResolver = $nodeNameResolver;
$this->nodeTypeResolver = $nodeTypeResolver;
Expand All @@ -125,7 +121,6 @@ public function autowire(
$this->currentFileProvider = $currentFileProvider;
$this->createdByRuleDecorator = $createdByRuleDecorator;
$this->changedNodeScopeRefresher = $changedNodeScopeRefresher;
$this->rectorOutput = $rectorOutput;
}

/**
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 7 additions & 1 deletion src/ValueObject/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
) {
}

Expand Down Expand Up @@ -89,4 +90,9 @@ public function getMemoryLimit(): ?string
{
return $this->memoryLimit;
}

public function isDebug(): bool
{
return $this->isDebug;
}
}

0 comments on commit 1593d00

Please sign in to comment.