Skip to content

Commit 8f396f6

Browse files
author
Test
committed
fix(cache:clear): resolve effective cache path per tool with adversarial coverage
cache:clear now reads the real cache location instead of relying on hard-coded defaults. Each job parses its native config (NEON, XML, PHP) where possible, falls back to the documented default otherwise, and surfaces a warning when the config uses an expression we can't resolve statically. Bugs fixed during the adversarial audit (would have hit real-world configs): - PHPStan tmpDir inside services: was misread as PHPStan's tmpDir; now requires top-level parameters: context. - PHPStan/Rector/PhpCsFixer regex picked the first match; now last-wins to match runtime behaviour, with comments stripped via token_get_all. - PHPStan: dynamic-last call after a static call now returns null instead of the stale path. - PHPUnit: cacheDirectory (10+) wins over deprecated cacheResultFile when both are present; previous order was inverted. - Rector default fixed to sys_get_temp_dir().'/rector_cached_files' (was /tmp/rector, also non-portable on Windows). - PHPMD default fixed to .phpmd.result.cache (was .phpmd.cache). - PHPCS multiple <arg name=cache>: last-wins. - Meta-args with whitespace-only values trim and fall back to the default. Last-resort meta-arg cache-dir on RectorJob (no CLI override exists in Rector); PHPStan emits a placeholder-warning when tmpDir uses %env.X% or similar unsupported placeholders. PhpCsFixer/PHPCS escape via their real CLI flags. Docs in docs/cli/cache-clear.md spell out the precedence per tool and warn that cache-dir is a last-resort with desync risk.
1 parent 0b0b25a commit 8f396f6

24 files changed

Lines changed: 2287 additions & 36 deletions

app/Commands/CacheClearCommand.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,16 +122,17 @@ private function clearJobCaches(array $jobs): int
122122
foreach ($jobs as $name => $jobConfig) {
123123
$jobInstance = $this->jobRegistry->create($jobConfig);
124124
$cachePaths = $jobInstance->getCachePaths();
125+
$resolutionWarning = $jobInstance->getCacheResolutionWarning();
125126

126127
foreach ($cachePaths as $path) {
127-
$cleared += $this->clearPath($name, $path);
128+
$cleared += $this->clearPath($name, $path, $resolutionWarning);
128129
}
129130
}
130131

131132
return $cleared;
132133
}
133134

134-
private function clearPath(string $jobName, string $path): int
135+
private function clearPath(string $jobName, string $path, ?string $resolutionWarning = null): int
135136
{
136137
if (is_dir($path)) {
137138
$this->deleteDirectory($path);
@@ -145,7 +146,8 @@ private function clearPath(string $jobName, string $path): int
145146
return 1;
146147
}
147148

148-
$this->line(" \e[90m- $jobName: $path (not found)\e[0m");
149+
$suffix = $resolutionWarning !== null ? "$resolutionWarning" : '';
150+
$this->line(" \e[90m- $jobName: $path (not found)$suffix\e[0m");
149151
return 0;
150152
}
151153

docs/changelog.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ All notable changes to this project are documented here.
77
### Fixed
88

