Skip to content

Commit ee81a32

Browse files
ondrejmirtesondrejmirtes
committed
Fix infinite recursion when caching types
1 parent dfc004e commit ee81a32

File tree

4 files changed

+56
-145
lines changed

4 files changed

+56
-145
lines changed

‎src/Type/FileTypeMapper.php

Lines changed: 1 addition & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,8 @@
88
use PHPStan\Analyser\NameScope;
99
use PHPStan\BetterReflection\Util\GetLastDocComment;
1010
use PHPStan\Broker\AnonymousClassNameHelper;
11-
use PHPStan\Cache\Cache;
1211
use PHPStan\File\FileHelper;
1312
use PHPStan\Parser\Parser;
14-
use PHPStan\Php\PhpVersion;
1513
use PHPStan\PhpDoc\PhpDocNodeResolver;
1614
use PHPStan\PhpDoc\PhpDocStringResolver;
1715
use PHPStan\PhpDoc\ResolvedPhpDocBlock;
@@ -23,16 +21,13 @@
2321
use PHPStan\Type\Generic\TemplateTypeFactory;
2422
use PHPStan\Type\Generic\TemplateTypeHelper;
2523
use PHPStan\Type\Generic\TemplateTypeMap;
26-
use ReflectionClass;
2724
use function array_key_exists;
2825
use function array_keys;
2926
use function array_map;
3027
use function array_merge;
3128
use function array_pop;
3229
use function array_slice;
3330
use function count;
34-
use function filemtime;
35-
use function implode;
3631
use function is_array;
3732
use function is_callable;
3833
use function is_file;
@@ -41,8 +36,6 @@
4136
use function sprintf;
4237
use function strpos;
4338
use function strtolower;
44-
use function time;
45-
use function trait_exists;
4639

