Keep the result cache when composer.lock changes but no package version did by SanderMuller · Pull Request #6086 · phpstan/phpstan-src

SanderMuller · 2026-07-22T12:27:20Z

Follow-up to phpstan/phpstan#14984, reported after a discussion with @staabm about result-cache misses on CI for projects that do not commit composer.lock.

When the result-cache metadata differs only in the Composer lock/installed files, ResultCacheManager asks PackageDependencyResolver::getChangedComposerPackages() which packages changed and re-seeds only the files depending on them. An empty result (no installed package version or reference changed) was treated identically to null (installed.php could not be parsed), so both discarded the whole cache and re-analysed every file.

That empty case is exactly the situation the surrounding comment already calls safe: the generated container and the analysis are unchanged when no package version changed. It is easy to hit in CI where composer.lock is not committed, because every job regenerates the lock, and any run-to-run difference that is not a package-version change (a different Composer patch version, dist/time metadata, or a content-hash change from an unrelated composer.json edit) flips the composerLocks hash and triggers a full re-analysis on top of a cache that was restored correctly.

This distinguishes the two cases:

null (undetermined, installed.php unparseable) still falls back to a full re-analysis.
[] (determined: nothing changed) keeps the restored cache and continues with the normal incremental analysis.

Added an e2e test (result-cache-composer-lock) that analyses cold, changes composer.lock without touching vendor/composer/installed.php, and asserts the cache is kept and no files are reanalysed. It fails before this change (full re-analysis, metadata do not match: composerLocks) and passes after.

Closes phpstan/phpstan#14984

…on did When the result-cache metadata differed only in the Composer lock/installed files, PackageDependencyResolver::getChangedComposerPackages() was consulted to re-seed just the files depending on a changed package. An empty result (no package version or reference changed) was treated the same as null (installed.php unparseable), so a composer.lock whose bytes changed without any package change discarded the entire cache and re-analysed every file. This is common in CI for projects that do not commit composer.lock: every job regenerates the lock, and any run-to-run difference that is not a package-version change forced a full re-analysis on top of a cache that was restored correctly. Distinguish the two: null still falls back to a full re-analysis, while an empty change set keeps the restored cache and continues with the normal incremental analysis. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

phpstan-bot · 2026-07-22T13:13:01Z

This pull request has been marked as ready for review.

staabm · 2026-07-22T14:14:54Z

+              jq '.["content-hash"]="0000000000000000000000000000000000000000"' composer.lock > composer.lock.new
+              mv composer.lock.new composer.lock
+              OUTPUT=$(../bashunit -a exit_code "0" "../../bin/phpstan analyse -vv")
+              echo "$OUTPUT"
+              ../bashunit -a contains 'Composer metadata changed but no package versions changed; keeping the result cache.' "$OUTPUT"
+              ../bashunit -a contains 'Result cache restored. 0 files will be reanalysed.' "$OUTPUT"


please add another test in which we change the content hash and in addition a single php file.
after the test we should assert 'Result cache restored. 1 files will be reanalysed.'

do we have a test which verifies the cache gets invalidated, after we updated a single package?

Done in a82d77d:

result-cache-composer-lock now also changes the content hash together with a single source file and asserts Result cache restored. 1 file will be reanalysed. (PHPStan prints 1 file singular at count 1). So the kept cache still reanalyses genuinely-changed files.

Added result-cache-package-update: a source file depends on psr/log, and changing the package's reference in vendor/composer/installed.php invalidates only the files depending on it (Composer packages changed (psr/log); re-analysing only the files depending on them. + 1 file will be reanalysed.). The package-change path was previously only covered by PackageDependencyResolverTest at the unit level.

staabm · 2026-07-22T14:34:23Z

were you able to test this on the tempest repo and verified the fix will make the result-cache work for them in PRs after the 1st commit?

…t-cache e2e Addresses review feedback on the composer-lock result cache tests: - result-cache-composer-lock: after the lock-only change (cache kept, 0 files), also change the content hash together with a single source file and assert "Result cache restored. 1 file will be reanalysed." so the kept cache still reanalyses genuinely changed files. - result-cache-package-update: a new e2e where a source file depends on psr/log and the package's recorded reference in vendor/composer/installed.php changes. Asserts the cache is invalidated only for files depending on that package (the package-granular path), not for the whole project. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

SanderMuller · 2026-07-22T15:00:45Z