99
- `--fast` / `--fast-branch` no longer leave non-accelerable jobs (`phpunit`, `paratest`, `phpcpd`, `script`, `custom`, `composer-*`) running their full suites when the effective input set is empty (no staged files / no diff vs base). The skip is now universal: any job — accelerable or not, with or without `paths` declared — is skipped with reason `no changes to validate` when the mode produced no input. Restores parity with the v2.x contract ("nothing changed = nothing to run"). Implemented in [`FlowPreparer::filterJobForMode()`](../src/Execution/FlowPreparer.php#L244) via a new universal guard backed by [`ExecutionContext::isEffectiveSetEmpty()`](../src/Execution/ExecutionContext.php). Decision-table coverage in [`tests/Unit/Execution/FlowPreparerTest.php`](../tests/Unit/Execution/FlowPreparerTest.php) (`it_filters_jobs_per_mode_decision_table` with 20 rows).
10+
- `cache:clear` now resolves the **effective** cache path for each job instead of relying on hard-coded defaults. Previously the command read a regex on the top-level `.neon` (PHPStan, ignoring `includes:` and placeholders) and used hard-coded literals for every other tool — `cache:clear` silently reported "not found" while the real cache lived elsewhere. After the fix:
11+
- PHPStan: `tmpDir:` is followed through `includes:` recursively (cycle-safe) and `%currentWorkingDirectory%` / `%rootDir%` are expanded.
12+
- Psalm: reads `cacheDirectory` from `psalm.xml`, resolved relative to the XML.
13+
- PHPCS: reads job arg `cache`, then `<arg name="cache" value="..."/>` from the ruleset.
14+
- PHPUnit: reads `cacheResultFile` and `cacheDirectory` (10+) from `phpunit.xml` / `phpunit.xml.dist`.
15+
- Rector: best-effort regex over `cacheDirectory(...)` in `rector.php` (literal, `__DIR__ . '/literal'`, `sys_get_temp_dir() . '/literal'`). Default fixed to `sys_get_temp_dir() . '/rector_cached_files'` (was incorrect `/tmp/rector`, also non-portable on Windows).
16+
- PHP-CS-Fixer: best-effort regex over `setCacheFile(...)` in `.php-cs-fixer.php`; respects job arg `cache-file` over the config (matching what php-cs-fixer itself does).
17+
- PHPMD: default fixed to `.phpmd.result.cache` (was incorrect `.phpmd.cache`).
18+
- When Rector / PHP-CS-Fixer config uses a dynamic expression for the cache path (variable, helper, env), `cache:clear` falls back to the default and surfaces a warning explaining why and how to override. Last-resort meta-arg `cache-dir` on the Rector job lets users force a path; PHP-CS-Fixer relies on its existing `cache-file` arg (which the tool itself respects as `--cache-file`). See [`docs/cli/cache-clear.md`](cli/cache-clear.md).
1019

1120
## [3.3.0]
1221

docs/cli/cache-clear.md

Lines changed: 77 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,83 @@ githooks cache:clear [names...] [--config=PATH]
1212

1313
Accepts job names, flow names (resolved to their jobs), or a mix. Without arguments, clears caches for all configured jobs.
1414

15-
## Default cache locations
16-
17-
| Tool | Default cache |
18-
|---|---|
19-
| PHPStan | `{sys_get_temp_dir}/phpstan/` (or `tmpDir` from `.neon` config) |
20-
| PHPMD | `.phpmd.cache` (or `cache-file` from job config) |
21-
| PHPCS | `.phpcs.cache` |
22-
| Psalm | `.psalm/cache/` |
23-
| PHPUnit | `.phpunit.result.cache` |
15+
## How the cache path is resolved
16+
17+
GitHooks tries to find the **effective** cache path for each job, in this order of precedence:
18+
19+
1. **Job arg in `githooks.php`** — if the tool accepts a CLI flag for the cache, declaring it on the job is the most reliable option.
20+
2. **Tool's native config file** — for tools whose cache config is declarative (NEON, XML), the path is parsed from there. Includes are followed and placeholders expanded where applicable.
21+
3. **Default** — the tool's documented default.
22+
23+
| Tool | Native config source | Default if nothing declared |
24+
|---|---|---|
25+
| **PHPStan** | `tmpDir:` in the `.neon` (follows `includes:` recursively, expands `%currentWorkingDirectory%` and `%rootDir%`) | `{sys_get_temp_dir}/phpstan/` |
26+
| **Psalm** | `cacheDirectory` attribute in `psalm.xml` (resolved relative to the XML) | `.psalm/cache/` |
27+
| **PHPCS** | Job arg `cache``<arg name="cache" value="..."/>` in the ruleset XML | `.phpcs.cache` |
28+
| **PHPUnit** | `cacheResultFile` / `cacheDirectory` attribute in `phpunit.xml` (or `phpunit.xml.dist`, resolved relative to the XML) | `.phpunit.result.cache` |
29+
| **PHPMD** | Job arg `cache-file` | `.phpmd.result.cache` |
30+
| **Rector** | Best-effort regex over `cacheDirectory(...)` in `rector.php` (recognises literals, `__DIR__ . '/literal'`, `sys_get_temp_dir() . '/literal'`) | `{sys_get_temp_dir}/rector_cached_files` |
31+
| **PHP-CS-Fixer** | Job arg `cache-file` (passed as `--cache-file` to the tool, which respects it over the config) → best-effort regex over `setCacheFile(...)` in `.php-cs-fixer.php` | `.php-cs-fixer.cache` |
32+
33+
## Last-resort override: `cache-dir` (Rector only)
34+
35+
When `rector.php` sets `cacheDirectory()` with a dynamic expression — a variable, an environment helper, a custom function — GitHooks **cannot** resolve the path statically. In that case `cache:clear` falls back to the default and prints a warning:
36+
37+
```
38+
- rector_src: /tmp/rector_cached_files (not found — could not parse cacheDirectory() in rector.php (uses a variable or helper); declare 'cache-dir' on the job to override (last-resort, see docs))
39+
```
40+
41+
To make `cache:clear` work in this scenario, declare `cache-dir` on the job in `githooks.php` with the **same path** that `rector.php` ends up resolving:
42+
43+
```php
44+
'rector_src' => [
45+
'type' => 'rector',
46+
'config' => 'rector.php',
47+
'cache-dir' => '.rector-cache', // last-resort override
48+
'paths' => ['src'],
49+
],
50+
```
51+
52+
!!! warning "Use only when strictly necessary"
53+
54+
`cache-dir` duplicates information that already lives in `rector.php`. If you change the path in one file and forget the other, **`cache:clear` will silently delete the wrong (or no) directory** while the real cache stays intact. The default behaviour — let GitHooks parse the config — is always preferable. Reach for `cache-dir` only when:
55+
56+
- your `rector.php` uses a variable, environment helper, or custom function for `cacheDirectory()`, and
57+
- you have a pipeline (CI, Makefile, hook) that relies on `cache:clear` doing the right thing.
58+
59+
Otherwise prefer rewriting `cacheDirectory()` to a literal or `__DIR__ . '/literal'` form, which GitHooks parses automatically.
60+
61+
## PHPCS with chained rulesets
62+
63+
If your `phpcs.xml` includes another ruleset with `<rule ref="Vendor/Other.xml"/>` and that included ruleset defines `<arg name="cache" value="..."/>`, **GitHooks does not follow refs** — only the cache declaration in the directly referenced ruleset is read.
64+
65+
The fix is the same single-source-of-truth pattern as PHP-CS-Fixer: declare `cache` on the job. PHPCS treats it as the CLI flag `--cache=PATH`, which **wins over any `<arg name="cache">`** in any ruleset (including the included ones). Both runtime and `cache:clear` end up using the path you declared:
66+
67+
```php
68+
'phpcs_src' => [
69+
'type' => 'phpcs',
70+
'standard' => 'qa/phpcs.xml',
71+
'cache' => 'qa/phpcs.cache', // CLI flag, wins over ruleset (incl. <rule ref>)
72+
'paths' => ['src'],
73+
],
74+
```
75+
76+
No duplication risk: phpcs **uses** the path you declared, so the runtime cache and the path that `cache:clear` deletes are guaranteed to match.
77+
78+
## PHP-CS-Fixer caveat
79+
80+
If `.php-cs-fixer.php` sets `setCacheFile()` with a dynamic expression, the same warning is emitted. **Do not** add a meta-arg here — instead use `cache-file` on the job:
81+
82+
```php
83+
'php_cs_fixer' => [
84+
'type' => 'php-cs-fixer',
85+
'config' => '.php-cs-fixer.php',
86+
'cache-file' => '.cache/php-cs-fixer.cache',
87+
'paths' => ['src'],
88+
],
89+
```
90+
91+
`cache-file` is a real CLI flag (`--cache-file=...`) that the tool itself respects over `setCacheFile()`. There is no duplication risk: php-cs-fixer **uses** the path you declared, so the runtime cache and the path that `cache:clear` deletes are guaranteed to match.
2492

2593
## Examples
2694

src/Configuration/JobConfiguration.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ private static function validateArguments(
365365
// 'time-budget' is recognised so the unknown-key loop doesn't emit a
366366
// duplicate warning; the dedicated check in validateTimeBudgetKeys()
367367
// already rejects it as an error (BUG-10).
368-
['executable-path', 'other-arguments', 'ignore-errors-on-exit', 'fail-fast', 'paths', 'rules', 'script', 'accelerable', 'execution', 'executable-prefix', 'cores', 'warn-after', 'fail-after', 'memory', 'time-budget']
368+
['executable-path', 'other-arguments', 'ignore-errors-on-exit', 'fail-fast', 'paths', 'rules', 'script', 'accelerable', 'execution', 'executable-prefix', 'cores', 'warn-after', 'fail-after', 'memory', 'time-budget', 'cache-dir']
369369
);
370370

371371
self::reportUnknownAndCliOnlyJobKeys($name, $type, $config, $knownKeys, $result);
Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Wtyd\GitHooks\Jobs\CacheResolver;
6+
7+
/**
8+
* Static-best-effort cache path extractor for PHP config files (rector.php,
9+
* .php-cs-fixer.php).
10+
*
11+
* These tools store cache config inside PHP code, which is not statically
12+
* parseable in the general case. We cover the three patterns that show up
13+
* in real projects:
14+
*
15+
* $config->method('absolute/path') → 'absolute/path'
16+
* $config->method(__DIR__ . '/literal') → dir(configFile)/literal
17+
* $config->method(\sys_get_temp_dir() . '/literal') → sys_get_temp_dir()/literal
18+
*
19+
* Anything dynamic (variables, helpers, env vars) is not extractable. The
20+
* caller distinguishes "match" (returns a string) from "no match"
21+
* (returns null) — when null, falling back to a default and emitting a
22+
* "could not parse" warning is up to the caller.
23+
*
24+
* The resolver intentionally does not load or execute the config file.
25+
*/
26+
final class PhpConfigCacheResolver
27+
{
28+
private const STRING_PATTERN = '(?:\'((?:\\\\.|[^\'\\\\])*)\'|"((?:\\\\.|[^"\\\\])*)")';
29+
30+
/**
31+
* @return string|null Resolved cache path, or null when no recognized
32+
* pattern matches (or the file is unreadable).
33+
*/
34+
public static function resolve(string $configPath, string $methodName): ?string
35+
{
36+
$content = self::readSanitizedContent($configPath);
37+
if ($content === null) {
38+
return null;
39+
}
40+
$methodEscaped = preg_quote($methodName, '/');
41+
42+
$allCallOffsets = self::allInvocationOffsets($content, $methodEscaped);
43+
if ($allCallOffsets === []) {
44+
return null;
45+
}
46+
$lastCallOffset = max($allCallOffsets);
47+
48+
$candidates = array_merge(
49+
self::collectDirConcat($content, $methodEscaped, $configPath),
50+
self::collectSysTempConcat($content, $methodEscaped),
51+
self::collectLiteral($content, $methodEscaped)
52+
);
53+
54+
foreach ($candidates as $hit) {
55+
if ($hit['pos'] === $lastCallOffset) {
56+
return $hit['path'];
57+
}
58+
}
59+
return null;
60+
}
61+
62+
/** @return int[] */
63+
private static function allInvocationOffsets(string $content, string $methodEscaped): array
64+
{
65+
if (preg_match_all('/->\s*' . $methodEscaped . '\s*\(/', $content, $matches, PREG_OFFSET_CAPTURE) === false) {
66+
return [];
67+
}
68+
$offsets = [];
69+
foreach ($matches[0] as $match) {
70+
$offsets[] = (int) $match[1];
71+
}
72+
return $offsets;
73+
}
74+
75+
/**
76+
* Reads the file and strips PHP comments so commented-out method calls
77+
* don't pollute the regex match.
78+
*/
79+
private static function readSanitizedContent(string $configPath): ?string
80+
{
81+
if (!is_file($configPath) || !is_readable($configPath)) {
82+
return null;
83+
}
84+
$content = file_get_contents($configPath);
85+
if ($content === false) {
86+
return null;
87+
}
88+
return self::stripComments($content);
89+
}
90+
91+
private static function stripComments(string $content): string
92+
{
93+
$tokens = token_get_all($content);
94+
if ($tokens === []) {
95+
return $content;
96+
}
97+
$out = '';
98+
foreach ($tokens as $token) {
99+
if (is_array($token)) {
100+
if (in_array($token[0], [T_COMMENT, T_DOC_COMMENT], true)) {
101+
continue;
102+
}
103+
$out .= $token[1];
104+
continue;
105+
}
106+
$out .= $token;
107+
}
108+
return $out;
109+
}
110+
111+
/** @return array<int, array{pos: int, path: string}> */
112+
private static function collectDirConcat(string $content, string $methodEscaped, string $configPath): array
113+
{
114+
$regex = '/->\s*' . $methodEscaped . '\s*\(\s*__DIR__\s*\.\s*' . self::STRING_PATTERN . '\s*\)/';
115+
$base = dirname(realpath($configPath) ?: $configPath);
116+
return self::collectMatches($regex, $content, static function (array $m) use ($base): string {
117+
return $base . self::pickLiteral($m);
118+
});
119+
}
120+
121+
/** @return array<int, array{pos: int, path: string}> */
122+
private static function collectSysTempConcat(string $content, string $methodEscaped): array
123+
{
124+
$regex = '/->\s*' . $methodEscaped . '\s*\(\s*\\\\?sys_get_temp_dir\s*\(\s*\)\s*\.\s*' . self::STRING_PATTERN . '\s*\)/';
125+
$tmp = sys_get_temp_dir();
126+
return self::collectMatches($regex, $content, static function (array $m) use ($tmp): string {
127+
return $tmp . self::pickLiteral($m);
128+
});
129+
}
130+
131+
/** @return array<int, array{pos: int, path: string}> */
132+
private static function collectLiteral(string $content, string $methodEscaped): array
133+
{
134+
$regex = '/->\s*' . $methodEscaped . '\s*\(\s*' . self::STRING_PATTERN . '\s*\)/';
135+
return self::collectMatches($regex, $content, static function (array $m): string {
136+
return self::pickLiteral($m);
137+
});
138+
}
139+
140+
/**
141+
* @param callable(array<int, string>): string $resolver
142+
* @return array<int, array{pos: int, path: string}>
143+
*/
144+
private static function collectMatches(string $regex, string $content, callable $resolver): array
145+
{
146+
if (preg_match_all($regex, $content, $matches, PREG_OFFSET_CAPTURE) === false) {
147+
return [];
148+
}
149+
$hits = [];
150+
$count = isset($matches[0]) ? count($matches[0]) : 0;
151+
for ($i = 0; $i < $count; $i++) {
152+
$offset = (int) $matches[0][$i][1];
153+
$captured = [
154+
$matches[0][$i][0],
155+
isset($matches[1][$i]) ? (string) $matches[1][$i][0] : '',
156+
isset($matches[2][$i]) ? (string) $matches[2][$i][0] : '',
157+
];
158+
$hits[] = ['pos' => $offset, 'path' => $resolver($captured)];
159+
}
160+
return $hits;
161+
}
162+
163+
/** @param array<int, string> $matches */
164+
private static function pickLiteral(array $matches): string
165+
{
166+
return $matches[1] !== '' ? $matches[1] : ($matches[2] ?? '');
167+
}
168+
169+
/**
170+
* Whether the config file declares the given method but with an expression
171+
* that we can't resolve statically (variables, function calls other than
172+
* __DIR__/sys_get_temp_dir, env vars, ...). Used by callers to decide
173+
* whether to surface a "could not parse" warning.
174+
*/
175+
public static function declaresUnresolvable(string $configPath, string $methodName): bool
176+
{
177+
$content = self::readSanitizedContent($configPath);
178+
if ($content === null) {
179+
return false;
180+
}
181+
$methodEscaped = preg_quote($methodName, '/');
182+
return preg_match('/->\s*' . $methodEscaped . '\s*\(/', $content) === 1
183+
&& self::resolve($configPath, $methodName) === null;
184+
}
185+
}

0 commit comments

Comments
 (0)