Sort the result cache's package dependencies before writing them

.github/workflows/e2e-tests.yml

@@ -890,6 +890,17 @@ jobs:
               echo "$OUTPUT"
               ../bashunit -a contains 'Composer packages changed (test/logger); re-analysing the files depending on them and the files with errors.' "$OUTPUT"
               ../bashunit -a contains 'Result cache restored. 1 file will be reanalysed.' "$OUTPUT"
+          - script: |
+              cd e2e/result-cache-deterministic-order
+              composer install
+              # Nothing in the written file may depend on the order the parallel workers happened to
+              # finish in: a result cache that is not reproducible cannot be hashed, compared or
+              # deduplicated between two machines. The fixture splits four files into two jobs, so no
+              # completion order puts them in ascending order by accident, and assert-sorted.php fails
+              # if any entry section of the cache is out of key order.
+              ../../bin/phpstan analyse
+              OUTPUT=$(php assert-sorted.php tmp/resultCache.php 2>&1) || { echo "$OUTPUT"; exit 1; }
+              echo "$OUTPUT"
           - script: |
               cd e2e/result-cache-path-repository
               composer install

e2e/result-cache-deterministic-order/.gitignore

@@ -0,0 +1,2 @@
+/vendor
+/composer.lock

e2e/result-cache-deterministic-order/assert-sorted.php

@@ -0,0 +1,107 @@
+<?php declare(strict_types = 1);
+
+// Reads the framed result cache and fails if any of its entry sections is not in key order.
+// The order the workers happen to finish in must not reach the file: a cache that is not
+// reproducible cannot be hashed, compared or deduplicated between two machines.
+
+$path = $argv[1] ?? null;
+if ($path === null || !is_file($path)) {
+	fwrite(STDERR, sprintf("Result cache %s does not exist.\n", $path ?? '<missing argument>'));
+	exit(1);
+}
+
+$handle = fopen($path, 'r');
+if ($handle === false) {
+	fwrite(STDERR, sprintf("Cannot open %s.\n", $path));
+	exit(1);
+}
+
+fgets($handle); // the PHP prefix line that keeps the file inert when included
+
+$unsorted = [];
+$sections = 0;
+$packageLists = [];
+while (($header = fgets($handle)) !== false) {
+	$header = rtrim($header, "\n");
+	if ($header === '') {
+		continue;
+	}
+
+	$parts = explode(' ', $header, 2);
+	if (count($parts) !== 2) {
+		fwrite(STDERR, sprintf("Malformed frame header \"%s\".\n", $header));
+		exit(1);
+	}
+
+	[$name, $size] = $parts;
+	if (!str_ends_with($name, '*')) {
+		fread($handle, (int) $size);
+
+		continue;
+	}
+
+	$name = substr($name, 0, -1);
+	$keys = [];
+	for ($i = 0; $i < (int) $size; $i++) {
+		$length = (int) rtrim((string) fgets($handle), "\n");
+		$entry = unserialize((string) fread($handle, $length));
+		$key = (string) array_key_first($entry);
+		$keys[] = $key;
+		if ($name !== 'packageDependencies') {
+			continue;
+		}
+
+		// The packages of one file are collected in the order its dependencies are reflected,
+		// so their order has to be fixed too, not just the order of the files. Unlike the key
+		// order, this does not depend on how the scheduler composes jobs, so it still fails
+		// without the sort even when the whole project runs as a single job.
+		$packageLists[$key] = $entry[array_key_first($entry)];
+	}
+
+	$sections++;
+	$sorted = $keys;
+	sort($sorted, SORT_STRING);
+	if ($keys === $sorted) {
+		continue;
+	}
+
+	$unsorted[$name] = $keys;
+}
+
+fclose($handle);
+
+if ($sections === 0) {
+	fwrite(STDERR, "The result cache has no entry sections, so nothing was checked.\n");
+	exit(1);
+}
+
+if ($packageLists === []) {
+	fwrite(STDERR, "The result cache recorded no package dependencies, so nothing was checked.\n");
+	exit(1);
+}
+
+$problems = [];
+foreach ($unsorted as $name => $keys) {
+	$problems[] = sprintf('Section "%s" is not in key order: %s', $name, implode(', ', array_map('basename', $keys)));
+}
+
+foreach ($packageLists as $file => $packages) {
+	$sortedPackages = $packages;
+	sort($sortedPackages, SORT_STRING);
+	if ($packages === $sortedPackages) {
+		continue;
+	}
+
+	$problems[] = sprintf('The packages of %s are not in order: %s', basename($file), implode(', ', $packages));
+}
+
+if ($problems !== []) {
+	fwrite(STDERR, implode("\n", $problems) . "\n");
+	exit(1);
+}
+
+echo sprintf(
+	"All %d entry sections of the result cache are in key order, and so are the packages of all %d files that have package dependencies.\n",
+	$sections,
+	count($packageLists),
+);

e2e/result-cache-deterministic-order/composer.json

