Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Parallel] Fix missing process RemovedAndAddedFilesProcessor->run() on parallel process on WorkerRunner #3218

Merged
merged 9 commits into from
Dec 19, 2022
1 change: 1 addition & 0 deletions .github/workflows/e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ jobs:
- 'e2e/use-rector-configurator'
- 'e2e/applied-rule-removed-node'
- 'e2e/parallel with space'
- 'e2e/removed-and-added-files-collector'

name: End to end test - ${{ matrix.directory }}

Expand Down
1 change: 1 addition & 0 deletions e2e/e2eTestRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

exec($e2eCommand, $output, $exitCode);
$output = trim(implode("\n", $output));
$output = str_replace(__DIR__, '.', $output);

$expectedDiff = 'expected-output.diff';
if (!file_exists($expectedDiff)) {
Expand Down
1 change: 1 addition & 0 deletions e2e/removed-and-added-files-collector/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/vendor
66 changes: 66 additions & 0 deletions e2e/removed-and-added-files-collector/TestRector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

declare(strict_types=1);

namespace Maintenance;

use PhpParser\Node;
use PhpParser\Node\Stmt\Function_;
use Rector\Core\Application\FileSystem\RemovedAndAddedFilesCollector;
use Rector\Core\Rector\AbstractRector;
use Rector\FileSystemRector\ValueObject\AddedFileWithContent;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

final class TestRector extends AbstractRector
{
private RemovedAndAddedFilesCollector $removedAndAddedFilesCollector;

public function __construct(
RemovedAndAddedFilesCollector $removedAndAddedFilesCollector,
) {
$this->removedAndAddedFilesCollector = $removedAndAddedFilesCollector;
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Test adding file.',
[
new CodeSample(
<<<'CODE_SAMPLE'
function test() {}
CODE_SAMPLE,
<<<'CODE_SAMPLE'
function test() {}
/* file named “test” is added */
CODE_SAMPLE
),
]
);
}

/**
* @return array<class-string<Node>>
*/
public function getNodeTypes(): array
{
return [Function_::class];
}

/**
* @param Function_ $node
*/
public function refactor(Node $node): ?Node
{
// Create a fixture.
$this->removedAndAddedFilesCollector->addAddedFile(
new AddedFileWithContent(
__DIR__ . '/test',
'test'
)
);

return $new;
}
}
10 changes: 10 additions & 0 deletions e2e/removed-and-added-files-collector/composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"require": {
"php": "^8.1"
},
"autoload-dev": {
"psr-4": {
"Maintenance\\": "."
}
}
}
7 changes: 7 additions & 0 deletions e2e/removed-and-added-files-collector/expected-output.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
! [NOTE] File
! "./removed-and-added-files-c
! ollector/test" will be added



[OK] Rector is done!
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this [OK] Rector is done! printed message should actually not exists with run with parallel along with:

$this->removedAndAddedFilesProcessor->run($configuration)

as it is running with --dry-run, but probably need in separate PRs for improvement 👍 :)

14 changes: 14 additions & 0 deletions e2e/removed-and-added-files-collector/rector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

use Maintenance\TestRector;

require_once __DIR__ . '/TestRector.php';

return static function (Rector\Config\RectorConfig $rectorConfig): void {
$rectorConfig->rule(TestRector::class);
$rectorConfig->paths([
__DIR__ . '/src',
]);
};
4 changes: 4 additions & 0 deletions e2e/removed-and-added-files-collector/src/Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?php

