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

Add the possibility to process files in batches #6312

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
44 changes: 44 additions & 0 deletions .github/workflows/batch_rector.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Test workflow to show how this feature works. DO NOT MERGE, REMOVE IN FINAL VERSION OF PR
name: Rector

on:
pull_request: null

jobs:
no_batch_rector:

runs-on: ubuntu-latest

steps:
-
uses: actions/checkout@v4

-
uses: shivammathur/setup-php@v2
with:
php-version: 8.2
coverage: none

- run: composer install --no-progress --ansi

- run: bin/rector process --ansi
batch_rector:
strategy:
matrix:
batches: [0, 1, 2, 3]

runs-on: ubuntu-latest

steps:
-
uses: actions/checkout@v4

-
uses: shivammathur/setup-php@v2
with:
php-version: 8.2
coverage: none

- run: composer install --no-progress --ansi

- run: bin/rector process --batch-index=${{ strategy.job-index }} --batch-total=${{ strategy.job-total }} --ansi
42 changes: 42 additions & 0 deletions .github/workflows/e2e_with_batches.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# This workflow runs system tests: Use the Rector application from the source
# checkout to process "fixture" projects in e2e/batch-run directory
# to see if those can be processed successfully in batches
name: End to End tests with batches

on:
pull_request: null

env:
# see https://github.com/composer/composer/issues/9368#issuecomment-718112361
COMPOSER_ROOT_VERSION: "dev-main"

jobs:
end_to_end:
runs-on: ubuntu-latest
timeout-minutes: 3
strategy:
fail-fast: false
matrix:
batches: [0, 1]

name: End to end test with batches - batch ${{ strategy.job-index }}

steps:
- uses: actions/checkout@v4

- uses: shivammathur/setup-php@v2
with:
php-version: 8.2
coverage: none

# run in root rector-src
- run: composer install --ansi

# run in e2e subdir
-
run: composer install --ansi
working-directory: e2e/batch-run

# run e2e test
- run: php ../e2eTestRunnerWithBatches.php ${{ strategy.job-index }} ${{ strategy.job-total }}
working-directory: e2e/batch-run
1 change: 1 addition & 0 deletions e2e/batch-run/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/vendor
7 changes: 7 additions & 0 deletions e2e/batch-run/composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"require": {
"php": "^8.1"
},
"minimum-stability": "dev",
"prefer-stable": true
}
22 changes: 22 additions & 0 deletions e2e/batch-run/expected-output-0.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
1 file with changes
===================

1) src/RenameDocblock.php:0

---------- begin diff ----------
@@ @@
<?php

/**
- * @param DateTime $someOldClass
+ * @param \DateTimeInterface $someOldClass
*/
function someFunction($someOldClass)
{
----------- end diff -----------

Applied rules:
* RenameClassRector


[OK] 1 file would have been changed (dry-run) by Rector
22 changes: 22 additions & 0 deletions e2e/batch-run/expected-output-1.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
1 file with changes
===================

1) src/UselessVarTag.php:1

---------- begin diff ----------
@@ @@

final class UselessVarTag
{
- /**
- * @var string
- */
public string $name = 'name';
}
----------- end diff -----------

Applied rules:
* RemoveUselessVarTagRector


[OK] 1 file would have been changed (dry-run) by Rector
24 changes: 24 additions & 0 deletions e2e/batch-run/rector.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\DeadCode\Rector\Property\RemoveUselessVarTagRector;
use Rector\DowngradePhp80\Rector\Class_\DowngradeAttributeToAnnotationRector;
use Rector\DowngradePhp80\ValueObject\DowngradeAttributeToAnnotation;
use Rector\Renaming\Rector\Name\RenameClassRector;

return static function (RectorConfig $rectorConfig): void {
$rectorConfig->paths([
__DIR__ . '/src',
]);

$rectorConfig->ruleWithConfiguration(RenameClassRector::class, [
'DateTime' => 'DateTimeInterface'
]);
$rectorConfig->ruleWithConfiguration(DowngradeAttributeToAnnotationRector::class, [
new DowngradeAttributeToAnnotation('Symfony\Component\Routing\Annotation\Route')
]);

$rectorConfig->rule(RemoveUselessVarTagRector::class);
};
8 changes: 8 additions & 0 deletions e2e/batch-run/src/AlreadyChangedDocblock.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

/**
* @param DateTimeInterface $someOldClass
*/
function someFunction($someOldClass)
{
}
8 changes: 8 additions & 0 deletions e2e/batch-run/src/RenameDocblock.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

/**
* @param DateTime $someOldClass
*/
function someFunction($someOldClass)
{
}
9 changes: 9 additions & 0 deletions e2e/batch-run/src/UselessVarTag.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

final class UselessVarTag
{
/**
* @var string
*/
public string $name = 'name';
}
51 changes: 51 additions & 0 deletions e2e/e2eTestRunnerWithBatches.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#!/usr/bin/env php
<?php

// runs a rector e2e test.
// checks whether we expect a certain output, or alternatively that rector just processed everything without errors

use Rector\Console\Formatter\ColorConsoleDiffFormatter;
use Rector\Console\Formatter\ConsoleDiffer;
use Rector\Console\Style\SymfonyStyleFactory;
use Rector\Util\Reflection\PrivatesAccessor;
use Symfony\Component\Console\Command\Command;

$projectRoot = __DIR__ .'/..';
$rectorBin = $projectRoot . '/bin/rector';
$autoloadFile = $projectRoot . '/vendor/autoload.php';

// so we can use helper classes here
require_once __DIR__ . '/../vendor/autoload.php';