@@ -0,0 +1,26 @@
+{
+	"name": "phpstan/result-cache-deterministic-order-e2e",
+	"repositories": [
+		{
+			"type": "path",
+			"url": "./logger",
+			"options": {
+				"symlink": false
+			}
+		},
+		{
+			"type": "path",
+			"url": "./mailer",
+			"options": {
+				"symlink": false
+			}
+		}
+	],
+	"require": {
+		"test/logger": "*",
+		"test/mailer": "*"
+	},
+	"autoload": {
+		"classmap": ["src"]
+	}
+}

e2e/result-cache-deterministic-order/logger/composer.json

@@ -0,0 +1 @@
+{ "name": "test/logger", "version": "1.0.0", "autoload": { "classmap": ["src"] } }

e2e/result-cache-deterministic-order/logger/src/Logger.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace Test\Logger;
+
+class Logger
+{
+
+	public function info(string $message): void
+	{
+	}
+
+}

e2e/result-cache-deterministic-order/mailer/composer.json

@@ -0,0 +1 @@
+{ "name": "test/mailer", "version": "1.0.0", "autoload": { "classmap": ["src"] } }

e2e/result-cache-deterministic-order/mailer/src/Mailer.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace Test\Mailer;
+
+class Mailer
+{
+
+	public function send(string $message): void
+	{
+	}
+
+}

e2e/result-cache-deterministic-order/phpstan.neon

@@ -0,0 +1,12 @@
+parameters:
+	level: 5
+	tmpDir: tmp
+	paths:
+		- src
+	parallel:
+		# Two jobs of two files each, so the four files reach the main process interleaved
+		# (F1, F3 in one job, F2, F4 in the other) and no worker completion order can put
+		# them in ascending order by accident.
+		jobSize: 2
+		maximumNumberOfProcesses: 4
+		minimumNumberOfJobsPerProcess: 1

e2e/result-cache-deterministic-order/src/F1.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace ResultCacheDeterministicOrderE2E;
+
+use Test\Logger\Logger;
+use Test\Mailer\Mailer;
+
+// Mailer is declared before Logger on purpose: the packages of one file are collected in the
+// order its dependencies are reflected, so without a sort this file records test/mailer first.
+class F1
+{
+
+	public function __construct(private Mailer $mailer, private Logger $logger)
+	{
+	}
+
+	public function doFoo(): void
+	{
+		$this->mailer->send('hello');
+		$this->logger->info('hello');
+	}
+
+}

e2e/result-cache-deterministic-order/src/F2.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace ResultCacheDeterministicOrderE2E;
+
+use Test\Logger\Logger;
+use Test\Mailer\Mailer;
+
+// Mailer is declared before Logger on purpose: the packages of one file are collected in the
+// order its dependencies are reflected, so without a sort this file records test/mailer first.
+class F2
+{
+
+	public function __construct(private Mailer $mailer, private Logger $logger)
+	{
+	}
+
+	public function doFoo(): void
+	{
+		$this->mailer->send('hello');
+		$this->logger->info('hello');
+	}
+
+}

e2e/result-cache-deterministic-order/src/F3.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace ResultCacheDeterministicOrderE2E;
+
+use Test\Logger\Logger;
+use Test\Mailer\Mailer;
+
+// Mailer is declared before Logger on purpose: the packages of one file are collected in the
+// order its dependencies are reflected, so without a sort this file records test/mailer first.
+class F3
+{
+
+	public function __construct(private Mailer $mailer, private Logger $logger)
+	{
+	}
+
+	public function doFoo(): void
+	{
+		$this->mailer->send('hello');
+		$this->logger->info('hello');
+	}
+
+}

e2e/result-cache-deterministic-order/src/F4.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace ResultCacheDeterministicOrderE2E;
+
+use Test\Logger\Logger;
+use Test\Mailer\Mailer;
+
+// Mailer is declared before Logger on purpose: the packages of one file are collected in the
+// order its dependencies are reflected, so without a sort this file records test/mailer first.
+class F4
+{
+
+	public function __construct(private Mailer $mailer, private Logger $logger)
+	{
+	}
+
+	public function doFoo(): void
+	{
+		$this->mailer->send('hello');
+		$this->logger->info('hello');
+	}
+
+}

e2e/result-cache-deterministic-order/tmp/.gitignore

@@ -0,0 +1,2 @@
+*
+!.*

src/Analyser/ResultCache/ResultCacheManager.php

@@ -1473,6 +1473,19 @@ private function save(
 
 		ksort($exportedNodes);
 
+		// Every other section above is ordered before it is written; this one was not, so its order
+		// followed the order the workers happened to finish in. Two identical runs of a 4524-file
+		// project produced 74.9 MB caches of the same size differing in 468652 bytes; with this sort
+		// they differ in the 2 bytes of lastFullAnalysisTime, which is meant to differ. That costs
+		// nothing to a run reading the file back, but it means the cache cannot be hashed, compared or
+		// deduplicated between two machines, and "the same analysis produces the same cache" is worth
+		// having before anyone ships one as a CI artifact.
+		ksort($packageDependencies);
+		foreach ($packageDependencies as $file => $packages) {
+			sort($packages);
+			$packageDependencies[$file] = $packages;
+		}
+
 		// The only point where the StubFilesExtensions may run: the analysis is over, bootstrapFiles
 		// have been executed, so the extensions can rely on them. restore() reads these hashes back
 		// instead of running the extensions again.