Skip to content

Commit 1a1cece

Browse files
Testclaude
andcommitted
fix(execution): reinterpret tools' empty-input failures as skipped (BUG-1)
PHPStan and PHPCS exit non-zero when their internal exclusion lists (excludePaths.analyse / --ignore) strip every input file, breaking pipelines on mono-domain MRs where the wrapper feeds them files that match a job's paths but fall entirely inside the tool's own exclusions. Each accelerable Job now declares isEmptyInputTolerated(exitCode, output) mirroring the existing isFixApplied() pattern. JobExecutor and FlowExecutor reinterpret matching results as JobResult::skipped with skipReason, emit onJobSkipped instead of onJobError, and bypass threshold evaluation — the tool didn't do real work, so comparing its near-zero duration against warn-after/fail-after would be meaningless. PhpstanJob: exit==1 + "No files found to analyse". PhpcsJob: exit in {1,2,3,16} + "All specified files were excluded" / "No files were checked" (defensive across PHPCS 3.x and the PHPCSStandards fork; the marker check is what actually decides). PHPMD already tolerates this natively (exit 0 on empty set); the other accelerable tools (parallel-lint, psalm, rector, php-cs-fixer) silently ignore non-matching inputs and need no override. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 683ccce commit 1a1cece

10 files changed

Lines changed: 486 additions & 9 deletions

File tree

docs/changelog.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22

33
All notable changes to this project are documented here.
44

5+
## [3.3.3]
6+
7+
### Fixed
8+
9+
- **Fast-branch / fast no longer fail with spurious "no files" errors when a job's tool config strips every input via its internal exclusion list.** Repro: a branch only touches files in one domain (e.g. `src/Gas/...`); the wrapper hands those files to a job whose `.neon` declares `excludePaths.analyse: [src/Gas]` (PHPStan) or whose `--ignore` CSV covers them (PHPCS). The tool then drops 100 % of the input and exits non-zero with `[ERROR] No files found to analyse.` (PHPStan, exit 1) or `ERROR: All specified files were excluded or did not match filtering rules.` (PHPCS, exit 16 on older versions and the PHPCSStandards fork). Before this fix the wrapper reported the job as failed, breaking mono-domain MRs (observed in consumer projects with per-domain flow structure). Each accelerable Job now declares an empty-input tolerance heuristic via `isEmptyInputTolerated(int $exitCode, string $output): bool` (mirror of the existing `isFixApplied()` pattern). `PhpstanJob` recognises `exit === 1` + `No files found to analyse`; `PhpcsJob` recognises `exit ∈ {1,2,3,16}` + `All specified files were excluded` / `No files were checked` (defensive across PHPCS 3.x and the PHPCSStandards fork). When tolerated, the JobResult is reinterpreted as `skipped: true` with `skipReason` instead of `success: false`, and the OutputHandler emits `onJobSkipped` rather than `onJobError`. Threshold evaluation is also bypassed for tolerated jobs — the tool didn't do real work, so comparing its near-zero duration against `warn-after`/`fail-after` would be meaningless. PHPMD already tolerates this case natively (`exit 0` when its `--exclude` empties the set); the other accelerable tools (parallel-lint, psalm, rector, php-cs-fixer) silently ignore non-matching inputs and do not need an override.
10+
511
## [3.3.2] ⚠️ Do not use — broken release
612

713
**This release is functionally identical to 3.3.1.** The git tag `v3.3.2` was published against a master commit whose bundled `.phar` binaries (`builds/githooks`, `builds/php7.4/githooks`) had never been updated from the `rc-3.3.2` branch where CI compiled them. Since GitHooks runs as a standalone `.phar`, installing `wtyd/githooks:3.3.2` ships the v3.3.1 binary under the v3.3.2 tag name. The fixes listed below are present in the source code of the tag but **not** in the executed binary.

src/Execution/FlowExecutor.php

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -669,6 +669,12 @@ private function collectResult(array $entry): JobResult
669669
return $this->buildResult($entry['job'], $entry['process'], $entry['start']);
670670
}
671671

