Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .github/workflows/e2e-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,40 @@ jobs:
OUTPUT=$(../bashunit -a exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
echo "$OUTPUT"
../bashunit -a contains 'ClassUsingDefine.php:13:Method ResultCacheE2EDefine\ClassUsingDefine::getMode() should return int<1, 3> but returns 5. [identifier=return.type]' "$OUTPUT"
- script: |
cd e2e/result-cache-composer-lock
composer install
../../bin/phpstan analyse
# 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please also update these tests to use a patch file instead

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to. Since #6086 is already merged, do you want this as a separate follow-up PR?

result-cache-package-update converts cleanly to the #6088 patch-file style. result-cache-composer-lock has a small wrinkle I'd like to talk through (its composer.lock is gitignored now, so there's nothing for a patch to apply against), but that can wait until you confirm you want the follow-up.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please followup

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"
Comment on lines +389 to +394

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a82d77d:

  1. 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.
  2. 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.

# Change the content hash again AND edit a single source file: the cache is still kept
# (no package changed), and only the one changed file is reanalysed.
printf '\n' >> src/Foo.php
jq '.["content-hash"]="1111111111111111111111111111111111111111"' 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. 1 file will be reanalysed.' "$OUTPUT"
- script: |
cd e2e/result-cache-package-update
composer install
../../bin/phpstan analyse
# Simulate a single Composer package being updated by changing its recorded reference in
# vendor/composer/installed.php. The cache must be invalidated only for files depending on
# that package (src/Foo.php uses psr/log), not for the whole project.
php -r '$f = "vendor/composer/installed.php"; $d = require $f; $d["versions"]["psr/log"]["reference"] = "0000000000000000000000000000000000000000"; file_put_contents($f, "<?php return " . var_export($d, true) . ";" . PHP_EOL);'
OUTPUT=$(../bashunit -a exit_code "0" "../../bin/phpstan analyse -vv")
echo "$OUTPUT"
../bashunit -a contains 'Composer packages changed (psr/log); re-analysing only the files depending on them.' "$OUTPUT"
../bashunit -a contains 'Result cache restored. 1 file will be reanalysed.' "$OUTPUT"
- script: |
cd e2e/bug-12606
export CONFIGTEST=test
Expand Down
2 changes: 2 additions & 0 deletions e2e/result-cache-composer-lock/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/vendor
/composer.lock
6 changes: 6 additions & 0 deletions e2e/result-cache-composer-lock/composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "phpstan/result-cache-composer-lock-e2e",
"autoload": {
"classmap": ["src"]
}
}
5 changes: 5 additions & 0 deletions e2e/result-cache-composer-lock/phpstan.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
parameters:
level: 5
tmpDir: tmp
paths:
- src
13 changes: 13 additions & 0 deletions e2e/result-cache-composer-lock/src/Foo.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

namespace ResultCacheComposerLockE2E;

class Foo
{

public function doFoo(): int
{
return 1;
}

}
2 changes: 2 additions & 0 deletions e2e/result-cache-composer-lock/tmp/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
*
!.*
2 changes: 2 additions & 0 deletions e2e/result-cache-package-update/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/vendor
/composer.lock
9 changes: 9 additions & 0 deletions e2e/result-cache-package-update/composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "phpstan/result-cache-package-update-e2e",
"require": {
"psr/log": "^3.0"
},
"autoload": {
"classmap": ["src"]
}
}
5 changes: 5 additions & 0 deletions e2e/result-cache-package-update/phpstan.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
parameters:
level: 5
tmpDir: tmp
paths:
- src
19 changes: 19 additions & 0 deletions e2e/result-cache-package-update/src/Foo.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace ResultCachePackageUpdateE2E;

use Psr\Log\LoggerInterface;

class Foo
{

public function __construct(private LoggerInterface $logger)
{
}

public function doFoo(): void
{
$this->logger->info('hello');
}

}
2 changes: 2 additions & 0 deletions e2e/result-cache-package-update/tmp/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
*
!.*
40 changes: 26 additions & 14 deletions src/Analyser/ResultCache/ResultCacheManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -268,12 +268,13 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
// and analysis are unchanged except for code coming from packages whose version actually
// changed. Re-analyse just the files depending on a changed package instead of everything;
// the existing incremental loop below then propagates to their dependents on signature change.
// Any other meta difference, or an undetermined change set, falls back to a full re-analysis.
// Any other meta difference, or an undetermined change set (installed.php cannot be parsed),
// falls back to a full re-analysis.
$changedPackages = array_diff($diffs, ['composerLocks', 'composerInstalled']) === []
? $this->packageDependencyResolver->getChangedComposerPackages($data['meta'], $meta)
: null;

if ($changedPackages === null || $changedPackages === []) {
if ($changedPackages === null) {
if ($output->isVeryVerbose()) {
$output->writeLineFormatted('Result cache not used because the metadata do not match: ' . implode(', ', $diffs));
}
Expand All @@ -296,18 +297,29 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
);
}

if ($output->isVeryVerbose()) {
$output->writeLineFormatted(sprintf(
'Composer packages changed (%s); re-analysing only the files depending on them.',
implode(', ', $changedPackages),
));
}
$changedPackagesLookup = array_fill_keys($changedPackages, true);
foreach ($packageDependencies as $packageDependentFile => $filePackages) {
foreach ($filePackages as $filePackage) {
if (isset($changedPackagesLookup[$filePackage])) {
$packageSeededFiles[] = $packageDependentFile;
break;
if ($changedPackages === []) {
// The Composer lock/installed metadata changed but no installed package's version or
// reference did (e.g. a composer.lock regenerated with different formatting or dist/time
// metadata, common in CI where composer.lock is not committed). Nothing analysis-relevant
// changed, so keep the restored cache and fall through to the normal incremental analysis
// instead of re-analysing everything.
if ($output->isVeryVerbose()) {
$output->writeLineFormatted('Composer metadata changed but no package versions changed; keeping the result cache.');
}
} else {
if ($output->isVeryVerbose()) {
$output->writeLineFormatted(sprintf(
'Composer packages changed (%s); re-analysing only the files depending on them.',
implode(', ', $changedPackages),
));
}
$changedPackagesLookup = array_fill_keys($changedPackages, true);
foreach ($packageDependencies as $packageDependentFile => $filePackages) {
foreach ($filePackages as $filePackage) {
if (isset($changedPackagesLookup[$filePackage])) {
$packageSeededFiles[] = $packageDependentFile;
break;
}
}
}
}
Expand Down
Loading