Fix #3367 - ensure --diff works after second run, not third

src/Psalm/Internal/Analyzer/ProjectAnalyzer.php

@@ -256,6 +256,8 @@ class ProjectAnalyzer
         $this->threads = $threads;
         $this->config = $config;
 
+        $this->clearCacheDirectoryIfConfigOrComposerLockfileChanged();
+
         $this->codebase = new Codebase(
             $config,
             $providers,
@@ -281,17 +283,38 @@ class ProjectAnalyzer
             $this->addProjectFile($file_path);
         }
 
-        if ($this->project_cache_provider && $this->project_cache_provider->hasLockfileChanged()) {
+        self::$instance = $this;
+    }
+
+    private function clearCacheDirectoryIfConfigOrComposerLockfileChanged() : void
+    {
+        if ($this->project_cache_provider
+            && $this->project_cache_provider->hasLockfileChanged()
+        ) {
             $this->progress->debug(
                 'Composer lockfile change detected, clearing cache' . "\n"
             );
 
-            Config::removeCacheDirectory($config->getCacheDirectory());
+            Config::removeCacheDirectory($this->config->getCacheDirectory());
+
+            if ($this->file_reference_provider->cache) {
+                $this->file_reference_provider->cache->hasConfigChanged();
+            }
 
             $this->project_cache_provider->updateComposerLockHash();
-        }
+        } elseif ($this->file_reference_provider->cache
+            && $this->file_reference_provider->cache->hasConfigChanged()
+        ) {
+            $this->progress->debug(
+                'Config change detected, clearing cache' . "\n"
+            );
 
-        self::$instance = $this;
+            Config::removeCacheDirectory($this->config->getCacheDirectory());
+
+            if ($this->project_cache_provider) {
+                $this->project_cache_provider->hasLockfileChanged();
+            }
+        }
     }
 
     /**

src/Psalm/Internal/Provider/FileReferenceCacheProvider.php

@@ -41,16 +41,16 @@ class FileReferenceCacheProvider
      */
     private $config;
 
-    /**
-     * @var bool
-     */
-    public $config_changed;
-
     public function __construct(Config $config)
     {
         $this->config = $config;
-        $this->config_changed = $config->hash !== $this->getConfigHashCache();
-        $this->setConfigHashCache($config->hash);
+    }
+
+    public function hasConfigChanged() : bool
+    {
+        $has_changed = $this->config->hash !== $this->getConfigHashCache();
+        $this->setConfigHashCache($this->config->hash);
+        return $has_changed;
     }
 
     /**
@@ -62,7 +62,7 @@ class FileReferenceCacheProvider
     {
         $cache_directory = $this->config->getCacheDirectory();
 
-        if (!$cache_directory || $this->config_changed) {
+        if (!$cache_directory) {
             return null;
         }
 
@@ -90,7 +90,7 @@ class FileReferenceCacheProvider
     {
         $cache_directory = $this->config->getCacheDirectory();
 
-        if (!$cache_directory || $this->config_changed) {
+        if (!$cache_directory) {
             return null;
         }
 
@@ -549,7 +549,6 @@ class FileReferenceCacheProvider
 
         if ($cache_directory
             && file_exists($analyzed_methods_cache_location)
-            && !$this->config_changed
         ) {
             /** @var array<string, array<string, int>> */
             return unserialize(file_get_contents($analyzed_methods_cache_location));
@@ -590,7 +589,6 @@ class FileReferenceCacheProvider
 
         if ($cache_directory
             && file_exists($file_maps_cache_location)
-            && !$this->config_changed
         ) {
             /**
              * @var array<string, FileMapType>
@@ -633,7 +631,6 @@ class FileReferenceCacheProvider
 
         if ($cache_directory
             && file_exists($type_coverage_cache_location)
-            && !$this->config_changed
         ) {
             /** @var array<string, array{int, int}> */
             $type_coverage_cache = unserialize(file_get_contents($type_coverage_cache_location));
@@ -676,7 +673,7 @@ class FileReferenceCacheProvider
             && file_exists($config_hash_cache_location)
         ) {
             /** @var string */
-            $file_maps_cache = unserialize(file_get_contents($config_hash_cache_location));
+            $file_maps_cache = file_get_contents($config_hash_cache_location);
 
             return $file_maps_cache;
         }