672+
/**
673+
* @SuppressWarnings(PHPMD.CyclomaticComplexity) Aggregates exit-code, fix-applied,
674+
* empty-input tolerance, ignoreErrorsOnExit, threshold and output dispatch in
675+
* a single pass; each branch corresponds to one independent outcome of a single job.
676+
* @SuppressWarnings(PHPMD.NPathComplexity) Same reason.
677+
*/
672678
private function buildResult(JobAbstract $job, Process $process, float $start): JobResult
673679
{
674680
$elapsed = $this->now() - $start;
@@ -677,7 +683,8 @@ private function buildResult(JobAbstract $job, Process $process, float $start):
677683
$stdout = $process->getOutput();
678684
$output = $stdout . $process->getErrorOutput();
679685
$fixApplied = $job->isFixApplied($exitCode);
680-
$success = $exitCode === 0 || $fixApplied;
686+
$emptyInputTolerated = $job->isEmptyInputTolerated($exitCode, $output);
687+
$success = $exitCode === 0 || $fixApplied || $emptyInputTolerated;
681688

682689
if ($job->isIgnoreErrorsOnExit() && !$success) {
683690
$success = true;
@@ -688,7 +695,14 @@ private function buildResult(JobAbstract $job, Process $process, float $start):
688695
$this->gitStager->stageTrackedFiles();
689696
}
690697

691-
[$thresholdState, $thresholdReason, $warnAfter, $failAfter] = $this->evaluateThreshold($job, $elapsed);
698+
// Skip threshold for empty-input-tolerated jobs: the tool didn't do real
699+
// work, comparing its (near-zero) duration against warn-after/fail-after
700+
// would be meaningless and could fail a legitimate skip.
701+
if ($emptyInputTolerated) {
702+
[$thresholdState, $thresholdReason, $warnAfter, $failAfter] = [JobResult::THRESHOLD_NONE, null, null, null];
703+
} else {
704+
[$thresholdState, $thresholdReason, $warnAfter, $failAfter] = $this->evaluateThreshold($job, $elapsed);
705+
}
692706

693707
// Per-job FAIL by threshold flips OK→KO ONLY when the tool itself succeeded;
694708
// if the tool already failed (KO real), the threshold is informational and
@@ -697,9 +711,13 @@ private function buildResult(JobAbstract $job, Process $process, float $start):
697711
$success = false;
698712
}
699713

714+
$skipReason = null;
700715
$displayName = $job->getDisplayName();
701716

