From 11a123f8c0280bba3d27238a78bd0d8d9c43b279 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Wed, 21 Oct 2026 15:16:09 -0300 Subject: [PATCH] fix: honor explicit Rector and PHPDoc scope --- .changelog/explicit-tool-scope.md | 6 ++ docs/commands/phpdoc.rst | 8 +- docs/commands/refactor.rst | 8 +- src/Console/Command/PhpDocCommand.php | 34 +++++++- src/Console/Command/RefactorCommand.php | 4 +- tests/Console/Command/PhpDocCommandTest.php | 83 +++++++++++++++++++ tests/Console/Command/RefactorCommandTest.php | 58 +++++++++++++ 7 files changed, 191 insertions(+), 10 deletions(-) create mode 100644 .changelog/explicit-tool-scope.md diff --git a/.changelog/explicit-tool-scope.md b/.changelog/explicit-tool-scope.md new file mode 100644 index 0000000000..8ce8ff13b8 --- /dev/null +++ b/.changelog/explicit-tool-scope.md @@ -0,0 +1,6 @@ +--- +category: fixed +type: patch +--- + +Honor explicit Rector configuration and forward PHPDoc file/directory selections to both underlying tools. Preserve configured scope when no paths are given and reject missing explicit PHPDoc paths before writing or running fixers. diff --git a/docs/commands/phpdoc.rst b/docs/commands/phpdoc.rst index cb5edb370e..ab2e5ce30e 100644 --- a/docs/commands/phpdoc.rst +++ b/docs/commands/phpdoc.rst @@ -27,8 +27,12 @@ Usage Arguments --------- -``path`` (optional) - Path to the file or directory to check. Default: ``.``. +``path...`` (optional) + Files or directories to check. Quote paths containing spaces. Without paths, + each tool uses its configured source paths. Explicit paths are passed to both + tools; PHP-CS-Fixer intersects them with its configured Finder, preserving + exclusions. Rector receives the paths as positional arguments and retains its + configured skips. Missing paths fail before the header or tools are invoked. Options ------- diff --git a/docs/commands/refactor.rst b/docs/commands/refactor.rst index 0e53b0c198..ea621be153 100644 --- a/docs/commands/refactor.rst +++ b/docs/commands/refactor.rst @@ -26,8 +26,9 @@ Options ``--fix, -f`` Automatically fix code refactoring issues. Without this option, runs in dry-run mode. -``--config, -c`` (optional) - Path to the Rector configuration file. Default: ``rector.php``. +``--config=FILE, -c FILE`` + Path to the Rector configuration file. Default: ``rector.php``. When supplied, + a value is required; an invalid path fails instead of using the default. ``--progress`` Enable progress output from Rector. @@ -70,7 +71,8 @@ Exit Codes Behavior --------- -- Local ``rector.php`` is preferred when present. +- Explicit ``--config`` is resolved and passed unchanged to Rector. Without it, + local ``rector.php`` is preferred when present. - Packaged default includes Fast Forward custom Rector rules plus shared Rector sets. - Progress output is disabled by default; use ``--progress`` to re-enable it in text mode. diff --git a/src/Console/Command/PhpDocCommand.php b/src/Console/Command/PhpDocCommand.php index 62270dc7c4..00eecfbdde 100644 --- a/src/Console/Command/PhpDocCommand.php +++ b/src/Console/Command/PhpDocCommand.php @@ -37,6 +37,7 @@ use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\BufferedOutput; use Symfony\Component\Console\Output\OutputInterface; @@ -114,9 +115,9 @@ protected function configure(): void ) ->addArgument( name: 'path', - mode: InputOption::VALUE_OPTIONAL, - description: 'Path to the file or directory to check.', - default: ['.'], + mode: InputArgument::OPTIONAL | InputArgument::IS_ARRAY, + description: 'Files or directories to check; omit to use configured paths.', + default: [], ) ->addOption( name: 'progress', @@ -152,6 +153,17 @@ protected function execute(InputInterface $input, OutputInterface $output): int $this->log('Checking and fixing PHPDocs...', $input); + $paths = (array) $input->getArgument('path'); + + foreach ($paths as $path) { + if ('' === $path || ! $this->filesystem->exists($path)) { + return $this->failure('PHPDoc path not found: {path}', $input, [ + 'path' => $path, + 'output' => $processOutput, + ]); + } + } + $this->ensureDocHeaderExists($input); $processBuilder = $this->processBuilder @@ -182,6 +194,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int $processBuilder = $processBuilder->withArgument('--dry-run'); } + if ([] !== $paths) { + $processBuilder = $processBuilder->withArgument('--path-mode=intersection')->withArgument('--'); + + foreach ($paths as $path) { + $processBuilder = $processBuilder->withArgument($path); + } + } + $phpCsFixer = $processBuilder->build([DevToolsPathResolver::getPreferredToolBinaryPath('php-cs-fixer'), 'fix']); $processBuilder = $this->processBuilder @@ -203,6 +223,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int $processBuilder = $processBuilder->withArgument('--dry-run'); } + if ([] !== $paths) { + $processBuilder = $processBuilder->withArgument('--'); + + foreach ($paths as $path) { + $processBuilder = $processBuilder->withArgument($path); + } + } + $rector = $processBuilder->build([DevToolsPathResolver::getPreferredToolBinaryPath('rector'), 'process']); $this->processQueue->add(process: $phpCsFixer, label: 'Fixing PHPDoc File Headers with PHP-CS-Fixer'); diff --git a/src/Console/Command/RefactorCommand.php b/src/Console/Command/RefactorCommand.php index 6da0bdd9cb..0abb1c809b 100644 --- a/src/Console/Command/RefactorCommand.php +++ b/src/Console/Command/RefactorCommand.php @@ -93,7 +93,7 @@ protected function configure(): void ->addOption( name: 'config', shortcut: 'c', - mode: InputOption::VALUE_OPTIONAL, + mode: InputOption::VALUE_REQUIRED, description: 'The path to the Rector configuration file.', default: self::CONFIG ); @@ -122,7 +122,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $processBuilder = $this->processBuilder ->withArgument('--ansi') ->withArgument('--config') - ->withArgument($this->fileLocator->locate(self::CONFIG)); + ->withArgument($this->fileLocator->locate($input->getOption('config') ?? self::CONFIG)); if (! $progress) { $processBuilder = $processBuilder->withArgument('--no-progress-bar'); diff --git a/tests/Console/Command/PhpDocCommandTest.php b/tests/Console/Command/PhpDocCommandTest.php index f1b1f65b37..92c7be9b6a 100644 --- a/tests/Console/Command/PhpDocCommandTest.php +++ b/tests/Console/Command/PhpDocCommandTest.php @@ -30,6 +30,7 @@ use FastForward\DevTools\Filesystem\FilesystemInterface; use FastForward\DevTools\Path\DevToolsPathResolver; use FastForward\DevTools\Process\ProcessBuilderInterface; +use FastForward\DevTools\Process\ProcessBuilder; use FastForward\DevTools\Process\ProcessQueueInterface; use FastForward\DevTools\Path\ManagedWorkspace; use FastForward\DevTools\Path\WorkingProjectPathResolver; @@ -39,6 +40,7 @@ use FastForward\DevTools\Environment\RuntimeEnvironment; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\Attributes\UsesTrait; use PHPUnit\Framework\TestCase; @@ -61,6 +63,7 @@ #[UsesClass(DevToolsEnvironment::class)] #[UsesClass(RuntimeEnvironment::class)] #[CoversClass(PhpDocCommand::class)] +#[UsesClass(ProcessBuilder::class)] #[UsesClass(Author::class)] #[UsesClass(Support::class)] #[UsesClass(ManagedWorkspace::class)] @@ -118,6 +121,7 @@ protected function setUp(): void $this->process->setEnv(Argument::type('array')) ->willReturn($this->process->reveal()); + $this->input->getArgument('path')->willReturn([]); $this->input->getOption('fix') ->willReturn(false); $this->input->getOption('cache') @@ -333,6 +337,85 @@ public function executeWillEnableProgressWhenRequested(): void self::assertSame(PhpDocCommand::SUCCESS, $this->invokeExecute()); } + /** + * @param list $paths + */ + #[Test] + #[DataProvider('selectedPaths')] + public function executeWillForwardSelectedPathsToBothTools(array $paths): void + { + $queue = $this->prophesize(ProcessQueueInterface::class); + $this->processQueue->add($this->process->reveal(), Argument::cetera())->shouldNotBeCalled(); + $this->command = new PhpDocCommand( + new ProcessBuilder(), + $queue->reveal(), + $this->composer->reveal(), + $this->fileLocator->reveal(), + $this->filesystem->reveal(), + $this->renderer->reveal(), + $this->clock->reveal(), + ); + $this->input->getArgument('path')->willReturn($paths); + $this->input->getOption('fix')->willReturn(true); + foreach ($paths as $path) { + $this->filesystem->exists($path)->willReturn(true); + } + $suffix = (new Process(['--', ...$paths]))->getCommandLine(); + $queue->add( + Argument::that(static fn(Process $process): bool => str_ends_with($process->getCommandLine(), $suffix) + && str_contains($process->getCommandLine(), '--path-mode=intersection') + && str_contains($process->getCommandLine(), DevToolsPathResolver::getPreferredToolBinaryPath('php-cs-fixer')) + && ! str_contains($process->getCommandLine(), '--dry-run')), + false, + false, + 'Fixing PHPDoc File Headers with PHP-CS-Fixer', + )->shouldBeCalledOnce(); + $queue->add( + Argument::that(static fn(Process $process): bool => str_ends_with($process->getCommandLine(), $suffix) + && str_contains($process->getCommandLine(), DevToolsPathResolver::getPreferredToolBinaryPath('rector')) + && ! str_contains($process->getCommandLine(), '--path-mode') + && ! str_contains($process->getCommandLine(), '--dry-run')), + false, + false, + 'Adding Missing PHPDoc with Rector', + )->shouldBeCalledOnce(); + $this->filesystem->dumpFile(PhpDocCommand::FILENAME, 'docheader')->shouldBeCalledOnce(); + $queue->run($this->output->reveal())->willReturn(PhpDocCommand::SUCCESS); + $this->logger->log('info', 'Checking and fixing PHPDocs...', Argument::type('array'))->shouldBeCalledOnce(); + $this->logger->log('info', 'Created .docheader from repository template.', Argument::type('array'))->shouldBeCalledOnce(); + $this->logger->log('info', 'PHPDoc checks completed successfully.', Argument::type('array'))->shouldBeCalledOnce(); + + self::assertSame(PhpDocCommand::SUCCESS, $this->invokeExecute()); + } + + /** + * @return iterable}> + */ + public static function selectedPaths(): iterable + { + yield 'file' => [['src/Feature.php']]; + yield 'directory' => [['src']]; + yield 'multiple paths with spaces' => [['source files/Feature.php', 'tests']]; + yield 'option-like filename' => [['--rules.php']]; + } + + /** + * @return void + */ + #[Test] + public function executeWillRejectMissingPathsBeforeWritingOrStartingTools(): void + { + $this->input->getArgument('path')->willReturn(['missing.php']); + $this->filesystem->exists('missing.php')->willReturn(false); + $this->filesystem->dumpFile(Argument::cetera())->shouldNotBeCalled(); + $this->processQueue->add($this->process->reveal(), Argument::cetera())->shouldNotBeCalled(); + $this->processQueue->run(Argument::cetera())->shouldNotBeCalled(); + $this->logger->log('info', 'Checking and fixing PHPDocs...', Argument::type('array'))->shouldBeCalledOnce(); + $this->logger->error('PHPDoc path not found: {path}', Argument::type('array'))->shouldBeCalledOnce(); + + self::assertSame(PhpDocCommand::FAILURE, $this->invokeExecute()); + } + /** * @return int */ diff --git a/tests/Console/Command/RefactorCommandTest.php b/tests/Console/Command/RefactorCommandTest.php index f28d5cd9dc..25b25d1047 100644 --- a/tests/Console/Command/RefactorCommandTest.php +++ b/tests/Console/Command/RefactorCommandTest.php @@ -24,6 +24,7 @@ use FastForward\DevTools\Path\DevToolsPathResolver; use FastForward\DevTools\Path\WorkingProjectPathResolver; use FastForward\DevTools\Process\ProcessBuilderInterface; +use FastForward\DevTools\Process\ProcessBuilder; use FastForward\DevTools\Process\ProcessQueueInterface; use FastForward\DevTools\Container\ContainerFactory; use FastForward\DevTools\Container\ServiceProvider\DevToolsServiceProvider; @@ -40,6 +41,7 @@ use Psr\Log\LoggerInterface; use FastForward\DevTools\Tests\Container\UsesContainerFactory; use ReflectionMethod; +use InvalidArgumentException; use Symfony\Component\Config\FileLocatorInterface; use Symfony\Component\Console\Formatter\OutputFormatter; use Symfony\Component\Console\Input\InputInterface; @@ -53,6 +55,7 @@ #[CoversClass(RefactorCommand::class)] #[UsesClass(DevToolsPathResolver::class)] #[UsesClass(WorkingProjectPathResolver::class)] +#[UsesClass(ProcessBuilder::class)] #[UsesTrait(LogsCommandResults::class)] final class RefactorCommandTest extends TestCase { @@ -89,6 +92,7 @@ protected function setUp(): void $this->logger = $this->prophesize(LoggerInterface::class); $this->setContainerEntry(LoggerInterface::class, $this->logger->reveal()); + $this->input->getOption('config')->willReturn(RefactorCommand::CONFIG); $this->input->getOption('fix') ->willReturn(false); $this->input->getOption('progress') @@ -211,6 +215,60 @@ public function executeWillEnableProgressWhenRequested(): void self::assertSame(RefactorCommand::SUCCESS, $this->executeCommand()); } + /** + * @return void + */ + #[Test] + public function executeWillHonorExplicitConfigurationWithSpaces(): void + { + $queue = $this->prophesize(ProcessQueueInterface::class); + $this->processQueue->add($this->process->reveal(), Argument::cetera())->shouldNotBeCalled(); + $this->command = new RefactorCommand($this->fileLocator->reveal(), new ProcessBuilder(), $queue->reveal()); + $this->input->getOption('config')->willReturn('custom rules/limited.php'); + $this->fileLocator->locate('custom rules/limited.php') + ->willReturn('/repo/custom rules/limited.php')->shouldBeCalledOnce(); + $this->fileLocator->locate(RefactorCommand::CONFIG)->shouldNotBeCalled(); + $expected = new Process([ + DevToolsPathResolver::getPreferredToolBinaryPath('rector'), + 'process', + '--ansi', + '--config', + '/repo/custom rules/limited.php', + '--no-progress-bar', + '--dry-run', + ]); + $queue->add( + Argument::that(static fn(Process $process): bool => $expected->getCommandLine() === $process->getCommandLine()), + false, + false, + 'Refactoring Code with Rector', + )->shouldBeCalledOnce(); + $queue->run($this->output->reveal())->willReturn(RefactorCommand::SUCCESS); + $this->logger->log('info', 'Running Rector for code refactoring...', Argument::type('array'))->shouldBeCalledOnce(); + $this->logger->log('info', 'Code refactoring checks completed successfully.', Argument::type('array'))->shouldBeCalledOnce(); + + self::assertSame(RefactorCommand::SUCCESS, $this->executeCommand()); + } + + /** + * @return void + */ + #[Test] + public function executeWillRejectInvalidExplicitConfigurationWithoutDefaultFallback(): void + { + $this->input->getOption('config')->willReturn('missing/config.php'); + $this->fileLocator->locate('missing/config.php') + ->willThrow(new InvalidArgumentException('Configuration not found: missing/config.php')); + $this->fileLocator->locate(RefactorCommand::CONFIG)->shouldNotBeCalled(); + $this->processQueue->add($this->process->reveal(), Argument::cetera())->shouldNotBeCalled(); + $this->processQueue->run(Argument::cetera())->shouldNotBeCalled(); + $this->logger->log('info', 'Running Rector for code refactoring...', Argument::type('array'))->shouldBeCalledOnce(); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Configuration not found: missing/config.php'); + + $this->executeCommand(); + } + /** * @return int */