From de01bb12209ca1f4fae5c1611f7299fe220eba25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Sun, 25 Oct 2026 13:31:02 -0300 Subject: [PATCH 1/2] fix: verify residual violations after explicit formatter writes --- .changelog/verify-fix-result.md | 7 +++++++ README.md | 4 ++++ src/Standards/StandardsRunner.php | 6 +++++- tests/Integration/StandardsTest.php | 19 +++++++++++++++++ tests/Standards/StandardsRunnerTest.php | 27 ++++++++++++++++++++++--- 5 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 .changelog/verify-fix-result.md diff --git a/.changelog/verify-fix-result.md b/.changelog/verify-fix-result.md new file mode 100644 index 0000000..32b7f10 --- /dev/null +++ b/.changelog/verify-fix-result.md @@ -0,0 +1,7 @@ +--- +category: fixed +type: patch +--- + +Verify each explicit formatter batch with a read-only check and propagate residual +violations rather than reporting successful completion of a partially normalized file. diff --git a/README.md b/README.md index 726d1ec..f690a8d 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,10 @@ Different checkouts cannot invalidate each other's cache. Native `withCache()` overrides remain supported and apply last; share cache directories only when their invalidation and concurrent use are intentionally coordinated. `fix` explicitly applies ECS fixes. It does not commit, push or update dependencies. +After each successful formatter batch, a read-only check verifies the result. +Any remaining violation or tool error is returned as a nonzero status. A formatter +pass can expose another rule, such as EOF normalization preceding opening-tag +spacing; inspect the partial diff before explicitly requesting another fix. Recheck and inspect the diff afterward; failures can leave already applied fixes. An empty PHP selection is a successful no-op. Invalid options, configuration paths, Git history or head mismatch fail closed with exit 2. Syntax/style tool failures, diff --git a/src/Standards/StandardsRunner.php b/src/Standards/StandardsRunner.php index 67d6975..35a7fe4 100644 --- a/src/Standards/StandardsRunner.php +++ b/src/Standards/StandardsRunner.php @@ -32,7 +32,8 @@ public function run(string $directory, array $files, string $config, bool $fix): // Bounded batches avoid ARG_MAX. Stop at the first failure; never report partial success. foreach (array_chunk($files, 50) as $batch) { - $command = [PHP_BINARY, $this->ecs, 'check', '--config=' . $config, '--no-progress-bar', '--no-interaction']; + $check = [PHP_BINARY, $this->ecs, 'check', '--config=' . $config, '--no-progress-bar', '--no-interaction']; + $command = $check; if ($fix) { $command[] = '--fix'; } @@ -40,6 +41,9 @@ public function run(string $directory, array $files, string $config, bool $fix): if (0 !== $status) { return $status; } + if ($fix && 0 !== $status = $this->process->run([...$check, '--', ...$batch], $directory)) { + return $status; + } } return 0; diff --git a/tests/Integration/StandardsTest.php b/tests/Integration/StandardsTest.php index 84b93ff..11d5a18 100644 --- a/tests/Integration/StandardsTest.php +++ b/tests/Integration/StandardsTest.php @@ -421,6 +421,25 @@ public function testDefaultCachesDoNotInvalidateAnotherCheckout(): void self::assertSame('project A cache', file_get_contents($cache . '/owned-marker')); } + /** A successful explicit fix must not hide a remaining violation after an EOF-sensitive formatter pass. */ + public function testSuccessfulFixCannotHideRemainingStyleViolations(): void + { + $this->put('source.php', 'commit(); + $fix = $this->cli(['fix', '--all']); + $after = file_get_contents($this->directory . '/source.php'); + $check = $this->cli(['check', '--all']); + + self::assertSame($check->getExitCode(), $fix->getExitCode(), $fix->getOutput() . $check->getOutput()); + self::assertSame($after, file_get_contents($this->directory . '/source.php')); + self::assertSame(0, $this->cli(['fix', '--all'])->getExitCode()); + $clean = file_get_contents($this->directory . '/source.php'); + + self::assertSame(0, $this->cli(['check', '--all'])->getExitCode()); + self::assertSame(0, $this->cli(['fix', '--all'])->getExitCode()); + self::assertSame($clean, file_get_contents($this->directory . '/source.php')); + } + /** Writes a synthetic fixture beneath the disposable test root, creating its parents. */ private function put(string $path, string $content): void { diff --git a/tests/Standards/StandardsRunnerTest.php b/tests/Standards/StandardsRunnerTest.php index 760ef22..4b44a73 100644 --- a/tests/Standards/StandardsRunnerTest.php +++ b/tests/Standards/StandardsRunnerTest.php @@ -34,6 +34,19 @@ public function testBatchesAreBoundedAndEcsFailureIsNotSwallowed(): void self::assertCount(57, $process->commands[101]); } + /** Propagates residual style failures after explicit fix without silently applying another write pass. */ + public function testFixSuccessIsVerifiedWithoutAnotherWritePass(): void + { + $process = new RecordingProcess(verificationStatus: 19); + $runner = new StandardsRunner($process, '/tools/ecs'); + + self::assertSame(19, $runner->run('/repo', ['/repo/a b.php'], '/config', true)); + self::assertCount(3, $process->commands); + self::assertContains('--fix', $process->commands[1]); + self::assertNotContains('--fix', $process->commands[2]); + self::assertSame('/repo/a b.php', $process->commands[2][array_key_last($process->commands[2])]); + } + /** Rejects malformed source before writes and makes empty selection a subprocess-free no-op. */ public function testSyntaxFailurePreventsFixAndNoFilesExecutesNothing(): void { @@ -51,8 +64,11 @@ final class RecordingProcess implements ProcessRunner public array $commands = []; /** Configures deterministic transport outcomes without executing real tools. */ - public function __construct(private readonly int $ecsStatus = 0, private readonly int $syntaxStatus = 0) - { + public function __construct( + private readonly int $ecsStatus = 0, + private readonly int $syntaxStatus = 0, + private readonly ?int $verificationStatus = null, + ) { } /** Fails if standards orchestration unexpectedly performs a capture operation. */ @@ -66,6 +82,11 @@ public function run(array $arguments, string $directory): int { $this->commands[] = $arguments; - return in_array('-l', $arguments, true) ? $this->syntaxStatus : $this->ecsStatus; + if (in_array('-l', $arguments, true)) { + return $this->syntaxStatus; + } + + return !in_array('--fix', $arguments, true) && null !== $this->verificationStatus + ? $this->verificationStatus : $this->ecsStatus; } } From afdd177722718263ca3a6d2c44acdd4f4fec0f88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Sun, 25 Oct 2026 13:46:53 -0300 Subject: [PATCH 2/2] test: require a residual violation in fix regression --- tests/Integration/StandardsTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Integration/StandardsTest.php b/tests/Integration/StandardsTest.php index 11d5a18..41028b9 100644 --- a/tests/Integration/StandardsTest.php +++ b/tests/Integration/StandardsTest.php @@ -430,6 +430,7 @@ public function testSuccessfulFixCannotHideRemainingStyleViolations(): void $after = file_get_contents($this->directory . '/source.php'); $check = $this->cli(['check', '--all']); + self::assertNotSame(0, $check->getExitCode(), $check->getOutput() . $check->getErrorOutput()); self::assertSame($check->getExitCode(), $fix->getExitCode(), $fix->getOutput() . $check->getOutput()); self::assertSame($after, file_get_contents($this->directory . '/source.php')); self::assertSame(0, $this->cli(['fix', '--all'])->getExitCode());