Yes. I built a phar from this branch and ran it against the Tempest repo with its real phpstan.neon.dist (level max, all its extensions, 445 files), simulating two CI runs. Cold run, then a warm run where composer.lock changed but no installed package reference did:

stock 2.2.5: Result cache not used because the metadata do not match: composerLocks -> all 445 files re-analysed.
this branch: Composer metadata changed but no package versions changed; keeping the result cache. -> Result cache restored. 0 files will be reanalysed. Same 9 errors reported both runs.

One honest caveat on when this actually helps Tempest, since I looked into it: two back-to-back composer update runs on Tempest produce a byte-identical lock, so for commits close in time with nothing changed the cache already matches and is reused. The fix kicks in when the regenerated lock differs without any package version/reference changing (a composer.json edit that resolves to the same versions, or a Composer version bump between runs). It does not change two cases: a dependency actually updating still goes through the package-granular path (only the dependent files), and PHPStan itself updating (^2.0 with no committed lock pulls a new release) still triggers a full re-analysis, which is correct. So this closes the over-invalidation gap, but committing composer.lock (or pinning phpstan) is still the most complete fix on their side.

staabm · 2026-07-22T15:05:29Z

(or pinning phpstan) is still the most complete fix on their side.

they already use a (outdated) but pinned PHPStan version, I think?
https://github.com/tempestphp/tempest-framework/blob/8c692db663176d95a32195ab24b7dcf1ad2ea5ca/composer.json#L80

staabm · 2026-07-22T15:07:27Z

@@ -0,0 +1,18 @@
+{


the initial reproducer (tempest) does not contain a composer.lock. I think our tests shouldn't have one either to reflect the real world?
(should be gitignored)

Good point, done in 9ce1fc9. Both fixtures now gitignore composer.lock instead of committing it; the e2e step's composer install regenerates it from composer.json before the test runs. I checked that composer install with no lock writes one, so the content-hash churn and the installed.php reference edit both still work.

The real-world case that motivated this (a framework whose CI regenerates the lock every job) gitignores composer.lock, so the fixtures shouldn't ship one either. `composer install` in the e2e step regenerates the lock from composer.json before the test runs, so the reproduction is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

staabm

I love it - thank you.

SanderMuller · 2026-07-22T15:23:28Z

You're right, thanks, scratch that. They pin phpstan/phpstan: 2.1.40, so phpstanVersion is stable and isn't the trigger. But 2.1.40 is 2.1.x, and the package-granular invalidation only shipped in 2.2.3 (PackageDependencyResolver doesn't exist before then), so on 2.1.40 any composer.lock change still invalidates the whole cache. On 2.2.x, #5933 already reanalyses only the files depending on a bumped package, and this PR keeps the cache when the lock changed but no package did.

staabm · 2026-07-22T17:03:41Z

+              # Change composer.lock without changing any installed package version/reference
+              # (vendor/composer/installed.php is left untouched), like a regenerated lock in CI
+              # when composer.lock is not committed. The result cache must be kept, not discarded.
+              jq '.["content-hash"]="0000000000000000000000000000000000000000"' composer.lock > composer.lock.new


please also update these tests to use a patch file instead

SanderMuller marked this pull request as draft July 22, 2026 12:41

SanderMuller marked this pull request as ready for review July 22, 2026 13:12

staabm reviewed Jul 22, 2026

View reviewed changes

staabm approved these changes Jul 22, 2026

View reviewed changes

staabm requested a review from VincentLanglet July 22, 2026 15:16

VincentLanglet approved these changes Jul 22, 2026

View reviewed changes

staabm merged commit 2d93205 into phpstan:2.2.x Jul 22, 2026
740 of 742 checks passed

SanderMuller mentioned this pull request Jul 22, 2026

Invalidate the whole result cache when a changed package registers a container class #6088

Open

staabm reviewed Jul 22, 2026

View reviewed changes

Uh oh!

Conversation

SanderMuller commented Jul 22, 2026

Uh oh!

phpstan-bot commented Jul 22, 2026

Uh oh!

staabm Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

staabm Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

SanderMuller Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

staabm commented Jul 22, 2026

Uh oh!

SanderMuller commented Jul 22, 2026

Uh oh!

staabm commented Jul 22, 2026

Uh oh!

staabm Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SanderMuller Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

staabm left a comment

Choose a reason for hiding this comment

Uh oh!

SanderMuller commented Jul 22, 2026

Uh oh!

Uh oh!

staabm Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

staabm Jul 22, 2026 •

edited

Loading