From 1b5f766f3eb3853b59b015f7b6f9138db5427add Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Wed, 28 Oct 2026 18:21:21 -0300 Subject: [PATCH 1/4] refactor: reuse native Rector report validation --- .../extract-rector-report-validation.md | 6 + .../OptionalAnalysis/NativeReportParser.php | 78 ++----------- docs/process-contracts.rst | 18 +++ src/Refactoring/RectorJsonReportParser.php | 108 ++++++++++++++++++ .../Automation/OptionalAnalysisInputTest.php | 3 +- .../Automation/OptionalAnalysisReportTest.php | 15 +-- .../OptionalMagicNumbersReportTest.php | 3 +- .../Integration/RectorJsonIntegrationTest.php | 7 +- .../RectorJsonReportParserTest.php | 64 +++++++++++ 9 files changed, 224 insertions(+), 78 deletions(-) create mode 100644 .changelog/extract-rector-report-validation.md create mode 100644 src/Refactoring/RectorJsonReportParser.php create mode 100644 tests/Refactoring/RectorJsonReportParserTest.php diff --git a/.changelog/extract-rector-report-validation.md b/.changelog/extract-rector-report-validation.md new file mode 100644 index 0000000..264b63e --- /dev/null +++ b/.changelog/extract-rector-report-validation.md @@ -0,0 +1,6 @@ +--- +category: changed +type: patch +--- + +Reuse native Rector preview report validation from the source library while preserving CLI output, native exits and optional-workflow classification and policy. diff --git a/.github/OptionalAnalysis/NativeReportParser.php b/.github/OptionalAnalysis/NativeReportParser.php index 010f820..6a2566c 100644 --- a/.github/OptionalAnalysis/NativeReportParser.php +++ b/.github/OptionalAnalysis/NativeReportParser.php @@ -7,12 +7,15 @@ use DOMDocument; use DOMElement; use FastForward\CodeStandards\Process\ProcessResult; +use FastForward\CodeStandards\Refactoring\RectorJsonReportParser; use InvalidArgumentException; -use JsonException; /** Classifies qualified native schemas together with their exits; human output is never scraped. */ final class NativeReportParser { + /** Reuses native Rector validation while keeping workflow classification and presentation here. */ + public function __construct(private readonly RectorJsonReportParser $rectorReports) {} + /** * Requires complete, consistent native XML before accepting exit 1 as findings. * @@ -64,35 +67,13 @@ 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)]; + $report = $this->rectorReports->parse($process); + $changes = $report['totals']['changed_files']; + $errorCount = $report['totals']['errors']; + $diagnostics = [...$this->rectorErrors($report['errors'] ?? []), ...$this->rectorChanges($report['changed_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); } @@ -160,26 +141,6 @@ private function integer(string $value): int 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. * @@ -190,17 +151,6 @@ 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']]; } @@ -208,19 +158,13 @@ private function rectorErrors(array $errors): array } /** - * Lists native changed files without interpreting diff text as line or AST identity. + * Lists validated native paths without inferring diff lines 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. + * @param list $files Native changed paths corresponding to the validated total. * @return list */ - private function rectorChanges(array $changes, array $files): array + private function rectorChanges(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.']; diff --git a/docs/process-contracts.rst b/docs/process-contracts.rst index c234a64..a29e20f 100644 --- a/docs/process-contracts.rst +++ b/docs/process-contracts.rst @@ -69,3 +69,21 @@ Golden fixtures exercise descriptive contracts and inheritance markers through read-only checking, explicit fixing and idempotence. A native Rector regression changes array syntax while retaining the same documentation. Consumer-selected semantic rules can change PHPDoc; this is not a claim about every configuration. + +Native Rector report validation +------------------------------- + +``Refactoring\RectorJsonReportParser::parse()`` validates a completed +``ProcessResult`` against the native Rector preview schema and returns the +decoded native report. Totals, native lists, error locations, diff paths and +the exit code must agree. Malformed or inconsistent reports raise +``InvalidArgumentException`` with the established native validation diagnostic. +Optional fields and native record ordering remain intact, including absent +empty lists and opaque diff text. No source line is inferred from a diff. + +The caller owns capture budgets. The optional workflow keeps its existing +report-size limit before parsing, then maps validated native data into its +existing statuses, diagnostic presentation and ``report``/``fail`` policies. +The parser does not execute Rector or evaluate configuration. The public +``refactor --format=json`` command continues to stream its native preview +output with the same options and exits. diff --git a/src/Refactoring/RectorJsonReportParser.php b/src/Refactoring/RectorJsonReportParser.php new file mode 100644 index 0000000..0dd0261 --- /dev/null +++ b/src/Refactoring/RectorJsonReportParser.php @@ -0,0 +1,108 @@ + The decoded native report, retaining all fields and native ordering. + * @throws InvalidArgumentException When the report is malformed or inconsistent with its completed exit. + */ + public function parse(ProcessResult $process): array + { + 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.'); + } + $this->validateErrors($errors); + $this->validateChanges($this->list($report['file_diffs'] ?? []), $files); + if (0 !== $errorCount) { + if (1 !== $process->exitCode) { + throw new InvalidArgumentException('Rector error report contradicts its exit.'); + } + + return $report; + } + if ((0 === $changes && 0 !== $process->exitCode) || (0 !== $changes && 2 !== $process->exitCode)) { + throw new InvalidArgumentException('Rector preview exit contradicts its native totals.'); + } + + return $report; + } + + /** 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; + } + + /** @param list $errors Native error locations; absent locations remain absent. */ + private function validateErrors(array $errors): void + { + 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.'); + } + } + } + + /** + * Validates native change records without inferring source locations from diff text. + * + * @param list $changes Optional native file diffs. + * @param list $files Changed paths reported by the same native invocation. + */ + private function validateChanges(array $changes, array $files): void + { + 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.'); + } + } + } +} diff --git a/tests/Automation/OptionalAnalysisInputTest.php b/tests/Automation/OptionalAnalysisInputTest.php index 69697be..76f8297 100644 --- a/tests/Automation/OptionalAnalysisInputTest.php +++ b/tests/Automation/OptionalAnalysisInputTest.php @@ -10,6 +10,7 @@ use FastForward\CodeStandards\Project\ProjectDirectoryResolver; use FastForward\CodeStandards\Project\ProjectScopeResolver; use FastForward\CodeStandards\Workflow\BoundedProcess; +use FastForward\CodeStandards\Refactoring\RectorJsonReportParser; use FastForward\CodeStandards\Workflow\NativeReportParser; use FastForward\CodeStandards\Workflow\OptionalAnalysisRunner; use FastForward\CodeStandards\Workflow\WorkflowInput; @@ -120,7 +121,7 @@ public function testEnvironmentInputsAreExplicitAndStrict(): void /** 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'); + return new OptionalAnalysisRunner(new FileSelection($process), new PhpFiles(), new ProjectScopeResolver('/unused', new ProjectDirectoryResolver()), new BoundedProcess(), new NativeReportParser(new RectorJsonReportParser()), '/nonexistent-tool'); } /** Produces only explicit fixture options; no native analysis executes inside this test class. */ diff --git a/tests/Automation/OptionalAnalysisReportTest.php b/tests/Automation/OptionalAnalysisReportTest.php index 3957cc0..3d34521 100644 --- a/tests/Automation/OptionalAnalysisReportTest.php +++ b/tests/Automation/OptionalAnalysisReportTest.php @@ -5,6 +5,7 @@ namespace FastForward\CodeStandards\Tests\Automation; use FastForward\CodeStandards\Process\ProcessResult; +use FastForward\CodeStandards\Refactoring\RectorJsonReportParser; use FastForward\CodeStandards\Workflow\AnalysisResult; use FastForward\CodeStandards\Workflow\NativeReportParser; use FastForward\CodeStandards\Workflow\SummaryRenderer; @@ -25,7 +26,7 @@ public function testMagicNumbersUsesTheSelectionForCoverageAndNativeEntriesForFi if (!extension_loaded('dom')) { self::markTestSkipped('Native XML classification requires the real DOM extension.'); } - $parser = new NativeReportParser(); + $parser = new NativeReportParser(new RectorJsonReportParser()); $clean = $parser->magicNumbers(new ProcessResult(0, $this->xml(), ''), ['/consumer/a.php', '/consumer/b.php']); self::assertSame('clean', $clean->status); self::assertSame(2, $clean->selectedCount); @@ -56,7 +57,7 @@ public static function invalidMagicReports(): iterable public function testInvalidMagicReportIsOperationalFailure(string $report, int $status): void { $this->expectException(InvalidArgumentException::class); - (new NativeReportParser())->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); + (new NativeReportParser(new RectorJsonReportParser()))->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); } /** @return iterable Unexpected nodes are rejected equally at all traversed structural boundaries. */ @@ -79,7 +80,7 @@ public static function unexpectedStructuralNodes(): iterable public function testUnexpectedStructuralNodesRejectClassification(string $report, int $status): void { $this->expectException(InvalidArgumentException::class); - (new NativeReportParser())->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); + (new NativeReportParser(new RectorJsonReportParser()))->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); } /** Structural whitespace/comments are harmless; actual snippet CDATA remains native source payload. */ @@ -90,7 +91,7 @@ public function testStructuralWhitespaceAndCommentsPreserveLegitimateSnippetCdat . '' . "\n\n" . '";]]>' . "\n\n\n"; - $result = (new NativeReportParser())->magicNumbers(new ProcessResult(1, $report, ''), ['/consumer/a.php']); + $result = (new NativeReportParser(new RectorJsonReportParser()))->magicNumbers(new ProcessResult(1, $report, ''), ['/consumer/a.php']); self::assertSame('findings', $result->status); self::assertSame(1, $result->findingCount); self::assertSame(7, $result->diagnostics[0]['line']); @@ -111,7 +112,7 @@ public function testXmlRejectsExternalEntitiesWithoutOpeningTheirSources(): void ] as $declaration) { $report = $declaration . '&source;'; try { - (new NativeReportParser())->magicNumbers(new ProcessResult(0, $report, ''), []); + (new NativeReportParser(new RectorJsonReportParser()))->magicNumbers(new ProcessResult(0, $report, ''), []); self::fail('DTD-bearing reports must be rejected.'); } catch (InvalidArgumentException) { self::assertSame(0, ExternalEntityProbe::$opens); @@ -125,7 +126,7 @@ public function testXmlRejectsExternalEntitiesWithoutOpeningTheirSources(): void /** Native errors dominate report-only policy even when changes were also proposed. */ public function testRectorNativeChangesAndErrorsHaveSeparateMeaning(): void { - $parser = new NativeReportParser(); + $parser = new NativeReportParser(new RectorJsonReportParser()); $clean = $parser->rector(new ProcessResult(0, '{"totals":{"changed_files":0,"errors":0}}', '')); self::assertSame('clean', $clean->status); self::assertNull($clean->selectedCount); @@ -161,7 +162,7 @@ public static function invalidRectorReports(): iterable public function testInvalidRectorReportIsOperationalFailure(string $report, int $status): void { $this->expectException(InvalidArgumentException::class); - (new NativeReportParser())->rector(new ProcessResult($status, $report, '')); + (new NativeReportParser(new RectorJsonReportParser()))->rector(new ProcessResult($status, $report, '')); } /** Plain preformatted HTML keeps Markdown/commands inert and bounds display without changing native totals. */ diff --git a/tests/Integration/OptionalMagicNumbersReportTest.php b/tests/Integration/OptionalMagicNumbersReportTest.php index e7c8752..9d5d1b8 100644 --- a/tests/Integration/OptionalMagicNumbersReportTest.php +++ b/tests/Integration/OptionalMagicNumbersReportTest.php @@ -5,6 +5,7 @@ namespace FastForward\CodeStandards\Tests\Integration; use FastForward\CodeStandards\Process\ProcessResult; +use FastForward\CodeStandards\Refactoring\RectorJsonReportParser; use FastForward\CodeStandards\Workflow\NativeReportParser; use PHPUnit\Framework\TestCase; use Symfony\Component\Filesystem\Filesystem; @@ -33,7 +34,7 @@ public function testClassifierAcceptsActualNativeReportsAndExits(): void '--directory=' . $directory, '--all', '--format=xml', ], $directory, timeout: 30); self::assertSame($expected, $process->run(), $process->getErrorOutput()); - $classified = (new NativeReportParser())->magicNumbers(new ProcessResult( + $classified = (new NativeReportParser(new RectorJsonReportParser()))->magicNumbers(new ProcessResult( $expected, $process->getOutput(), $process->getErrorOutput(), ), [$path]); self::assertSame(0 === $expected ? 'clean' : 'findings', $classified->status); diff --git a/tests/Integration/RectorJsonIntegrationTest.php b/tests/Integration/RectorJsonIntegrationTest.php index e346ff6..bd18c05 100644 --- a/tests/Integration/RectorJsonIntegrationTest.php +++ b/tests/Integration/RectorJsonIntegrationTest.php @@ -5,6 +5,7 @@ namespace FastForward\CodeStandards\Tests\Integration; use FastForward\CodeStandards\Process\ProcessResult; +use FastForward\CodeStandards\Refactoring\RectorJsonReportParser; use FastForward\CodeStandards\Workflow\NativeReportParser; use PHPUnit\Framework\TestCase; use Symfony\Component\Filesystem\Filesystem; @@ -52,9 +53,10 @@ public function testNativeJsonDistinguishesFindingsAndClean(): void $process = $this->preview(); self::assertSame($status, $process->getExitCode(), $process->getErrorOutput()); $report = json_decode($process->getOutput(), true, flags: JSON_THROW_ON_ERROR); + self::assertSame($report, (new RectorJsonReportParser())->parse(new ProcessResult($status, $process->getOutput(), $process->getErrorOutput()))); self::assertSame($changes, $report['totals']['changed_files']); self::assertSame(0, $report['totals']['errors']); - $classified = (new NativeReportParser())->rector(new ProcessResult($status, $process->getOutput(), $process->getErrorOutput())); + $classified = (new NativeReportParser(new RectorJsonReportParser()))->rector(new ProcessResult($status, $process->getOutput(), $process->getErrorOutput())); self::assertSame(0 === $changes ? 'clean' : 'findings', $classified->status); self::assertSame($changes, $classified->findingCount); self::assertSame($source, file_get_contents($this->directory . '/source.php')); @@ -73,10 +75,11 @@ public function testNativeJsonRetainsOperationalErrors(): void $process = $this->preview(); self::assertSame(1, $process->getExitCode(), $process->getErrorOutput()); $report = json_decode($process->getOutput(), true, flags: JSON_THROW_ON_ERROR); + self::assertSame($report, (new RectorJsonReportParser())->parse(new ProcessResult(1, $process->getOutput(), $process->getErrorOutput()))); self::assertGreaterThan(0, $report['totals']['errors']); self::assertNotEmpty($report['errors']); self::assertArrayHasKey('file', $report['errors'][0]); - $classified = (new NativeReportParser())->rector(new ProcessResult(1, $process->getOutput(), $process->getErrorOutput())); + $classified = (new NativeReportParser(new RectorJsonReportParser()))->rector(new ProcessResult(1, $process->getOutput(), $process->getErrorOutput())); self::assertSame('operational-error', $classified->status); self::assertTrue($classified->blocks('report')); self::assertSame($source, file_get_contents($this->directory . '/source.php')); diff --git a/tests/Refactoring/RectorJsonReportParserTest.php b/tests/Refactoring/RectorJsonReportParserTest.php new file mode 100644 index 0000000..b7aaaf6 --- /dev/null +++ b/tests/Refactoring/RectorJsonReportParserTest.php @@ -0,0 +1,64 @@ + [['file' => $path, 'diff' => "@@ -1 +2 @@\n-\n+\n", 'applied_rectors' => ['NativeRule']]], + 'errors' => [['message' => 'Native processing failure', 'file' => null, 'line' => null]], + 'changed_files' => [$path], + 'totals' => ['changed_files' => 1, 'errors' => 1], + 'native_extension' => ['retained' => true], + ]; + $process = new ProcessResult(1, json_encode($report, JSON_THROW_ON_ERROR), 'native stderr'); + + self::assertSame($report, (new RectorJsonReportParser())->parse($process)); + self::assertSame('native stderr', $process->stderr); + } + + /** Native clean reports may omit empty optional lists; validation does not invent fields. */ + public function testCleanReportRetainsAbsentOptionalLists(): void + { + $report = ['totals' => ['changed_files' => 0, 'errors' => 0]]; + self::assertSame($report, (new RectorJsonReportParser())->parse(new ProcessResult(0, json_encode($report, JSON_THROW_ON_ERROR), ''))); + } + + /** @return iterable Existing native validation errors remain objective. */ + public static function invalidReports(): iterable + { + yield 'malformed JSON' => ['{"totals":', 1, 'Rector did not return a complete native JSON document.']; + yield 'human preflight failure' => ['Error: missing explicit PHP target', 2, 'Rector did not return a complete native JSON document.']; + yield 'missing totals' => ['{}', 0, 'Rector JSON totals are unavailable.']; + yield 'count coercion' => ['{"totals":{"changed_files":"0","errors":0}}', 0, 'Rector JSON count is invalid.']; + yield 'error total mismatch' => ['{"totals":{"changed_files":0,"errors":1}}', 1, 'Rector error total is inconsistent.']; + yield 'changed paths mismatch' => ['{"totals":{"changed_files":1,"errors":0}}', 2, 'Rector changed-file list is inconsistent.']; + yield 'invalid error location' => ['{"totals":{"changed_files":0,"errors":1},"errors":[{"message":"failure","line":0}]}', 1, 'Rector JSON error location is invalid.']; + yield 'invalid diff location' => ['{"totals":{"changed_files":0,"errors":0},"file_diffs":[{"file":"other.php","diff":"opaque"}]}', 0, 'Rector JSON change location is invalid.']; + yield 'keyed list' => ['{"totals":{"changed_files":0,"errors":0},"errors":{"unexpected":{}}}', 0, 'Rector JSON entries must be a list.']; + yield 'error exit mismatch' => ['{"totals":{"changed_files":0,"errors":1},"errors":[{"message":"failure"}]}', 2, 'Rector error report contradicts its exit.']; + yield 'clean exit mismatch' => ['{"totals":{"changed_files":0,"errors":0}}', 1, 'Rector preview exit contradicts its native totals.']; + } + + /** Rejected reports keep the established exception class and diagnostic text. */ + #[DataProvider('invalidReports')] + public function testInvalidReportsRetainExistingErrors(string $json, int $exit, string $message): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage($message); + (new RectorJsonReportParser())->parse(new ProcessResult($exit, $json, '')); + } +} From 07d5bb466d7d9c26121e611be15ddec31711e77e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Wed, 28 Oct 2026 18:26:12 -0300 Subject: [PATCH 2/4] refactor: clarify extracted parser arguments --- .github/OptionalAnalysis/NativeReportParser.php | 4 ++-- src/Refactoring/RectorJsonReportParser.php | 4 ++-- tests/Automation/OptionalAnalysisInputTest.php | 2 +- tests/Automation/OptionalAnalysisReportTest.php | 14 +++++++------- .../Integration/OptionalMagicNumbersReportTest.php | 2 +- tests/Integration/RectorJsonIntegrationTest.php | 8 ++++---- tests/Refactoring/RectorJsonReportParserTest.php | 6 +++--- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/.github/OptionalAnalysis/NativeReportParser.php b/.github/OptionalAnalysis/NativeReportParser.php index 6a2566c..8eb0f3b 100644 --- a/.github/OptionalAnalysis/NativeReportParser.php +++ b/.github/OptionalAnalysis/NativeReportParser.php @@ -72,9 +72,9 @@ public function rector(ProcessResult $process): AnalysisResult $errorCount = $report['totals']['errors']; $diagnostics = [...$this->rectorErrors($report['errors'] ?? []), ...$this->rectorChanges($report['changed_files'] ?? [])]; if (0 !== $errorCount) { - return new AnalysisResult('Rector', 'operational-error', findingCount: $changes, diagnostics: $diagnostics, details: $errorCount . " native processing error(s).\n" . $process->stderr, nativeExit: $process->exitCode); + return new AnalysisResult(tool: 'Rector', status: 'operational-error', findingCount: $changes, diagnostics: $diagnostics, details: $errorCount . " native processing error(s).\n" . $process->stderr, nativeExit: $process->exitCode); } - 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); + return new AnalysisResult(tool: 'Rector', status: 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. */ diff --git a/src/Refactoring/RectorJsonReportParser.php b/src/Refactoring/RectorJsonReportParser.php index 0dd0261..ce9a2f3 100644 --- a/src/Refactoring/RectorJsonReportParser.php +++ b/src/Refactoring/RectorJsonReportParser.php @@ -20,9 +20,9 @@ final class RectorJsonReportParser public function parse(ProcessResult $process): array { try { - $report = json_decode($process->stdout, true, flags: JSON_THROW_ON_ERROR); + $report = json_decode(json: $process->stdout, associative: true, flags: JSON_THROW_ON_ERROR); } catch (JsonException $exception) { - throw new InvalidArgumentException('Rector did not return a complete native JSON document.', previous: $exception); + throw new InvalidArgumentException(message: '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.'); diff --git a/tests/Automation/OptionalAnalysisInputTest.php b/tests/Automation/OptionalAnalysisInputTest.php index 76f8297..8778e78 100644 --- a/tests/Automation/OptionalAnalysisInputTest.php +++ b/tests/Automation/OptionalAnalysisInputTest.php @@ -121,7 +121,7 @@ public function testEnvironmentInputsAreExplicitAndStrict(): void /** 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(new RectorJsonReportParser()), '/nonexistent-tool'); + return new OptionalAnalysisRunner(new FileSelection($process), new PhpFiles(), new ProjectScopeResolver('/unused', new ProjectDirectoryResolver()), new BoundedProcess(), new NativeReportParser(rectorReports: new RectorJsonReportParser()), '/nonexistent-tool'); } /** Produces only explicit fixture options; no native analysis executes inside this test class. */ diff --git a/tests/Automation/OptionalAnalysisReportTest.php b/tests/Automation/OptionalAnalysisReportTest.php index 3d34521..bcbb538 100644 --- a/tests/Automation/OptionalAnalysisReportTest.php +++ b/tests/Automation/OptionalAnalysisReportTest.php @@ -26,7 +26,7 @@ public function testMagicNumbersUsesTheSelectionForCoverageAndNativeEntriesForFi if (!extension_loaded('dom')) { self::markTestSkipped('Native XML classification requires the real DOM extension.'); } - $parser = new NativeReportParser(new RectorJsonReportParser()); + $parser = new NativeReportParser(rectorReports: new RectorJsonReportParser()); $clean = $parser->magicNumbers(new ProcessResult(0, $this->xml(), ''), ['/consumer/a.php', '/consumer/b.php']); self::assertSame('clean', $clean->status); self::assertSame(2, $clean->selectedCount); @@ -57,7 +57,7 @@ public static function invalidMagicReports(): iterable public function testInvalidMagicReportIsOperationalFailure(string $report, int $status): void { $this->expectException(InvalidArgumentException::class); - (new NativeReportParser(new RectorJsonReportParser()))->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); + (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); } /** @return iterable Unexpected nodes are rejected equally at all traversed structural boundaries. */ @@ -80,7 +80,7 @@ public static function unexpectedStructuralNodes(): iterable public function testUnexpectedStructuralNodesRejectClassification(string $report, int $status): void { $this->expectException(InvalidArgumentException::class); - (new NativeReportParser(new RectorJsonReportParser()))->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); + (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); } /** Structural whitespace/comments are harmless; actual snippet CDATA remains native source payload. */ @@ -91,7 +91,7 @@ public function testStructuralWhitespaceAndCommentsPreserveLegitimateSnippetCdat . '' . "\n\n" . '";]]>' . "\n\n\n"; - $result = (new NativeReportParser(new RectorJsonReportParser()))->magicNumbers(new ProcessResult(1, $report, ''), ['/consumer/a.php']); + $result = (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers(new ProcessResult(1, $report, ''), ['/consumer/a.php']); self::assertSame('findings', $result->status); self::assertSame(1, $result->findingCount); self::assertSame(7, $result->diagnostics[0]['line']); @@ -112,7 +112,7 @@ public function testXmlRejectsExternalEntitiesWithoutOpeningTheirSources(): void ] as $declaration) { $report = $declaration . '&source;'; try { - (new NativeReportParser(new RectorJsonReportParser()))->magicNumbers(new ProcessResult(0, $report, ''), []); + (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers(new ProcessResult(0, $report, ''), []); self::fail('DTD-bearing reports must be rejected.'); } catch (InvalidArgumentException) { self::assertSame(0, ExternalEntityProbe::$opens); @@ -126,7 +126,7 @@ public function testXmlRejectsExternalEntitiesWithoutOpeningTheirSources(): void /** Native errors dominate report-only policy even when changes were also proposed. */ public function testRectorNativeChangesAndErrorsHaveSeparateMeaning(): void { - $parser = new NativeReportParser(new RectorJsonReportParser()); + $parser = new NativeReportParser(rectorReports: new RectorJsonReportParser()); $clean = $parser->rector(new ProcessResult(0, '{"totals":{"changed_files":0,"errors":0}}', '')); self::assertSame('clean', $clean->status); self::assertNull($clean->selectedCount); @@ -162,7 +162,7 @@ public static function invalidRectorReports(): iterable public function testInvalidRectorReportIsOperationalFailure(string $report, int $status): void { $this->expectException(InvalidArgumentException::class); - (new NativeReportParser(new RectorJsonReportParser()))->rector(new ProcessResult($status, $report, '')); + (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->rector(new ProcessResult($status, $report, '')); } /** Plain preformatted HTML keeps Markdown/commands inert and bounds display without changing native totals. */ diff --git a/tests/Integration/OptionalMagicNumbersReportTest.php b/tests/Integration/OptionalMagicNumbersReportTest.php index 9d5d1b8..90b0365 100644 --- a/tests/Integration/OptionalMagicNumbersReportTest.php +++ b/tests/Integration/OptionalMagicNumbersReportTest.php @@ -34,7 +34,7 @@ public function testClassifierAcceptsActualNativeReportsAndExits(): void '--directory=' . $directory, '--all', '--format=xml', ], $directory, timeout: 30); self::assertSame($expected, $process->run(), $process->getErrorOutput()); - $classified = (new NativeReportParser(new RectorJsonReportParser()))->magicNumbers(new ProcessResult( + $classified = (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers(new ProcessResult( $expected, $process->getOutput(), $process->getErrorOutput(), ), [$path]); self::assertSame(0 === $expected ? 'clean' : 'findings', $classified->status); diff --git a/tests/Integration/RectorJsonIntegrationTest.php b/tests/Integration/RectorJsonIntegrationTest.php index bd18c05..293137a 100644 --- a/tests/Integration/RectorJsonIntegrationTest.php +++ b/tests/Integration/RectorJsonIntegrationTest.php @@ -53,10 +53,10 @@ public function testNativeJsonDistinguishesFindingsAndClean(): void $process = $this->preview(); self::assertSame($status, $process->getExitCode(), $process->getErrorOutput()); $report = json_decode($process->getOutput(), true, flags: JSON_THROW_ON_ERROR); - self::assertSame($report, (new RectorJsonReportParser())->parse(new ProcessResult($status, $process->getOutput(), $process->getErrorOutput()))); + self::assertSame($report, (new RectorJsonReportParser())->parse(new ProcessResult(exitCode: $status, stdout: $process->getOutput(), stderr: $process->getErrorOutput()))); self::assertSame($changes, $report['totals']['changed_files']); self::assertSame(0, $report['totals']['errors']); - $classified = (new NativeReportParser(new RectorJsonReportParser()))->rector(new ProcessResult($status, $process->getOutput(), $process->getErrorOutput())); + $classified = (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->rector(new ProcessResult(exitCode: $status, stdout: $process->getOutput(), stderr: $process->getErrorOutput())); self::assertSame(0 === $changes ? 'clean' : 'findings', $classified->status); self::assertSame($changes, $classified->findingCount); self::assertSame($source, file_get_contents($this->directory . '/source.php')); @@ -75,11 +75,11 @@ public function testNativeJsonRetainsOperationalErrors(): void $process = $this->preview(); self::assertSame(1, $process->getExitCode(), $process->getErrorOutput()); $report = json_decode($process->getOutput(), true, flags: JSON_THROW_ON_ERROR); - self::assertSame($report, (new RectorJsonReportParser())->parse(new ProcessResult(1, $process->getOutput(), $process->getErrorOutput()))); + self::assertSame($report, (new RectorJsonReportParser())->parse(new ProcessResult(exitCode: 1, stdout: $process->getOutput(), stderr: $process->getErrorOutput()))); self::assertGreaterThan(0, $report['totals']['errors']); self::assertNotEmpty($report['errors']); self::assertArrayHasKey('file', $report['errors'][0]); - $classified = (new NativeReportParser(new RectorJsonReportParser()))->rector(new ProcessResult(1, $process->getOutput(), $process->getErrorOutput())); + $classified = (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->rector(new ProcessResult(exitCode: 1, stdout: $process->getOutput(), stderr: $process->getErrorOutput())); self::assertSame('operational-error', $classified->status); self::assertTrue($classified->blocks('report')); self::assertSame($source, file_get_contents($this->directory . '/source.php')); diff --git a/tests/Refactoring/RectorJsonReportParserTest.php b/tests/Refactoring/RectorJsonReportParserTest.php index b7aaaf6..c61ae97 100644 --- a/tests/Refactoring/RectorJsonReportParserTest.php +++ b/tests/Refactoring/RectorJsonReportParserTest.php @@ -24,7 +24,7 @@ public function testNativeReportIsRetainedWithoutNormalization(): void 'totals' => ['changed_files' => 1, 'errors' => 1], 'native_extension' => ['retained' => true], ]; - $process = new ProcessResult(1, json_encode($report, JSON_THROW_ON_ERROR), 'native stderr'); + $process = new ProcessResult(exitCode: 1, stdout: json_encode(value: $report, flags: JSON_THROW_ON_ERROR), stderr: 'native stderr'); self::assertSame($report, (new RectorJsonReportParser())->parse($process)); self::assertSame('native stderr', $process->stderr); @@ -34,7 +34,7 @@ public function testNativeReportIsRetainedWithoutNormalization(): void public function testCleanReportRetainsAbsentOptionalLists(): void { $report = ['totals' => ['changed_files' => 0, 'errors' => 0]]; - self::assertSame($report, (new RectorJsonReportParser())->parse(new ProcessResult(0, json_encode($report, JSON_THROW_ON_ERROR), ''))); + self::assertSame($report, (new RectorJsonReportParser())->parse(new ProcessResult(exitCode: 0, stdout: json_encode(value: $report, flags: JSON_THROW_ON_ERROR), stderr: ''))); } /** @return iterable Existing native validation errors remain objective. */ @@ -59,6 +59,6 @@ public function testInvalidReportsRetainExistingErrors(string $json, int $exit, { $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage($message); - (new RectorJsonReportParser())->parse(new ProcessResult($exit, $json, '')); + (new RectorJsonReportParser())->parse(new ProcessResult(exitCode: $exit, stdout: $json, stderr: '')); } } From d70341643b0edc0c200f5147f39e6e389fb433cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Wed, 28 Oct 2026 19:16:36 -0300 Subject: [PATCH 3/4] style: expand Rector report calls and name fixture arguments --- .../OptionalAnalysis/NativeReportParser.php | 61 +++++++- src/Refactoring/RectorJsonReportParser.php | 27 +++- .../Automation/OptionalAnalysisInputTest.php | 75 +++++++++- .../Automation/OptionalAnalysisReportTest.php | 134 ++++++++++++++---- .../OptionalMagicNumbersReportTest.php | 27 ++-- .../Integration/RectorJsonIntegrationTest.php | 39 +++-- .../RectorJsonReportParserTest.php | 73 ++++++++-- 7 files changed, 364 insertions(+), 72 deletions(-) diff --git a/.github/OptionalAnalysis/NativeReportParser.php b/.github/OptionalAnalysis/NativeReportParser.php index 8eb0f3b..e902c79 100644 --- a/.github/OptionalAnalysis/NativeReportParser.php +++ b/.github/OptionalAnalysis/NativeReportParser.php @@ -58,7 +58,17 @@ public function magicNumbers(ProcessResult $process, array $selectedFiles): Anal 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); + return new AnalysisResult( + tool: 'Magic Numbers', + status: 0 === $count ? 'clean' : 'findings', + selectedCount: count( + $selectedFiles, + ), + findingCount: $count, + diagnostics: $diagnostics, + details: $process->stderr, + nativeExit: $process->exitCode, + ); } /** Native Rector errors remain blocking even when the same report also proposes changes. */ @@ -70,18 +80,48 @@ public function rector(ProcessResult $process): AnalysisResult $report = $this->rectorReports->parse($process); $changes = $report['totals']['changed_files']; $errorCount = $report['totals']['errors']; - $diagnostics = [...$this->rectorErrors($report['errors'] ?? []), ...$this->rectorChanges($report['changed_files'] ?? [])]; + $diagnostics = [ + ...$this->rectorErrors($report['errors'] ?? []), + ...$this->rectorChanges($report['changed_files'] ?? []), + ]; if (0 !== $errorCount) { - return new AnalysisResult(tool: 'Rector', status: 'operational-error', findingCount: $changes, diagnostics: $diagnostics, details: $errorCount . " native processing error(s).\n" . $process->stderr, nativeExit: $process->exitCode); + return new AnalysisResult( + tool: 'Rector', + status: 'operational-error', + findingCount: $changes, + diagnostics: $diagnostics, + details: $errorCount . " native processing error(s).\n" . $process->stderr, + nativeExit: $process->exitCode, + ); } - return new AnalysisResult(tool: 'Rector', status: 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); + return new AnalysisResult( + tool: 'Rector', + status: 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.'); + 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 { @@ -89,7 +129,14 @@ private function xml(string $output): DOMElement $document->resolveExternals = false; $document->substituteEntities = false; $document->validateOnParse = false; - if (!$document->loadXML($output, LIBXML_NONET) || null !== $document->doctype || 'phpmnd' !== $document->documentElement?->tagName) { + 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')) { diff --git a/src/Refactoring/RectorJsonReportParser.php b/src/Refactoring/RectorJsonReportParser.php index ce9a2f3..c8434a2 100644 --- a/src/Refactoring/RectorJsonReportParser.php +++ b/src/Refactoring/RectorJsonReportParser.php @@ -22,7 +22,10 @@ public function parse(ProcessResult $process): array try { $report = json_decode(json: $process->stdout, associative: true, flags: JSON_THROW_ON_ERROR); } catch (JsonException $exception) { - throw new InvalidArgumentException(message: 'Rector did not return a complete native JSON document.', previous: $exception); + throw new InvalidArgumentException( + message: '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.'); @@ -34,7 +37,14 @@ public function parse(ProcessResult $process): array 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)) { + 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.'); } $this->validateErrors($errors); @@ -100,7 +110,18 @@ private function validateErrors(array $errors): void private function validateChanges(array $changes, array $files): void { foreach ($changes as $change) { - if (!is_array($change) || !is_string($change['file'] ?? null) || !in_array($change['file'], $files, true) || !is_string($change['diff'] ?? null)) { + 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.'); } } diff --git a/tests/Automation/OptionalAnalysisInputTest.php b/tests/Automation/OptionalAnalysisInputTest.php index 8778e78..0f83b20 100644 --- a/tests/Automation/OptionalAnalysisInputTest.php +++ b/tests/Automation/OptionalAnalysisInputTest.php @@ -18,7 +18,13 @@ use PHPUnit\Framework\TestCase; use RuntimeException; -foreach (['AnalysisResult', 'BoundedProcess', 'NativeReportParser', 'WorkflowInput', 'OptionalAnalysisRunner'] as $helper) { +foreach ([ + 'AnalysisResult', + 'BoundedProcess', + 'NativeReportParser', + 'WorkflowInput', + 'OptionalAnalysisRunner', +] as $helper) { require_once dirname(__DIR__, 2) . '/.github/OptionalAnalysis/' . $helper . '.php'; } @@ -44,7 +50,10 @@ public function testEmptySelectionDoesNotStartTheAnalyzer(): void $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", + in_array('rev-parse', $arguments, true) && in_array('HEAD', $arguments, true) => str_repeat( + 'b', + 40, + ) . "\n", default => '', }; }); @@ -75,7 +84,17 @@ public function testRectorOnlyDoesNotSelectMagicNumbersFiles(): void $process->expects(self::never())->method('capture'); $process->expects(self::never())->method('run'); $results = $this->runner($process)->run(new WorkflowInput( - realpath(sys_get_temp_dir()), str_repeat('a', 40), str_repeat('b', 40), false, 'report', true, 'report', '', + directory: realpath(sys_get_temp_dir()), + base: str_repeat('a', 40), + head: str_repeat( + 'b', + 40, + ), + magicNumbers: false, + magicNumbersPolicy: 'report', + rector: true, + rectorPolicy: 'report', + rectorConfig: '', )); self::assertSame('disabled', $results[0]->status); self::assertSame('operational-error', $results[1]->status); @@ -89,7 +108,17 @@ public function testMagicNumbersSelectionFailureDoesNotPreventRectorValidation() $process->method('capture')->willThrowException(new RuntimeException('Selection unavailable.')); $process->expects(self::never())->method('run'); $results = $this->runner($process)->run(new WorkflowInput( - realpath(sys_get_temp_dir()), str_repeat('a', 40), str_repeat('b', 40), true, 'report', true, 'report', '', + directory: realpath(sys_get_temp_dir()), + base: str_repeat('a', 40), + head: str_repeat( + 'b', + 40, + ), + magicNumbers: true, + magicNumbersPolicy: 'report', + rector: true, + rectorPolicy: 'report', + rectorConfig: '', )); self::assertSame('operational-error', $results[0]->status); self::assertSame('Selection unavailable.', $results[0]->details); @@ -103,7 +132,13 @@ public function testMagicNumbersSelectionFailureDoesNotPreventRectorValidation() public function testEnvironmentInputsAreExplicitAndStrict(): void { $environment = [ - 'STANDARDS_DIRECTORY' => '/consumer', 'STANDARDS_BASE' => str_repeat('a', 40), 'STANDARDS_HEAD' => str_repeat('b', 40), + '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' => '', ]; @@ -121,12 +156,38 @@ public function testEnvironmentInputsAreExplicitAndStrict(): void /** 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(rectorReports: new RectorJsonReportParser()), '/nonexistent-tool'); + return new OptionalAnalysisRunner( + selection: new FileSelection( + process: $process, + ), + phpFiles: new PhpFiles(), + projects: new ProjectScopeResolver( + defaultConfig: '/unused', + directories: new ProjectDirectoryResolver(), + ), + process: new BoundedProcess(), + reports: new NativeReportParser( + rectorReports: new RectorJsonReportParser(), + ), + root: '/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', ''); + return new WorkflowInput( + directory: $directory, + base: str_repeat('a', 40), + head: str_repeat( + 'b', + 40, + ), + magicNumbers: $magic, + magicNumbersPolicy: 'report', + rector: false, + rectorPolicy: 'report', + rectorConfig: '', + ); } } diff --git a/tests/Automation/OptionalAnalysisReportTest.php b/tests/Automation/OptionalAnalysisReportTest.php index bcbb538..2713c16 100644 --- a/tests/Automation/OptionalAnalysisReportTest.php +++ b/tests/Automation/OptionalAnalysisReportTest.php @@ -27,12 +27,26 @@ public function testMagicNumbersUsesTheSelectionForCoverageAndNativeEntriesForFi self::markTestSkipped('Native XML classification requires the real DOM extension.'); } $parser = new NativeReportParser(rectorReports: new RectorJsonReportParser()); - $clean = $parser->magicNumbers(new ProcessResult(0, $this->xml(), ''), ['/consumer/a.php', '/consumer/b.php']); + $clean = $parser->magicNumbers( + new ProcessResult(exitCode: 0, stdout: $this->xml(), stderr: ''), + ['/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']); + $found = $parser->magicNumbers( + new ProcessResult( + exitCode: 1, + stdout: $this->xml( + '";]]>', + 1, + 1, + ), + stderr: '', + ), + ['/consumer/a.php'], + ); self::assertSame('findings', $found->status); self::assertSame(1, $found->findingCount); self::assertSame(7, $found->diagnostics[0]['line']); @@ -46,10 +60,19 @@ public static function invalidMagicReports(): iterable yield 'not XML' => ['not XML', 1]; yield 'truncated XML' => ['', 1]; yield 'wrong exit' => ['', 1]; - yield 'overflow' => ['', 0]; + yield 'overflow' => [ + '', + 0, + ]; yield 'counts' => ['', 1]; - yield 'unselected path' => ['', 1]; - yield 'entity declaration' => [']>', 0]; + yield 'unselected path' => [ + '', + 1, + ]; + yield 'entity declaration' => [ + ']>', + 0, + ]; } /** XML schema/status failures reject the report rather than returning a clean zero. */ @@ -57,7 +80,10 @@ public static function invalidMagicReports(): iterable public function testInvalidMagicReportIsOperationalFailure(string $report, int $status): void { $this->expectException(InvalidArgumentException::class); - (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); + (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers( + new ProcessResult(exitCode: $status, stdout: $report, stderr: ''), + ['/consumer/a.php'], + ); } /** @return iterable Unexpected nodes are rejected equally at all traversed structural boundaries. */ @@ -69,9 +95,18 @@ public static function unexpectedStructuralNodes(): iterable 'whitespace CDATA' => '', 'processing instruction' => '', ] as $label => $node) { - yield 'root ' . $label => ['' . $node . '', 0]; - yield 'files ' . $label => ['' . $node . '', 0]; - yield 'file ' . $label => ['' . $node . '', 1]; + yield 'root ' . $label => [ + '' . $node . '', + 0, + ]; + yield 'files ' . $label => [ + '' . $node . '', + 0, + ]; + yield 'file ' . $label => [ + '' . $node . '', + 1, + ]; } } @@ -80,7 +115,10 @@ public static function unexpectedStructuralNodes(): iterable public function testUnexpectedStructuralNodesRejectClassification(string $report, int $status): void { $this->expectException(InvalidArgumentException::class); - (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers(new ProcessResult($status, $report, ''), ['/consumer/a.php']); + (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers( + new ProcessResult(exitCode: $status, stdout: $report, stderr: ''), + ['/consumer/a.php'], + ); } /** Structural whitespace/comments are harmless; actual snippet CDATA remains native source payload. */ @@ -91,7 +129,10 @@ public function testStructuralWhitespaceAndCommentsPreserveLegitimateSnippetCdat . '' . "\n\n" . '";]]>' . "\n\n\n"; - $result = (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers(new ProcessResult(1, $report, ''), ['/consumer/a.php']); + $result = (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers( + new ProcessResult(exitCode: 1, stdout: $report, stderr: ''), + ['/consumer/a.php'], + ); self::assertSame('findings', $result->status); self::assertSame(1, $result->findingCount); self::assertSame(7, $result->diagnostics[0]['line']); @@ -112,7 +153,10 @@ public function testXmlRejectsExternalEntitiesWithoutOpeningTheirSources(): void ] as $declaration) { $report = $declaration . '&source;'; try { - (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers(new ProcessResult(0, $report, ''), []); + (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->magicNumbers( + new ProcessResult(exitCode: 0, stdout: $report, stderr: ''), + [], + ); self::fail('DTD-bearing reports must be rejected.'); } catch (InvalidArgumentException) { self::assertSame(0, ExternalEntityProbe::$opens); @@ -127,18 +171,29 @@ public function testXmlRejectsExternalEntitiesWithoutOpeningTheirSources(): void public function testRectorNativeChangesAndErrorsHaveSeparateMeaning(): void { $parser = new NativeReportParser(rectorReports: new RectorJsonReportParser()); - $clean = $parser->rector(new ProcessResult(0, '{"totals":{"changed_files":0,"errors":0}}', '')); + $clean = $parser->rector( + new ProcessResult(exitCode: 0, stdout: '{"totals":{"changed_files":0,"errors":0}}', stderr: ''), + ); 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, '')); + $changes = $parser->rector(new ProcessResult(exitCode: 2, stdout: $json, stderr: '')); 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::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"}]}', '')); + $error = $parser->rector( + new ProcessResult( + exitCode: 1, + stdout: '{"totals":{"changed_files":0,"errors":1},"errors":[{"file":"src/b.php","line":12,"message":"native failure"}]}', + stderr: '', + ), + ); self::assertSame('operational-error', $error->status); self::assertSame(12, $error->diagnostics[0]['line']); self::assertTrue($error->blocks('report')); @@ -162,17 +217,31 @@ public static function invalidRectorReports(): iterable public function testInvalidRectorReportIsOperationalFailure(string $report, int $status): void { $this->expectException(InvalidArgumentException::class); - (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->rector(new ProcessResult($status, $report, '')); + (new NativeReportParser(rectorReports: new RectorJsonReportParser()))->rector( + new ProcessResult(exitCode: $status, stdout: $report, stderr: ''), + ); } /** 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]); + $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'), + new AnalysisResult( + tool: 'Magic Numbers', + status: 'findings', + selectedCount: 1, + findingCount: 1000, + diagnostics: $diagnostics, + details: $malicious, + nativeExit: 1, + ), + new AnalysisResult(tool: 'Rector', status: 'disabled'), ]); self::assertLessThanOrEqual(65536, strlen($summary)); self::assertStringNotContainsString('