function test() {
}
45 changes: 32 additions & 13 deletions packages/Parallel/WorkerRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@
use Clue\React\NDJson\Encoder;
use Nette\Utils\FileSystem;
use PHPStan\Analyser\NodeScopeResolver;
use Rector\Core\Application\FileProcessor\PhpFileProcessor;
use Rector\Core\Application\FileSystem\RemovedAndAddedFilesProcessor;
use Rector\Core\Console\Style\RectorConsoleOutputStyle;
use Rector\Core\Contract\Processor\FileProcessorInterface;
use Rector\Core\Provider\CurrentFileProvider;
use Rector\Core\StaticReflection\DynamicSourceLocatorDecorator;
use Rector\Core\Util\ArrayParametersMerger;
use Rector\Core\ValueObject\Application\File;
use Rector\Core\ValueObject\Configuration;
use Rector\Core\ValueObject\Error\SystemError;
use Rector\Core\ValueObject\Reporting\FileDiff;
use Rector\Parallel\ValueObject\Bridge;
use Symplify\EasyParallel\Enum\Action;
use Symplify\EasyParallel\Enum\ReactCommand;
Expand All @@ -29,13 +31,17 @@ final class WorkerRunner
*/
private const RESULT = 'result';

/**
* @param FileProcessorInterface[] $fileProcessors
*/
public function __construct(
private readonly ArrayParametersMerger $arrayParametersMerger,
private readonly CurrentFileProvider $currentFileProvider,
private readonly PhpFileProcessor $phpFileProcessor,
private readonly NodeScopeResolver $nodeScopeResolver,
private readonly DynamicSourceLocatorDecorator $dynamicSourceLocatorDecorator,
private readonly RectorConsoleOutputStyle $rectorConsoleOutputStyle
private readonly RectorConsoleOutputStyle $rectorConsoleOutputStyle,
private readonly RemovedAndAddedFilesProcessor $removedAndAddedFilesProcessor,
private readonly array $fileProcessors = []
) {
}

Expand Down Expand Up @@ -83,16 +89,7 @@ public function run(Encoder $encoder, Decoder $decoder, Configuration $configura
$file = new File($filePath, FileSystem::read($filePath));
$this->currentFileProvider->setFile($file);

if (! $this->phpFileProcessor->supports($file, $configuration)) {
continue;
}

$currentErrorsAndFileDiffs = $this->phpFileProcessor->process($file, $configuration);

$errorAndFileDiffs = $this->arrayParametersMerger->merge(
$errorAndFileDiffs,
$currentErrorsAndFileDiffs
);
$errorAndFileDiffs = $this->processFiles($file, $configuration, $errorAndFileDiffs);

// warn about deprecated @noRector annotation
if (! str_ends_with($file->getFilePath(), 'WorkerRunner.php')
Expand Down Expand Up @@ -130,6 +127,28 @@ public function run(Encoder $encoder, Decoder $decoder, Configuration $configura
$decoder->on(ReactEvent::ERROR, $handleErrorCallback);
}

/**
* @param array{system_errors: SystemError[], file_diffs: FileDiff[]}|mixed[] $errorAndFileDiffs
* @return array{system_errors: SystemError[], file_diffs: FileDiff[]}
*/
public function processFiles(File $file, Configuration $configuration, array $errorAndFileDiffs): array
{
foreach ($this->fileProcessors as $fileProcessor) {
if (! $fileProcessor->supports($file, $configuration)) {
continue;
}

$currentErrorsAndFileDiffs = $fileProcessor->process($file, $configuration);
$errorAndFileDiffs = $this->arrayParametersMerger->merge(
$errorAndFileDiffs,
$currentErrorsAndFileDiffs
);
}

$this->removedAndAddedFilesProcessor->run($configuration);
return $errorAndFileDiffs;
}

/**
* @param SystemError[] $systemErrors
* @return SystemError[]
Expand Down
1 change: 1 addition & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ parameters:
- packages/BetterPhpDocParser/ValueObject/Parser/BetterTokenIterator.php
# reported only in non-CI, for some reason; keep it there even if it's reported as fixed error
- packages/Parallel/Application/ParallelFileProcessor.php
- packages/Parallel/WorkerRunner.php

# skipped on purpose, as ctor overrie
- '#Rector\\StaticTypeMapper\\ValueObject\\Type\\SimpleStaticType\:\:__construct\(\) does not call parent constructor from PHPStan\\Type\\StaticType#'
Expand Down