702-
if ($success) {
717+
if ($emptyInputTolerated) {
718+
$skipReason = 'tool reported no input files after applying internal exclusions';
719+
$this->outputHandler->onJobSkipped($displayName, $skipReason);
720+
} elseif ($success) {
703721
$this->outputHandler->onJobSuccess($displayName, $time);
704722
} else {
705723
$this->outputHandler->onJobError($displayName, $time, $output);
@@ -717,8 +735,8 @@ private function buildResult(JobAbstract $job, Process $process, float $start):
717735
$job->getType(),
718736
$exitCode,
719737
$job->getConfiguredPaths(),
720-
false,
721-
null,
738+
$emptyInputTolerated,
739+
$skipReason,
722740
$stdout,
723741
$this->buildPerJobInputFiles($job),
724742
$elapsed,

src/Execution/JobExecutor.php

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,23 @@ public function execute(JobAbstract $job): JobResult
3333
$stdout = $process->getOutput();
3434
$output = $stdout . $process->getErrorOutput();
3535
$fixApplied = $job->isFixApplied($exitCode);
36+
$emptyInputTolerated = $job->isEmptyInputTolerated($exitCode, $output);
3637

37-
$success = $exitCode === 0 || $fixApplied;
38+
$success = $exitCode === 0 || $fixApplied || $emptyInputTolerated;
3839

3940
if ($job->isIgnoreErrorsOnExit() && !$success) {
4041
$success = true;
4142
}
4243

44+
$skipReason = $emptyInputTolerated
45+
? 'tool reported no input files after applying internal exclusions'
46+
: null;
47+
4348
$displayName = $job->getDisplayName();
4449

45-
if ($success) {
50+
if ($emptyInputTolerated) {
51+
$this->printer->warning("$displayName - SKIP. Time: $time ($skipReason)");
52+
} elseif ($success) {
4653
$this->printer->success("$displayName - OK. Time: $time");
4754
} else {
4855
$this->printer->error("$displayName - KO. Time: $time");
@@ -63,8 +70,8 @@ public function execute(JobAbstract $job): JobResult
6370
$job->getType(),
6471
$exitCode,
6572
$job->getConfiguredPaths(),
66-
false,
67-
null,
73+
$emptyInputTolerated,
74+
$skipReason,
6875
$stdout
6976
);
7077
}

src/Jobs/JobAbstract.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,23 @@ public function isFixApplied(int $exitCode): bool
395395
return false;
396396
}
397397

398+
/**
399+
* Whether a non-zero exit code reflects the tool refusing to operate
400+
* on an empty input set (everything the wrapper passed got dropped by
401+
* the tool's internal exclusions). Subclasses override to recognise
402+
* the tool-specific exit code + output signature.
403+
*
404+
* When this returns true, the executor reinterprets the JobResult as
405+
* skipped rather than failed: the tool didn't fail — it had nothing to do.
406+
*
407+
* @SuppressWarnings(PHPMD.UnusedFormalParameter) Default implementation
408+
* ignores both arguments; subclasses inspect them.
409+
*/
410+
public function isEmptyInputTolerated(int $exitCode, string $output): bool
411+
{
412+
return false;
413+
}
414+
398415
/**
399416
* Whether this tool type supports structured output parsing (JSON).
400417
*/

src/Jobs/PhpcsJob.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,26 @@ class PhpcsJob extends JobAbstract
1010
{
1111
public const SUPPORTS_FAST = true;
1212

13+
/**
14+
* PHPCS variants of "every input was excluded" — observed across versions
15+
* (Squizlabs 2.x → 3.x and the PHPCSStandards fork). Both phrases have
16+
* appeared as stdout when `--ignore` (CLI) or `<exclude-pattern>` (ruleset)
17+
* filters out 100% of the files passed as arguments.
18+
*/
19+
private const EMPTY_INPUT_MARKERS = [
20+
'All specified files were excluded',
21+
'No files were checked',
22+
];
23+
24+
/**
25+
* Defensive exit-code set: PHPCS 3.13.x (the version we vendor) returns 0
26+
* silently when --ignore drops every input; older versions and the
27+
* PHPCSStandards fork return 16. We accept the {1,2,3,16} range so a future
28+
* version bump doesn't silently regress — the marker check below is what
29+
* actually decides whether the failure is real or "nothing to do".
30+
*/
31+
private const EMPTY_INPUT_EXIT_CODES = [1, 2, 3, 16];
32+
1333
protected const ARGUMENT_MAP = [
1434
'standard' => ['flag' => '--standard', 'type' => 'value'],
1535
'ignore' => ['flag' => '--ignore', 'type' => 'csv'],
@@ -88,6 +108,19 @@ public function getThreadCapability(): ?ThreadCapability
88108
return new ThreadCapability('parallel', $current);
89109
}
90110

111+
public function isEmptyInputTolerated(int $exitCode, string $output): bool
112+
{
113+
if (!in_array($exitCode, self::EMPTY_INPUT_EXIT_CODES, true)) {
114+
return false;
115+
}
116+
foreach (self::EMPTY_INPUT_MARKERS as $marker) {
117+
if (strpos($output, $marker) !== false) {
118+
return true;
119+
}
120+
}
121+
return false;
122+
}
123+
91124
public function applyThreadLimit(int $threads): void
92125
{
93126
$this->args['parallel'] = $threads;

src/Jobs/PhpstanJob.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@ class PhpstanJob extends JobAbstract
1111
{
1212
public const SUPPORTS_FAST = true;
1313

14+
/**
15+
* PHPStan emits this on stderr when every input file is filtered out by
16+
* `excludePaths.analyse` of the active config. The wrapper concatenates
17+
* stderr after stdout in JobExecutor/FlowExecutor before consulting
18+
* isEmptyInputTolerated(), so a single str_contains over $output is enough.
19+
*/
20+
private const EMPTY_INPUT_MARKER = 'No files found to analyse';
21+
1422
protected const ARGUMENT_MAP = [
1523
'config' => ['flag' => '-c', 'type' => 'value', 'separator' => ' '],
1624
'level' => ['flag' => '-l', 'type' => 'value', 'separator' => ' '],
@@ -49,6 +57,11 @@ public function getThreadCapability(): ?ThreadCapability
4957
return new ThreadCapability('_phpstan_internal', $workers, 1, false);
5058
}
5159

60+
public function isEmptyInputTolerated(int $exitCode, string $output): bool
61+
{
62+
return $exitCode === 1 && strpos($output, self::EMPTY_INPUT_MARKER) !== false;
63+
}
64+
5265
/**
5366
* Public accessor for the worker count declared in the .neon. Used by
5467
* cross-flow validators (ConfigurationParser) that must compare the
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
<?php
2+
3+
namespace Tests\System\Release;
4+
5+
use Tests\ReleaseTestCase;
6+
7+
/**
8+
* @group release
9+
*
10+
* BUG-1: when a job's tool config strips every input via its internal
11+
* exclusion list (`excludePaths.analyse` for PHPStan, `--ignore` for PHPCS),
12+
* the tool exits non-zero with a recognisable marker. The .phar must
13+
* reinterpret that as `skipped: true` instead of failing the flow.
14+
*
15+
* This test ships a `.neon` whose `excludePaths.analyse` covers the very file
16+
* the job is asked to analyse. Without the fix, PHPStan returns exit 1 +
17+
* "[ERROR] No files found to analyse." and the job is reported as failed.
18+
* With the fix, the JSON output reports `skipped: true` and `success: true`.
19+
*/
20+
class EmptyInputToleranceReleaseTest extends ReleaseTestCase
21+
{
22+
protected function setUp(): void
23+
{
24+
parent::setUp();
25+
26+
// A valid PHP file in src/. The .neon will exclude src/ entirely, so
27+
// PHPStan will report "No files found to analyse" with exit 1.
28+
file_put_contents(
29+
self::TESTS_PATH . '/src/File.php',
30+
$this->phpFileBuilder->build()
31+
);
32+
33+
$qaDir = self::TESTS_PATH . '/qa';
34+
if (!is_dir($qaDir)) {
35+
mkdir($qaDir, 0777, true);
36+
}
37+
38+
// .neon paths are relative to the .neon itself.
39+
// From testsDir/qa/, "../src" points to testsDir/src.
40+
file_put_contents(
41+
"$qaDir/phpstan-exclude-all.neon",
42+
"parameters:\n level: 0\n paths:\n - ../src\n excludePaths:\n analyse:\n - ../src\n"
43+
);
44+
}
45+
46+
/** @test */
47+
public function phpstan_job_with_all_inputs_excluded_is_reported_as_skipped_in_json()
48+
{
49+
// Build a v3-style githooks.php declaring a single phpstan job that
50+
// analyses testsDir/src — and whose .neon excludes that path entirely.
51+
// Without the BUG-1 fix this returns failed; with the fix it returns skipped.
52+
$githooksPhp = $this->configurationFileBuilder
53+
->enableV3Mode()
54+
->setV3Hooks([])
55+
->setV3Flows([])
56+
->setV3Jobs([
57+
'phpstan_excl' => [
58+
'type' => 'phpstan',
59+
'executable-path' => 'vendor/bin/phpstan',
60+
'config' => self::TESTS_PATH . '/qa/phpstan-exclude-all.neon',
61+
'paths' => [self::TESTS_PATH . '/src'],
62+
'level' => 0,
63+
],
64+
])
65+
->buildV3Php();
66+
67+
// ReleaseTestCase::tearDown() cleans this up.
68+
file_put_contents('githooks.php', $githooksPhp);
69+
70+
$output = [];
71+
$exitCode = 1;
72+
exec("$this->githooks job phpstan_excl --format=json 2>/dev/null", $output, $exitCode);
73+
$json = implode("\n", $output);
74+
75+
$this->assertSame(0, $exitCode, "githooks job exit code (raw output:\n$json)");
76+
77+
$decoded = json_decode($json, true);
78+
$this->assertIsArray($decoded, "JSON output must parse (raw:\n$json)");
79+
$this->assertArrayHasKey('jobs', $decoded);
80+
$this->assertCount(1, $decoded['jobs']);
81+
82+
$job = $decoded['jobs'][0];
83+
$this->assertSame('phpstan_excl', $job['name']);
84+
$this->assertTrue(
85+
$job['skipped'] ?? false,
86+
"Job must be reported as skipped — raw job entry:\n" . json_encode($job, JSON_PRETTY_PRINT)
87+
);
88+
$this->assertTrue($job['success'] ?? false, 'Skipped jobs do not fail the flow');
89+
$this->assertNotEmpty($job['skipReason'] ?? '', 'skipReason must be populated');
90+
}
91+
}

0 commit comments

Comments
 (0)