diff --git a/.changelog/structured-check-json.md b/.changelog/structured-check-json.md new file mode 100644 index 0000000..98ef483 --- /dev/null +++ b/.changelog/structured-check-json.md @@ -0,0 +1,6 @@ +--- +category: added +type: minor +--- + +Add explicit JSON check reports with native ECS findings, syntax results, incomplete-batch accounting and bounded nonstreaming capture; preserve default text and explicit fix behavior. diff --git a/README.md b/README.md index f9dbc27..81573b5 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,10 @@ composer test ## 🛠️ Usage `check` reports violations with a nonzero status and does not change source. +Use `check --all --format=json` for an explicit single-document machine report, +including native ECS findings, failure classification and remaining-work counts. +See the [structured check contract](docs/structured-check.rst). Text is the +default; `fix` uses text output. The default preset stores its ECS cache beneath the system temporary directory, in `fast-forward-code-standards/`. Different checkouts cannot invalidate each other's cache. Native diff --git a/bin/ecs-worker b/bin/ecs-worker index aec78cd..f7d3e96 100644 --- a/bin/ecs-worker +++ b/bin/ecs-worker @@ -5,11 +5,19 @@ declare(strict_types=1); use Symplify\EasyCodingStandard\Console\Command\CheckCommand; use Symplify\EasyCodingStandard\Console\Output\ConsoleOutputFormatter; +use Symplify\EasyCodingStandard\Console\Output\JsonOutputFormatter; use Symplify\EasyCodingStandard\DependencyInjection\EasyCodingStandardContainerFactory; // Internal process protocol: own autoloader, installed engine, config, mode, --, absolute files. try { - if ($argc < 7 || '--' !== $argv[5] || !in_array($argv[4], ['--check', '--fix'], true)) { + $json = '--format=json' === ($argv[5] ?? null); + $separator = $json ? 6 : 5; + if ( + $argc < $separator + 2 + || '--' !== $argv[$separator] + || !in_array($argv[4], ['--check', '--fix'], true) + || ($json && '--check' !== $argv[4]) + ) { throw new InvalidArgumentException('ECS worker requires installation paths, an explicit mode and selected files.'); } [$script, $autoload, $engine, $config, $mode] = array_slice($argv, 0, 5); @@ -18,7 +26,7 @@ try { throw new InvalidArgumentException('ECS worker requires readable absolute installation and config paths.'); } } - $files = array_slice($argv, 6); + $files = array_slice($argv, $separator + 1); foreach ($files as $file) { if (!str_starts_with($file, '/')) { throw new InvalidArgumentException('ECS worker requires absolute selected file paths.'); @@ -29,6 +37,8 @@ try { require $autoload; require_once $engine . '/vendor/squizlabs/php_codesniffer/autoload.php'; + $format = $json ? JsonOutputFormatter::getName() : ConsoleOutputFormatter::NAME; + $container = (new EasyCodingStandardContainerFactory())->createFromArgv(['ecs', 'check', '--config=' . $config]); $command = $container->make(CheckCommand::class); @@ -42,7 +52,7 @@ try { false, false, $config, - ConsoleOutputFormatter::NAME, + $format, '', '', '', diff --git a/docs/index.rst b/docs/index.rst index 03015bf..4b1c6ef 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -34,4 +34,5 @@ Architecture, policy and evidence rector resolved-preset static-analysis-evaluation + structured-check twig-evaluation diff --git a/docs/structured-check.rst b/docs/structured-check.rst new file mode 100644 index 0000000..2a845ec --- /dev/null +++ b/docs/structured-check.rst @@ -0,0 +1,101 @@ +Structured check output +======================= + +``check --format=json`` selects one nonstreaming JSON report on stdout. The default +``--format=text`` retains human output and existing exit behavior. ``fix`` does not +accept this option. Formatting and semantic refactoring remain separate operations. + +.. code-block:: console + + vendor/bin/code-standards check --all --format=json + vendor/bin/code-standards check --base=BASE_SHA --head=HEAD_SHA --format=json + +Use existing complete immutable commits for ``BASE_SHA`` and ``HEAD_SHA``. Selection, +consumer ignore rules, explicit ECS configuration and PHP runtime requirements are +the same as text checking. JSON checking never requests ``--fix``. Executable +configuration is not sandboxed; use the same unprivileged environment as text mode. + +Report contract +--------------- + +The report includes ``schema_version`` (1), ``command`` (``check``), ``status``, ``exit_code``, ``complete``, +``selection``, ``files``, ``errors`` and ``diagnostics.stderr``. ``files`` is an object +keyed by exact absolute paths. Native ``errors`` records retain line, file path, +message and checker identity; native ``diffs`` retain diff text and applied checkers. +No line numbers are guessed from a diff or a human PHP diagnostic. + +``selection.files`` counts selected files; ``syntax_checked`` counts completed +native PHP syntax invocations; ``style_submitted`` counts files submitted in +completed, consistently reported ECS batches. The corresponding ``syntax_remaining`` +and ``style_remaining`` counts identify unfinished work. ECS may reuse its cache or +honor consumer skips, so submitted is not a claim of fresh parsing of every file. + +``complete`` means all selected syntax invocations and ECS batches completed with +consistent reports. Checking stops at the first nonzero syntax result, findings +batch or operational error, matching the bounded text-mode policy. A findings +report can be incomplete when later batches remain. An empty selection is complete +and clean without starting a checker. + +.. list-table:: Result states + :header-rows: 1 + + * - Status + - Exit + - Meaning + * - ``clean`` + - 0 + - All selected work completed consistently without findings, or selection is empty. + * - ``findings`` + - 2 + - Native, consistently reported style violations or diffs; check ``complete``. + * - ``syntax-failed`` + - Native nonzero status + - ``php -n -l`` did not validate a selected file; exact diagnostics are data, not parsed classifications. + * - ``operational-error`` + - 1, or 2 for controlled selection failures + - Selection, execution, report validation or encoding could not establish a trustworthy result. + +Controlled selection failures retain text-mode status 2, including invalid input, +missing configuration, unavailable Git history, a mismatched HEAD or a dirty tracked +checkout. Unexpected failures and checker/report errors use status 1. + +Native stderr remains separate diagnostic data and is also written to stderr; +warnings alone do not imply failure. Operation errors additionally produce a fixed +stderr diagnostic. Invalid format names fail before execution with status 2 and +stderr output. Symfony binding/help errors and global quiet/silent flags retain +their native Console behavior; they are not JSON-report requests to an executed +check. + +Validation and limits +--------------------- + +The isolated worker uses the tool's own autoloader and native ECS JSON formatter. +Custom process overrides must implement the optional ``LimitedResultProcessRunner`` +capability for JSON checking. Existing ``ProcessRunner`` implementations retain +their text-mode contract; an unsupported JSON transport fails before a checker starts. +The parser checks the whole JSON document, native status, record shapes, selected +paths and counts. It does not strip banners or search for a later JSON fragment. +ECS totals can include system errors absent from its file inventory; mismatches +are operational errors, never clean results or ordinary style findings. + +Each ECS batch contains at most 50 paths, with a 300-second execution timeout, +2 MiB retained stdout and 64 KiB retained stderr limits. Syntax invocations use +300 seconds and 64 KiB per channel. Combined retained diagnostics are limited to +64 KiB. Output is limited during native process callbacks; overflow fails instead +of returning a truncated report. Symfony's transient pipe reads and the child's +memory are outside this retained-output bound. Termination covers the owned +process; no sandbox or descendant-process isolation is promised. + +Report strings must be representable as JSON without UTF-8 replacement. Invalid +native JSON, unsupported bytes or an oversized report fail objectively. A complete +machine report can be larger than human output because ECS JSON contains diffs; +``--no-diffs`` is not used as a false size guarantee. Native caches and explicit +consumer overrides remain active, as described in the configuration contract. + +This format covers checking, not automatic fixes, Rector reports, type analysis or +agent detection. Select it explicitly; environment detection never grants write +authority or overrides the requested format. + +The broader `structured-output contract (#10) `__ +also covers shared pretty/color controls and CWD/cache metadata. Those fields and +cross-command policies are outside this check schema. diff --git a/docs/usage/commands.rst b/docs/usage/commands.rst index 1a23ec3..1171634 100644 --- a/docs/usage/commands.rst +++ b/docs/usage/commands.rst @@ -7,6 +7,11 @@ accept ``--directory`` and ``--config``. Symfony Console supports both ``--option=value`` and ``--option value``. See ``help check`` and ``help fix`` for the installed command definitions. +``check`` additionally accepts ``--format=text|json`` with text as the default. +The explicit `structured check contract <../structured-check.rst>`__ documents +single-document stdout, native findings, operational failures and partial coverage. +``fix`` retains text output and does not accept this option. + The separate opt-in ``refactor`` command uses an explicit native Rector configuration's source paths rather than this Git changed-file selection. See the `semantic refactoring contract <../rector.rst>`__ for required target diff --git a/src/Console/Command/CheckCommand.php b/src/Console/Command/CheckCommand.php index 9e7c4a9..a0ceb68 100644 --- a/src/Console/Command/CheckCommand.php +++ b/src/Console/Command/CheckCommand.php @@ -6,8 +6,10 @@ use FastForward\CodeStandards\Console\Input\ScanInput; use FastForward\CodeStandards\Console\StandardsOperation; +use FastForward\CodeStandards\Console\StructuredCheckOperation; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Attribute\MapInput; +use Symfony\Component\Console\Attribute\Option; use Symfony\Component\Console\Style\SymfonyStyle; /** Exposes read-only checking as an invocable object adapted by Symfony. */ @@ -18,7 +20,7 @@ final readonly class CheckCommand { /** Injects the shared orchestration without parsing input or resolving a checkout. */ - public function __construct(private StandardsOperation $operation) {} + public function __construct(private StandardsOperation $operation, private StructuredCheckOperation $structured) {} /** Returns the exact check status; this entrypoint never grants write authority. */ public function __invoke( @@ -26,7 +28,19 @@ public function __invoke( #[MapInput] ScanInput $settings, + + #[Option(description: 'Output format: text or json. JSON is one nonstreaming check report.')] + string $format = 'text', ): int { + if ('json' === $format) { + return $this->structured->run($settings, $io); + } + if ('text' !== $format) { + $io->getErrorStyle()->error('Check format must be text or json.'); + + return 2; + } + return $this->operation->run($settings, $io, false); } } diff --git a/src/Console/StructuredCheckOperation.php b/src/Console/StructuredCheckOperation.php new file mode 100644 index 0000000..1cb518d --- /dev/null +++ b/src/Console/StructuredCheckOperation.php @@ -0,0 +1,70 @@ +projects->resolve($settings->all, $settings->base, $settings->head, $settings->directory, $settings->config); + $paths = $this->selection->select($scope->directory, $scope->base, $scope->head, $scope->config); + $files = $this->phpFiles->filter($scope->directory, $paths); + $result = $this->standards->run($scope->directory, $files, $scope->config); + } catch (Throwable $exception) { + $exitCode = $exception instanceof InvalidArgumentException || $exception instanceof RuntimeException ? 2 : 1; + $result = new CheckResult('operational-error', $exitCode, false, 0, 0, 0, errors: [ + ['phase' => 'selection', 'message' => $exception->getMessage()], + ]); + } + + return $this->present($result, $io); + } + + /** Rejects unrepresentable byte strings without silently replacing paths or truncating a report. */ + private function present(CheckResult $result, SymfonyStyle $io): int + { + try { + $json = json_encode($result->toArray(), JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES); + } catch (JsonException) { + $result = new CheckResult('operational-error', 1, false, $result->selected, $result->syntaxChecked, $result->styleSubmitted, errors: [ + ['phase' => 'encoding', 'message' => 'Check output contains bytes that cannot be represented as JSON.'], + ]); + $json = json_encode($result->toArray(), JSON_THROW_ON_ERROR); + } + if ([] !== $result->errors) { + $io->getErrorStyle()->writeln('Code Standards check did not complete successfully.', OutputInterface::OUTPUT_RAW); + } + if ('' !== $result->stderr) { + $io->getErrorStyle()->write($result->stderr, false, OutputInterface::OUTPUT_RAW); + } + $io->writeln($json, OutputInterface::OUTPUT_RAW); + + return $result->exitCode; + } +} diff --git a/src/Process/LimitedResultProcessRunner.php b/src/Process/LimitedResultProcessRunner.php new file mode 100644 index 0000000..5ab0988 --- /dev/null +++ b/src/Process/LimitedResultProcessRunner.php @@ -0,0 +1,25 @@ + $arguments Executable followed by literal arguments. + * @param string $directory Child working directory. + * @param positive-int $stdoutLimit Maximum retained stdout bytes. + * @param positive-int $stderrLimit Maximum retained stderr bytes. + * @param float $timeout Positive, finite execution limit in seconds. + * @return ProcessResult Completed status and exact output within the limits. + * @throws \Throwable When execution cannot complete within the supplied bounds. + */ + public function captureLimitedResult(array $arguments, string $directory, int $stdoutLimit, int $stderrLimit, float $timeout = 300): ProcessResult; +} diff --git a/src/Process/NativeProcessRunner.php b/src/Process/NativeProcessRunner.php index 0479349..46d3f36 100644 --- a/src/Process/NativeProcessRunner.php +++ b/src/Process/NativeProcessRunner.php @@ -5,10 +5,11 @@ namespace FastForward\CodeStandards\Process; use InvalidArgumentException; +use OverflowException; use Symfony\Component\Process\Process; /** Native transport; callers own selection and operation policy. */ -final class NativeProcessRunner implements ResultProcessRunner +final class NativeProcessRunner implements LimitedResultProcessRunner { /** Both captured and streamed configuration subprocesses receive the same credential policy. */ private const array ENVIRONMENT = ['GH_TOKEN' => false, 'GITHUB_TOKEN' => false, 'COMPOSER_AUTH' => false]; @@ -42,11 +43,46 @@ public function captureResult(array $arguments, string $directory, float $timeou return new ProcessResult($status, $process->getOutput(), $process->getErrorOutput()); } + /** Retains bounded channel bytes through Symfony's callback without its internal output buffers. */ + public function captureLimitedResult(array $arguments, string $directory, int $stdoutLimit, int $stderrLimit, float $timeout = 300): ProcessResult + { + if ($stdoutLimit < 1 || $stderrLimit < 1) { + throw new InvalidArgumentException('Captured output limits must be positive byte counts.'); + } + + $process = $this->createProcess($arguments, $directory, $timeout); + $process->disableOutput(); + $channels = [Process::OUT => '', Process::ERR => '']; + $limits = [Process::OUT => $stdoutLimit, Process::ERR => $stderrLimit]; + $collect = true; + + try { + $status = $process->run(static function (string $type, string $bytes) use (&$channels, $limits, &$collect): void { + if (!$collect) { + return; + } + if (strlen($bytes) > $limits[$type] - strlen($channels[$type])) { + $collect = false; + throw new OverflowException('Captured ' . $type . ' exceeded its byte limit.'); + } + + $channels[$type] .= $bytes; + }); + } finally { + $collect = false; + $process->stop(0); + } + + return new ProcessResult($status, $channels[Process::OUT], $channels[Process::ERR]); + } + /** @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) { + if ( + !is_finite($timeout) + || $timeout <= 0 + ) { throw new InvalidArgumentException('Subprocess timeout must be a positive finite number of seconds.'); } diff --git a/src/ServiceProvider/StandardsServiceProvider.php b/src/ServiceProvider/StandardsServiceProvider.php index 40cc472..8a1475a 100644 --- a/src/ServiceProvider/StandardsServiceProvider.php +++ b/src/ServiceProvider/StandardsServiceProvider.php @@ -11,6 +11,8 @@ use FastForward\CodeStandards\Project\ProjectScopeResolver; use FastForward\CodeStandards\Refactoring\RectorRunner; use FastForward\CodeStandards\Standards\StandardsRunner; +use FastForward\CodeStandards\Standards\StructuredCheckRunner; +use FastForward\CodeStandards\Standards\EcsJsonReportParser; use FastForward\Container\Factory\CallableFactory; use FastForward\Container\Factory\FactoryInterface; use FastForward\Container\Factory\InvokableFactory; @@ -37,7 +39,8 @@ public function __construct(private string $vendorDirectory, private OutputInter { $engine = realpath($vendorDirectory . '/symplify/easy-coding-standard'); $entrypoint = $engine . '/bin/ecs'; - if (!str_starts_with($vendorDirectory, '/') + if ( + !str_starts_with($vendorDirectory, '/') || false === $engine || !is_file($entrypoint) || !is_readable($entrypoint) @@ -73,6 +76,14 @@ public function getFactories(): array $this->vendorDirectory . '/autoload.php', $this->engineDirectory, ), + StructuredCheckRunner::class => new InvokableFactory( + StructuredCheckRunner::class, + ProcessRunner::class, + dirname(__DIR__, 2) . '/bin/ecs-worker', + $this->vendorDirectory . '/autoload.php', + $this->engineDirectory, + EcsJsonReportParser::class, + ), RectorRunner::class => new InvokableFactory( RectorRunner::class, ProcessRunner::class, diff --git a/src/Standards/CheckResult.php b/src/Standards/CheckResult.php new file mode 100644 index 0000000..cde4223 --- /dev/null +++ b/src/Standards/CheckResult.php @@ -0,0 +1,56 @@ + $files Validated native file diagnostics and diffs. + * @param list> $errors Operation diagnostics, separate from style findings. + */ + public function __construct( + public string $status, + public int $exitCode, + public bool $complete, + public int $selected, + public int $syntaxChecked, + public int $styleSubmitted, + public array $files = [], + public array $errors = [], + public string $stderr = '', + ) {} + + /** + * Serializes the outcome with explicit coverage and schema identity. + * + * @return array Versioned, single-document presentation without timing or human banners. + */ + public function toArray(): array + { + return [ + 'schema_version' => 1, + 'command' => 'check', + 'status' => $this->status, + 'exit_code' => $this->exitCode, + 'complete' => $this->complete, + 'selection' => [ + 'files' => $this->selected, + 'syntax_checked' => $this->syntaxChecked, + 'syntax_remaining' => $this->selected - $this->syntaxChecked, + 'style_submitted' => $this->styleSubmitted, + 'style_remaining' => $this->selected - $this->styleSubmitted, + ], + 'files' => (object) $this->files, + 'errors' => $this->errors, + 'diagnostics' => ['stderr' => $this->stderr], + ]; + } +} diff --git a/src/Standards/EcsJsonReportParser.php b/src/Standards/EcsJsonReportParser.php new file mode 100644 index 0000000..c4c6d5a --- /dev/null +++ b/src/Standards/EcsJsonReportParser.php @@ -0,0 +1,119 @@ + $selected Exact paths supplied to this worker invocation. + * @return array{errors:int,diffs:int,files:array>} Consistent native findings, never omitted system errors. + * @throws RuntimeException When status, schema, paths or totals cannot establish a complete report. + */ + public function parse(ProcessResult $result, array $selected): array + { + try { + $report = json_decode($result->stdout, true, 128, JSON_THROW_ON_ERROR); + } catch (JsonException $exception) { + throw new RuntimeException('ECS did not return one valid JSON report.', previous: $exception); + } + + if ( + !is_array($report) + || !$this->hasKeys($report, ['totals', 'files']) + || !is_array($report['totals']) + || !$this->hasKeys($report['totals'], ['errors', 'diffs']) + || !is_int($report['totals']['errors']) + || $report['totals']['errors'] < 0 + || !is_int($report['totals']['diffs']) + || $report['totals']['diffs'] < 0 + || !is_array($report['files']) + ) { + throw new RuntimeException('ECS returned an unsupported JSON report schema.'); + } + + $errors = $diffs = 0; + foreach ($report['files'] as $path => $file) { + if ( + !is_string($path) + || !in_array($path, $selected, true) + || !is_array($file) + || [] === $file + || [] !== array_diff(array_keys($file), ['errors', 'diffs']) + ) { + throw new RuntimeException('ECS returned an unexpected report path or file record.'); + } + foreach ($file as $kind => $records) { + if (!is_array($records) || !array_is_list($records) || [] === $records) { + throw new RuntimeException('ECS returned an invalid findings list.'); + } + foreach ($records as $record) { + $this->validateRecord($kind, $record, $path); + } + } + $errors += count($file['errors'] ?? []); + $diffs += count($file['diffs'] ?? []); + } + + if ($errors !== $report['totals']['errors'] || $diffs !== $report['totals']['diffs']) { + throw new RuntimeException('ECS totals include unreported or inconsistent findings; execution may have failed.'); + } + $expected = 0 === $errors + $diffs ? 0 : 2; + if ($expected !== $result->exitCode) { + throw new RuntimeException('ECS exit status does not agree with its JSON report.'); + } + + return ['errors' => $errors, 'diffs' => $diffs, 'files' => $report['files']]; + } + + /** Rejects malformed diagnostic records without parsing human messages or diffs. */ + private function validateRecord(string $kind, mixed $record, string $path): void + { + if (!is_array($record)) { + throw new RuntimeException('ECS returned an invalid finding record.'); + } + if ('errors' === $kind) { + if ( + !$this->hasKeys($record, ['line', 'file_path', 'message', 'source_class']) + || !is_int($record['line']) + || $record['line'] < 1 + || $path !== $record['file_path'] + || !is_string($record['message']) + || !is_string($record['source_class']) + || '' === $record['source_class'] + ) { + throw new RuntimeException('ECS returned an invalid diagnostic record.'); + } + + return; + } + if ( + !$this->hasKeys($record, ['diff', 'applied_checkers']) + || !is_string($record['diff']) + || !is_array($record['applied_checkers']) + || !array_is_list($record['applied_checkers']) + || [] === $record['applied_checkers'] + ) { + throw new RuntimeException('ECS returned an invalid diff record.'); + } + foreach ($record['applied_checkers'] as $checker) { + if (!is_string($checker) || '' === $checker) { + throw new RuntimeException('ECS returned an invalid checker identity.'); + } + } + } + + /** Validates native object fields without treating JSON member order as significant. */ + private function hasKeys(array $record, array $expected): bool + { + return count($record) === count($expected) && [] === array_diff(array_keys($record), $expected); + } +} diff --git a/src/Standards/EcsWorkerCommand.php b/src/Standards/EcsWorkerCommand.php new file mode 100644 index 0000000..a3812f9 --- /dev/null +++ b/src/Standards/EcsWorkerCommand.php @@ -0,0 +1,22 @@ + $files Absolute source paths, never shell-expanded. + * @return list Explicit installation, mode, format and selected-file arguments. + */ + public static function build(string $worker, string $autoload, string $engine, string $config, bool $fix, array $files, string $format = 'text'): array + { + $formatArguments = 'text' === $format ? [] : ['--format=' . $format]; + + return [PHP_BINARY, $worker, $autoload, $engine, $config, $fix ? '--fix' : '--check', ...$formatArguments, '--', ...$files]; + } +} diff --git a/src/Standards/StandardsRunner.php b/src/Standards/StandardsRunner.php index d063341..e024dc9 100644 --- a/src/Standards/StandardsRunner.php +++ b/src/Standards/StandardsRunner.php @@ -58,6 +58,6 @@ public function run(string $directory, array $files, string $config, bool $fix): */ private function formatterCommand(string $config, bool $fix, array $files): array { - return [PHP_BINARY, $this->worker, $this->autoload, $this->engine, $config, $fix ? '--fix' : '--check', '--', ...$files]; + return EcsWorkerCommand::build($this->worker, $this->autoload, $this->engine, $config, $fix, $files); } } diff --git a/src/Standards/StructuredCheckRunner.php b/src/Standards/StructuredCheckRunner.php new file mode 100644 index 0000000..30e7102 --- /dev/null +++ b/src/Standards/StructuredCheckRunner.php @@ -0,0 +1,85 @@ + $files Validated absolute PHP paths. + * @return CheckResult Complete findings or an honest partial result; no source-write authority is granted. + */ + public function run(string $directory, array $files, string $config): CheckResult + { + if ([] === $files) { + return new CheckResult('clean', 0, true, 0, 0, 0); + } + + $syntaxChecked = $styleSubmitted = 0; + $findings = []; + $phase = 'syntax'; + $stderr = ''; + + try { + if (!$this->process instanceof LimitedResultProcessRunner) { + throw new \RuntimeException('JSON checking requires limited result capture.'); + } + foreach ($files as $file) { + $result = $this->process->captureLimitedResult([PHP_BINARY, '-n', '-l', $file], $directory, 65_536, 65_536, 300); + $stderr = $this->appendDiagnostic($stderr, $result->stderr); + ++$syntaxChecked; + if (0 !== $result->exitCode) { + return new CheckResult('syntax-failed', $result->exitCode, false, count($files), $syntaxChecked, 0, errors: [ + ['phase' => 'syntax', 'file' => $file, 'stdout' => $result->stdout, 'stderr' => $result->stderr], + ], stderr: $stderr); + } + } + + $phase = 'ecs'; + foreach (array_chunk($files, 50) as $batch) { + $command = EcsWorkerCommand::build($this->worker, $this->autoload, $this->engine, $config, false, $batch, 'json'); + $result = $this->process->captureLimitedResult($command, $directory, 2_097_152, 65_536, 300); + $stderr = $this->appendDiagnostic($stderr, $result->stderr); + $report = $this->reports->parse($result, $batch); + $styleSubmitted += count($batch); + $findings += $report['files']; + if (0 !== $result->exitCode) { + return new CheckResult('findings', 2, count($files) === $styleSubmitted, count($files), $syntaxChecked, $styleSubmitted, $findings, stderr: $stderr); + } + } + + return new CheckResult('clean', 0, true, count($files), $syntaxChecked, $styleSubmitted, stderr: $stderr); + } catch (Throwable $exception) { + return new CheckResult('operational-error', 1, false, count($files), $syntaxChecked, $styleSubmitted, $findings, [ + ['phase' => $phase, 'message' => $exception->getMessage()], + ], $stderr); + } + } + + /** Keeps retained diagnostics bounded across the entire check without discarding warnings. */ + private function appendDiagnostic(string $previous, string $current): string + { + if (strlen($current) > 65_536 - strlen($previous)) { + throw new \OverflowException('Combined check diagnostics exceeded their byte limit.'); + } + + return $previous . $current; + } +} diff --git a/tests/Integration/StandardsTest.php b/tests/Integration/StandardsTest.php index ba4c308..39baa37 100644 --- a/tests/Integration/StandardsTest.php +++ b/tests/Integration/StandardsTest.php @@ -136,6 +136,35 @@ public function testInvalidHeadAndDirtyCheckoutFailClosed(): void self::assertStringContainsString('clean tracked checkout', $dirty->getErrorOutput()); } + /** Selection failures retain text-mode status while JSON reports remain incomplete and source stays unchanged. */ + public function testJsonSelectionFailuresPreserveTextModeExitStatus(): void + { + $this->put('a.php', 'commit(); + $this->put('a.php', 'commit(); + + foreach (['mismatch', 'dirty'] as $scenario) { + if ('dirty' === $scenario) { + $this->put('a.php', 'directory . '/a.php'); + $text = $this->cli($arguments); + $json = $this->cli([...$arguments, '--format=json']); + $report = json_decode($json->getOutput(), true, flags: JSON_THROW_ON_ERROR); + + self::assertSame(2, $text->getExitCode(), $scenario); + self::assertSame($text->getExitCode(), $json->getExitCode(), $scenario); + self::assertSame(2, $report['exit_code'], $scenario); + self::assertSame('operational-error', $report['status'], $scenario); + self::assertFalse($report['complete'], $scenario); + self::assertSame('selection', $report['errors'][0]['phase'], $scenario); + self::assertSame(0, $report['selection']['files'], $scenario); + self::assertSame($source, file_get_contents($this->directory . '/a.php'), $scenario); + } + } + /** Uses real ECS to prove violation status, source preservation and second-pass fix stability. */ public function testStyleCheckDoesNotWriteAndExplicitFixIsIdempotent(): void { @@ -156,6 +185,92 @@ public function testStyleCheckDoesNotWriteAndExplicitFixIsIdempotent(): void self::assertSame($fixed, file_get_contents($this->directory . '/' . $path)); } + /** Native JSON reports findings without banners or source changes, then clean output after explicit fix. */ + public function testExplicitJsonCheckUsesNativeReportsAndPreservesSource(): void + { + $path = "space and\nnewline.php"; + $bad = "put($path, $bad); + $this->commit(); + $check = $this->cli(['check', '--all', '--format=json']); + $report = json_decode($check->getOutput(), true, flags: JSON_THROW_ON_ERROR); + + self::assertSame(2, $check->getExitCode(), $check->getOutput() . $check->getErrorOutput()); + self::assertSame('check', $report['command']); + self::assertSame('findings', $report['status']); + self::assertTrue($report['complete']); + self::assertSame(0, $report['selection']['style_remaining']); + self::assertArrayHasKey($this->directory . '/' . $path, $report['files']); + self::assertSame($bad, file_get_contents($this->directory . '/' . $path)); + self::assertSame('', $check->getErrorOutput()); + self::assertSame('', $this->git(['diff'])); + + self::assertSame(0, $this->cli(['fix', '--all'])->getExitCode()); + $fixed = file_get_contents($this->directory . '/' . $path); + $clean = $this->cli(['check', '--all', '--format=json']); + $cleanReport = json_decode($clean->getOutput(), true, flags: JSON_THROW_ON_ERROR); + self::assertSame(0, $clean->getExitCode(), $clean->getOutput() . $clean->getErrorOutput()); + self::assertSame('clean', $cleanReport['status']); + self::assertSame($fixed, file_get_contents($this->directory . '/' . $path)); + } + + /** Syntax failures and executable-config output cannot masquerade as style findings or clean JSON. */ + public function testJsonCheckClassifiesSyntaxAndContaminatedConfigurationSeparately(): void + { + $broken = 'put('source.php', $broken); + $this->commit(); + $syntax = $this->cli(['check', '--all', '--format=json']); + $report = json_decode($syntax->getOutput(), true, flags: JSON_THROW_ON_ERROR); + self::assertNotSame(0, $syntax->getExitCode()); + self::assertSame('syntax-failed', $report['status']); + self::assertFalse($report['complete']); + self::assertSame(1, $report['selection']['style_remaining']); + self::assertSame($broken, file_get_contents($this->directory . '/source.php')); + + $this->put('source.php', "put('.build/ecs.php', 'cli(['check', '--all', '--format=json', '--config=.build/ecs.php']); + $report = json_decode($contaminated->getOutput(), true, flags: JSON_THROW_ON_ERROR); + self::assertSame(1, $contaminated->getExitCode()); + self::assertSame('operational-error', $report['status']); + self::assertFalse($report['complete']); + self::assertStringNotContainsString('consumer banner', $contaminated->getOutput()); + } + + /** Unsupported formats are rejected before config execution and fix does not acquire a JSON mode. */ + public function testCheckFormatsAreExplicitAndEmptySelectionHasACompleteReport(): void + { + $empty = $this->cli(['check', '--all', '--format=json']); + $report = json_decode($empty->getOutput(), true, flags: JSON_THROW_ON_ERROR); + self::assertSame(0, $empty->getExitCode()); + self::assertSame('check', $report['command']); + self::assertSame('clean', $report['status']); + self::assertSame(0, $report['selection']['files']); + self::assertTrue($report['complete']); + + $this->put('.build/ecs.php', 'cli(['check', '--all', '--format=xml', '--config=.build/ecs.php']); + self::assertSame(2, $invalid->getExitCode()); + self::assertSame('', $invalid->getOutput()); + self::assertFileDoesNotExist($this->directory . '/.build/ran'); + $fix = $this->cli(['fix', '--all', '--format=json']); + self::assertSame(2, $fix->getExitCode()); + } + + /** Unrepresentable source bytes cause an objective operational failure instead of lossy JSON coercion. */ + public function testJsonReportDoesNotReplaceInvalidUtf8InSourceDiffs(): void + { + $source = "put('source.php', $source); + $this->commit(); + $check = $this->cli(['check', '--all', '--format=json']); + $report = json_decode($check->getOutput(), true, flags: JSON_THROW_ON_ERROR); + self::assertSame(1, $check->getExitCode(), $check->getOutput()); + self::assertSame('operational-error', $report['status']); + self::assertSame($source, file_get_contents($this->directory . '/source.php')); + } + /** Preserves older parameter syntax by default, existing commas and explicit fix/check boundaries. */ public function testDefaultFormattingDoesNotIntroduceParameterCommasFromTheHostRuntime(): void { diff --git a/tests/Process/LimitedResultProcessRunnerTest.php b/tests/Process/LimitedResultProcessRunnerTest.php new file mode 100644 index 0000000..214f36d --- /dev/null +++ b/tests/Process/LimitedResultProcessRunnerTest.php @@ -0,0 +1,74 @@ +captureLimitedResult([ + PHP_BINARY, '-r', 'fwrite(STDOUT, "a\0\xff"); fwrite(STDERR, "b\0\xfe"); exit(23);', + ], sys_get_temp_dir(), 3, 3); + + self::assertSame(23, $result->exitCode); + self::assertSame("a\0\xff", $result->stdout); + self::assertSame("b\0\xfe", $result->stderr); + } + + /** Both overflow channels stop the known direct child; cleanup does not rethrow from its callback. */ + public function testOverflowTerminatesTheOwnedChild(): void + { + foreach (['STDOUT', 'STDERR'] as $channel) { + $marker = tempnam(sys_get_temp_dir(), 'limited-child-'); + try { + (new NativeProcessRunner())->captureLimitedResult([ + PHP_BINARY, '-r', + 'file_put_contents($argv[1], (string) getmypid()); fwrite(' . $channel . ', str_repeat("x", 65536)); usleep(2000000); file_put_contents($argv[1], "survived");', + $marker, + ], sys_get_temp_dir(), 64, 64, 3); + self::fail('Oversized output must raise an exception.'); + } catch (OverflowException $exception) { + self::assertStringContainsString('byte limit', $exception->getMessage()); + $pid = file_get_contents($marker); + self::assertMatchesRegularExpression('/^[1-9][0-9]*$/', $pid); + if (function_exists('posix_kill')) { + self::assertFalse(posix_kill((int) $pid, 0), 'The exact fixture child must have exited.'); + } + } finally { + unlink($marker); + } + } + } + + /** Native timeout and credential removal remain effective for limited capture. */ + public function testTimeoutAndEnvironmentUseTheExistingBoundary(): void + { + $runner = new NativeProcessRunner(); + $result = $runner->captureLimitedResult([ + PHP_BINARY, '-r', 'foreach (["GH_TOKEN", "GITHUB_TOKEN", "COMPOSER_AUTH"] as $key) { if (false !== getenv($key)) { exit(19); } } echo "ok";', + ], sys_get_temp_dir(), 2, 2); + self::assertSame(0, $result->exitCode); + self::assertSame('ok', $result->stdout); + $this->expectException(ProcessTimedOutException::class); + $runner->captureLimitedResult([PHP_BINARY, '-r', 'usleep(200000);'], sys_get_temp_dir(), 10, 10, 0.01); + } + + /** Invalid byte limits fail without starting the selected executable. */ + public function testInvalidOutputLimitsAreRejected(): void + { + $this->expectException(\InvalidArgumentException::class); + (new NativeProcessRunner())->captureLimitedResult([PHP_BINARY, '-r', 'exit(99);'], sys_get_temp_dir(), 0, 1); + } +} diff --git a/tests/Standards/EcsJsonReportParserTest.php b/tests/Standards/EcsJsonReportParserTest.php new file mode 100644 index 0000000..94c8140 --- /dev/null +++ b/tests/Standards/EcsJsonReportParserTest.php @@ -0,0 +1,63 @@ + 0, 'diffs' => 0, 'files' => []], (new EcsJsonReportParser())->parse($result, ['/source.php'])); + $reordered = new ProcessResult(0, '{"files":[],"totals":{"diffs":0,"errors":0}}', ''); + self::assertSame(['errors' => 0, 'diffs' => 0, 'files' => []], (new EcsJsonReportParser())->parse($reordered, ['/source.php'])); + } + + /** Valid diagnostics and diffs retain exact paths, messages and checker names. */ + public function testNativeFindingRecordsAreRetained(): void + { + $path = "/space and\nnewline.php"; + $file = [ + 'errors' => [['line' => 3, 'file_path' => $path, 'message' => 'Example', 'source_class' => 'ExampleSniff']], + 'diffs' => [['diff' => "--- original\n+++ new\n", 'applied_checkers' => ['ExampleFixer']]], + ]; + $result = new ProcessResult(2, json_encode(['totals' => ['errors' => 1, 'diffs' => 1], 'files' => [$path => $file]], JSON_THROW_ON_ERROR), ''); + $report = (new EcsJsonReportParser())->parse($result, [$path]); + self::assertSame($file, $report['files'][$path]); + self::assertSame(1, $report['errors']); + self::assertSame(1, $report['diffs']); + } + + /** Omitted system errors, unexpected status and contaminated/truncated reports fail closed. */ + public function testUnprovenOrMalformedReportsAreRejected(): void + { + $cases = [ + [2, '{"totals":{"errors":1,"diffs":0},"files":[]}'], + [2, '{"totals":{"errors":0,"diffs":0},"files":[]}'], + [1, '{"totals":{"errors":0,"diffs":0},"files":[]}'], + [0, 'banner {"totals":{"errors":0,"diffs":0},"files":[]}'], + [0, '{"totals":{"errors":0,"diffs":0},"files":[]} trailing'], + [0, '{"totals":'], + [0, '{"totals":{"errors":"0","diffs":0},"files":[]}'], + [0, '{"totals":{"errors":0,"diffs":0},"files":[],"extra":true}'], + [2, '{"totals":{"errors":0,"diffs":1},"files":{"/other.php":{"diffs":[{"diff":"x","applied_checkers":["X"]}]}}}'], + [2, '{"totals":{"errors":0,"diffs":1},"files":{"/source.php":{"diffs":[{"diff":"x","applied_checkers":[]}]}}}'], + ]; + foreach ($cases as [$status, $json]) { + try { + (new EcsJsonReportParser())->parse(new ProcessResult($status, $json, ''), ['/source.php']); + self::fail('Unproven report accepted: ' . $json); + } catch (RuntimeException $exception) { + self::assertStringContainsString('ECS', $exception->getMessage()); + } + } + } +} diff --git a/tests/Standards/QueuedReportProcess.php b/tests/Standards/QueuedReportProcess.php new file mode 100644 index 0000000..0d871e5 --- /dev/null +++ b/tests/Standards/QueuedReportProcess.php @@ -0,0 +1,45 @@ + $styleResults Completed ECS outcomes or transport failures. */ + public function __construct(private array $styleResults = [], private ?ProcessResult $syntaxResult = null) {} + + public function capture(array $arguments, string $directory, float $timeout = 120): string + { + throw new \LogicException('Legacy capture is not JSON transport.'); + } + + public function run(array $arguments, string $directory, float $timeout = 300): int + { + throw new \LogicException('JSON transport must not stream.'); + } + + public function captureResult(array $arguments, string $directory, float $timeout = 300): ProcessResult + { + throw new \LogicException('Unlimited result capture is not JSON transport.'); + } + + public function captureLimitedResult(array $arguments, string $directory, int $stdoutLimit, int $stderrLimit, float $timeout = 300): ProcessResult + { + $this->commands[] = $arguments; + $result = in_array('-l', $arguments, true) + ? ($this->syntaxResult ?? new ProcessResult(0, 'native lint success', '')) + : (array_shift($this->styleResults) ?? new ProcessResult(0, '{"totals":{"errors":0,"diffs":0},"files":[]}', '')); + if ($result instanceof \Throwable) { + throw $result; + } + + return $result; + } +} diff --git a/tests/Standards/StructuredCheckRunnerTest.php b/tests/Standards/StructuredCheckRunnerTest.php new file mode 100644 index 0000000..a3e3b44 --- /dev/null +++ b/tests/Standards/StructuredCheckRunnerTest.php @@ -0,0 +1,87 @@ +runner($process)->run('/repo', [], '/config'); + self::assertSame('clean', $result->status); + self::assertTrue($result->complete); + self::assertSame([], $process->commands); + } + + public function testSyntaxFailurePreventsEcsAndRetainsStatusWithoutParsingDiagnostics(): void + { + $process = new QueuedReportProcess(syntaxResult: new ProcessResult(255, 'native diagnostic', 'native stderr')); + $result = $this->runner($process)->run('/repo', ['/first.php', '/next.php'], '/config'); + self::assertSame('syntax-failed', $result->status); + self::assertSame(255, $result->exitCode); + self::assertFalse($result->complete); + self::assertSame(1, $result->syntaxChecked); + self::assertSame(2, $result->toArray()['selection']['style_remaining']); + self::assertCount(1, $process->commands); + } + + public function testFindingsStopAtTheFirstFailingBoundedBatch(): void + { + $files = array_map(static fn(int $number): string => '/repo/' . $number . '.php', range(1, 101)); + $findings = ['totals' => ['errors' => 0, 'diffs' => 1], 'files' => [$files[50] => ['diffs' => [['diff' => 'native diff', 'applied_checkers' => ['Fixer']]]]]]; + $process = new QueuedReportProcess([ + new ProcessResult(0, '{"totals":{"errors":0,"diffs":0},"files":[]}', ''), + new ProcessResult(2, json_encode($findings, JSON_THROW_ON_ERROR), ''), + ]); + $result = $this->runner($process)->run('/repo', $files, '/config'); + self::assertSame('findings', $result->status); + self::assertSame(2, $result->exitCode); + self::assertFalse($result->complete); + self::assertSame(101, $result->syntaxChecked); + self::assertSame(100, $result->styleSubmitted); + self::assertSame(1, $result->toArray()['selection']['style_remaining']); + self::assertCount(103, $process->commands); + self::assertContains('--format=json', $process->commands[101]); + self::assertNotContains('--fix', $process->commands[101]); + } + + public function testCleanBatchesAccountForEverySelectedFile(): void + { + foreach ([1, 50, 51, 100, 101] as $count) { + $files = array_map(static fn(int $number): string => '/repo/' . $number . '.php', range(1, $count)); + $process = new QueuedReportProcess(); + $result = $this->runner($process)->run('/repo', $files, '/config'); + self::assertSame('clean', $result->status); + self::assertTrue($result->complete); + self::assertSame($count, $result->syntaxChecked); + self::assertSame($count, $result->styleSubmitted); + self::assertSame(0, $result->toArray()['selection']['style_remaining']); + self::assertCount($count + (int) ceil($count / 50), $process->commands); + } + } + + public function testOperationalErrorsCannotBecomeFindings(): void + { + foreach ([new \OverflowException('too much output'), new ProcessResult(2, '{"totals":{"errors":1,"diffs":0},"files":[]}', 'native failure')] as $outcome) { + $result = $this->runner(new QueuedReportProcess([$outcome]))->run('/repo', ['/repo/source.php'], '/config'); + self::assertSame('operational-error', $result->status); + self::assertSame(1, $result->exitCode); + self::assertFalse($result->complete); + self::assertSame(0, $result->styleSubmitted); + self::assertSame([], $result->files); + } + } + + private function runner(QueuedReportProcess $process): StructuredCheckRunner + { + return new StructuredCheckRunner($process, '/worker', '/autoload', '/ecs', new EcsJsonReportParser()); + } +}