@@ -700,7 +697,7 @@ class FileReferenceCacheProvider
 
             file_put_contents(
                 $config_hash_cache_location,
-                serialize($hash)
+                $hash
             );
         }
     }

tests/EndToEnd/PsalmEndToEndTest.php

@@ -48,6 +48,7 @@ class PsalmEndToEndTest extends TestCase
         mkdir(self::$tmpDir . '/src');
 
         copy(__DIR__ . '/../fixtures/DummyProjectWithErrors/composer.json', self::$tmpDir . '/composer.json');
+        copy(__DIR__ . '/../fixtures/DummyProjectWithErrors/composer.lock', self::$tmpDir . '/composer.lock');
 
         (new Process(['composer', 'install', '--no-plugins'], self::$tmpDir))->mustRun();
     }
@@ -65,6 +66,14 @@ class PsalmEndToEndTest extends TestCase
         parent::setUp();
     }
 
+    public function tearDown(): void
+    {
+        if (\file_exists(self::$tmpDir . '/cache')) {
+            self::recursiveRemoveDirectory(self::$tmpDir . '/cache');
+        }
+        parent::tearDown();
+    }
+
     public function testHelpReturnsMessage(): void
     {
         $this->assertStringContainsString('Usage:', $this->runPsalm(['--help'], self::$tmpDir)['STDOUT']);
@@ -110,6 +119,27 @@ class PsalmEndToEndTest extends TestCase
         $this->assertSame(1, $result['CODE']);
     }
 
+    public function testPsalmDiff(): void
+    {
+        $this->runPsalmInit(1);
+        $result = $this->runPsalm(['--diff', '-m'], self::$tmpDir, true);
+        $this->assertStringContainsString('InvalidReturnType', $result['STDOUT']);
+        $this->assertStringContainsString('InvalidReturnStatement', $result['STDOUT']);
+        $this->assertStringContainsString('2 errors', $result['STDOUT']);
+        $this->assertStringContainsString('E', $result['STDERR']);
+
+        $this->assertSame(1, $result['CODE']);
+
+        $result = $this->runPsalm(['--diff', '-m'], self::$tmpDir, true);
+
+        $this->assertStringContainsString('InvalidReturnType', $result['STDOUT']);
+        $this->assertStringContainsString('InvalidReturnStatement', $result['STDOUT']);
+        $this->assertStringContainsString('2 errors', $result['STDOUT']);
+        $this->assertStringNotContainsString('E', $result['STDERR']);
+
+        $this->assertSame(1, $result['CODE']);
+    }
+
     public function testLegacyConfigWithoutresolveFromConfigFile(): void
     {
         $this->runPsalmInit(1);
@@ -137,7 +167,18 @@ class PsalmEndToEndTest extends TestCase
             $args[] = 'src';
             $args[] = (string) $level;
         }
-        return $this->runPsalm($args, self::$tmpDir, false, false);
+
+        $ret = $this->runPsalm($args, self::$tmpDir, false, false);
+
+        $psalm_config_contents = file_get_contents(self::$tmpDir . '/psalm.xml');
+        $psalm_config_contents = \str_replace(
+            'errorLevel="1"',
+            'errorLevel="1" cacheDirectory="' . self::$tmpDir . '/cache"',
+            $psalm_config_contents
+        );
+        file_put_contents(self::$tmpDir . '/psalm.xml', $psalm_config_contents);
+
+        return $ret;
     }
 
     /** from comment by itay at itgoldman dot com at

tests/Internal/Provider/FakeFileReferenceCacheProvider.php

@@ -57,7 +57,6 @@ class FakeFileReferenceCacheProvider extends \Psalm\Internal\Provider\FileRefere
 
     public function __construct()
     {
-        $this->config_changed = false;
     }
 
     /**

tests/fixtures/DummyProjectWithErrors/composer.json

@@ -2,7 +2,9 @@
     "name": "acme/sample-project",
     "description": "A sample project to be used when testing Psalm",
     "type": "project",
-    "require": {},
+    "require": {
+        "vimeo/psalm": "^3.11"
+    },
     "autoload": {
         "psr-4": {
             "Acme\\SampleProject\\": "src/"