From b2173b91d7d325a5d404f19e6278867e45bd2b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Tue, 27 Oct 2026 12:36:33 -0300 Subject: [PATCH 1/5] feat: summarize explicitly enabled optional analyses --- .changelog/optional-analysis-summaries.md | 6 + .github/OptionalAnalysis/AnalysisResult.php | 50 ++++ .github/OptionalAnalysis/BoundedProcess.php | 227 ++++++++++++++++++ .../OptionalAnalysis/NativeReportParser.php | 217 +++++++++++++++++ .../OptionalAnalysisRunner.php | 110 +++++++++ .github/OptionalAnalysis/SummaryRenderer.php | 57 +++++ .github/OptionalAnalysis/WorkflowInput.php | 69 ++++++ .../WorkflowServiceProvider.php | 38 +++ .github/OptionalAnalysis/launch.php | 23 ++ .github/actions/analyze/action.yml | 51 ++++ .github/run-optional-analysis.php | 37 +++ .github/workflows/code-standards-analysis.yml | 148 ++++++++++++ .github/workflows/code-standards.yml | 31 +++ .github/workflows/repository-ci.yml | 4 + README.md | 13 +- docs/index.rst | 1 + docs/optional-analysis.rst | 138 +++++++++++ docs/rector.rst | 29 +++ docs/usage/commands.rst | 3 + resources/workflows/code-standards.yml | 6 + src/Console/Command/RefactorCommand.php | 5 +- src/Refactoring/RectorRunner.php | 15 +- .../OptionalAnalysisEntrypointTest.php | 88 +++++++ .../Automation/OptionalAnalysisInputTest.php | 99 ++++++++ .../OptionalAnalysisProcessTest.php | 144 +++++++++++ .../Automation/OptionalAnalysisReportTest.php | 182 ++++++++++++++ .../OptionalAnalysisWorkflowTest.php | 100 ++++++++ tests/Automation/WorkflowContextTest.php | 17 +- tests/Automation/WorkflowTest.php | 22 +- .../OptionalMagicNumbersReportTest.php | 52 ++++ .../Integration/RectorJsonIntegrationTest.php | 96 ++++++++ tests/Refactoring/RectorJsonPreviewTest.php | 74 ++++++ 32 files changed, 2133 insertions(+), 19 deletions(-) create mode 100644 .changelog/optional-analysis-summaries.md create mode 100644 .github/OptionalAnalysis/AnalysisResult.php create mode 100644 .github/OptionalAnalysis/BoundedProcess.php create mode 100644 .github/OptionalAnalysis/NativeReportParser.php create mode 100644 .github/OptionalAnalysis/OptionalAnalysisRunner.php create mode 100644 .github/OptionalAnalysis/SummaryRenderer.php create mode 100644 .github/OptionalAnalysis/WorkflowInput.php create mode 100644 .github/OptionalAnalysis/WorkflowServiceProvider.php create mode 100644 .github/OptionalAnalysis/launch.php create mode 100644 .github/actions/analyze/action.yml create mode 100644 .github/run-optional-analysis.php create mode 100644 .github/workflows/code-standards-analysis.yml create mode 100644 docs/optional-analysis.rst create mode 100644 tests/Automation/OptionalAnalysisEntrypointTest.php create mode 100644 tests/Automation/OptionalAnalysisInputTest.php create mode 100644 tests/Automation/OptionalAnalysisProcessTest.php create mode 100644 tests/Automation/OptionalAnalysisReportTest.php create mode 100644 tests/Automation/OptionalAnalysisWorkflowTest.php create mode 100644 tests/Integration/OptionalMagicNumbersReportTest.php create mode 100644 tests/Integration/RectorJsonIntegrationTest.php create mode 100644 tests/Refactoring/RectorJsonPreviewTest.php diff --git a/.changelog/optional-analysis-summaries.md b/.changelog/optional-analysis-summaries.md new file mode 100644 index 0000000..5853948 --- /dev/null +++ b/.changelog/optional-analysis-summaries.md @@ -0,0 +1,6 @@ +--- +category: added +type: minor +--- + +Add explicitly enabled Magic Numbers and Rector workflow analyses with bounded native-report classification, advisory or blocking findings policies, always-visible summaries and failing operational errors. Expose native Rector JSON preview without granting semantic apply authority. diff --git a/.github/OptionalAnalysis/AnalysisResult.php b/.github/OptionalAnalysis/AnalysisResult.php new file mode 100644 index 0000000..8d91034 --- /dev/null +++ b/.github/OptionalAnalysis/AnalysisResult.php @@ -0,0 +1,50 @@ + $diagnostics Native locations/messages. + * @param string $details Bounded later by the renderer, and never interpreted as Markdown or workflow commands. + * @param int|null $nativeExit Captured Code Standards invocation status; null if completion is unavailable. + */ + public function __construct( + public string $tool, + public string $status, + public ?int $selectedCount = null, + public ?int $findingCount = null, + public array $diagnostics = [], + public string $details = '', + public ?int $nativeExit = null, + ) { + if (!in_array($tool, ['Magic Numbers', 'Rector'], true) + || !in_array($status, ['disabled', 'not-executed', 'clean', 'findings', 'operational-error'], true) + || (null !== $selectedCount && $selectedCount < 0) + || (null !== $findingCount && $findingCount < 0) + || (null !== $nativeExit && $nativeExit < 0)) { + throw new InvalidArgumentException('Invalid optional-analysis result.'); + } + } + + /** Only verified findings may become advisory; errors stay failing under either policy. */ + public function blocks(string $policy): bool + { + if (!in_array($policy, ['report', 'fail'], true)) { + throw new InvalidArgumentException('Findings policy must be report or fail.'); + } + + return 'operational-error' === $this->status || ('findings' === $this->status && 'fail' === $policy); + } +} diff --git a/.github/OptionalAnalysis/BoundedProcess.php b/.github/OptionalAnalysis/BoundedProcess.php new file mode 100644 index 0000000..d07e71f --- /dev/null +++ b/.github/OptionalAnalysis/BoundedProcess.php @@ -0,0 +1,227 @@ + $arguments Installed Code Standards entrypoint and explicitly validated options. + */ + public function capture(array $arguments, string $directory, float $timeout = self::TIMEOUT_SECONDS): ProcessResult + { + $this->assertPrerequisites($timeout); + $deadline = hrtime(true) + (int) ($timeout * 1_000_000_000); + $environment = getenv(); + foreach (self::PRIVATE_ENVIRONMENT_FIELDS as $name) { + unset($environment[$name]); + } + $temporary = $this->temporary(); + $environment['TMPDIR'] = $temporary; + try { + $process = @proc_open([ + '/usr/bin/setsid', '--wait', PHP_BINARY, __DIR__ . '/launch.php', ...$arguments, + ], [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w'], 3 => ['pipe', 'w']], $pipes, $directory, $environment); + } catch (Throwable $exception) { + $this->removeTemporary($temporary); + + throw $exception; + } + if (!is_resource($process)) { + $this->removeTemporary($temporary); + throw new RuntimeException('Cannot start the owned optional-analyzer process.'); + } + $pid = proc_get_status($process)['pid']; + $confirmed = false; + try { + $this->confirmGroup($pipes[3], $pid, min($deadline, hrtime(true) + 2_000_000_000)); + $confirmed = true; + if (1 !== fwrite($pipes[0], 'S')) { + throw new RuntimeException('Cannot authorize the owned analyzer to start.'); + } + fclose($pipes[0]); + fclose($pipes[3]); + unset($pipes[0], $pipes[3]); + [$stdout, $stderr, $status] = $this->read($process, $pipes, $deadline); + } finally { + if ($confirmed) { + // This group was created and verified before any consumer configuration could execute. + @posix_kill(-$pid, 15); + @posix_kill(-$pid, 9); + } else { + // Before the start handshake, only the trusted launcher can exist. + @proc_terminate($process, 9); + } + foreach ($pipes as $pipe) { + if (is_resource($pipe)) { + fclose($pipe); + } + } + $closed = proc_close($process); + $this->removeTemporary($temporary); + } + $status ??= $closed; + if ($status < 0) { + throw new RuntimeException('Optional analyzer completion status is unavailable.'); + } + + return new ProcessResult($status, $stdout, $stderr); + } + + /** Missing hosted-Linux isolation support fails before launching anything; there is no weaker fallback. */ + private function assertPrerequisites(float $timeout): void + { + if (!is_finite($timeout) || $timeout <= 0 || $timeout > self::TIMEOUT_SECONDS) { + throw new InvalidArgumentException('Optional analyzer timeout must be positive, finite and at most 300 seconds.'); + } + if ('Linux' !== PHP_OS_FAMILY + || !is_executable('/usr/bin/setsid') + || !function_exists('posix_getpgid') + || !function_exists('posix_getsid') + || !function_exists('posix_kill')) { + throw new RuntimeException('Optional workflow execution requires hosted Linux, setsid and PHP POSIX process-group support.'); + } + } + + /** The trusted launcher blocks before payload execution until its own PID/session/group have been checked. */ + private function confirmGroup($control, int $pid, int $deadline): void + { + stream_set_blocking($control, false); + $identity = ''; + while (!str_contains($identity, "\n")) { + if (hrtime(true) >= $deadline) { + throw new RuntimeException('Owned analyzer session handshake timed out.'); + } + $bytes = fread($control, 32); + if (false === $bytes || ('' === $bytes && feof($control))) { + throw new RuntimeException('Owned analyzer session handshake failed.'); + } + $identity .= $bytes; + if (strlen($identity) > 32) { + throw new RuntimeException('Owned analyzer session identity is invalid.'); + } + usleep(1000); + } + if ($identity !== $pid . "\n" || @posix_getpgid($pid) !== $pid || @posix_getsid($pid) !== $pid) { + throw new RuntimeException('Owned analyzer process group could not be confirmed.'); + } + } + + /** + * Reads fixed chunks fairly from both channels and checks the deadline between reads. + * + * @param resource $process The exclusively owned trusted launcher resource. + * @param array $pipes Nonblocking stdout/stderr pipes, closed as EOF is observed. + * @return array{string, string, int|null} Exact bounded channels and the observed completion status. + */ + private function read($process, array &$pipes, int $deadline): array + { + foreach ($pipes as $pipe) { + stream_set_blocking($pipe, false); + } + $captured = [1 => '', 2 => '']; + $status = null; + do { + if (hrtime(true) >= $deadline) { + throw new RuntimeException('Optional analyzer exceeded its total invocation timeout; no clean result is available.'); + } + $ready = array_values($pipes); + $write = $except = []; + if ([] !== $ready && false === @stream_select($ready, $write, $except, 0, 100000)) { + throw new RuntimeException('Cannot read optional-analyzer channels.'); + } + foreach ($ready as $pipe) { + $channel = array_search($pipe, $pipes, true); + if (false === $channel) { + throw new RuntimeException('Optional analyzer channel identity is unavailable.'); + } + $limit = 1 === $channel ? self::REPORT_BYTES : self::STDERR_BYTES; + $bytes = fread($pipe, min(self::CHUNK_BYTES, $limit - strlen($captured[$channel]) + 1)); + if (false === $bytes || strlen($captured[$channel]) + strlen($bytes) > $limit) { + throw new RuntimeException('Optional analyzer output exceeded its capture limit; the incomplete report is not classified.'); + } + $captured[$channel] .= $bytes; + if (feof($pipe)) { + fclose($pipe); + unset($pipes[$channel]); + } + } + $state = proc_get_status($process); + if (!$state['running']) { + $status = $state['signaled'] ? 128 + $state['termsig'] : ($state['exitcode'] >= 0 ? $state['exitcode'] : $status); + } + if ([] === $pipes && $state['running']) { + usleep(10000); + } + } while ([] !== $pipes || $state['running']); + + return [$captured[1], $captured[2], $status]; + } + + /** Native preflight/cache temporary files belong to this invocation, never the consumer or a shared cache. */ + private function temporary(): string + { + $base = realpath(getenv('RUNNER_TEMP') ?: sys_get_temp_dir()); + if (false === $base) { + throw new RuntimeException('Runner temporary directory is unavailable.'); + } + $path = $base . '/standards-optional-' . bin2hex(random_bytes(12)); + if (!mkdir($path, 0700)) { + throw new RuntimeException('Cannot create owned analyzer temporary directory.'); + } + + return $path; + } + + /** Traverses only the freshly owned directory and never follows links created by executable configuration. */ + private function removeTemporary(string $directory, int &$remaining = 4096): void + { + if (is_link($directory) || is_file($directory)) { + if (!unlink($directory)) { + throw new RuntimeException('Cannot remove the owned temporary path replacement.'); + } + + return; + } + if (!is_dir($directory)) { + return; + } + foreach (new \FilesystemIterator($directory, \FilesystemIterator::SKIP_DOTS) as $file) { + if (--$remaining < 0) { + throw new RuntimeException('Owned analyzer temporary cleanup exceeded its entry bound; runner teardown must remove the remainder.'); + } + if ($file->isLink() || !$file->isDir()) { + if (!unlink($file->getPathname())) { + throw new RuntimeException('Cannot remove an owned analyzer temporary entry.'); + } + } else { + $this->removeTemporary($file->getPathname(), $remaining); + } + } + if (!rmdir($directory)) { + throw new RuntimeException('Cannot remove the owned analyzer temporary directory.'); + } + } +} diff --git a/.github/OptionalAnalysis/NativeReportParser.php b/.github/OptionalAnalysis/NativeReportParser.php new file mode 100644 index 0000000..282f1bd --- /dev/null +++ b/.github/OptionalAnalysis/NativeReportParser.php @@ -0,0 +1,217 @@ + $selectedFiles Exact absolute files from the existing Git/PHP selector. + * @throws InvalidArgumentException When output, counts, paths or status contradict the qualified schema. + */ + public function magicNumbers(ProcessResult $process, array $selectedFiles): AnalysisResult + { + $root = $this->xml($process->stdout); + $reportedFiles = $this->children($root, 'files'); + if (1 !== count($reportedFiles)) { + throw new InvalidArgumentException('PHPMND XML must contain exactly one files element.'); + } + $allowed = array_fill_keys($selectedFiles, true); + $diagnostics = []; + $seen = []; + foreach ($this->children($reportedFiles[0], 'file') as $file) { + $path = $file->getAttribute('path'); + if (!isset($allowed[$path]) || isset($seen[$path])) { + throw new InvalidArgumentException('PHPMND reported an unexpected or repeated selected path.'); + } + $seen[$path] = true; + $entries = $this->children($file, 'entry'); + if ([] === $entries || count($entries) !== $this->integer($file->getAttribute('errors'))) { + throw new InvalidArgumentException('PHPMND file finding count is inconsistent.'); + } + foreach ($entries as $entry) { + $line = $this->integer($entry->getAttribute('line')); + if ($line < 1) { + throw new InvalidArgumentException('PHPMND finding line must be positive.'); + } + $diagnostics[] = ['file' => $path, 'line' => $line, 'message' => 'Numeric literal requires review.']; + } + } + $count = $this->integer($root->getAttribute('errorCount')); + if ($count !== count($diagnostics) || count($seen) !== $this->integer($root->getAttribute('fileCount'))) { + throw new InvalidArgumentException('PHPMND report totals are inconsistent.'); + } + if ((0 === $count && 0 !== $process->exitCode) || (0 !== $count && 1 !== $process->exitCode)) { + throw new InvalidArgumentException('PHPMND exit contradicts its complete native report.'); + } + + return new AnalysisResult('Magic Numbers', 0 === $count ? 'clean' : 'findings', count($selectedFiles), $count, $diagnostics, $process->stderr, $process->exitCode); + } + + /** Native Rector errors remain blocking even when the same report also proposes changes. */ + public function rector(ProcessResult $process): AnalysisResult + { + if (strlen($process->stdout) > BoundedProcess::REPORT_BYTES) { + throw new InvalidArgumentException('Rector native report exceeds the supported size.'); + } + try { + $report = json_decode($process->stdout, true, flags: JSON_THROW_ON_ERROR); + } catch (JsonException $exception) { + throw new InvalidArgumentException('Rector did not return a complete native JSON document.', previous: $exception); + } + if (!is_array($report) || !isset($report['totals']) || !is_array($report['totals'])) { + throw new InvalidArgumentException('Rector JSON totals are unavailable.'); + } + $changes = $this->jsonInteger($report['totals']['changed_files'] ?? null); + $errorCount = $this->jsonInteger($report['totals']['errors'] ?? null); + $errors = $this->list($report['errors'] ?? []); + if ($errorCount !== count($errors)) { + throw new InvalidArgumentException('Rector error total is inconsistent.'); + } + $files = $this->list($report['changed_files'] ?? []); + if (count($files) !== $changes || count(array_filter($files, static fn(mixed $file): bool => is_string($file) && '' !== $file)) !== count($files)) { + throw new InvalidArgumentException('Rector changed-file list is inconsistent.'); + } + $diagnostics = [...$this->rectorErrors($errors), ...$this->rectorChanges($this->list($report['file_diffs'] ?? []), $files)]; + if (0 !== $errorCount) { + if (1 !== $process->exitCode) { + throw new InvalidArgumentException('Rector error report contradicts its exit.'); + } + + return new AnalysisResult('Rector', 'operational-error', findingCount: $changes, diagnostics: $diagnostics, details: $errorCount . " native processing error(s).\n" . $process->stderr, nativeExit: $process->exitCode); + } + if ((0 === $changes && 0 !== $process->exitCode) || (0 !== $changes && 2 !== $process->exitCode)) { + throw new InvalidArgumentException('Rector preview exit contradicts its native totals.'); + } + return new AnalysisResult('Rector', 0 === $changes ? 'clean' : 'findings', findingCount: $changes, diagnostics: $diagnostics, details: "Scope is the explicit native configuration, not the formatter changed-file list.\n" . $process->stderr, nativeExit: $process->exitCode); + } + + /** Parses without external resolution/substitution, then rejects DTDs; CDATA remains ordinary source text. */ + private function xml(string $output): DOMElement + { + if (strlen($output) > BoundedProcess::REPORT_BYTES || !extension_loaded('dom') || 1 !== preg_match('//u', $output) || str_contains($output, "\0")) { + throw new InvalidArgumentException('PHPMND requires UTF-8 native XML without DTD/entity declarations and an available DOM extension.'); + } + $previous = libxml_use_internal_errors(true); + try { + $document = new DOMDocument(); + $document->resolveExternals = false; + $document->substituteEntities = false; + $document->validateOnParse = false; + if (!$document->loadXML($output, LIBXML_NONET) || null !== $document->doctype || 'phpmnd' !== $document->documentElement?->tagName) { + throw new InvalidArgumentException('PHPMND did not return a complete native XML document.'); + } + if ('' === $document->documentElement->getAttribute('version')) { + throw new InvalidArgumentException('PHPMND XML version is unavailable.'); + } + + return $document->documentElement; + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($previous); + } + } + + /** @return list Direct structural children only; snippets are not interpreted as XML schema. */ + private function children(DOMElement $parent, string $name): array + { + $elements = []; + foreach ($parent->childNodes as $child) { + if ($child instanceof DOMElement) { + if ($name !== $child->tagName) { + throw new InvalidArgumentException('Unexpected PHPMND XML structure.'); + } + $elements[] = $child; + } elseif (XML_TEXT_NODE === $child->nodeType && '' !== trim($child->textContent)) { + throw new InvalidArgumentException('Unexpected text in PHPMND XML structure.'); + } + } + + return $elements; + } + + /** Exact nonnegative decimal strings avoid coercion, overflow and misleading native counts. */ + private function integer(string $value): int + { + $integer = (int) $value; + if (1 !== preg_match('/\A(?:0|[1-9][0-9]*)\z/D', $value) || (string) $integer !== $value) { + throw new InvalidArgumentException('Native report count is invalid.'); + } + + return $integer; + } + + /** Native JSON counts must be integers rather than coerced strings or floating point values. */ + private function jsonInteger(mixed $value): int + { + if (!is_int($value) || $value < 0) { + throw new InvalidArgumentException('Rector JSON count is invalid.'); + } + + return $value; + } + + /** @return list Native ordered report entries, not an arbitrary keyed JSON object. */ + private function list(mixed $value): array + { + if (!is_array($value) || !array_is_list($value)) { + throw new InvalidArgumentException('Rector JSON entries must be a list.'); + } + + return $value; + } + + /** + * Retains actual native error locations; an unavailable path is not replaced with a guessed source file. + * + * @param list $errors Native error records. + * @return list + */ + private function rectorErrors(array $errors): array + { + $diagnostics = []; + foreach ($errors as $error) { + if (!is_array($error) + || (isset($error['file']) && !is_string($error['file'])) + || !is_string($error['message'] ?? null) + || (isset($error['line']) && (!is_int($error['line']) || $error['line'] < 1))) { + throw new InvalidArgumentException('Rector JSON error location is invalid.'); + } + $diagnostics[] = ['file' => $error['file'] ?? '[unavailable]', 'line' => $error['line'] ?? null, 'message' => $error['message']]; + } + + return $diagnostics; + } + + /** + * Lists native changed files without interpreting diff text as line or AST identity. + * + * @param list $changes Optional native file-diff records, validated without displaying source. + * @param list $files The native changed-file list corresponding to the total. + * @return list + */ + private function rectorChanges(array $changes, array $files): array + { + foreach ($changes as $change) { + if (!is_array($change) || !is_string($change['file'] ?? null) || !in_array($change['file'], $files, true) || !is_string($change['diff'] ?? null)) { + throw new InvalidArgumentException('Rector JSON change location is invalid.'); + } + } + $diagnostics = []; + foreach ($files as $file) { + $diagnostics[] = ['file' => $file, 'line' => null, 'message' => 'Semantic changes proposed; the native JSON does not provide a line field.']; + } + + return $diagnostics; + } +} diff --git a/.github/OptionalAnalysis/OptionalAnalysisRunner.php b/.github/OptionalAnalysis/OptionalAnalysisRunner.php new file mode 100644 index 0000000..ec75488 --- /dev/null +++ b/.github/OptionalAnalysis/OptionalAnalysisRunner.php @@ -0,0 +1,110 @@ + A result exists for both tools, including disabled/error paths. */ + public function run(WorkflowInput $input): array + { + $magic = new AnalysisResult('Magic Numbers', 'disabled', details: 'Not enabled; no analyzer executed.'); + $rector = new AnalysisResult('Rector', 'disabled', details: 'Not enabled; no analyzer executed.'); + if (!$input->magicNumbers && !$input->rector) { + return [$magic, $rector]; + } + try { + $directory = realpath($input->directory); + if (false === $directory || !is_dir($directory)) { + throw new InvalidArgumentException('Consumer directory is unavailable.'); + } + $paths = $this->selection->select($directory, $input->base, $input->head); + $files = $this->phpFiles->filter($directory, $paths); + } catch (Throwable $exception) { + return [ + $input->magicNumbers ? $this->failure('Magic Numbers', $exception) : $magic, + $input->rector ? $this->failure('Rector', $exception) : $rector, + ]; + } + if ($input->magicNumbers) { + $magic = $this->magicNumbers($input, $directory, $files); + } + if ($input->rector) { + $rector = $this->rector($input, $directory); + } + + return [$magic, $rector]; + } + + /** @param list $files Eligible paths selected by the existing exact-PR/ignore contract. */ + private function magicNumbers(WorkflowInput $input, string $directory, array $files): AnalysisResult + { + if ([] === $files) { + return new AnalysisResult('Magic Numbers', 'not-executed', 0, details: 'No eligible changed PHP files; the engine was not executed.'); + } + $result = null; + try { + $result = $this->process->capture([ + PHP_BINARY, '-d', 'memory_limit=256M', $this->root . '/bin/code-standards', 'magic-numbers', + '--directory=' . $directory, '--base=' . $input->base, '--head=' . $input->head, '--format=xml', + ], $directory); + + return $this->reports->magicNumbers($result, $files); + } catch (Throwable $exception) { + return $this->failure('Magic Numbers', $exception, count($files), $result); + } + } + + /** Rector retains explicitly configured whole-project paths and always runs preview, never apply. */ + private function rector(WorkflowInput $input, string $directory): AnalysisResult + { + $result = null; + try { + if ('' === $input->rectorConfig || str_starts_with($input->rectorConfig, '/')) { + throw new InvalidArgumentException('Enabled Rector requires a consumer-relative rector-config input.'); + } + $scope = $this->projects->resolve(true, null, null, $directory, $input->rectorConfig); + if (!str_starts_with($scope->config, $directory . '/')) { + throw new InvalidArgumentException('Workflow Rector configuration must resolve inside the consumer checkout.'); + } + $result = $this->process->capture([ + PHP_BINARY, '-d', 'memory_limit=256M', $this->root . '/bin/code-standards', 'refactor', + '--directory=' . $directory, '--config=' . $scope->config, '--format=json', + ], $directory); + + return $this->reports->rector($result); + } catch (Throwable $exception) { + return $this->failure('Rector', $exception, process: $result); + } + } + + /** Exceptions never turn into advisory findings or an unavailable zero count. */ + private function failure(string $tool, Throwable $exception, ?int $selected = null, ?ProcessResult $process = null): AnalysisResult + { + $details = $exception->getMessage(); + if (null !== $process && '' !== $process->stderr) { + $details .= "\nNative stderr: " . $process->stderr; + } + + return new AnalysisResult($tool, 'operational-error', $selected, details: $details, nativeExit: $process?->exitCode); + } +} diff --git a/.github/OptionalAnalysis/SummaryRenderer.php b/.github/OptionalAnalysis/SummaryRenderer.php new file mode 100644 index 0000000..aec9c2f --- /dev/null +++ b/.github/OptionalAnalysis/SummaryRenderer.php @@ -0,0 +1,57 @@ + $results Known tools/results, independent of findings severity. */ + public function render(array $results): string + { + $summary = "## Optional Code Standards analyses\n\n"; + $summary .= "| Tool | Status | Selected PHP files | Native count | Invocation exit |\n| --- | --- | ---: | ---: | ---: |\n"; + foreach ($results as $result) { + $label = 'Rector' === $result->tool ? 'changed files' : 'findings'; + $summary .= '| ' . $result->tool . ' | ' . $result->status . ' | ' . ($result->selectedCount ?? 'unknown/native scope') . ' | ' . ($result->findingCount ?? 'unavailable') . ' ' . $label . ' | ' . ($result->nativeExit ?? 'not available') . " |\n"; + } + $summary .= "\n"; + foreach ($results as $result) { + $summary .= '### ' . $result->tool . "\n\n"; + $summary .= "
\n";
+            $lines = '' === $result->details ? [] : [$result->details];
+            foreach (array_slice($result->diagnostics, 0, self::MAX_DIAGNOSTICS) as $diagnostic) {
+                $lines[] = $diagnostic['file'] . ':' . ($diagnostic['line'] ?? '?') . ' — ' . $diagnostic['message'];
+            }
+            foreach ($lines as $line) {
+                $escaped = $this->escape($line) . "\n";
+                if (strlen($summary) + strlen($escaped) > self::MAX_BYTES - 512) {
+                    return $summary . "
\n\nAdditional diagnostics omitted to keep the summary bounded. Native totals above are unchanged.\n"; + } + $summary .= $escaped; + } + $summary .= "\n\n"; + if (count($result->diagnostics) > self::MAX_DIAGNOSTICS) { + $summary .= "Additional diagnostics omitted; native totals are not truncated.\n\n"; + } + } + + return $summary; + } + + /** Control bytes remain visible and escaped; untrusted closing tags cannot end the preformatted block. */ + private function escape(string $text): string + { + $truncated = strlen($text) > self::MAX_FIELD_BYTES; + $text = substr($text, 0, self::MAX_FIELD_BYTES); + $quoted = json_encode($text, JSON_INVALID_UTF8_SUBSTITUTE | JSON_UNESCAPED_SLASHES); + $visible = substr($quoted, 1, -1) . ($truncated ? '… [truncated]' : ''); + + return htmlspecialchars($visible, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'); + } +} diff --git a/.github/OptionalAnalysis/WorkflowInput.php b/.github/OptionalAnalysis/WorkflowInput.php new file mode 100644 index 0000000..5f633c9 --- /dev/null +++ b/.github/OptionalAnalysis/WorkflowInput.php @@ -0,0 +1,69 @@ + $environment Literal environment fields supplied by the composite action. */ + public static function fromEnvironment(array $environment): self + { + return new self( + self::field($environment, 'STANDARDS_DIRECTORY'), + self::field($environment, 'STANDARDS_BASE'), + self::field($environment, 'STANDARDS_HEAD'), + self::boolean(self::field($environment, 'STANDARDS_MAGIC_NUMBERS')), + self::field($environment, 'STANDARDS_MAGIC_NUMBERS_POLICY'), + self::boolean(self::field($environment, 'STANDARDS_RECTOR')), + self::field($environment, 'STANDARDS_RECTOR_POLICY'), + self::field($environment, 'STANDARDS_RECTOR_CONFIG'), + ); + } + + /** Missing fields fail explicitly rather than becoming a silently disabled analysis. */ + private static function field(array $environment, string $name): string + { + if (!isset($environment[$name]) || !is_string($environment[$name])) { + throw new InvalidArgumentException('Missing optional-analysis input: ' . $name); + } + + return $environment[$name]; + } + + /** GitHub booleans have a literal true/false representation; other strings are not coerced. */ + private static function boolean(string $value): bool + { + if (!in_array($value, ['true', 'false'], true)) { + throw new InvalidArgumentException('Optional analysis enable inputs must be true or false.'); + } + + return 'true' === $value; + } +} diff --git a/.github/OptionalAnalysis/WorkflowServiceProvider.php b/.github/OptionalAnalysis/WorkflowServiceProvider.php new file mode 100644 index 0000000..e725761 --- /dev/null +++ b/.github/OptionalAnalysis/WorkflowServiceProvider.php @@ -0,0 +1,38 @@ + The scalar checkout identity cannot be inferred by autowiring. */ + public function getFactories(): array + { + return [OptionalAnalysisRunner::class => new InvokableFactory( + OptionalAnalysisRunner::class, + FileSelection::class, + PhpFiles::class, + ProjectScopeResolver::class, + BoundedProcess::class, + NativeReportParser::class, + $this->root, + )]; + } + + /** @return array This helper decorates no public runtime service. */ + public function getExtensions(): array + { + return []; + } +} diff --git a/.github/OptionalAnalysis/launch.php b/.github/OptionalAnalysis/launch.php new file mode 100644 index 0000000..dde3693 --- /dev/null +++ b/.github/OptionalAnalysis/launch.php @@ -0,0 +1,23 @@ + ['file', '/dev/null', 'r'], 1 => STDOUT, 2 => STDERR], $pipes); +if (!is_resource($process)) { + exit(1); +} +exit(proc_close($process)); diff --git a/.github/actions/analyze/action.yml b/.github/actions/analyze/action.yml new file mode 100644 index 0000000..f9880dc --- /dev/null +++ b/.github/actions/analyze/action.yml @@ -0,0 +1,51 @@ +name: Analyze optional PHP quality +description: Classify explicitly enabled native analyzers and write a bounded job summary. +inputs: + directory: + required: true + description: Absolute exact consumer checkout directory. + base-sha: + required: true + description: Exact PR base commit. + head-sha: + required: true + description: Exact PR head commit. + magic-numbers: + default: 'false' + description: Explicitly enable native Magic Numbers analysis. + magic-numbers-policy: + default: report + description: report or fail for verified Magic Numbers findings; operational errors always block. + rector: + default: 'false' + description: Explicitly enable native Rector preview, never semantic apply. + rector-policy: + default: report + description: report or fail for verified Rector changes; operational errors always block. + rector-config: + default: '' + description: Required consumer-relative native PHP configuration when Rector is enabled. +outputs: + summary-written: + description: True only after the classified results were written to the job summary. + value: ${{ steps.report.outputs.summary-written }} + blocking: + description: Explicit policy decision; the workflow must enforce it in its final gate. + value: ${{ steps.report.outputs.blocking }} +runs: + using: composite + steps: + - name: Classify optional results and write summary + id: report + shell: bash + env: + STANDARDS_ROOT: ${{ github.action_path }}/../../.. + STANDARDS_DIRECTORY: ${{ inputs.directory }} + STANDARDS_BASE: ${{ inputs.base-sha }} + STANDARDS_HEAD: ${{ inputs.head-sha }} + STANDARDS_MAGIC_NUMBERS: ${{ inputs.magic-numbers }} + STANDARDS_MAGIC_NUMBERS_POLICY: ${{ inputs.magic-numbers-policy }} + STANDARDS_RECTOR: ${{ inputs.rector }} + STANDARDS_RECTOR_POLICY: ${{ inputs.rector-policy }} + STANDARDS_RECTOR_CONFIG: ${{ inputs.rector-config }} + run: php "$STANDARDS_ROOT/.github/run-optional-analysis.php" diff --git a/.github/run-optional-analysis.php b/.github/run-optional-analysis.php new file mode 100644 index 0000000..de2bf68 --- /dev/null +++ b/.github/run-optional-analysis.php @@ -0,0 +1,37 @@ +get(OptionalAnalysisRunner::class)->run($input); +$summary = $container->get(SummaryRenderer::class)->render($results); +$path = getenv('GITHUB_STEP_SUMMARY'); +if (!is_string($path) || '' === $path || strlen($summary) !== file_put_contents($path, $summary, FILE_APPEND)) { + throw new RuntimeException('Cannot write the optional-analysis summary.'); +} +$policies = ['Magic Numbers' => $input->magicNumbersPolicy, 'Rector' => $input->rectorPolicy]; +$failure = false; +foreach ($results as $result) { + $failure = $result->blocks($policies[$result->tool]) || $failure; +} +$output = getenv('GITHUB_OUTPUT'); +$values = "summary-written=true\nblocking=" . ($failure ? 'true' : 'false') . "\n"; +if (!is_string($output) || '' === $output || strlen($values) !== file_put_contents($output, $values, FILE_APPEND)) { + throw new RuntimeException('Cannot record optional-analysis summary completion and policy.'); +} +// Classification completed; the workflow's mandatory final gate applies this explicit blocking decision. +exit(0); diff --git a/.github/workflows/code-standards-analysis.yml b/.github/workflows/code-standards-analysis.yml new file mode 100644 index 0000000..1f56e6c --- /dev/null +++ b/.github/workflows/code-standards-analysis.yml @@ -0,0 +1,148 @@ +name: 'Internal: Optional PHP Analysis' +on: + workflow_call: + inputs: + php-version: + default: '8.3' + type: string + magic-numbers: + default: false + type: boolean + magic-numbers-policy: + default: report + type: string + rector: + default: false + type: boolean + rector-policy: + default: report + type: string + rector-config: + default: '' + type: string +permissions: + contents: read +jobs: + analyze: + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Reject privileged events and invalid refs + id: runtime + env: + STANDARDS_EVENT: ${{ github.event_name }} + STANDARDS_JOB_JSON: ${{ toJSON(job) }} + STANDARDS_BASE: ${{ github.event.pull_request.base.sha }} + STANDARDS_HEAD: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + [[ "$STANDARDS_EVENT" == pull_request ]] || exit 1 + [[ "$STANDARDS_BASE" =~ ^[a-fA-F0-9]{40}$ ]] || exit 1 + [[ "$STANDARDS_HEAD" =~ ^[a-fA-F0-9]{40}$ ]] || exit 1 + runtime=$(jq --slurp --raw-output --exit-status ' + select(length == 1) | .[0] | select(type == "object") + | select(.workflow_repository | type == "string") + | select(.workflow_repository == "php-fast-forward/code-standards") + | select(.workflow_sha | type == "string") + | select(.workflow_sha | length == 40 and test("^[a-f0-9]+$")) + | .workflow_sha + ' <<< "$STANDARDS_JOB_JSON") || { + echo '::error::Invalid executing workflow identity in native job context.' >&2 + exit 1 + } + { + printf 'repository=php-fast-forward/code-standards\n' + printf 'sha=%s\n' "$runtime" + } >> "$GITHUB_OUTPUT" + - name: Validate explicit optional findings policies + env: + STANDARDS_MAGIC_POLICY: ${{ inputs.magic-numbers-policy }} + STANDARDS_RECTOR_POLICY: ${{ inputs.rector-policy }} + run: | + set -euo pipefail + for policy in "$STANDARDS_MAGIC_POLICY" "$STANDARDS_RECTOR_POLICY"; do + [[ "$policy" == report || "$policy" == fail ]] || { + echo '::error::Optional findings policies must be report or fail.' >&2 + exit 1 + } + done + - name: Check out reviewed tool runtime + if: inputs.magic-numbers || inputs.rector + uses: actions/checkout@v7 + with: + repository: ${{ steps.runtime.outputs.repository }} + ref: ${{ steps.runtime.outputs.sha }} + path: tool + persist-credentials: false + - name: Set up PHP + if: inputs.magic-numbers || inputs.rector + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ inputs.php-version }} + extensions: dom, posix + coverage: none + tools: composer:v2 + - name: Install only the reviewed tool + if: inputs.magic-numbers || inputs.rector + working-directory: tool + env: + COMPOSER_HOME: ${{ runner.temp }}/standards-analysis-composer-home + COMPOSER_CACHE_DIR: ${{ runner.temp }}/standards-analysis-composer-cache + run: composer install --no-dev --no-plugins --no-scripts --no-interaction --prefer-dist --no-progress + - name: Check out exact consumer head + if: inputs.magic-numbers || inputs.rector + uses: actions/checkout@v7 + with: + ref: ${{ github.event.pull_request.head.sha }} + path: consumer + fetch-depth: 0 + persist-credentials: false + - name: Classify optional analyses + id: analysis + if: inputs.magic-numbers || inputs.rector + uses: ./tool/.github/actions/analyze + with: + directory: ${{ github.workspace }}/consumer + base-sha: ${{ github.event.pull_request.base.sha }} + head-sha: ${{ github.event.pull_request.head.sha }} + magic-numbers: ${{ inputs.magic-numbers }} + magic-numbers-policy: ${{ inputs.magic-numbers-policy }} + rector: ${{ inputs.rector }} + rector-policy: ${{ inputs.rector-policy }} + rector-config: ${{ inputs.rector-config }} + - name: Always summarize unavailable or disabled analyses + if: always() && steps.analysis.outputs.summary-written != 'true' + env: + STANDARDS_MAGIC_ENABLED: ${{ inputs.magic-numbers }} + STANDARDS_RECTOR_ENABLED: ${{ inputs.rector }} + run: | + set -euo pipefail + { + printf '## Optional Code Standards analyses\n\n' + printf '| Tool | Status | Findings |\n| --- | --- | --- |\n' + if [[ "$STANDARDS_MAGIC_ENABLED" == true ]]; then + printf '| Magic Numbers | operational-error | unavailable |\n' + else + printf '| Magic Numbers | disabled | not executed |\n' + fi + if [[ "$STANDARDS_RECTOR_ENABLED" == true ]]; then + printf '| Rector | operational-error | unavailable |\n' + else + printf '| Rector | disabled | not executed |\n' + fi + printf '\nEnabled analyses without a complete classified result are operational errors, never clean passes.\n' + } >> "$GITHUB_STEP_SUMMARY" + - name: Enforce explicit optional-analysis result + if: always() + env: + STANDARDS_MAGIC_ENABLED: ${{ inputs.magic-numbers }} + STANDARDS_RECTOR_ENABLED: ${{ inputs.rector }} + STANDARDS_SUMMARY_WRITTEN: ${{ steps.analysis.outputs.summary-written }} + STANDARDS_BLOCKING: ${{ steps.analysis.outputs.blocking }} + run: | + set -euo pipefail + if [[ "$STANDARDS_MAGIC_ENABLED" != true && "$STANDARDS_RECTOR_ENABLED" != true ]]; then + exit 0 + fi + [[ "$STANDARDS_SUMMARY_WRITTEN" == true ]] || exit 1 + [[ "$STANDARDS_BLOCKING" == false ]] diff --git a/.github/workflows/code-standards.yml b/.github/workflows/code-standards.yml index 0f2fd20..64a300c 100644 --- a/.github/workflows/code-standards.yml +++ b/.github/workflows/code-standards.yml @@ -10,6 +10,26 @@ on: description: Optional consumer-relative ECS config. default: '' type: string + magic-numbers: + description: Explicitly enable native Magic Numbers analysis of the exact PR selection. + default: false + type: boolean + magic-numbers-policy: + description: report or fail for verified findings; operational errors always fail. + default: report + type: string + rector: + description: Explicitly enable native semantic preview using rector-config, never apply. + default: false + type: boolean + rector-policy: + description: report or fail for verified proposed changes; operational errors always fail. + default: report + type: string + rector-config: + description: Consumer-relative native PHP configuration with explicit target and rules. + default: '' + type: string permissions: {} jobs: check: @@ -19,3 +39,14 @@ jobs: with: php-version: ${{ inputs.php-version }} config: ${{ inputs.config }} + optional-analysis: + permissions: + contents: read + uses: ./.github/workflows/code-standards-analysis.yml + with: + php-version: ${{ inputs.php-version }} + magic-numbers: ${{ inputs.magic-numbers }} + magic-numbers-policy: ${{ inputs.magic-numbers-policy }} + rector: ${{ inputs.rector }} + rector-policy: ${{ inputs.rector-policy }} + rector-config: ${{ inputs.rector-config }} diff --git a/.github/workflows/repository-ci.yml b/.github/workflows/repository-ci.yml index 849c88b..7145879 100644 --- a/.github/workflows/repository-ci.yml +++ b/.github/workflows/repository-ci.yml @@ -10,6 +10,9 @@ jobs: reusable-check: if: github.event_name == 'pull_request' uses: ./.github/workflows/code-standards.yml + with: + magic-numbers: true + magic-numbers-policy: report tests: runs-on: ubuntu-latest timeout-minutes: 15 @@ -24,6 +27,7 @@ jobs: - uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} + extensions: dom, posix coverage: none tools: composer:v2 - run: composer install --no-plugins --no-scripts --no-interaction --prefer-dist --no-progress diff --git a/README.md b/README.md index 5a4d393..a894c50 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ type analysis, application tests and coverage belong to their respective tools. - Read-only syntax/style checking and explicitly requested local fixes. - Exact PR merge-base selection with binary-safe file paths. - Invocable Symfony Console commands composed with Fast Forward Container. -- An unprivileged reusable workflow and a portable consumption skill. +- An unprivileged reusable workflow with explicit optional analysis summaries and a portable consumption skill. ## 📦 Installation and requirements @@ -156,12 +156,23 @@ static analysis remain separate; no DevTools extraction parity is asserted. ## 🧰 API summary +The reusable keeps optional analyses disabled by default. Enable +`magic-numbers: true` or `rector: true` explicitly; Rector also requires +`rector-config`. Each tool's `*-policy` accepts `report` or `fail` for verified +findings. Operational failures always fail, including invalid/incomplete native +reports. A visible summary distinguishes disabled, not executed, clean, findings +and operational error, with native counts and available file/line information. +See [optional workflow analyses](docs/optional-analysis.rst) for inputs, limits, +trust boundaries and consumer examples. Verify that the selected workflow ref +declares these inputs before copying optional settings. + | Entry point | Contract | | --- | --- | | `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. | | [`code-standards magic-numbers`](docs/magic-numbers.rst) | Analyze selected source with opt-in PHPMND, independently of check/fix; findings keep a nonzero status. | +| `code-standards refactor --config=rector.php --format=json` | Emit native JSON for preview only; JSON cannot be combined with `--apply`. | | `ECSConfig::configure(?callable $customize)` | Return the native ECS builder; apply the optional callback last. | | `help check` / `help fix` / `help refactor` / `help magic-numbers` | Show the installed Symfony option definitions. | diff --git a/docs/index.rst b/docs/index.rst index 03015bf..43416ba 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -27,6 +27,7 @@ Architecture, policy and evidence architecture automation instruction-map + optional-analysis legacy-fixer-comparison magic-numbers phpdoc-comparison diff --git a/docs/optional-analysis.rst b/docs/optional-analysis.rst new file mode 100644 index 0000000..d7352b0 --- /dev/null +++ b/docs/optional-analysis.rst @@ -0,0 +1,138 @@ +Optional workflow analyses and summaries +======================================= + +The public reusable keeps the mandatory style/syntax check unchanged. A separate +hosted job reports optional analysis without making ``check`` or ``fix`` invoke +semantic or quality tools. Both analyzers are **disabled by default**. With no +optional tool enabled, that job writes disabled statuses without checking out, +installing or executing another tool graph. + +Inputs and adoption +------------------- + +The existing ``php-version`` and ``config`` inputs retain their contracts. +``config`` belongs to the formatter; it does not enable or configure Rector. +The additional inputs are: + +* ``magic-numbers``: boolean, default ``false``; analyze the exact eligible PHP + selection through the separate ``magic-numbers`` command. +* ``magic-numbers-policy``: ``report`` (default) or ``fail`` for verified findings. +* ``rector``: boolean, default ``false``; run native semantic preview only. +* ``rector-config``: required when Rector is enabled; an existing consumer-relative + PHP configuration resolving inside the checkout, with explicit target/rules. + Native configuration owns Rector's whole-project paths; these are not restricted + to the formatter changed-file list. No automatic ``--apply`` is possible. +* ``rector-policy``: ``report`` (default) or ``fail`` for verified proposed changes. + +Policies are exact strings; unsupported values fail explicitly. A policy only +changes the treatment of **verified findings**. Operational errors always block, +including when the selected policy is ``report``. Tools are not enabled because +their dependency is installed or a configuration filename happens to exist. + +Start with the consumer wrapper in ``resources/workflows/code-standards.yml``. +Keep its existing unprivileged PR event and released/ref provenance contract. +Verify that the selected workflow ref declares these inputs before uncommenting +optional settings. The GitHub ``v0`` channel and Composer constraints remain +separate distribution choices; no unexisting release or automatic upgrade is +implied by copying an example. + +.. code-block:: yaml + + with: + php-version: '8.3' + magic-numbers: true + magic-numbers-policy: report + rector: true + rector-config: rector.php + rector-policy: fail + +Result states and counts +----------------------- + +The job summary distinguishes: + +* ``disabled``: not requested; no optional analyzer execution occurred. +* ``not-executed``: valid Magic Numbers selection contains no eligible PHP files; + no engine execution or clean native result is claimed. +* ``clean``: an executed, complete native report and exit agree on zero findings. +* ``findings``: complete native findings/differences; report-only policy informs, + while fail policy blocks. +* ``operational-error``: failed prerequisites, execution/timeout, malformed or + inconsistent output, missing capability or exceeded limits. Missing results + are not silently replaced with zero counts. + +For Magic Numbers, ``selected PHP files`` comes from the existing +``FileSelection``/``PhpFiles`` contract, not XML ``fileCount``. Native ``fileCount`` +counts only files containing findings. ``Findings`` counts native entries; their +real file/line values are shown. XML columns are not safe AST-patch identities. +An empty eligible selection is checked before launching the analyzer. + +For Rector, ``changed files`` is the native total, not a count of individual +rules, lines or defects. Available native error locations retain their line +field. Proposed changes have no native JSON line field; the summary marks that +location unavailable rather than parsing a unified diff or inventing precision. +The explicit CLI JSON representation retains the native diff for reviewed use. + +Classification requires the complete structured report **and** the process exit. +PHPMND exit 1 alone can mean findings or failure. Rector exit 2 alone can mean +proposed changes or a controlled invocation error. Unsupported combinations fail +closed. A mixed Rector report containing errors remains operational failure even +when it also includes proposed changes. + +Execution, summary and trust limits +---------------------------------- + +The workflow uses only ``pull_request``, ``contents: read``, GitHub-hosted Ubuntu, +the called job's verified repository/SHA, exact consumer head, separate checkouts +and no persisted checkout credentials. Only the tool's lock is installed, with +plugins/scripts disabled; consumer dependencies are not installed. Configured +optional tools do not change protections, required checks, releases or consumers. + +Each optional invocation has a total 300-second budget, a 2 MiB stdout report cap +and a separate 64 KiB diagnostic cap. The total Rector budget includes its native +inventory and processing, whereas the standalone CLI timeout is per subprocess. +The existing Magic Numbers argv bound is preserved. Overflow discards classification +of the incomplete report and reports operational error with unavailable counts; +it is not a truncated clean result. + +The workflow starts the Code Standards PHP CLI with ``memory_limit=256M``. +This constrains that PHP process, including its buffered native Rector preflight +output. It is not a global process-tree or native Rector memory limit: PHP +subprocesses have their own configuration, and executable configuration may alter +its own INI settings. No persistent INI file or standalone CLI default is changed. + +The workflow-private Linux executor starts a trusted launcher in its own +``setsid`` session, checks the child's PID/session/process-group identity and only +then permits the payload. It reads fixed nonblocking chunks with deadline checks; +it does not depend on a Symfony callback after unbounded intermediate pipe reads. +Only that confirmed owned group is terminated on completion/failure, including +ordinary analyzer descendants. Missing ``setsid`` or PHP POSIX support fails before +launch instead of choosing a weaker fallback. Invocation temporary files belong +to an owned temporary directory; cleanup does not follow symlinks. These are +workflow requirements, not new global Composer/CLI platform requirements. +The private child environment omits the existing tool credential keys plus +GitHub command-file paths and known Actions token fields. CI/workspace metadata +remain available. This narrows ambient capabilities; it does not claim that +arbitrary PHP is sandboxed from the runner filesystem. + +Reports are parsed without network/external-entity resolution and XML DTDs are +rejected. The summary displays bounded, escaped preformatted text; source snippets, +full diffs and untrusted HTML/Markdown are not interpreted. Control/non-ASCII +characters use visible JSON escapes. Display is limited to 100 diagnostics per +tool and 64 KiB overall with explicit omission/truncation notices; validated +native totals are not reduced to the displayed subset. Raw reports are not +uploaded as artifacts or shared automatically. + +The classification step writes its summary and a controlled blocking decision. +A mandatory final gate fails for operational errors, configured blocking findings, +or incomplete/missing outputs; there is no blanket ``continue-on-error``. +An ``always()`` fallback writes unavailable/error statuses if setup or the helper +fails before it can provide a complete result. Platform cancellation or outages +can prevent cleanup/final steps and never establish a successful analysis. + +Rector configuration, custom rules and bootstraps are executable PHP and can +mutate files even during preview. This is not a security/filesystem sandbox or +a guarantee against malicious code which detaches another session. The boundary +is the unprivileged ephemeral hosted job and finite runtime. Never call it from +a privileged target event or supply secrets. Review the +`native refactoring contract `__ before enabling it. diff --git a/docs/rector.rst b/docs/rector.rst index 9bca61b..947873d 100644 --- a/docs/rector.rst +++ b/docs/rector.rst @@ -34,6 +34,35 @@ Preview passes native ``--dry-run``. Explicit apply performs one write pass and ``--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. +Native JSON preview +------------------- + +``--format`` accepts ``text`` (the default) or ``json``. JSON selects Rector's +native output formatter for preview, preserving its totals, errors, +``changed_files`` and ``file_diffs`` without scraping or rewriting them. +JSON and ``--apply`` are rejected together before evaluating configuration or +starting native work: apply plus verification would otherwise concatenate two +documents. Text apply and its verification keep the existing behavior. + +.. code-block:: console + + php bin/code-standards refactor --config=rector.php --format=json > rector-preview.json + +The redirection writes only the explicitly selected report file. Native JSON +has ``totals.changed_files`` and ``totals.errors``; available processing errors +carry file/message/line fields. Proposed changes carry files and native diffs, +not an AST-location field. The reusable does not infer source lines from diff +text. Clean preview returns 0; native changes return 2; processing errors +return 1. Controlled invocation errors can also return 2, so automation must +validate the complete report together with its exit status. An unavailable or +malformed report is an operational error, never a clean or advisory finding. + +This representation adds no inventory or processing invocation. Executable +configuration and its stdout can still affect execution/report validity; JSON +preview retains the trust and namespace boundaries below. The +`optional workflow guide `__ describes explicitly enabled +Rector preview, report/fail policies and bounded summaries. + Executable configuration and initialization boundary ---------------------------------------------------- diff --git a/docs/usage/commands.rst b/docs/usage/commands.rst index 1a23ec3..20b64aa 100644 --- a/docs/usage/commands.rst +++ b/docs/usage/commands.rst @@ -12,6 +12,9 @@ configuration's source paths rather than this Git changed-file selection. See the `semantic refactoring contract <../rector.rst>`__ for required target PHP, optional native cache configuration, bounded timeouts and execution limits. Use ``help refactor`` for its installed options. +Native JSON preview is explicit with ``--format=json``; it cannot be combined +with ``--apply``. Human text remains the default. See the +`optional workflow analyses <../optional-analysis.rst>`__ for CI policies and summaries. The opt-in ``magic-numbers`` command uses the same Git selection as ``check`` and ``fix``, independently of formatting. It accepts ``--all`` or ``--base``/``--head`` diff --git a/resources/workflows/code-standards.yml b/resources/workflows/code-standards.yml index 8e9e31b..88d2b35 100644 --- a/resources/workflows/code-standards.yml +++ b/resources/workflows/code-standards.yml @@ -9,3 +9,9 @@ jobs: uses: php-fast-forward/code-standards/.github/workflows/code-standards.yml@v0 with: php-version: '8.3' + # Enable only after verifying these inputs exist in the selected workflow ref. + # magic-numbers: true + # magic-numbers-policy: report + # rector: true + # rector-config: rector.php + # rector-policy: fail diff --git a/src/Console/Command/RefactorCommand.php b/src/Console/Command/RefactorCommand.php index 439be77..5ec154e 100644 --- a/src/Console/Command/RefactorCommand.php +++ b/src/Console/Command/RefactorCommand.php @@ -40,6 +40,9 @@ public function __invoke( #[Option(description: 'Positive whole seconds per Rector subprocess, including preflight and verification.')] string $timeout = '300', + + #[Option(description: 'Native output: text, or json for preview only; json cannot be combined with --apply.')] + string $format = 'text', ): int { try { if (null === $config) { @@ -49,7 +52,7 @@ public function __invoke( $seconds = $this->timeoutSeconds($timeout); $scope = $this->projects->resolve(true, null, null, $directory, $config); - return $this->rector->run($scope->directory, $scope->config, $apply, $seconds); + return $this->rector->run($scope->directory, $scope->config, $apply, $seconds, $format); } catch (ProcessFailedException $error) { $io->getErrorStyle()->error($error->getMessage()); diff --git a/src/Refactoring/RectorRunner.php b/src/Refactoring/RectorRunner.php index adbc67d..6790d45 100644 --- a/src/Refactoring/RectorRunner.php +++ b/src/Refactoring/RectorRunner.php @@ -22,12 +22,25 @@ public function __construct(private ProcessRunner $process, private string $rect * successful apply adds verification. Native Rector may evaluate configuration more than once per invocation. * This transport is not a filesystem sandbox. * + * JSON is native preview output only; apply plus verification would produce multiple documents. + * Invalid output/write combinations fail before evaluating executable configuration. + * * @return int Native process status, including 2 when dry-run proposes changes. */ - public function run(string $directory, string $config, bool $apply, float $timeout = 300): int + public function run(string $directory, string $config, bool $apply, float $timeout = 300, string $format = 'text'): int { + if (!in_array($format, ['text', 'json'], true)) { + throw new InvalidArgumentException('Rector format must be text or json.'); + } + if ($apply && 'json' === $format) { + throw new InvalidArgumentException('JSON output is preview-only; use text output for explicit --apply.'); + } + $this->assertRules($directory, $config, $timeout); $check = [PHP_BINARY, $this->rector, 'process', '--config=' . $config, '--no-progress-bar']; + if ('json' === $format) { + $check[] = '--output-format=json'; + } $status = $this->process->run($apply ? $check : [...$check, '--dry-run'], $directory, $timeout); if (!$apply || 0 !== $status) { return $status; diff --git a/tests/Automation/OptionalAnalysisEntrypointTest.php b/tests/Automation/OptionalAnalysisEntrypointTest.php new file mode 100644 index 0000000..63a631e --- /dev/null +++ b/tests/Automation/OptionalAnalysisEntrypointTest.php @@ -0,0 +1,88 @@ +directory = realpath(sys_get_temp_dir()) . '/optional-entrypoint-' . bin2hex(random_bytes(8)); + mkdir($this->directory); + } + + protected function tearDown(): void + { + (new Filesystem())->remove($this->directory); + } + + /** Disabled status requires neither an existing consumer directory nor Git/native execution. */ + public function testCompositionWritesBothDisabledStatesAndACompleteControlledDecision(): void + { + $process = $this->process(dirname(__DIR__, 2) . '/.github/run-optional-analysis.php'); + self::assertSame(0, $process->run(), $process->getErrorOutput()); + self::assertSame("summary-written=true\nblocking=false\n", file_get_contents($this->directory . '/outputs')); + $summary = file_get_contents($this->directory . '/summary'); + self::assertStringContainsString('| Magic Numbers | disabled |', $summary); + self::assertStringContainsString('| Rector | disabled |', $summary); + self::assertStringNotContainsString('| clean |', $summary); + } + + /** @return iterable Each runner command file must be written completely. */ + public static function partialTargets(): iterable + { + yield 'summary' => ['GITHUB_STEP_SUMMARY']; + yield 'outputs' => ['GITHUB_OUTPUT']; + } + + /** A partial write is not a successfully summarized/classified invocation. */ + #[DataProvider('partialTargets')] + public function testPartialRunnerFileWritesFailInsteadOfClaimingCompletion(string $target): void + { + $source = <<<'PHP' + remaining, strlen($data)); + $this->remaining -= $written; + return $written; + } + public function stream_flush(): bool { return true; } + } + stream_wrapper_register('standards-partial', PartialRunnerFile::class); + PHP; + $bootstrap = $this->directory . '/partial.php'; + file_put_contents($bootstrap, $source . "\nrequire " . var_export(dirname(__DIR__, 2) . '/.github/run-optional-analysis.php', true) . ';'); + $process = $this->process($bootstrap, [$target => 'standards-partial://runner-file']); + self::assertNotSame(0, $process->run()); + self::assertStringContainsString('Cannot ' . ('GITHUB_OUTPUT' === $target ? 'record' : 'write'), $process->getErrorOutput()); + if ('GITHUB_STEP_SUMMARY' === $target) { + self::assertFileDoesNotExist($this->directory . '/outputs'); + } + } + + /** Literal fixture inputs disable both analyzers; the absent consumer cannot be used accidentally. */ + private function process(string $entrypoint, array $overrides = []): Process + { + return new Process([PHP_BINARY, $entrypoint], dirname(__DIR__, 2), [ + 'STANDARDS_DIRECTORY' => '/nonexistent-optional-consumer', + 'STANDARDS_BASE' => str_repeat('a', 40), 'STANDARDS_HEAD' => str_repeat('b', 40), + 'STANDARDS_MAGIC_NUMBERS' => 'false', 'STANDARDS_MAGIC_NUMBERS_POLICY' => 'report', + 'STANDARDS_RECTOR' => 'false', 'STANDARDS_RECTOR_POLICY' => 'report', 'STANDARDS_RECTOR_CONFIG' => '', + 'GITHUB_STEP_SUMMARY' => $this->directory . '/summary', 'GITHUB_OUTPUT' => $this->directory . '/outputs', + ...$overrides, + ], timeout: 10); + } +} diff --git a/tests/Automation/OptionalAnalysisInputTest.php b/tests/Automation/OptionalAnalysisInputTest.php new file mode 100644 index 0000000..77547a3 --- /dev/null +++ b/tests/Automation/OptionalAnalysisInputTest.php @@ -0,0 +1,99 @@ +createMock(ProcessRunner::class); + $process->expects(self::never())->method('capture'); + $process->expects(self::never())->method('run'); + $runner = $this->runner($process); + $results = $runner->run($this->input('/directory-that-does-not-exist', false)); + self::assertSame(['disabled', 'disabled'], array_column($results, 'status')); + } + + /** A valid Git selection with no PHP produces not-executed, never a fabricated engine-clean result. */ + public function testEmptySelectionDoesNotStartTheAnalyzer(): void + { + $directory = realpath(sys_get_temp_dir()); + $process = $this->createMock(ProcessRunner::class); + $process->method('capture')->willReturnCallback(static function (array $arguments): string { + return match (true) { + in_array('--is-inside-work-tree', $arguments, true) => "true\n", + in_array('rev-parse', $arguments, true) && in_array('HEAD', $arguments, true) => str_repeat('b', 40) . "\n", + default => '', + }; + }); + $process->expects(self::never())->method('run'); + $results = $this->runner($process)->run($this->input($directory, true)); + self::assertSame('not-executed', $results[0]->status); + self::assertSame(0, $results[0]->selectedCount); + self::assertNull($results[0]->findingCount); + self::assertNull($results[0]->nativeExit); + self::assertSame('disabled', $results[1]->status); + } + + /** Unavailable checkout prerequisites are errors even when findings would be advisory. */ + public function testUnavailableConsumerCannotBecomeCleanOrAdvisoryFindings(): void + { + $process = $this->createMock(ProcessRunner::class); + $process->expects(self::never())->method('capture'); + $results = $this->runner($process)->run($this->input('/directory-that-does-not-exist', true)); + self::assertSame('operational-error', $results[0]->status); + self::assertTrue($results[0]->blocks('report')); + self::assertNull($results[0]->findingCount); + } + + /** Missing/stringly enablement and unrecognized severity policies fail instead of being coerced. */ + public function testEnvironmentInputsAreExplicitAndStrict(): void + { + $environment = [ + 'STANDARDS_DIRECTORY' => '/consumer', 'STANDARDS_BASE' => str_repeat('a', 40), 'STANDARDS_HEAD' => str_repeat('b', 40), + 'STANDARDS_MAGIC_NUMBERS' => 'false', 'STANDARDS_MAGIC_NUMBERS_POLICY' => 'report', + 'STANDARDS_RECTOR' => 'false', 'STANDARDS_RECTOR_POLICY' => 'report', 'STANDARDS_RECTOR_CONFIG' => '', + ]; + self::assertFalse(WorkflowInput::fromEnvironment($environment)->magicNumbers); + foreach (['STANDARDS_MAGIC_NUMBERS' => 'yes', 'STANDARDS_MAGIC_NUMBERS_POLICY' => 'ignore', 'STANDARDS_HEAD' => 'main'] as $key => $value) { + try { + WorkflowInput::fromEnvironment([...$environment, $key => $value]); + self::fail('Malformed optional input must not be coerced.'); + } catch (InvalidArgumentException $exception) { + self::assertNotSame('', $exception->getMessage()); + } + } + } + + /** The fake installation is deliberately absent: tests which accidentally launch it must fail. */ + private function runner(ProcessRunner $process): OptionalAnalysisRunner + { + return new OptionalAnalysisRunner(new FileSelection($process), new PhpFiles(), new ProjectScopeResolver('/unused', new ProjectDirectoryResolver()), new BoundedProcess(), new NativeReportParser(), '/nonexistent-tool'); + } + + /** Produces only explicit fixture options; no native analysis executes inside this test class. */ + private function input(string $directory, bool $magic): WorkflowInput + { + return new WorkflowInput($directory, str_repeat('a', 40), str_repeat('b', 40), $magic, 'report', false, 'report', ''); + } +} diff --git a/tests/Automation/OptionalAnalysisProcessTest.php b/tests/Automation/OptionalAnalysisProcessTest.php new file mode 100644 index 0000000..53482c4 --- /dev/null +++ b/tests/Automation/OptionalAnalysisProcessTest.php @@ -0,0 +1,144 @@ +directory = realpath(sys_get_temp_dir()) . '/optional process fixture-' . bin2hex(random_bytes(8)); + mkdir($this->directory); + } + + protected function tearDown(): void + { + if (isset($this->directory)) { + (new Filesystem())->remove($this->directory); + } + } + + /** Literal arguments, exact raw channels and nonzero status survive without inherited tool credentials. */ + public function testBoundedCaptureRetainsChannelsAndStatus(): void + { + $previous = []; + $fields = ['GH_TOKEN', 'GITHUB_TOKEN', 'COMPOSER_AUTH', 'GITHUB_OUTPUT', 'GITHUB_STEP_SUMMARY', 'GITHUB_ENV', 'GITHUB_PATH', 'GITHUB_STATE', 'ACTIONS_RUNTIME_TOKEN', 'ACTIONS_ID_TOKEN_REQUEST_TOKEN', 'ACTIONS_ID_TOKEN_REQUEST_URL']; + foreach ($fields as $name) { + $previous[$name] = getenv($name); + putenv($name . '=synthetic-test-value'); + } + $metadata = ['CI' => 'true', 'GITHUB_ACTIONS' => 'true', 'GITHUB_WORKSPACE' => $this->directory]; + foreach ($metadata as $name => $value) { + $previous[$name] = getenv($name); + putenv($name . '=' . $value); + } + try { + $source = 'foreach (' . var_export($fields, true) . ' as $name) { if (getenv($name)!==false) exit(19); } ' + . 'foreach (' . var_export($metadata, true) . ' as $name => $value) { if (getenv($name)!==$value) exit(20); } ' + . 'fwrite(STDOUT,$argv[1]); fwrite(STDERR,"error\0\xff"); exit(2);'; + $argument = "quote' space\nliteral"; + $result = (new BoundedProcess())->capture([PHP_BINARY, '-r', $source, $argument], $this->directory, 5); + self::assertSame(2, $result->exitCode); + self::assertSame($argument, $result->stdout); + self::assertSame("error\0\xff", $result->stderr); + } finally { + foreach ($previous as $name => $value) { + putenv(false === $value ? $name : $name . '=' . $value); + } + } + } + + /** A continuous producer cannot fill an unbounded intermediate buffer or leave its sleeping child alive. */ + public function testContinuousProducerAndOwnedDescendantStopAtTheCaptureLimit(): void + { + $pid = $this->directory . '/overflow-child.pid'; + try { + (new BoundedProcess())->capture([PHP_BINARY, '-r', $this->descendantSource('while (true) { fwrite(STDOUT, str_repeat("x", 8192)); }'), $pid], $this->directory, 5); + self::fail('Continuous output must fail at the capture cap.'); + } catch (RuntimeException $exception) { + self::assertStringContainsString('capture limit', $exception->getMessage()); + } + $this->assertOwnedChildTerminated($pid); + } + + /** A silent parent and sleeping child share the verified group and cannot evade the total timeout. */ + public function testTimeoutStopsOnlyTheOwnedProcessGroup(): void + { + $pid = $this->directory . '/timeout-child.pid'; + try { + (new BoundedProcess())->capture([PHP_BINARY, '-r', $this->descendantSource('sleep(60);'), $pid], $this->directory, 1); + self::fail('A silent invocation must have a finite total budget.'); + } catch (RuntimeException $exception) { + self::assertStringContainsString('timeout', $exception->getMessage()); + } + $this->assertOwnedChildTerminated($pid); + } + + /** stderr has its own small cap, independent of an otherwise valid stdout document. */ + public function testDiagnosticCaptureIsBoundedSeparately(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('capture limit'); + (new BoundedProcess())->capture([PHP_BINARY, '-r', 'fwrite(STDOUT,"{}"); while (true) { fwrite(STDERR,str_repeat("e",8192)); }'], $this->directory, 5); + } + + /** Cleanup unlinks owned path replacements without following a link to another fixture's contents. */ + public function testOwnedTemporaryCleanupNeverFollowsSymlinks(): void + { + $outside = $this->directory . '/preserved'; + mkdir($outside); + file_put_contents($outside . '/marker', 'preserve this owned fixture'); + foreach (['child', 'root'] as $placement) { + $source = '$own=getenv("TMPDIR"); if($argv[2]==="root"){rmdir($own);symlink($argv[1],$own);}else{symlink($argv[1],$own."/link");file_put_contents($own."/temporary","owned");} echo $own;'; + $result = (new BoundedProcess())->capture([PHP_BINARY, '-r', $source, $outside, $placement], $this->directory, 5); + self::assertSame(0, $result->exitCode); + self::assertFileDoesNotExist($result->stdout); + self::assertFalse(is_link($result->stdout)); + self::assertSame('preserve this owned fixture', file_get_contents($outside . '/marker')); + } + } + + /** The PID is written only by an exclusively owned synthetic sleeper, not discovered through a process listing. */ + private function descendantSource(string $parent): string + { + return '$child=proc_open([PHP_BINARY,"-r",\'file_put_contents($argv[1],(string)getmypid());sleep(60);\',$argv[1]],[0=>["file","/dev/null","r"],1=>["file","/dev/null","w"],2=>["file","/dev/null","w"]],$pipes);' + . '$deadline=microtime(true)+2;while(!is_file($argv[1])&µtime(true)<$deadline){usleep(1000);}if(!is_file($argv[1]))exit(20);' + . $parent; + } + + /** Reads only the known fixture PID's Linux state; a terminated zombie is not treated as a live orphan. */ + private function assertOwnedChildTerminated(string $file): void + { + self::assertFileExists($file); + $pid = file_get_contents($file); + self::assertMatchesRegularExpression('/\A[1-9][0-9]*\z/D', $pid); + $deadline = microtime(true) + 2; + do { + $state = @file_get_contents('/proc/' . $pid . '/stat'); + if (false === $state || in_array(substr($state, strrpos($state, ') ') + 2, 1), ['Z', 'X'], true)) { + self::addToAssertionCount(1); + + return; + } + usleep(10000); + } while (microtime(true) < $deadline); + + self::fail('The exclusively owned fixture descendant remains alive after cleanup.'); + } +} diff --git a/tests/Automation/OptionalAnalysisReportTest.php b/tests/Automation/OptionalAnalysisReportTest.php new file mode 100644 index 0000000..35caee2 --- /dev/null +++ b/tests/Automation/OptionalAnalysisReportTest.php @@ -0,0 +1,182 @@ +magicNumbers(new ProcessResult(0, $this->xml(), ''), ['/consumer/a.php', '/consumer/b.php']); + self::assertSame('clean', $clean->status); + self::assertSame(2, $clean->selectedCount); + self::assertSame(0, $clean->findingCount); + + $found = $parser->magicNumbers(new ProcessResult(1, $this->xml('";]]>', 1, 1), ''), ['/consumer/a.php']); + self::assertSame('findings', $found->status); + self::assertSame(1, $found->findingCount); + self::assertSame(7, $found->diagnostics[0]['line']); + self::assertFalse($found->blocks('report')); + self::assertTrue($found->blocks('fail')); + } + + /** @return iterable Invalid/partial output must never be accepted as advisory debt. */ + public static function invalidMagicReports(): iterable + { + yield 'not XML' => ['not XML', 1]; + yield 'truncated XML' => ['', 1]; + yield 'wrong exit' => ['', 1]; + yield 'overflow' => ['', 0]; + yield 'counts' => ['', 1]; + yield 'unselected path' => ['', 1]; + yield 'entity declaration' => [']>', 0]; + } + + /** XML schema/status failures reject the report rather than returning a clean zero. */ + #[DataProvider('invalidMagicReports')] + public function testInvalidMagicReportIsOperationalFailure(string $report, int $status): void + { + $this->expectException(InvalidArgumentException::class); + (new NativeReportParser())->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); + } + + /** Rejecting a DTD must not first open its external subset or entity through a PHP stream wrapper. */ + public function testXmlRejectsExternalEntitiesWithoutOpeningTheirSources(): void + { + if (!extension_loaded('dom')) { + self::markTestSkipped('External-entity qualification requires the real DOM extension.'); + } + self::assertTrue(stream_wrapper_register('standards-report-probe', ExternalEntityProbe::class)); + ExternalEntityProbe::$opens = 0; + try { + foreach ([ + '', + ']>', + ] as $declaration) { + $report = $declaration . '&source;'; + try { + (new NativeReportParser())->magicNumbers(new ProcessResult(0, $report, ''), []); + self::fail('DTD-bearing reports must be rejected.'); + } catch (InvalidArgumentException) { + self::assertSame(0, ExternalEntityProbe::$opens); + } + } + } finally { + self::assertTrue(stream_wrapper_unregister('standards-report-probe')); + } + } + + /** Native errors dominate report-only policy even when changes were also proposed. */ + public function testRectorNativeChangesAndErrorsHaveSeparateMeaning(): void + { + $parser = new NativeReportParser(); + $clean = $parser->rector(new ProcessResult(0, '{"totals":{"changed_files":0,"errors":0}}', '')); + self::assertSame('clean', $clean->status); + self::assertNull($clean->selectedCount); + + $json = '{"totals":{"changed_files":1,"errors":0},"changed_files":["src/a.php"],"file_diffs":[{"file":"src/a.php","diff":"@@ -1 +9 @@\\n-old\\n+new"}]}'; + $changes = $parser->rector(new ProcessResult(2, $json, '')); + self::assertSame('findings', $changes->status); + self::assertSame(1, $changes->findingCount); + self::assertNull($changes->diagnostics[0]['line'], 'Do not infer a line from a diff or fabricate AST identity.'); + self::assertFalse($changes->blocks('report')); + + $error = $parser->rector(new ProcessResult(1, '{"totals":{"changed_files":0,"errors":1},"errors":[{"file":"src/b.php","line":12,"message":"native failure"}]}', '')); + self::assertSame('operational-error', $error->status); + self::assertSame(12, $error->diagnostics[0]['line']); + self::assertTrue($error->blocks('report')); + self::assertTrue($error->blocks('fail')); + self::assertSame(1, $error->nativeExit); + } + + /** @return iterable ANSI, extra documents and mismatched types are not scraped away. */ + public static function invalidRectorReports(): iterable + { + yield 'human output' => ['Error: malformed config', 2]; + yield 'multiple JSON documents' => ['{"totals":{"changed_files":0,"errors":0}}{}', 0]; + yield 'coerced count' => ['{"totals":{"changed_files":"0","errors":0}}', 0]; + yield 'findings without list' => ['{"totals":{"changed_files":1,"errors":0}}', 2]; + yield 'clean with failure exit' => ['{"totals":{"changed_files":0,"errors":0}}', 1]; + yield 'errors without entries' => ['{"totals":{"changed_files":0,"errors":1}}', 1]; + } + + /** Controlled invocation errors sharing status 2 do not become Rector findings. */ + #[DataProvider('invalidRectorReports')] + public function testInvalidRectorReportIsOperationalFailure(string $report, int $status): void + { + $this->expectException(InvalidArgumentException::class); + (new NativeReportParser())->rector(new ProcessResult($status, $report, '')); + } + + /** Plain preformatted HTML keeps Markdown/commands inert and bounds display without changing native totals. */ + public function testSummaryEscapesAndTruncatesUntrustedDiagnostics(): void + { + $malicious = "\n[link](https://example.invalid) ::error::"; + $diagnostics = array_fill(0, 1000, ['file' => $malicious . str_repeat('x', 5000), 'line' => 7, 'message' => $malicious]); + $summary = (new SummaryRenderer())->render([ + new AnalysisResult('Magic Numbers', 'findings', 1, 1000, $diagnostics, $malicious, 1), + new AnalysisResult('Rector', 'disabled'), + ]); + self::assertLessThanOrEqual(65536, strlen($summary)); + self::assertStringNotContainsString('