From 4f284db499242d3c7d35669599df34d77a57bc9a Mon Sep 17 00:00:00 2001 From: Sven Herrmann Date: Thu, 30 Jul 2026 18:07:52 +0200 Subject: [PATCH] Store PhpStorm stub paths relative to the stubs package in the reflection cache --- .../BetterReflectionSourceLocatorFactory.php | 3 + .../CachedPhpInternalSourceLocator.php | 84 +++++++- .../CachedPhpInternalSourceLocatorTest.php | 194 ++++++++++++++++++ .../SourceLocator/SpyCacheStorage.php | 29 +++ 4 files changed, 307 insertions(+), 3 deletions(-) create mode 100644 tests/PHPStan/Reflection/BetterReflection/SourceLocator/CachedPhpInternalSourceLocatorTest.php create mode 100644 tests/PHPStan/Reflection/BetterReflection/SourceLocator/SpyCacheStorage.php diff --git a/src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php b/src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php index 47d29ca36d4..cec81b10dc3 100644 --- a/src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php +++ b/src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php @@ -15,6 +15,7 @@ use PHPStan\Cache\Cache; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; +use PHPStan\File\FileHelper; use PHPStan\Php\PhpVersion; use PHPStan\Reflection\BetterReflection\SourceLocator\AutoloadFunctionsSourceLocator; use PHPStan\Reflection\BetterReflection\SourceLocator\AutoloadSourceLocator; @@ -57,6 +58,7 @@ public function __construct( private Parser $php8Parser, private Cache $cache, private PhpVersion $phpVersion, + private FileHelper $fileHelper, private PhpStormStubsSourceStubber $phpstormStubsSourceStubber, private ReflectionSourceStubber $reflectionSourceStubber, private OptimizedSingleFileSourceLocatorRepository $optimizedSingleFileSourceLocatorRepository, @@ -182,6 +184,7 @@ public function create(): SourceLocator new PhpInternalSourceLocator($astPhp8Locator, $this->phpstormStubsSourceStubber), $this->cache, $this->phpVersion, + $this->fileHelper, )); $locators[] = new AutoloadSourceLocator($this->fileNodesFetcher, true); diff --git a/src/Reflection/BetterReflection/SourceLocator/CachedPhpInternalSourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/CachedPhpInternalSourceLocator.php index 51980b74dc1..a91b7d39061 100644 --- a/src/Reflection/BetterReflection/SourceLocator/CachedPhpInternalSourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/CachedPhpInternalSourceLocator.php @@ -2,6 +2,7 @@ namespace PHPStan\Reflection\BetterReflection\SourceLocator; +use JetBrains\PHPStormStub\PhpStormStubsMap; use Override; use PHPStan\BetterReflection\Identifier\Identifier; use PHPStan\BetterReflection\Identifier\IdentifierType; @@ -13,12 +14,18 @@ use PHPStan\BetterReflection\Reflector\Reflector; use PHPStan\BetterReflection\SourceLocator\Type\SourceLocator; use PHPStan\Cache\Cache; +use PHPStan\File\FileHelper; use PHPStan\Internal\ComposerHelper; use PHPStan\Php\PhpVersion; use function array_key_exists; use function is_array; +use function is_file; +use function is_string; use function sprintf; +use function str_starts_with; +use function strlen; use function strtolower; +use function substr; /** * Caches the reflections built from the PhpStorm stubs the way the optimized @@ -29,17 +36,26 @@ * * The stubber's output depends on the stubs package, the reflection library * and the target PHP version (stub members are version-filtered), so all - * three are part of the cache key. + * three are part of the cache key. The key deliberately contains no + * installation path — the same cache directory may be shared by installations + * seeing the project at different absolute paths (host vs. Docker container). + * The persisted blob must be just as portable, so the stub file name is + * stored relative to the stubs package and resolved against the current + * installation on import; an entry whose stub file cannot be resolved here + * (e.g. written by a phar, read by a vendor install) counts as a cache miss. */ final class CachedPhpInternalSourceLocator implements SourceLocator { private ?string $variableCacheKey = null; + private ?string $stubsDirectory = null; + public function __construct( private SourceLocator $inner, private Cache $cache, private PhpVersion $phpVersion, + private FileHelper $fileHelper, ) { } @@ -55,6 +71,9 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): $variableCacheKey = $this->getVariableCacheKey(); $cached = $this->cache->load($cacheKey, $variableCacheKey); + if (is_array($cached)) { + $cached = $this->resolveStubFilename($cached); + } if (is_array($cached)) { if ($identifier->isConstant()) { return ReflectionConstant::importFromCache($reflector, $cached); @@ -77,12 +96,71 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier): || $reflection instanceof ReflectionFunction || $reflection instanceof ReflectionConstant ) { - $this->cache->save($cacheKey, $variableCacheKey, $reflection->exportToCache()); + $export = $this->relativizeStubFilename($reflection->exportToCache()); + if ($export !== null) { + $this->cache->save($cacheKey, $variableCacheKey, $export); + } } return $reflection; } + /** + * @param array $export + * @return array|null + */ + private function relativizeStubFilename(array $export): ?array + { + if (!is_array($export['locatedSource'] ?? null) || !is_array($export['locatedSource']['data'] ?? null)) { + return null; + } + + $filename = $export['locatedSource']['data']['filename'] ?? null; + if (!is_string($filename)) { + return null; + } + + $stubsDirectoryPrefix = $this->getStubsDirectory() . '/'; + $normalizedFilename = $this->fileHelper->normalizePath($filename, '/'); + if (!str_starts_with($normalizedFilename, $stubsDirectoryPrefix)) { + return null; + } + + $export['locatedSource']['data']['filename'] = substr($normalizedFilename, strlen($stubsDirectoryPrefix)); + + return $export; + } + + /** + * @param array $cached + * @return array|null + */ + private function resolveStubFilename(array $cached): ?array + { + if (!is_array($cached['locatedSource'] ?? null) || !is_array($cached['locatedSource']['data'] ?? null)) { + return null; + } + + $relativeFilename = $cached['locatedSource']['data']['filename'] ?? null; + if (!is_string($relativeFilename)) { + return null; + } + + $filename = $this->getStubsDirectory() . '/' . $relativeFilename; + if (!is_file($filename)) { + return null; + } + + $cached['locatedSource']['data']['filename'] = $filename; + + return $cached; + } + + private function getStubsDirectory(): string + { + return $this->stubsDirectory ??= $this->fileHelper->normalizePath(PhpStormStubsMap::DIR, '/'); + } + /** * @return list */ @@ -95,7 +173,7 @@ public function locateIdentifiersByType(Reflector $reflector, IdentifierType $id private function getVariableCacheKey(): string { return $this->variableCacheKey ??= sprintf( - 'v1-%s-%s-%s', + 'v2-%s-%s-%s', ComposerHelper::getBetterReflectionVersion(), ComposerHelper::getPhpStormStubsVersion(), $this->phpVersion->getVersionString(), diff --git a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/CachedPhpInternalSourceLocatorTest.php b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/CachedPhpInternalSourceLocatorTest.php new file mode 100644 index 00000000000..8f213392a9e --- /dev/null +++ b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/CachedPhpInternalSourceLocatorTest.php @@ -0,0 +1,194 @@ + [new Identifier('Countable', new IdentifierType(IdentifierType::IDENTIFIER_CLASS))]; + yield 'function' => [new Identifier('strlen', new IdentifierType(IdentifierType::IDENTIFIER_FUNCTION))]; + yield 'constant' => [new Identifier('PREG_PATTERN_ORDER', new IdentifierType(IdentifierType::IDENTIFIER_CONSTANT))]; + } + + private static function createEnumIdentifier(): Identifier + { + // Random\IntervalBoundary is the first enum bundled with PHP (8.3); + // under older versions the stubber filters it out + return new Identifier('Random\IntervalBoundary', new IdentifierType(IdentifierType::IDENTIFIER_CLASS)); + } + + #[RequiresPhp('>= 8.3.0')] + public function testPersistedEnumEntryIsPortableAcrossInstallations(): void + { + $this->testPersistedEntryIsPortableAcrossInstallations(self::createEnumIdentifier()); + } + + #[RequiresPhp('>= 8.3.0')] + public function testWarmCacheEnumImportResolvesAgainstCurrentInstallation(): void + { + $this->testWarmCacheImportResolvesAgainstCurrentInstallation(self::createEnumIdentifier()); + } + + #[RequiresPhp('>= 8.3.0')] + public function testEnumEntryWithUnresolvableStubPathIsTreatedAsCacheMiss(): void + { + $this->testEntryWithUnresolvableStubPathIsTreatedAsCacheMiss(self::createEnumIdentifier()); + } + + /** + * The cache key is environment-independent, so an entry written by one + * installation may be read by another where the project is mounted at a + * different absolute path (host vs. Docker container). The persisted + * blob must therefore not contain this installation's absolute paths. + */ + #[DataProvider('dataIdentifiers')] + public function testPersistedEntryIsPortableAcrossInstallations(Identifier $identifier): void + { + $storage = $this->createSpyStorage(); + $locator = $this->createLocator($this->createInnerLocator(), $storage); + $reflection = $locator->locateIdentifier(new DefaultReflector($locator), $identifier); + + $this->assertInstanceOf(Reflection::class, $reflection); + $this->assertNotEmpty($storage->items); + + $fileHelper = self::getContainer()->getByType(FileHelper::class); + $stubsDirectory = $fileHelper->normalizePath(PhpStormStubsMap::DIR, '/'); + foreach ($storage->items as $key => $item) { + $filename = $item['locatedSource']['data']['filename']; + $this->assertIsString($filename); + $this->assertFalse(str_starts_with($filename, '/'), $key . ': stored stub path must be relative, got ' . $filename); + $this->assertStringNotContainsString('://', $filename, $key . ': stored stub path must be relative, got ' . $filename); + $this->assertTrue(is_file($stubsDirectory . '/' . $filename), $key . ': stored stub path must resolve against the stubs directory, got ' . $filename); + } + } + + #[DataProvider('dataIdentifiers')] + public function testWarmCacheImportResolvesAgainstCurrentInstallation(Identifier $identifier): void + { + $storage = $this->createSpyStorage(); + $coldLocator = $this->createLocator($this->createInnerLocator(), $storage); + $coldReflection = $coldLocator->locateIdentifier(new DefaultReflector($coldLocator), $identifier); + $this->assertInstanceOf(Reflection::class, $coldReflection); + + $innerMustNotBeHit = new class ($this) implements SourceLocator { + + public function __construct(private CachedPhpInternalSourceLocatorTest $test) + { + } + + #[Override] + public function locateIdentifier(Reflector $reflector, Identifier $identifier): ?Reflection + { + $this->test->fail('Warm cache must not fall back to the inner locator'); + } + + #[Override] + public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array + { + return []; + } + + }; + + $warmLocator = $this->createLocator($innerMustNotBeHit, $storage); + $reflection = $warmLocator->locateIdentifier(new DefaultReflector($warmLocator), $identifier); + + $this->assertInstanceOf(Reflection::class, $reflection); + $this->assertSame($identifier->getName(), $reflection->getName()); + + if ( + !$reflection instanceof ReflectionClass + && !$reflection instanceof ReflectionFunction + && !$reflection instanceof ReflectionConstant + ) { + $this->fail('Unexpected reflection class ' . $reflection::class); + } + + $fileHelper = self::getContainer()->getByType(FileHelper::class); + $stubsDirectory = $fileHelper->normalizePath(PhpStormStubsMap::DIR, '/'); + $fileName = $reflection->getLocatedSource()->getFileName(); + $this->assertNotNull($fileName); + $this->assertTrue(str_starts_with($fileHelper->normalizePath($fileName, '/'), $stubsDirectory . '/'), 'Imported stub path must point into this installation, got ' . $fileName); + } + + /** + * An entry whose stub file does not exist in this installation (written + * by a distribution with a different stub layout, e.g. phar vs. vendor) + * must be treated as a cache miss instead of crashing the analysis. + */ + #[DataProvider('dataIdentifiers')] + public function testEntryWithUnresolvableStubPathIsTreatedAsCacheMiss(Identifier $identifier): void + { + $storage = $this->createSpyStorage(); + $coldLocator = $this->createLocator($this->createInnerLocator(), $storage); + $coldReflection = $coldLocator->locateIdentifier(new DefaultReflector($coldLocator), $identifier); + $this->assertInstanceOf(Reflection::class, $coldReflection); + + foreach ($storage->items as $key => $item) { + $item['locatedSource']['data']['filename'] = 'Core/DoesNotExist.php'; + $storage->items[$key] = $item; + } + + $locator = $this->createLocator($this->createInnerLocator(), $storage); + $reflection = $locator->locateIdentifier(new DefaultReflector($locator), $identifier); + + $this->assertInstanceOf(Reflection::class, $reflection); + $this->assertSame($identifier->getName(), $reflection->getName()); + } + + private function createLocator(SourceLocator $inner, SpyCacheStorage $storage): CachedPhpInternalSourceLocator + { + return new CachedPhpInternalSourceLocator( + $inner, + new Cache($storage), + self::getContainer()->getByType(PhpVersion::class), + self::getContainer()->getByType(FileHelper::class), + ); + } + + private function createInnerLocator(): PhpInternalSourceLocator + { + $php8Parser = self::getContainer()->getService('php8PhpParser'); + if (!$php8Parser instanceof Parser) { + throw new ShouldNotHappenException(); + } + + return new PhpInternalSourceLocator( + new Locator($php8Parser), + self::getContainer()->getByType(PhpStormStubsSourceStubber::class), + ); + } + + private function createSpyStorage(): SpyCacheStorage + { + return new SpyCacheStorage(); + } + +} diff --git a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/SpyCacheStorage.php b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/SpyCacheStorage.php new file mode 100644 index 00000000000..bb3ed419919 --- /dev/null +++ b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/SpyCacheStorage.php @@ -0,0 +1,29 @@ + */ + public array $items = []; + + /** + * @return mixed|null + */ + public function load(string $key, string $variableKey) + { + return $this->items[$key] ?? null; + } + + /** + * @param mixed $data + */ + public function save(string $key, string $variableKey, $data): void + { + $this->items[$key] = $data; + } + +}