$batchIndex = $argv[1];
$batchTotal = $argv[2];

$e2eCommand = 'php '. $rectorBin .' process --dry-run --no-ansi -a '. $autoloadFile
. ' --batch-index=' . $batchIndex . ' --batch-total=' . $batchTotal;

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

$expectedDiff = 'expected-output-' . $batchIndex . '.diff';
if (!file_exists($expectedDiff)) {
echo $output;
exit($exitCode);
}

$symfonyStyleFactory = new SymfonyStyleFactory(new PrivatesAccessor());
$symfonyStyle = $symfonyStyleFactory->create();

$matchedExpectedOutput = false;
$expectedOutput = trim(file_get_contents($expectedDiff));
if ($output === $expectedOutput) {
$symfonyStyle->success('End-to-end test successfully completed');
exit(Command::SUCCESS);
}

// print color diff, to make easy find the differences
$consoleDiffer = new ConsoleDiffer(new ColorConsoleDiffFormatter());
$diff = $consoleDiffer->diff($output, $expectedOutput);
$symfonyStyle->writeln($diff);

exit(Command::FAILURE);
6 changes: 5 additions & 1 deletion rector.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Rector\DeadCode\Rector\ConstFetch\RemovePhpVersionIdCheckRector;
use Rector\DeadCode\Rector\Property\RemoveUnusedPrivatePropertyRector;
use Rector\Php55\Rector\String_\StringClassNameToClassConstantRector;
use Rector\TypeDeclaration\Rector\Expression\InlineVarDocTagToAssertRector;

return RectorConfig::configure()
->withPreparedSets(
Expand Down Expand Up @@ -57,4 +58,7 @@
],

RemoveUnusedPrivatePropertyRector::class => [__DIR__ . '/src/Configuration/RectorConfigBuilder.php'],
]);
])
// This is just to show how it works in github actions. DO NOT MERGE, REMOVE IN FINAL VERSION OF PR
->withRules([InlineVarDocTagToAssertRector::class])
;
8 changes: 8 additions & 0 deletions src/Application/ApplicationFileProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Rector\Reporting\MissConfigurationReporter;
use Rector\Testing\PHPUnit\StaticPHPUnitEnvironment;
use Rector\Util\ArrayParametersMerger;
use Rector\Util\BatchSplitter;
use Rector\ValueObject\Application\File;
use Rector\ValueObject\Configuration;
use Rector\ValueObject\Error\SystemError;
Expand Down Expand Up @@ -61,6 +62,13 @@ public function run(Configuration $configuration, InputInterface $input): Proces
$this->missConfigurationReporter->reportVendorInPaths($filePaths);
$this->missConfigurationReporter->reportStartWithShortOpenTag();

$batchSplitter = new BatchSplitter();
$filePaths = $batchSplitter->getItemsInBatch(
$filePaths,
$configuration->getBatchIndex(),
$configuration->getBatchTotal()
);

// no files found
if ($filePaths === []) {
return new ProcessResult([], []);
Expand Down
11 changes: 9 additions & 2 deletions src/Configuration/ConfigurationFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ public function createForTests(array $paths): Configuration
false,
null,
false,
false
false,
0,
0,
);
}

Expand All @@ -65,11 +67,14 @@ 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);
$batchIndex = (int) $input->getOption(Option::BATCH_INDEX);
$batchTotal = (int) $input->getOption(Option::BATCH_TOTAL);
$isDebug = (bool) $input->getOption(Option::DEBUG);

// using debug disables parallel, so emitting exception is straightforward and easier to debug
// using debug disables parallel and batch running, so emitting exception is straightforward and easier to debug
if ($isDebug) {
$isParallel = false;
$batchTotal = 0;
}

$memoryLimit = $this->resolveMemoryLimit($input);
Expand All @@ -90,6 +95,8 @@ public function createFromInput(InputInterface $input): Configuration
$memoryLimit,
$isDebug,
$isReportingWithRealPath,
$batchIndex,
$batchTotal,
);
}

Expand Down
10 changes: 10 additions & 0 deletions src/Configuration/Option.php
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,16 @@ final class Option
*/
public const PARALLEL_PORT = 'port';

/**
* @var string
*/
public const BATCH_INDEX = 'batch-index';

/**
* @var string
*/
public const BATCH_TOTAL = 'batch-total';

/**
* @internal Use @see \Rector\Config\RectorConfig::parallel() instead with pass int $jobSize parameter
* @var string
Expand Down
6 changes: 6 additions & 0 deletions src/Console/Command/ProcessCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

$configuration = $this->configurationFactory->createFromInput($input);

if ($configuration->getBatchTotal() !== 0 && $configuration->getBatchIndex() >= $configuration->getBatchTotal()) {
$this->symfonyStyle->error('The job index needs to be less than the job total');
return ExitCode::FAILURE;
}

$this->memoryLimiter->adjust($configuration);

// disable console output in case of json output formatter
Expand Down
14 changes: 14 additions & 0 deletions src/Console/ConsoleApplication.php
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,20 @@ private function addCustomOptions(InputDefinition $inputDefinition): void
InputOption::VALUE_NONE,
'Clear cache'
));

$inputDefinition->addOption(new InputOption(
Option::BATCH_INDEX,
null,
InputOption::VALUE_REQUIRED,
'Index of the current job when running in batch mode'
));

$inputDefinition->addOption(new InputOption(
Option::BATCH_TOTAL,
null,
InputOption::VALUE_REQUIRED,
'Total number of jobs when running in batch mode'
));
}

private function getDefaultConfigPath(): string
Expand Down
Loading