4740
class FileTypeMapper
4841
{
@@ -63,17 +56,12 @@ class FileTypeMapper
6356

6457
private int $resolvedPhpDocBlockCacheCount = 0;
6558

66-
/** @var array<string, bool> */
67-
private array $alreadyProcessedDependentFiles = [];
68-
6959
public function __construct(
7060
private ReflectionProviderProvider $reflectionProviderProvider,
7161
private Parser $phpParser,
7262
private PhpDocStringResolver $phpDocStringResolver,
7363
private PhpDocNodeResolver $phpDocNodeResolver,
74-
private Cache $cache,
7564
private AnonymousClassNameHelper $anonymousClassNameHelper,
76-
private PhpVersion $phpVersion,
7765
private FileHelper $fileHelper,
7866
)
7967
{
@@ -180,15 +168,7 @@ private function resolvePhpDocStringToDocNode(string $phpDocString): PhpDocNode
180168
private function getNameScopeMap(string $fileName): array
181169
{
182170
if (!isset($this->memoryCache[$fileName])) {
183-
$cacheKey = sprintf('%s-phpdocstring-v22-trait-bug', $fileName);
184-
$variableCacheKey = sprintf('%s-%s', implode(',', array_map(static fn (array $file): string => sprintf('%s-%d', $file['filename'], $file['modifiedTime']), $this->getCachedDependentFilesWithTimestamps($fileName))), $this->phpVersion->getVersionString());
185-
$map = $this->cache->load($cacheKey, $variableCacheKey);
186-
187-
if ($map === null) {
188-
$map = $this->createResolvedPhpDocMap($fileName);
189-
$this->cache->save($cacheKey, $variableCacheKey, $map);
190-
}
191-
171+
$map = $this->createResolvedPhpDocMap($fileName);
192172
if ($this->memoryCacheCount >= 512) {
193173
$this->memoryCache = array_slice(
194174
$this->memoryCache,
@@ -607,126 +587,4 @@ private function getNameScopeKey(
607587
return md5(sprintf('%s-%s-%s-%s', $file, $class, $trait, $function));
608588
}
609589

610-
/**
611-
* @return array<array{filename: string, modifiedTime: int}>
612-
*/
613-
private function getCachedDependentFilesWithTimestamps(string $fileName): array
614-
{
615-
$cacheKey = sprintf('dependentFilesTimestamps-%s-v3-filter-ast', $fileName);
616-
$fileModifiedTime = filemtime($fileName);
617-
if ($fileModifiedTime === false) {
618-
$fileModifiedTime = time();
619-
}
620-
$variableCacheKey = sprintf('%d-%s', $fileModifiedTime, $this->phpVersion->getVersionString());
621-
/** @var array<array{filename: string, modifiedTime: int}>|null $cachedFilesTimestamps */
622-
$cachedFilesTimestamps = $this->cache->load($cacheKey, $variableCacheKey);
623-
if ($cachedFilesTimestamps !== null) {
624-
$useCached = true;
625-
foreach ($cachedFilesTimestamps as $cachedFile) {
626-
$cachedFilename = $cachedFile['filename'];
627-
$cachedTimestamp = $cachedFile['modifiedTime'];
628-
629-
if (!is_file($cachedFilename)) {
630-
$useCached = false;
631-
break;
632-
}
633-
634-
$currentTimestamp = filemtime($cachedFilename);
635-
if ($currentTimestamp === false) {
636-
$useCached = false;
637-
break;
638-
}
639-
640-
if ($currentTimestamp !== $cachedTimestamp) {
641-
$useCached = false;
642-
break;
643-
}
644-
}
645-
646-
if ($useCached) {
647-
return $cachedFilesTimestamps;
648-
}
649-
}
650-
651-
$filesTimestamps = [];
652-
foreach ($this->getDependentFiles($fileName) as $dependentFile) {
653-
$dependentFileModifiedTime = filemtime($dependentFile);
654-
if ($dependentFileModifiedTime === false) {
655-
$dependentFileModifiedTime = time();
656-
}
657-
658-
$filesTimestamps[] = [
659-
'filename' => $dependentFile,
660-
'modifiedTime' => $dependentFileModifiedTime,
661-
];
662-
}
663-
664-
$this->cache->save($cacheKey, $variableCacheKey, $filesTimestamps);
665-
666-
return $filesTimestamps;
667-
}
668-
669-
/**
670-
* @return string[]
671-
*/
672-
private function getDependentFiles(string $fileName): array
673-
{
674-
$dependentFiles = [$fileName];
675-
676-
if (isset($this->alreadyProcessedDependentFiles[$fileName])) {
677-
return $dependentFiles;
678-
}
679-
680-
$this->alreadyProcessedDependentFiles[$fileName] = true;
681-
682-
$this->processNodes(
683-
$this->phpParser->parseFile($fileName),
684-
function (Node $node) use (&$dependentFiles) {
685-
if ($node instanceof Node\Stmt\Declare_) {
686-
return null;
687-
}
688-
if ($node instanceof Node\Stmt\Namespace_) {
689-
return null;
690-
}
691-
692-
if (!$node instanceof Node\Stmt\Class_ && !$node instanceof Node\Stmt\Trait_ && !$node instanceof Node\Stmt\Enum_) {
693-
return null;
694-
}
695-
696-
foreach ($node->stmts as $stmt) {
697-
if (!$stmt instanceof Node\Stmt\TraitUse) {
698-
continue;
699-
}
700-
701-
foreach ($stmt->traits as $traitName) {
702-
$traitName = (string) $traitName;
703-
if (!trait_exists($traitName)) {
704-
continue;
705-
}
706-
707-
$traitReflection = new ReflectionClass($traitName);
708-
if ($traitReflection->getFileName() === false) {
709-
continue;
710-
}
711-
if (!is_file($traitReflection->getFileName())) {
712-
continue;
713-
}
714-
715-
foreach ($this->getDependentFiles($traitReflection->getFileName()) as $traitFileName) {
716-
$dependentFiles[] = $traitFileName;
717-
}
718-
}
719-
}
720-
721-
return null;
722-
},
723-
static function (): void {
724-
},
725-
);
726-
727-
unset($this->alreadyProcessedDependentFiles[$fileName]);
728-
729-
return $dependentFiles;
730-
}
731-
732590
}

‎tests/PHPStan/Analyser/AnalyserIntegrationTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,16 @@ public function testBug6842(): void
609609
$this->assertSame(28, $errors[0]->getLine());
610610
}
611611

612+
public function testBug6896(): void
613+
{
614+
if (PHP_VERSION_ID < 80000) {
615+
$this->markTestSkipped('Test requires PHP 8.0.');
616+
}
617+
618+
$errors = $this->runAnalyse(__DIR__ . '/data/bug-6896.php');
619+
$this->assertCount(6, $errors);
620+
}
621+
612622
/**
613623
* @param string[]|null $allAnalysedFiles
614624
* @return Error[]

‎tests/PHPStan/Analyser/data/bug-6896.php

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php declare(strict_types=1); // lint >= 8.0
2+
3+
namespace Bug6896;
4+
5+
use IteratorIterator;
6+
use LimitIterator;
7+
use Traversable;
8+
use ArrayObject;
9+
10+
/**
11+
* @template TKey as array-key
12+
* @template TValue
13+
*
14+
* @extends ArrayObject<TKey, TValue>
15+
*
16+
* Basic generic iterator, with additional helper functions.
17+
*/
18+
abstract class XIterator extends ArrayObject
19+
{
20+
}
21+
22+
final class RandHelper
23+
{
24+
25+
/**
26+
* @template TRandKey as array-key
27+
* @template TRandVal
28+
* @template TRandList as array<TRandKey, TRandVal>|XIterator<TRandKey, TRandVal>|Traversable<TRandKey, TRandVal>
29+
*
30+
* @param TRandList $list
31+
*
32+
* @return (
33+
* TRandList is array ? array<TRandKey, TRandVal> : (
34+
* TRandList is XIterator ? XIterator<TRandKey, TRandVal> :
35+
* IteratorIterator<TRandKey, TRandVal>|LimitIterator<TRandKey, TRandVal>
36+
* ))
37+
*/
38+
public static function getPseudoRandomWithUrl(
39+
array|XIterator|Traversable $list,
40+
): array|XIterator|IteratorIterator|LimitIterator
41+
{
42+
return $list;
43+
}
44+
}

‎tests/PHPStan/Broker/BrokerTest.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
use PhpParser\Node\Name;
66
use PHPStan\Analyser\Scope;
77
use PHPStan\BetterReflection\SourceLocator\SourceStubber\PhpStormStubsSourceStubber;
8-
use PHPStan\Cache\Cache;
98
use PHPStan\DependencyInjection\Reflection\DirectClassReflectionExtensionRegistryProvider;
109
use PHPStan\File\FileHelper;
1110
use PHPStan\File\SimpleRelativePathHelper;
@@ -44,7 +43,7 @@ protected function setUp(): void
4443
$setterReflectionProviderProvider,
4544
$classReflectionExtensionRegistryProvider,
4645
$this->createMock(FunctionReflectionFactory::class),
47-
new FileTypeMapper($setterReflectionProviderProvider, $this->getParser(), $phpDocStringResolver, $phpDocNodeResolver, $this->createMock(Cache::class), $anonymousClassNameHelper, self::getContainer()->getByType(PhpVersion::class), self::getContainer()->getByType(FileHelper::class)),
46+
new FileTypeMapper($setterReflectionProviderProvider, $this->getParser(), $phpDocStringResolver, $phpDocNodeResolver, $anonymousClassNameHelper, self::getContainer()->getByType(FileHelper::class)),
4847
self::getContainer()->getByType(PhpDocInheritanceResolver::class),
4948
self::getContainer()->getByType(PhpVersion::class),
5049
self::getContainer()->getByType(NativeFunctionReflectionProvider::class),

0 commit comments

Comments
 (0)