diff --git a/.changelog/explicit-rector-command.md b/.changelog/explicit-rector-command.md new file mode 100644 index 0000000..25372e5 --- /dev/null +++ b/.changelog/explicit-rector-command.md @@ -0,0 +1,12 @@ +--- +category: added +type: minor +--- + +Add a separate opt-in native Rector command with dry-run by default, explicit +configuration and apply authority, empty-rule initialization protection and +post-apply verification. Require an explicit native consumer PHP target before +processing; keep target PHP, rules and caches consumer-owned; +formatting check/fix never invoke semantic refactoring. +Allow an explicit positive per-subprocess timeout without disabling bounds; +execution timeouts fail with status 1, while cache configuration remains optional. diff --git a/AGENTS.md b/AGENTS.md index d7ab52c..96d1bc8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,6 +66,7 @@ for the exact local base/head before publication; do not claim remote CI evidenc - [README.md](README.md): library/CLI adoption, explicit fix and tool scope. - [Policy comparison](docs/policy.md): evidence, rule differences and extraction gates. +- [Explicit Rector](docs/rector.md): opt-in native semantic preview/application; executable configuration owns target PHP, rules, source paths and caches. - [Resolved preset](docs/resolved-preset.md): compiled rules, golden contracts and dependency-update review. - [Legacy fixer comparison](docs/legacy-fixer-comparison.md): fixed-source inventory, loaded dependencies and remaining profile decisions. - [PHPDoc comparison](docs/phpdoc-comparison.md): actual custom-rule registration, source contracts and remaining golden decisions. diff --git a/README.md b/README.md index 510ed57..c3fa7d0 100644 --- a/README.md +++ b/README.md @@ -147,7 +147,11 @@ and the [consumer skill](resources/skills/fast-forward-code-standards/SKILL.md). The [architecture decision](docs/architecture.md) defines library/tool dependency directions and CPX/autoload limits. The formatter preserves descriptive PHPDoc and inherited documentation. -Semantic refactoring and type analysis are outside the formatter scope. +The separate opt-in [`refactor` command](docs/rector.md) previews an explicit +native Rector configuration and requires `--apply` for semantic writes. It is +never invoked by formatting `check` or `fix`; target PHP and cache remain native +consumer configuration decisions. Custom PHPDoc transformations and deeper +static analysis remain separate; no DevTools extraction parity is asserted. ## 🧰 API summary @@ -155,8 +159,9 @@ Semantic refactoring and type analysis are outside the formatter scope. | --- | --- | | `code-standards check` | Parse selected PHP and check style without changing source. | | `code-standards fix` | Explicitly format selected source, then verify; failures may leave partial changes. | +| `code-standards refactor --config=rector.php` | Preview native semantic rules; only `--apply` permits writes, followed by verification. | | `ECSConfig::configure(?callable $customize)` | Return the native ECS builder; apply the optional callback last. | -| `help check` / `help fix` | Show the installed Symfony option definitions. | +| `help check` / `help fix` / `help refactor` | Show the installed Symfony option definitions. | Internal CLI services are implementation details. Supported configuration uses native ECS rule imports, skips and configured rules; there is no second DSL. @@ -189,9 +194,10 @@ Composer package constraint. The CLI selects working-tree files from Git, not staged index blobs. Changed mode requires a clean tracked checkout and cannot certify staged-only content. -**Does it run PHPStan, Rector or my application's tests?** -No. The formatter checks syntax and style. Type analysis, semantic refactoring -and application testing have separate scopes and compatibility requirements. +**Do formatting check/fix run PHPStan, Rector or my application's tests?** +No. Formatting checks syntax and style. The separate opt-in `refactor` command +uses an explicit native Rector configuration; it never runs through check/fix. +Type analysis and application testing remain separate scopes. **Does a successful fix always mean the source is clean?** The CLI verifies each formatted batch and reports remaining violations. Rules diff --git a/composer.json b/composer.json index b730dd6..d76c4a2 100644 --- a/composer.json +++ b/composer.json @@ -41,7 +41,9 @@ "composer-runtime-api": "^2.2", "container-interop/service-provider": "^0.4.1", "fast-forward/container": "~1.10.1", + "phpstan/phpstan": "~2.1.51", "psr/container": "^1.1 || ^2.0", + "rector/rector": "~2.4.2", "symfony/console": "^7.4 || ^8.0", "symfony/process": "^7.4 || ^8.0", "symplify/easy-coding-standard": "^13.3.8" diff --git a/composer.lock b/composer.lock index 30dcd4f..43a7920 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "a57a9274f932194060fca44700f0e7cb", + "content-hash": "dbded3ef4a3e25aa79065ba546255638", "packages": [ { "name": "brick/varexporter", @@ -420,6 +420,59 @@ }, "time": "2026-09-13T18:51:52+00:00" }, + { + "name": "phpstan/phpstan", + "version": "2.1.51", + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/phpstan/phpstan/zipball/dc3b523c45e714c70de2ac5113b958223b55dc59", + "reference": "dc3b523c45e714c70de2ac5113b958223b55dc59", + "shasum": "" + }, + "require": { + "php": "^7.4|^8.0" + }, + "conflict": { + "phpstan/phpstan-shim": "*" + }, + "bin": [ + "phpstan", + "phpstan.phar" + ], + "type": "library", + "autoload": { + "files": [ + "bootstrap.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "PHPStan - PHP Static Analysis Tool", + "keywords": [ + "dev", + "static analysis" + ], + "support": { + "docs": "https://phpstan.org/user-guide/getting-started", + "forum": "https://github.com/phpstan/phpstan/discussions", + "issues": "https://github.com/phpstan/phpstan/issues", + "security": "https://github.com/phpstan/phpstan/security/policy", + "source": "https://github.com/phpstan/phpstan-src" + }, + "funding": [ + { + "url": "https://github.com/ondrejmirtes", + "type": "github" + }, + { + "url": "https://github.com/phpstan", + "type": "github" + } + ], + "time": "2026-04-21T18:22:01+00:00" + }, { "name": "psr/container", "version": "2.0.2", @@ -524,6 +577,66 @@ }, "time": "2021-10-29T13:26:27+00:00" }, + { + "name": "rector/rector", + "version": "2.4.2", + "source": { + "type": "git", + "url": "https://github.com/rectorphp/rector.git", + "reference": "e645b6463c6a88ea5b44b17d3387d35a912c7946" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/rectorphp/rector/zipball/e645b6463c6a88ea5b44b17d3387d35a912c7946", + "reference": "e645b6463c6a88ea5b44b17d3387d35a912c7946", + "shasum": "" + }, + "require": { + "php": "^7.4|^8.0", + "phpstan/phpstan": "^2.1.48" + }, + "conflict": { + "rector/rector-doctrine": "*", + "rector/rector-downgrade-php": "*", + "rector/rector-phpunit": "*", + "rector/rector-symfony": "*" + }, + "suggest": { + "ext-dom": "To manipulate phpunit.xml via the custom-rule command" + }, + "bin": [ + "bin/rector" + ], + "type": "library", + "autoload": { + "files": [ + "bootstrap.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "Instant Upgrade and Automated Refactoring of any PHP code", + "homepage": "https://getrector.com/", + "keywords": [ + "automation", + "dev", + "migration", + "refactoring" + ], + "support": { + "issues": "https://github.com/rectorphp/rector/issues", + "source": "https://github.com/rectorphp/rector/tree/2.4.2" + }, + "funding": [ + { + "url": "https://github.com/tomasvotruba", + "type": "github" + } + ], + "time": "2026-04-16T13:07:34+00:00" + }, { "name": "symfony/console", "version": "v7.4.21", diff --git a/docs/architecture.md b/docs/architecture.md index 8c9cdf0..f5f86d6 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -46,8 +46,10 @@ Ordinary Composer installation inside the consumer intentionally shares that installation's autoloader through its binary proxy. `composer-runtime-api` and `$_composer_autoload_path` remain part of this contract. Removing an extra import from a different target graph does not remove normal Composer bootstrap. -PHPUnit, Rector and type-analysis adapters still require their own compatibility -tests; the formatter's process boundary does not prove their isolation. +PHPUnit and type-analysis adapters require their own compatibility tests; the +formatter's process boundary does not prove their isolation. The separate Rector +command's executable configuration and child-autoload limits are documented in +[the native refactoring contract](rector.md). ## Bootstrap and compatibility diff --git a/docs/instruction-map.md b/docs/instruction-map.md index 096a778..db73a9e 100644 --- a/docs/instruction-map.md +++ b/docs/instruction-map.md @@ -10,5 +10,9 @@ consumer procedure. It is not copied to `.agents/skills` and needs no host alias Read root instructions, then only the linked domain guide or skill needed by the task. The root Child DOX Index provides the reading route. +The opt-in refactoring guide remains under this same root authority. It records +the native Rector invocation and executable-config boundary; it introduces no +new instruction subtree, automatic semantic profile or consumer installer. + No previous instruction tree is migrated. Rollback restores the root contract, its index and this map together through Git; it changes no host configuration. diff --git a/docs/policy.md b/docs/policy.md index 49ea750..2f7d653 100644 --- a/docs/policy.md +++ b/docs/policy.md @@ -27,7 +27,7 @@ from blocked PR #376 or composer-installers #15 was executed or changed. | Scope | source resolver excludes generated/resources/public/vendor | consumer Finder, hidden files, VCS ignores, path exclusions | Tracked PHP/bin candidates filtered by root/nested consumer .gitignore (including tracked matches); only .git structurally excluded; PHP symlinks rejected | | Overrides | customize callback last | extraRules last | customize callback last; explicit config selection | | Check | combined orchestration has independent mutation risks | dry-run fixer | syntax parse then ECS check, no source writes | -| Semantic transformations | Rector seven sets, PHP migration, Safe migration, Faker and custom PHPDoc | mixed into risky fixer | Not implemented; requires explicit target, opt-in and independent tests | +| Semantic transformations | Rector seven sets, PHP migration, Safe migration, Faker and custom PHPDoc | mixed into risky fixer | Not implemented in the initial extraction snapshot; required explicit target, opt-in and independent tests | The explicit rule deltas above are intentional. Transitive upstream preset membership is dependency-version specific and fixed for contributor builds by @@ -53,9 +53,17 @@ exercise headers and overrides in golden fixtures and decide which semantic transformations belong in an opt-in profile. Optional Twig/magic-value policy, Composer metadata normalization and existing public DevTools APIs stay untouched. -Static quality in this first increment means parseable PHP and formatting -conformance. Type analysis, dead-code analysis and semantic refactors are not -claimed. Application tests, coverage and complete reports remain outside this domain. +The initial extraction snapshot limited static quality to parseable PHP and +formatting conformance; it did not claim type analysis, dead-code analysis or +semantic refactors. Application tests, coverage and complete reports remain +outside this domain. + +The current default formatter does not select a semantic profile. The separately +opted-in [native Rector command](rector.md) exists and requires explicit native +configuration, reviewed rules and a consumer PHP target; formatting check/fix +never invoke it. Native cache configuration is optional and remains consumer-owned, +including Rector's in-memory cache selection in CI. This command does not claim +parity with the three legacy custom PHPDoc transformations; #4 remains partial. The [PHPDoc comparison](phpdoc-comparison.md) records actual custom Rector activation and the fixtures required before adopting a documentation policy. diff --git a/docs/rector.md b/docs/rector.md new file mode 100644 index 0000000..b9886cb --- /dev/null +++ b/docs/rector.md @@ -0,0 +1,113 @@ +# Explicit semantic refactoring + +The opt-in `refactor` command previews native semantic transformations and +applies them only when explicitly requested. It does not select a migration +profile or authorize publication. `check` and `fix` never invoke the semantic +runner. The separate invocable command uses the existing container factories +and process transport. + +The qualified lock selects published Rector 2.4.2, reference +`e645b6463c6a88ea5b44b17d3387d35a912c7946`, with PHPStan 2.1.51. The manifest +restricts both dependencies to their respective 2.4/2.1 patch lines. Rector's +upstream PHPStan constraint also permits 2.3.2, but that combination reproduced +a fatal missing `RichParser::$container` error before rule execution. This +compatibility restriction applies to the shared Composer graph; a consumer +requiring a newer PHPStan must use a separate compatible tool installation. +PHPStan is an engine dependency here, not a newly exposed analysis command. +Dependency updates require the real integration fixtures below. Native configuration is the only +extension point; no PHP parser, configuration translator or engine framework is +introduced. Configuration paths resolve from `--directory` or the current +directory. This command deliberately uses the configuration's whole-project +paths rather than borrowing the formatter's changed-file list. + +```php +withPhpVersion(PhpVersion::PHP_74) + ->withPaths([__DIR__ . '/src']) + ->withRules([LongArrayToShortArrayRector::class]) + ->withCache(sys_get_temp_dir() . '/my-project-rector/' . hash('sha256', __DIR__)) + ->withoutParallel(); +``` + +The target is a required consumer decision, independent of the host PHP. Declare +it with native `withPhpVersion()` or `RectorConfig::phpVersion()`, directly or in +an imported native configuration. Composer constraints and Rector's runtime +fallback do not satisfy this explicit target requirement. `withPhpVersion` +limits supported transformations; it does not downgrade existing code or select +PHP migration sets. Review any later `withPhpSets` selection separately. The +command does not infer or overwrite target, rule, cache, bootstrap, parallelism +or path choices. Configure generated caches separately from owned source, with +a directory unique to the checkout. No consumer dependency/lock update occurs. +Rector 2.4.2 defaults to memory cache storage when it detects CI, so setting a +cache directory does not guarantee that directory will be created. Use native +`withCache` backend options if persistent file storage is explicitly required; +the command preserves Rector's selection. + +```sh +php bin/code-standards refactor --config=rector.php --directory=/project +# After reviewing the proposed semantic diff: +php bin/code-standards refactor --config=rector.php --directory=/project --apply +# For a larger project, choose an explicit finite per-subprocess budget: +php bin/code-standards refactor --config=rector.php --directory=/project --timeout=900 +``` + +Preview passes native `--dry-run`. Explicit apply performs one write pass and +then a dry-run verification; remaining differences fail. Native exits are 0 for +clean/success, 1 for processing errors or subprocess timeouts and 2 for proposed changes. Controlled +invocation, empty-rule and invalid-inventory errors also return 2, with an error +diagnostic. Other native fatal exits propagate unchanged. Rector 2.4.2 uses `-n` for dry-run and does not expose the ECS +`--no-interaction` option; its native flags are not translated blindly. + +`--timeout` accepts a positive whole number of seconds and defaults to 300. +It bounds each Rector subprocess separately: preflight inventory, processing and +post-apply verification receive the same limit. Fractional, negative, zero, +nonfinite and overflowing values are rejected; no unlimited mode is exposed. +The option changes this refactor invocation, leaving Git's 120-second captured +transport default and formatting's 300-second default unchanged. Native Rector +process/parallel-worker limits remain native configuration decisions. + +## Executable configuration and initialization boundary + +The command first asks native `list-rules --output-format=json` for the selected +configuration's rules. A temporary, tool-owned native PHP wrapper imports that +configuration into Rector's own initialized container and checks its public +parameter API for an explicit PHP target. The wrapper is removed after preflight, +including failures; selected source and consumer configuration are not rewritten. +The actual `process` commands keep the original config path and cache identity. +Missing or invalid native targets fail without semantic writes. An empty or invalid inventory is rejected without calling +`process`: native Rector otherwise can generate `rector.php` and return success +even during dry-run. Missing explicit configuration is rejected before launch. + +This is a contract for **trusted, deterministic configuration**. Preview runs +one inventory and one processing invocation; successful apply adds a verification +invocation. Native Rector may evaluate configuration multiple times within an +invocation. The guard does not add another configuration-evaluation process. +A configuration that changes its target or rules between invocations can defeat that preflight. Arbitrary +configuration, Composer autoload files, custom rules and configured bootstraps +can execute and mutate files even in dry-run. The command is not a filesystem or +security sandbox, and cannot promise read-only behavior for such executable +extensions. Do not use them with privileged events, secrets or an untrusted +consumer. Changes to the native initialization contract require regression +review before changing the pinned Rector version. + +The subprocess prevents Rector from sharing the parent CLI's in-memory state. +It does not guarantee namespace isolation in the child: Rector loads its +installation autoloader and may load the consumer's `vendor/autoload.php` too. +The collision regression requires failure rather than a false clean result; +configuration/bootstrap compatibility remains a consumer qualification gate. +Captured preflight and streamed processing use the same credential stripping. +No generic promise of semantic safety follows from a zero result. + +The integration fixtures cover no-write preview, explicit apply/verification, empty custom +configuration, missing/invalid targets, a PHP 7.4 target under a newer host, +imported targets and native service bindings, configured cache ownership, +configuration context/evaluation counts, native bootstrap failures and consumer/tool autoload collisions. Run the +consumer's affected behavioral tests and review API/side-effect changes before +accepting any semantic patch. Type analysis and application testing are not +implicitly added to this command. diff --git a/resources/skills/fast-forward-code-standards/SKILL.md b/resources/skills/fast-forward-code-standards/SKILL.md index 8255eb1..034b01c 100644 --- a/resources/skills/fast-forward-code-standards/SKILL.md +++ b/resources/skills/fast-forward-code-standards/SKILL.md @@ -24,7 +24,12 @@ metadata: privileged token or on a privileged target event. No incidental dependency update. 5. Only when fixing is requested run `code-standards fix --all`, inspect the diff and rerun check. Do not commit/push automatically. Semantic migrations need a - separate reviewed procedure; none are enabled by this initial package. + separate reviewed procedure: the opt-in `refactor` command requires explicit + native Rector configuration, an explicit consumer PHP target and reviewed + rules. Native cache configuration is optional; Rector may use memory in CI. + Preview is default; only explicit `--apply` grants semantic writes. + Read `docs/rector.md` for executable-config/autoload limits. Never substitute + this command for ordinary formatting `fix` or automatically migrate consumers. 6. For requested CI adoption copy `resources/workflows/code-standards.yml` from the selected ref after v0 points to a confirmed 0.x release, and use `pull_request` with read-only permissions and a hosted runner. No secrets/inherit, write token, diff --git a/src/Console/Application.php b/src/Console/Application.php index 031fef0..0bdbbe4 100644 --- a/src/Console/Application.php +++ b/src/Console/Application.php @@ -6,6 +6,7 @@ use FastForward\CodeStandards\Console\Command\CheckCommand; use FastForward\CodeStandards\Console\Command\FixCommand; +use FastForward\CodeStandards\Console\Command\RefactorCommand; use Symfony\Component\Console\Application as SymfonyApplication; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Exception\ExceptionInterface; @@ -16,13 +17,14 @@ /** Native Symfony application; standard binding errors retain the CLI's invalid-input status. */ final class Application extends SymfonyApplication { - /** Registers the two explicitly composed operations without resolving project state or executing tools. */ - public function __construct(CheckCommand $check, FixCommand $fix) + /** Registers independent operations without resolving project state or executing tools. */ + public function __construct(CheckCommand $check, FixCommand $fix, RefactorCommand $refactor) { parent::__construct('Fast Forward Code Standards'); $this->addCommand($check); $this->addCommand($fix); + $this->addCommand($refactor); $this->setAutoExit(false); } diff --git a/src/Console/Command/RefactorCommand.php b/src/Console/Command/RefactorCommand.php new file mode 100644 index 0000000..439be77 --- /dev/null +++ b/src/Console/Command/RefactorCommand.php @@ -0,0 +1,80 @@ +timeoutSeconds($timeout); + $scope = $this->projects->resolve(true, null, null, $directory, $config); + + return $this->rector->run($scope->directory, $scope->config, $apply, $seconds); + } catch (ProcessFailedException $error) { + $io->getErrorStyle()->error($error->getMessage()); + + return $error->getProcess()->getExitCode() ?? Command::FAILURE; + } catch (ProcessTimedOutException $error) { + $io->getErrorStyle()->error($error->getMessage()); + + return Command::FAILURE; + } catch (InvalidArgumentException|RuntimeException $error) { + $io->getErrorStyle()->error($error->getMessage()); + + return Command::INVALID; + } + } + + /** Preserves the literal option contract instead of silently coercing decimal, exponent or overflowing input. */ + private function timeoutSeconds(string $timeout): int + { + $seconds = (int) $timeout; + + if (1 !== preg_match('/^[1-9][0-9]*$/D', $timeout) + || (string) $seconds !== $timeout) { + throw new InvalidArgumentException('--timeout must be a positive whole number of seconds.'); + } + + return $seconds; + } +} diff --git a/src/Process/NativeProcessRunner.php b/src/Process/NativeProcessRunner.php index f4f301c..d987717 100644 --- a/src/Process/NativeProcessRunner.php +++ b/src/Process/NativeProcessRunner.php @@ -4,32 +4,43 @@ namespace FastForward\CodeStandards\Process; +use InvalidArgumentException; use Symfony\Component\Process\Process; /** Native transport; callers own selection and operation policy. */ final class NativeProcessRunner implements ProcessRunner { + /** Both captured and streamed configuration subprocesses receive the same credential policy. */ + private const array ENVIRONMENT = ['GH_TOKEN' => false, 'GITHUB_TOKEN' => false, 'COMPOSER_AUTH' => false]; + /** @param \Closure(string,string):void|null $write Explicit output sink; null keeps this library transport silent. */ public function __construct(private readonly ?\Closure $write = null) {} - /** Captures Git output as bytes, including NUL-delimited paths. */ - public function capture(array $arguments, string $directory): string + /** Captures native output as bytes, including Git paths and Rector's JSON inventory. */ + public function capture(array $arguments, string $directory, float $timeout = 120): string { - $process = new Process($arguments, $directory, timeout: 120); + $process = $this->createProcess($arguments, $directory, $timeout); $process->mustRun(); return $process->getOutput(); } /** Runs one tool, streaming its output without changing its failure status. */ - public function run(array $arguments, string $directory): int + public function run(array $arguments, string $directory, float $timeout = 300): int { - $process = new Process($arguments, $directory, [ - 'GH_TOKEN' => false, - 'GITHUB_TOKEN' => false, - 'COMPOSER_AUTH' => false, - ], timeout: 300); + $process = $this->createProcess($arguments, $directory, $timeout); return $process->run($this->write); } + + /** @param list $arguments Positive finite bounds never become Symfony's zero/null unlimited mode. */ + private function createProcess(array $arguments, string $directory, float $timeout): Process + { + if (!is_finite($timeout) + || $timeout <= 0) { + throw new InvalidArgumentException('Subprocess timeout must be a positive finite number of seconds.'); + } + + return new Process($arguments, $directory, self::ENVIRONMENT, timeout: $timeout); + } } diff --git a/src/Process/ProcessRunner.php b/src/Process/ProcessRunner.php index 698523b..a4e7bda 100644 --- a/src/Process/ProcessRunner.php +++ b/src/Process/ProcessRunner.php @@ -7,9 +7,15 @@ /** Executes bounded argument arrays without shell interpolation. */ interface ProcessRunner { - /** @param list $arguments Returns stdout or throws on failure. */ - public function capture(array $arguments, string $directory): string; + /** + * @param list $arguments Returns stdout or throws on failure. + * @param positive-float $timeout Finite maximum seconds for this subprocess; defaults retain Git's existing limit. + */ + public function capture(array $arguments, string $directory, float $timeout = 120): string; - /** @param list $arguments Streams diagnostics and returns the actual exit code. */ - public function run(array $arguments, string $directory): int; + /** + * @param list $arguments Streams diagnostics and returns the actual exit code. + * @param positive-float $timeout Finite maximum seconds for this subprocess; default formatting limit is unchanged. + */ + public function run(array $arguments, string $directory, float $timeout = 300): int; } diff --git a/src/Refactoring/RectorRunner.php b/src/Refactoring/RectorRunner.php new file mode 100644 index 0000000..adbc67d --- /dev/null +++ b/src/Refactoring/RectorRunner.php @@ -0,0 +1,102 @@ +assertRules($directory, $config, $timeout); + $check = [PHP_BINARY, $this->rector, 'process', '--config=' . $config, '--no-progress-bar']; + $status = $this->process->run($apply ? $check : [...$check, '--dry-run'], $directory, $timeout); + if (!$apply || 0 !== $status) { + return $status; + } + + return $this->process->run([...$check, '--dry-run'], $directory, $timeout); + } + + /** Uses Rector's own JSON inventory instead of evaluating or parsing consumer PHP in the parent process. */ + private function assertRules(string $directory, string $config, float $timeout): void + { + $output = $this->nativeInventory($directory, $config, $timeout); + try { + $inventory = json_decode($output, true, flags: JSON_THROW_ON_ERROR); + } catch (JsonException $error) { + throw new InvalidArgumentException('Rector did not return a valid native rule inventory.', previous: $error); + } + $rules = is_array($inventory) ? ($inventory['rectors'] ?? null) : null; + if (!is_array($rules) + || [] === $rules + || !array_is_list($rules) + || count(array_filter($rules, static fn(mixed $rule): bool => is_string($rule) && '' !== $rule)) !== count($rules)) { + throw new InvalidArgumentException('Select a native Rector configuration with at least one rule; automatic initialization is disabled.'); + } + } + + /** Imports the original config in Rector's native container; no application/config source is copied or parsed. */ + private function nativeInventory(string $directory, string $config, float $timeout): string + { + $temporary = sys_get_temp_dir() . '/code-standards-rector-' . bin2hex(random_bytes(16)); + if (!mkdir($temporary, 0700)) { + throw new RuntimeException('Cannot create the native Rector preflight configuration.'); + } + $preflight = $temporary . '/preflight.php'; + try { + $source = $this->targetGuard($config); + if (strlen($source) !== file_put_contents($preflight, $source)) { + throw new RuntimeException('Cannot write the native Rector preflight configuration.'); + } + + return $this->process->capture([ + PHP_BINARY, $this->rector, 'list-rules', '--config=' . $preflight, '--output-format=json', + ], $directory, $timeout); + } finally { + if (is_file($preflight)) { + unlink($preflight); + } + rmdir($temporary); + } + } + + /** Uses public native configuration/parameter APIs; actual process commands retain the original config identity. */ + private function targetGuard(string $config): string + { + return sprintf(<<<'PHP' + import(%s); + if ($implicitTarget + || !SimpleParameterProvider::hasParameter(Option::PHP_VERSION_FEATURES) + || SimpleParameterProvider::provideIntParameter(Option::PHP_VERSION_FEATURES) <= 0) { + throw new InvalidArgumentException('Select an explicit PHP target with native RectorConfig::phpVersion() or withPhpVersion().'); + } + }; + PHP, var_export($config, true)); + } +} diff --git a/src/ServiceProvider/StandardsServiceProvider.php b/src/ServiceProvider/StandardsServiceProvider.php index 060d549..051cd3a 100644 --- a/src/ServiceProvider/StandardsServiceProvider.php +++ b/src/ServiceProvider/StandardsServiceProvider.php @@ -7,6 +7,7 @@ use FastForward\CodeStandards\Process\NativeProcessRunner; use FastForward\CodeStandards\Process\ProcessRunner; use FastForward\CodeStandards\Project\ProjectScopeResolver; +use FastForward\CodeStandards\Refactoring\RectorRunner; use FastForward\CodeStandards\Standards\StandardsRunner; use FastForward\Container\Factory\CallableFactory; use FastForward\Container\Factory\FactoryInterface; @@ -60,6 +61,11 @@ public function getFactories(): array $this->vendorDirectory . '/autoload.php', $this->engineDirectory, ), + RectorRunner::class => new InvokableFactory( + RectorRunner::class, + ProcessRunner::class, + $this->vendorDirectory . '/rector/rector/bin/rector', + ), ]; } diff --git a/tests/Integration/RectorTest.php b/tests/Integration/RectorTest.php new file mode 100644 index 0000000..85df401 --- /dev/null +++ b/tests/Integration/RectorTest.php @@ -0,0 +1,260 @@ +directory = realpath(sys_get_temp_dir()) . '/rector-consumer-' . bin2hex(random_bytes(8)); + mkdir($this->directory); + file_put_contents($this->directory . '/composer.lock', '{"fixture":"do not update"}'); + } + + protected function tearDown(): void + { + $files = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->directory, \FilesystemIterator::SKIP_DOTS), \RecursiveIteratorIterator::CHILD_FIRST); + foreach ($files as $file) { + $file->isDir() && !$file->isLink() ? rmdir($file->getPathname()) : unlink($file->getPathname()); + } + rmdir($this->directory); + } + + /** Native preview leaves source/config/lock bytes intact; explicit apply is verified and repeatable. */ + public function testPreviewApplyAndSecondPassWithExplicitNativeConfiguration(): void + { + $source = "directory . '/source.php', $source); + $config = $this->configuration('\\Rector\\Php54\\Rector\\Array_\\LongArrayToShortArrayRector', 70400); + $before = $this->ownedBytes(); + + $preview = $this->cli(environment: ['CI' => 'true', 'GITHUB_ACTIONS' => 'true']); + self::assertSame(2, $preview->getExitCode(), $preview->getOutput() . $preview->getErrorOutput()); + self::assertSame($before, $this->ownedBytes()); + // Native Rector chooses memory storage in CI even with a configured cache directory. + self::assertDirectoryDoesNotExist($this->directory . '/.cache'); + self::assertSame(0, $this->cli(['--apply'])->getExitCode()); + self::assertStringContainsString('[1, 2]', file_get_contents($this->directory . '/source.php')); + $after = $this->ownedBytes(); + self::assertSame(0, $this->cli()->getExitCode()); + self::assertSame(0, $this->cli(['--apply'])->getExitCode()); + self::assertSame($after, $this->ownedBytes()); + self::assertSame($config, file_get_contents($this->directory . '/policy.php')); + } + + /** Empty native custom config is rejected before Rector's successful-but-writing initializer. */ + public function testEmptyConfigDoesNotGenerateRectorFile(): void + { + file_put_contents($this->directory . '/source.php', 'directory . '/policy.php', 'withPhpVersion(70400);'); + $before = $this->ownedBytes(); + $result = $this->cli(); + + self::assertSame(2, $result->getExitCode()); + self::assertStringContainsString('at least one rule', $result->getErrorOutput()); + self::assertFileDoesNotExist($this->directory . '/rector.php'); + self::assertSame($before, $this->ownedBytes()); + } + + /** @return iterable}> Both preview and application require a declared target. */ + public static function executionModes(): iterable + { + yield 'preview' => [[]]; + yield 'apply' => [['--apply']]; + } + + /** Valid rules must not silently inherit the executing runtime as the consumer PHP target. */ + #[DataProvider('executionModes')] + public function testMissingExplicitTargetRejectsBeforeProcessing(array $options): void + { + file_put_contents($this->directory . '/source.php', 'directory . '/composer.json', '{"require":{"php":"^7.4"}}'); + $config = $this->configuration('\\Rector\\Php54\\Rector\\Array_\\LongArrayToShortArrayRector', 70400); + file_put_contents($this->directory . '/policy.php', str_replace('->withPhpVersion(70400)', '', $config)); + $before = $this->ownedBytes(); + $result = $this->cli($options); + + self::assertSame(1, $result->getExitCode(), $result->getOutput() . $result->getErrorOutput()); + self::assertStringContainsString('explicit PHP target', $result->getErrorOutput()); + self::assertFileDoesNotExist($this->directory . '/rector.php'); + self::assertSame($before, $this->ownedBytes()); + } + + /** A target below the executing PHP suppresses a versioned transformation without selecting migration sets. */ + public function testConsumerTargetControlsVersionedRule(): void + { + $source = 'name = $name; } }'; + file_put_contents($this->directory . '/source.php', $source); + $rule = '\\Rector\\Php80\\Rector\\Class_\\ClassPropertyAssignToConstructorPromotionRector'; + $this->configuration($rule, 70400); + self::assertSame(0, $this->cli()->getExitCode()); + $this->configuration($rule, 80000); + self::assertSame(2, $this->cli()->getExitCode()); + self::assertSame($source, file_get_contents($this->directory . '/source.php')); + } + + /** Native import and service bindings preserve cwd/config context and evaluation counts through the preflight. */ + public function testImportedExplicitTargetAndNativeServicesPreserveConfigurationContext(): void + { + file_put_contents($this->directory . '/source.php', 'directory . '/relative.txt', 'consumer context'); + file_put_contents($this->directory . '/parts.php', <<<'PHP' + phpVersion(70400); + $config->paths([__DIR__ . '/source.php']); + $config->rule(\Rector\Php54\Rector\Array_\LongArrayToShortArrayRector::class); + $config->disableParallel(); + }; + PHP); + file_put_contents($this->directory . '/policy.php', <<<'PHP' + import(__DIR__ . '/parts.php'); + $config->make(\PHPStan\Parser\Parser::class); + file_put_contents(__DIR__ . '/context.jsonl', json_encode([ + 'cwd' => getcwd(), + 'directory' => __DIR__, + 'relative' => file_get_contents('relative.txt'), + ], JSON_THROW_ON_ERROR) . "\n", FILE_APPEND); + }; + PHP); + $before = $this->ownedBytes(); + $native = [PHP_BINARY, dirname(__DIR__, 2) . '/vendor/rector/rector/bin/rector']; + $inventory = new Process([...$native, 'list-rules', '--config=policy.php', '--output-format=json'], $this->directory, timeout: 90); + $inventory->mustRun(); + $inventoryLoads = count(file($this->directory . '/context.jsonl', FILE_IGNORE_NEW_LINES)); + unlink($this->directory . '/context.jsonl'); + $processing = new Process([...$native, 'process', '--config=policy.php', '--dry-run', '--no-progress-bar'], $this->directory, timeout: 90); + self::assertSame(2, $processing->run(), $processing->getOutput() . $processing->getErrorOutput()); + $processLoads = count(file($this->directory . '/context.jsonl', FILE_IGNORE_NEW_LINES)); + unlink($this->directory . '/context.jsonl'); + $preview = $this->cli(); + self::assertSame(2, $preview->getExitCode(), $preview->getOutput() . $preview->getErrorOutput()); + self::assertSame($before, $this->ownedBytes()); + $lines = file($this->directory . '/context.jsonl', FILE_IGNORE_NEW_LINES); + self::assertCount($inventoryLoads + $processLoads, $lines); + foreach ($lines as $line) { + self::assertSame([ + 'cwd' => $this->directory, + 'directory' => $this->directory, + 'relative' => 'consumer context', + ], json_decode($line, true, flags: JSON_THROW_ON_ERROR)); + } + $apply = $this->cli(['--apply']); + self::assertSame(0, $apply->getExitCode(), $apply->getOutput() . $apply->getErrorOutput()); + self::assertCount(2 * $inventoryLoads + 3 * $processLoads, file($this->directory . '/context.jsonl', FILE_IGNORE_NEW_LINES)); + self::assertStringContainsString('[1]', file_get_contents($this->directory . '/source.php')); + } + + /** Native invalid target/config errors must fail without processing or mutating selected source. */ + public function testInvalidExplicitTargetFailsWithoutWriting(): void + { + file_put_contents($this->directory . '/source.php', 'configuration('\\Rector\\Php54\\Rector\\Array_\\LongArrayToShortArrayRector', $target); + $before = $this->ownedBytes(); + $result = $this->cli(['--apply']); + self::assertNotSame(0, $result->getExitCode(), $result->getOutput() . $result->getErrorOutput()); + self::assertSame($before, $this->ownedBytes()); + self::assertFileDoesNotExist($this->directory . '/rector.php'); + } + } + + /** The subprocess boundary protects the parent, but a consumer autoload collision still fails in the child. */ + public function testConsumerAutoloadCollisionIsNotClaimedToBeIsolated(): void + { + file_put_contents($this->directory . '/source.php', 'configuration('\\Rector\\Php54\\Rector\\Array_\\LongArrayToShortArrayRector', 70400); + mkdir($this->directory . '/vendor'); + file_put_contents($this->directory . '/vendor/autoload.php', 'ownedBytes(); + $result = $this->cli(); + + self::assertNotSame(0, $result->getExitCode()); + self::assertSame($before, $this->ownedBytes()); + unlink($this->directory . '/vendor/autoload.php'); + self::assertSame(2, $this->cli()->getExitCode()); + } + + /** Native bootstrap failures preserve their status instead of being mistaken for no changes. */ + public function testExecutableConfigFailureAndRequiredOption(): void + { + file_put_contents($this->directory . '/policy.php', 'cli(); + self::assertSame(1, $result->getExitCode()); + self::assertStringContainsString('native config failure', $result->getErrorOutput()); + $missing = new Process([PHP_BINARY, dirname(__DIR__, 2) . '/bin/code-standards', 'refactor'], $this->directory, timeout: 20); + self::assertSame(2, $missing->run()); + } + + /** A real native preflight timeout is an execution failure and removes its owned guard without source writes. */ + public function testNativePreflightTimeoutPreservesConsumerAndCleansGuard(): void + { + file_put_contents($this->directory . '/source.php', 'configuration('\\Rector\\Php54\\Rector\\Array_\\LongArrayToShortArrayRector', 70400); + $slow = 'directory . '/policy.php', $slow); + $before = $this->ownedBytes(); + $result = $this->cli(['--timeout=2']); + + self::assertSame(1, $result->getExitCode(), $result->getOutput() . $result->getErrorOutput()); + self::assertStringContainsString('exceeded the timeout', $result->getErrorOutput()); + self::assertSame($before, $this->ownedBytes()); + $preflight = file_get_contents($this->directory . '/preflight.path'); + self::assertStringContainsString('/code-standards-rector-', $preflight); + self::assertFileDoesNotExist($preflight); + self::assertDirectoryDoesNotExist(dirname($preflight)); + + file_put_contents($this->directory . '/policy.php', $config); + $accepted = $this->cli(['--timeout=5']); + self::assertSame(2, $accepted->getExitCode(), $accepted->getOutput() . $accepted->getErrorOutput()); + } + + /** Returns native builder PHP unchanged; target and cache remain consumer-owned options. */ + private function configuration(string $rule, int $target): string + { + $config = 'withPhpVersion(' . $target . ')' + . '->withoutParallel()->withCache(__DIR__."/.cache")->withPaths([__DIR__."/source.php"])' + . '->withRules([' . $rule . '::class]);'; + file_put_contents($this->directory . '/policy.php', $config); + + return $config; + } + + /** @return array Bytes owned by the consumer; configured generated cache is deliberately separate. */ + private function ownedBytes(): array + { + $bytes = []; + foreach (['source.php', 'policy.php', 'composer.json', 'composer.lock'] as $name) { + if (is_file($this->directory . '/' . $name)) { + $bytes[$name] = file_get_contents($this->directory . '/' . $name); + } + } + + return $bytes; + } + + /** + * @param list $options Additional explicit command options, never shell fragments. + * + * @param array $environment Explicit fixture environment, inherited by the native child. + */ + private function cli(array $options = [], array $environment = []): Process + { + $process = new Process([PHP_BINARY, dirname(__DIR__, 2) . '/bin/code-standards', 'refactor', '--config=policy.php', ...$options], $this->directory, $environment, timeout: 90); + $process->run(); + + return $process; + } +} diff --git a/tests/Process/NativeProcessRunnerTest.php b/tests/Process/NativeProcessRunnerTest.php new file mode 100644 index 0000000..5aa9371 --- /dev/null +++ b/tests/Process/NativeProcessRunnerTest.php @@ -0,0 +1,51 @@ +{$method}([PHP_BINARY, '-r', 'usleep(250000);'], sys_get_temp_dir(), 0.01); + self::fail('A controlled long subprocess must time out.'); + } catch (ProcessTimedOutException $exception) { + self::assertSame(0.01, $exception->getExceededTimeout()); + } + } + $arguments = [PHP_BINARY, '-r', 'usleep(50000); echo "completed";']; + self::assertSame('completed', $process->capture($arguments, sys_get_temp_dir(), 2)); + $output = ''; + $streamed = new NativeProcessRunner(static function (string $type, string $bytes) use (&$output): void { + $output .= $bytes; + }); + self::assertSame(0, $streamed->run($arguments, sys_get_temp_dir(), 2)); + self::assertSame('completed', $output); + } + + /** Zero, negative and nonfinite values cannot silently become Symfony's unlimited mode. */ + public function testInvalidBoundsFailBeforeLaunchingEitherChannel(): void + { + foreach (['capture', 'run'] as $method) { + foreach ([0.0, -0.1, INF, -INF, NAN] as $timeout) { + try { + (new NativeProcessRunner())->{$method}([PHP_BINARY, '-r', 'echo "unexpected";'], sys_get_temp_dir(), $timeout); + self::fail('Invalid subprocess limits must be rejected.'); + } catch (InvalidArgumentException $exception) { + self::assertStringContainsString('positive finite', $exception->getMessage()); + } + } + } + } +} diff --git a/tests/Refactoring/RectorRunnerTest.php b/tests/Refactoring/RectorRunnerTest.php new file mode 100644 index 0000000..7aec365 --- /dev/null +++ b/tests/Refactoring/RectorRunnerTest.php @@ -0,0 +1,127 @@ +run('/consumer', '/consumer/my policy.php', false)); + self::assertCount(1, $process->commands); + self::assertContains('--dry-run', $process->commands[0]); + self::assertContains('--config=/consumer/my policy.php', $process->commands[0]); + self::assertContains('list-rules', $process->captures[0]); + self::assertNotContains('--no-interaction', $process->commands[0]); + self::assertSame([300.0], $process->captureTimeouts); + self::assertSame([300.0], $process->runTimeouts); + } + + /** Apply is a single write pass followed by a read-only verification whose failure remains observable. */ + public function testApplyVerifiesAndPropagatesResidualChanges(): void + { + $process = new RectorTransport(statuses: [0, 2]); + self::assertSame(2, (new RectorRunner($process, '/tool/rector'))->run('/consumer', '/consumer/policy.php', true)); + self::assertCount(2, $process->commands); + self::assertNotContains('--dry-run', $process->commands[0]); + self::assertContains('--dry-run', $process->commands[1]); + + $failed = new RectorTransport(statuses: [1]); + self::assertSame(1, (new RectorRunner($failed, '/tool/rector'))->run('/consumer', '/consumer/policy.php', true)); + self::assertCount(1, $failed->commands); + } + + /** Empty or malformed inventory must prevent process from auto-generating rector.php. */ + public function testEmptyOrMalformedRulesNeverReachProcess(): void + { + foreach (['{"rectors":[]}', '{}', 'not JSON', '{"rectors":[1]}'] as $inventory) { + $process = new RectorTransport($inventory); + try { + (new RectorRunner($process, '/tool/rector'))->run('/consumer', '/consumer/policy.php', false); + self::fail('Invalid rule inventory must fail closed.'); + } catch (InvalidArgumentException) { + self::assertSame([], $process->commands); + } + } + } + + /** Only the temporary preflight is removed; original config identity stays on every process command. */ + public function testTargetGuardEscapesOriginalPathAndCleansItsOwnFile(): void + { + $process = new RectorTransport(); + $config = "/consumer/quote'line\npolicy.php"; + (new RectorRunner($process, '/tool/rector'))->run('/consumer', $config, false); + + self::assertCount(1, $process->preflights); + self::assertStringContainsString('$config->import(' . var_export($config, true) . ');', $process->guards[0]); + self::assertFileDoesNotExist($process->preflights[0]); + self::assertContains('--config=' . $config, $process->commands[0]); + + $failure = new RectorTransport(); + $failure->failCapture = true; + try { + (new RectorRunner($failure, '/tool/rector'))->run('/consumer', $config, true); + self::fail('Native bootstrap failure must remain observable.'); + } catch (RuntimeException $exception) { + self::assertSame('fixture capture failure', $exception->getMessage()); + self::assertFileDoesNotExist($failure->preflights[0]); + self::assertSame([], $failure->commands); + } + } + + /** One genuine operation parameter bounds inventory, apply and verification without changing global defaults. */ + public function testExplicitTimeoutReachesEveryRectorSubprocess(): void + { + $process = new RectorTransport(statuses: [0, 0]); + self::assertSame(0, (new RectorRunner($process, '/tool/rector'))->run('/consumer', '/consumer/policy.php', true, 600)); + self::assertSame([600.0], $process->captureTimeouts); + self::assertSame([600.0, 600.0], $process->runTimeouts); + } +} + +/** Records only the execution boundary; no native configuration or source is evaluated here. */ +final class RectorTransport implements ProcessRunner +{ + public array $commands = []; + public array $captures = []; + public array $preflights = []; + public array $guards = []; + public bool $failCapture = false; + public array $captureTimeouts = []; + public array $runTimeouts = []; + + /** @param list $statuses Native process/verification outcomes in execution order. */ + public function __construct(private string $inventory = '{"rectors":["ReviewedRule"]}', private array $statuses = [0]) {} + + public function capture(array $arguments, string $directory, float $timeout = 120): string + { + $this->captures[] = $arguments; + $this->captureTimeouts[] = $timeout; + $preflight = substr($arguments[3], strlen('--config=')); + $this->preflights[] = $preflight; + $this->guards[] = file_get_contents($preflight); + if ($this->failCapture) { + throw new RuntimeException('fixture capture failure'); + } + + return $this->inventory; + } + + public function run(array $arguments, string $directory, float $timeout = 300): int + { + $this->commands[] = $arguments; + $this->runTimeouts[] = $timeout; + + return array_shift($this->statuses); + } +} diff --git a/tests/Refactoring/RefactorCommandTest.php b/tests/Refactoring/RefactorCommandTest.php new file mode 100644 index 0000000..96f4d7f --- /dev/null +++ b/tests/Refactoring/RefactorCommandTest.php @@ -0,0 +1,166 @@ +createMock(ProcessRunner::class); + $process->expects(self::once())->method('capture')->willReturn('{"rectors":["ReviewedRule"]}'); + $process->expects(self::once())->method('run')->with(self::callback(static fn(array $arguments): bool => in_array('--dry-run', $arguments, true)), $directory)->willReturn(2); + $application = new Application(); + $application->setAutoExit(false); + $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default'), new RectorRunner($process, '/tool/rector'))); + $tester = new ApplicationTester($application); + + try { + self::assertSame(2, $tester->run(['command' => 'refactor'])); + self::assertStringContainsString('explicit native Rector configuration', $tester->getDisplay()); + self::assertSame(2, $tester->run(['command' => 'refactor', '--config' => $config, '--directory' => $directory])); + self::assertStringNotContainsString('ERROR', $tester->getDisplay()); + } finally { + unlink($config); + } + } + + /** Both channels preserve credential filtering when a caller supplies a finite timeout override. */ + public function testConfigurationSubprocessesDoNotReceiveToolCredentials(): void + { + $previous = []; + foreach (['GH_TOKEN', 'GITHUB_TOKEN', 'COMPOSER_AUTH'] as $name) { + $previous[$name] = getenv($name); + putenv($name . '=synthetic-test-value'); + } + + try { + $arguments = [ + PHP_BINARY, '-r', 'foreach (["GH_TOKEN", "GITHUB_TOKEN", "COMPOSER_AUTH"] as $name) { echo getenv($name) === false ? "absent\n" : "present\n"; }', + ]; + $output = (new NativeProcessRunner())->capture($arguments, sys_get_temp_dir(), 2); + self::assertSame("absent\nabsent\nabsent\n", $output); + $streamed = ''; + $process = new NativeProcessRunner(static function (string $type, string $bytes) use (&$streamed): void { + $streamed .= $bytes; + }); + self::assertSame(0, $process->run($arguments, sys_get_temp_dir(), 2)); + self::assertSame("absent\nabsent\nabsent\n", $streamed); + } finally { + foreach ($previous as $name => $value) { + putenv(false === $value ? $name : $name . '=' . $value); + } + } + } + + /** @return iterable Widths exercise narrow, hosted-CI and wide rendering without private terminal state. */ + public static function timeoutTerminalWidths(): iterable + { + yield 'narrow' => [20]; + yield 'hosted CI' => [80]; + yield 'wide' => [160]; + } + + /** An actual short subprocess timeout remains a failure with its complete diagnostic and owned preflight cleanup. */ + #[DataProvider('timeoutTerminalWidths')] + public function testTimeoutIsFailureAndCleansThePreflight(int $columns): void + { + $native = new Process([PHP_BINARY, '-r', 'usleep(250000);'], timeout: 0.01); + try { + $native->mustRun(); + self::fail('The controlled subprocess must time out.'); + } catch (ProcessTimedOutException $failure) { + $timeout = $failure; + } + $config = tempnam(sys_get_temp_dir(), 'rector-policy-'); + $preflight = null; + $process = $this->createMock(ProcessRunner::class); + $process->expects(self::once())->method('capture')->willReturnCallback(static function (array $arguments) use (&$preflight, $timeout): never { + $preflight = substr($arguments[3], strlen('--config=')); + throw $timeout; + }); + $process->expects(self::never())->method('run'); + $application = new Application(); + $application->setAutoExit(false); + $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default'), new RectorRunner($process, '/tool/rector'))); + $tester = new ApplicationTester($application); + + $previousColumns = getenv('COLUMNS'); + try { + putenv('COLUMNS=' . $columns); + self::assertSame(1, $tester->run(['command' => 'refactor', '--config' => $config, '--directory' => realpath(sys_get_temp_dir())])); + self::assertMatchesRegularExpression('/exceeded\s+the\s+timeout\s+of\s+0\.01\s+seconds\./', $tester->getDisplay()); + self::assertIsString($preflight); + self::assertFileDoesNotExist($preflight); + self::assertDirectoryDoesNotExist(dirname($preflight)); + } finally { + putenv(false === $previousColumns ? 'COLUMNS' : 'COLUMNS=' . $previousColumns); + unlink($config); + } + self::assertSame($previousColumns, getenv('COLUMNS')); + } + + /** @return iterable Inputs that must never be coerced to a disabled or unintended limit. */ + public static function invalidTimeouts(): iterable + { + foreach (['bad', '1.5', '0.5', '0', '-1', '1e3', 'INF', 'NAN', ' 5', '5 ', '+5', '01', '999999999999999999999'] as $input) { + yield $input => [$input]; + } + } + + /** Rejects malformed bounds through the actual Symfony option binding before touching transport. */ + #[DataProvider('invalidTimeouts')] + public function testInvalidTimeoutIsRejectedWithoutLaunchingProcesses(string $timeout): void + { + $config = tempnam(sys_get_temp_dir(), 'rector-policy-'); + $process = $this->createMock(ProcessRunner::class); + $process->expects(self::never())->method('capture'); + $process->expects(self::never())->method('run'); + $application = new Application(); + $application->setAutoExit(false); + $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default'), new RectorRunner($process, '/tool/rector'))); + $tester = new ApplicationTester($application); + try { + self::assertSame(2, $tester->run(['command' => 'refactor', '--config' => $config, '--timeout' => $timeout])); + self::assertStringContainsString('--timeout must be a positive whole number', $tester->getDisplay()); + } finally { + unlink($config); + } + } + + /** The actual option binding forwards the requested positive limit to the native preflight and dry-run. */ + public function testPositiveTimeoutOptionIsForwardedWithoutCoercion(): void + { + $config = tempnam(sys_get_temp_dir(), 'rector-policy-'); + $directory = realpath(sys_get_temp_dir()); + $process = $this->createMock(ProcessRunner::class); + $process->expects(self::once())->method('capture')->with(self::isArray(), $directory, 600.0)->willReturn('{"rectors":["ReviewedRule"]}'); + $process->expects(self::once())->method('run')->with(self::isArray(), $directory, 600.0)->willReturn(0); + $application = new Application(); + $application->setAutoExit(false); + $application->addCommand(new RefactorCommand(new ProjectScopeResolver('/unused-default'), new RectorRunner($process, '/tool/rector'))); + $tester = new ApplicationTester($application); + try { + self::assertSame(0, $tester->run(['command' => 'refactor', '--config' => $config, '--directory' => $directory, '--timeout' => '600'])); + } finally { + unlink($config); + } + } +} diff --git a/tests/ServiceProvider/CompositionTest.php b/tests/ServiceProvider/CompositionTest.php index 1e58fef..73571e4 100644 --- a/tests/ServiceProvider/CompositionTest.php +++ b/tests/ServiceProvider/CompositionTest.php @@ -7,6 +7,7 @@ use FastForward\CodeStandards\Console\Application; use FastForward\CodeStandards\Console\Command\CheckCommand; use FastForward\CodeStandards\Console\Command\FixCommand; +use FastForward\CodeStandards\Console\Command\RefactorCommand; use FastForward\CodeStandards\Process\NativeProcessRunner; use FastForward\CodeStandards\Process\ProcessRunner; use FastForward\CodeStandards\ServiceProvider\StandardsServiceProvider; @@ -49,6 +50,8 @@ public function testAutowiringAndTransportOverrideComposeDistinctCommands(): voi self::assertInstanceOf(FixCommand::class, $container->get(FixCommand::class)); self::assertFalse(is_subclass_of(CheckCommand::class, Command::class)); self::assertFalse(is_subclass_of(FixCommand::class, Command::class)); + self::assertInstanceOf(RefactorCommand::class, $container->get(RefactorCommand::class)); + self::assertFalse(is_subclass_of(RefactorCommand::class, Command::class)); $tester = new ApplicationTester($application); self::assertSame(23, $tester->run(['command' => $name, '--all' => true, '--directory' => dirname(__DIR__, 2)])); @@ -110,13 +113,13 @@ final class RecordingTransport implements ProcessRunner public array $commands = []; /** Reads only Git metadata through the production shell-free transport. */ - public function capture(array $arguments, string $directory): string + public function capture(array $arguments, string $directory, float $timeout = 120): string { - return (new NativeProcessRunner())->capture($arguments, $directory); + return (new NativeProcessRunner())->capture($arguments, $directory, $timeout); } /** Returns a synthetic style failure to verify end-to-end exit-code propagation. */ - public function run(array $arguments, string $directory): int + public function run(array $arguments, string $directory, float $timeout = 300): int { $this->commands[] = $arguments; diff --git a/tests/Standards/StandardsRunnerTest.php b/tests/Standards/StandardsRunnerTest.php index c024387..91af4c8 100644 --- a/tests/Standards/StandardsRunnerTest.php +++ b/tests/Standards/StandardsRunnerTest.php @@ -71,13 +71,13 @@ public function __construct( ) {} /** Fails if standards orchestration unexpectedly performs a capture operation. */ - public function capture(array $arguments, string $directory): string + public function capture(array $arguments, string $directory, float $timeout = 120): string { throw new \LogicException('Not expected.'); } /** Records argument boundaries and returns the selected synthetic tool status. */ - public function run(array $arguments, string $directory): int + public function run(array $arguments, string $directory, float $timeout = 300): int { $this->commands[] = $arguments;