Don't reuse cleaned ASTs for analysed files

.github/workflows/e2e-tests.yml

@@ -346,6 +346,12 @@ jobs:
               cd e2e/bug-11857
               composer install
               ../../bin/phpstan
+          - script: |
+              cd e2e/early-reflection-extension
+              composer install
+              # The regression happens on the first reflection parse, before the reflection cache under tmp exists.
+              rm -rf tmp
+              ../../bin/phpstan
           - script: |
               cd e2e/in-trait
               OUTPUT=$(../bashunit -a exit_code "1" "../../bin/phpstan --error-format=raw")

e2e/early-reflection-extension/.gitignore

@@ -0,0 +1,2 @@
+/tmp
+/vendor

e2e/early-reflection-extension/composer.json

@@ -0,0 +1,8 @@
+{
+	"autoload-dev": {
+		"classmap": [
+			"extension/",
+			"src/"
+		]
+	}
+}

e2e/early-reflection-extension/extension/EarlyReflectionMethodsClassReflectionExtension.php

@@ -0,0 +1,33 @@
+<?php declare(strict_types = 1);
+
+namespace EarlyReflectionExtension;
+
+use Override;
+use PHPStan\Reflection\ClassReflection;
+use PHPStan\Reflection\MethodReflection;
+use PHPStan\Reflection\MethodsClassReflectionExtension;
+use PHPStan\Reflection\ReflectionProvider;
+use PHPStan\ShouldNotHappenException;
+
+final class EarlyReflectionMethodsClassReflectionExtension implements MethodsClassReflectionExtension
+{
+
+	public function __construct(ReflectionProvider $reflectionProvider)
+	{
+		// Without a cache upgrade, this cleaned AST is reused during analysis and reports return.missing.
+		$reflectionProvider->getClass(AnalysedClass::class)->getNativeReflection();
+	}
+
+	#[Override]
+	public function hasMethod(ClassReflection $classReflection, string $methodName): bool
+	{
+		return false;
+	}
+
+	#[Override]
+	public function getMethod(ClassReflection $classReflection, string $methodName): MethodReflection
+	{
+		throw new ShouldNotHappenException();
+	}
+
+}

e2e/early-reflection-extension/phpstan.neon

@@ -0,0 +1,11 @@
+parameters:
+	level: 8
+	tmpDir: tmp
+	paths:
+		- src
+
+services:
+	-
+		class: EarlyReflectionExtension\EarlyReflectionMethodsClassReflectionExtension
+		tags:
+			- phpstan.broker.methodsClassReflectionExtension

e2e/early-reflection-extension/src/AnalysedClass.php

@@ -0,0 +1,13 @@
+<?php declare(strict_types = 1);
+
+namespace EarlyReflectionExtension;
+
+final class AnalysedClass
+{
+
+	public function getValue(): string
+	{
+		return 'value';
+	}
+
+}

src/Parser/CachedParser.php

@@ -41,8 +41,8 @@ final class CachedParser implements Parser
 	/** @var LruCache<Node\Stmt[]> keyed by source code, weighing the length of that source */
 	private LruCache $cachedNodesByString;
 
-	/** @var array<string, true> */
-	private array $parsedByString = [];
+	/** @var array<string, true> source code parsed into the richest file representation */
+	private array $cachedNodesByStringAreRich = [];
 
 	/** @var LruCache<array{int, int, string}> path => [mtime, size, source code] */
 	private LruCache $cachedSourceByFile;
@@ -78,7 +78,18 @@ public function parseFile(string $file): array
 	{
 		$sourceCode = $this->readFile($file);
 		$cachedNodes = $this->cachedNodesByString->get($sourceCode);
-		if ($cachedNodes !== null && !isset($this->parsedByString[$sourceCode])) {
+		if ($cachedNodes !== null && isset($this->cachedNodesByStringAreRich[$sourceCode])) {
+			return $cachedNodes;
+		}
+
+		// An entry the routing parser produced before the analysed-file list
+		// arrived holds a cleaned AST. Serving it to a request that has to see
+		// the file in full is the bug this marker prevents.
+		$richRequest = !$this->originalParser instanceof PathRoutingParser
+			|| $this->originalParser->shouldUseRichParser($file);
+		if ($cachedNodes !== null && !$richRequest) {
+			// Repeated file parses have to return the same node objects. A fixable rule can
+			// locate a node through another parser caller and match it by object identity.
 			return $cachedNodes;
 		}
 
@@ -87,12 +98,13 @@ public function parseFile(string $file): array
 			// upgrade an entry previously produced by parseString() in place -
 			// no net change to the entry count, just refresh its LRU position
 			$this->cachedNodesByString->replace($sourceCode, $nodes);
-			unset($this->parsedByString[$sourceCode]);
-
-			return $nodes;
+		} else {
+			$this->store($sourceCode, $nodes);
 		}
 
-		$this->store($sourceCode, $nodes);
+		if ($richRequest) {
+			$this->cachedNodesByStringAreRich[$sourceCode] = true;
+		}
 
 		return $nodes;
 	}
@@ -109,7 +121,6 @@ public function parseString(string $sourceCode): array
 
 		$nodes = $this->originalParser->parseString($sourceCode);
 		$this->store($sourceCode, $nodes);
-		$this->parsedByString[$sourceCode] = true;
 
 		return $nodes;
 	}
@@ -120,7 +131,7 @@ public function parseString(string $sourceCode): array
 	private function store(string $sourceCode, array $nodes): void
 	{
 		foreach ($this->cachedNodesByString->set($sourceCode, $nodes, strlen($sourceCode)) as $evictedSourceCode) {
-			unset($this->parsedByString[$evictedSourceCode]);
+			unset($this->cachedNodesByStringAreRich[$evictedSourceCode]);
 		}
 	}
 

src/Parser/PathRoutingParser.php

@@ -21,6 +21,9 @@ final class PathRoutingParser implements Parser
 	/** @var array<string, true> filePath(string) => bool(true) */
 	private array $analysedFiles = [];
 
+	/** @var array<string, bool> */
+	private array $shouldUseRichParserCache = [];
+
 	public function __construct(
 		private FileHelper $fileHelper,
 		private Parser $currentPhpVersionRichParser,
@@ -38,16 +41,31 @@ public function __construct(
 	public function setAnalysedFiles(array $files): void
 	{
 		$this->analysedFiles = array_fill_keys($files, true);
+		$this->shouldUseRichParserCache = [];
 	}
 
 	public function parseFile(string $file): array
 	{
-		$normalizedPath = $this->fileHelper->normalizePath($file, '/');
-		if (str_contains($normalizedPath, 'vendor/jetbrains/phpstorm-stubs')) {
+		if ($this->isPhp8StubFile($file)) {
 			return $this->php8Parser->parseFile($file);
 		}
-		if (str_contains($normalizedPath, 'vendor/phpstan/php-8-stubs/stubs')) {
-			return $this->php8Parser->parseFile($file);
+
+		$parser = $this->shouldUseRichParser($file)
+			? $this->currentPhpVersionRichParser
+			: $this->currentPhpVersionSimpleParser;
+
+		return $parser->parseFile($this->fileHelper->normalizePath($file));
+	}
+
+	public function shouldUseRichParser(string $file): bool
+	{
+		return $this->shouldUseRichParserCache[$file] ??= $this->resolveShouldUseRichParser($file);
+	}
+
+	private function resolveShouldUseRichParser(string $file): bool
+	{
+		if ($this->isPhp8StubFile($file)) {
+			return false;
 		}
 
 		$file = $this->fileHelper->normalizePath($file);
@@ -64,21 +82,29 @@ public function parseFile(string $file): array
 				if ($realFilePath !== false) {
 					$normalizedRealFilePath = $this->fileHelper->normalizePath($realFilePath);
 					if (isset($this->analysedFiles[$normalizedRealFilePath])) {
-						return $this->currentPhpVersionRichParser->parseFile($file);
+						return true;
 					}
 				}
 				break;
 			}
 
-			return $this->currentPhpVersionSimpleParser->parseFile($file);
+			return false;
 		}
 
-		return $this->currentPhpVersionRichParser->parseFile($file);
+		return true;
 	}
 
 	public function parseString(string $sourceCode): array
 	{
 		return $this->currentPhpVersionSimpleParser->parseString($sourceCode);
 	}
 
+	private function isPhp8StubFile(string $file): bool
+	{
+		$normalizedPath = $this->fileHelper->normalizePath($file, '/');
+
+		return str_contains($normalizedPath, 'vendor/jetbrains/phpstorm-stubs')
+			|| str_contains($normalizedPath, 'vendor/phpstan/php-8-stubs/stubs');
+	}
+
 }

tests/PHPStan/Parser/CachedParserTest.php

@@ -198,6 +198,68 @@ public function testParseTheSameFileWithDifferentMethod(): void
 		$this->assertSame(2, $stmts[0]->stmts[1]->expr->expr->class->getAttribute(AnonymousClassVisitor::ATTRIBUTE_LINE_INDEX));
 	}
 
+	public function testParseFileBeforeAnalysedFilesAreSet(): void
+	{
+		$fileHelper = self::getContainer()->getByType(FileHelper::class);
+		$pathRoutingParser = new PathRoutingParser(
+			$fileHelper,
+			self::getContainer()->getService('currentPhpVersionRichParser'),
+			self::getContainer()->getService('currentPhpVersionSimpleDirectParser'),
+			self::getContainer()->getService('php8Parser'),
+			null,
+		);
+		$parser = new CachedParser($pathRoutingParser, 500, 4194304);
+		$path = $fileHelper->normalizePath(__DIR__ . '/data/test.php');
+
+		$stmts = $parser->parseFile($path);
+		$this->assertInstanceOf(Namespace_::class, $stmts[0]);
+		$this->assertInstanceOf(Node\Stmt\Expression::class, $stmts[0]->stmts[0]);
+		$this->assertInstanceOf(Node\Expr\Assign::class, $stmts[0]->stmts[0]->expr);
+		$this->assertInstanceOf(Node\Expr\New_::class, $stmts[0]->stmts[0]->expr->expr);
+		$this->assertNull($stmts[0]->stmts[0]->expr->expr->class->getAttribute(AnonymousClassVisitor::ATTRIBUTE_LINE_INDEX));
+
+		$pathRoutingParser->setAnalysedFiles([$path]);
+
+		$stmts = $parser->parseFile($path);
+		$this->assertInstanceOf(Namespace_::class, $stmts[0]);
+		$this->assertInstanceOf(Node\Stmt\Expression::class, $stmts[0]->stmts[0]);
+		$this->assertInstanceOf(Node\Expr\Assign::class, $stmts[0]->stmts[0]->expr);
+		$this->assertInstanceOf(Node\Expr\New_::class, $stmts[0]->stmts[0]->expr->expr);
+		$this->assertSame(1, $stmts[0]->stmts[0]->expr->expr->class->getAttribute(AnonymousClassVisitor::ATTRIBUTE_LINE_INDEX));
+	}
+
+	public function testParseStringEntryIsNotUpgradedBeforeAnalysedFilesAreSet(): void
+	{
+		$fileHelper = self::getContainer()->getByType(FileHelper::class);
+		$pathRoutingParser = new PathRoutingParser(
+			$fileHelper,
+			self::getContainer()->getService('currentPhpVersionRichParser'),
+			self::getContainer()->getService('currentPhpVersionSimpleDirectParser'),
+			self::getContainer()->getService('php8Parser'),
+			null,
+		);
+		$parser = new CachedParser($pathRoutingParser, 500, 4194304);
+		$path = $fileHelper->normalizePath(__DIR__ . '/data/test.php');
+
+		$stringStmts = $parser->parseString(FileReader::read($path));
+		$stmts = $parser->parseFile($path);
+		$this->assertSame($stringStmts, $stmts);
+		$this->assertInstanceOf(Namespace_::class, $stmts[0]);
+		$this->assertInstanceOf(Node\Stmt\Expression::class, $stmts[0]->stmts[0]);
+		$this->assertInstanceOf(Node\Expr\Assign::class, $stmts[0]->stmts[0]->expr);
+		$this->assertInstanceOf(Node\Expr\New_::class, $stmts[0]->stmts[0]->expr->expr);
+		$this->assertNull($stmts[0]->stmts[0]->expr->expr->class->getAttribute(AnonymousClassVisitor::ATTRIBUTE_LINE_INDEX));
+
+		$pathRoutingParser->setAnalysedFiles([$path]);
+
+		$stmts = $parser->parseFile($path);
+		$this->assertInstanceOf(Namespace_::class, $stmts[0]);
+		$this->assertInstanceOf(Node\Stmt\Expression::class, $stmts[0]->stmts[0]);
+		$this->assertInstanceOf(Node\Expr\Assign::class, $stmts[0]->stmts[0]->expr);
+		$this->assertInstanceOf(Node\Expr\New_::class, $stmts[0]->stmts[0]->expr->expr);
+		$this->assertSame(1, $stmts[0]->stmts[0]->expr->expr->class->getAttribute(AnonymousClassVisitor::ATTRIBUTE_LINE_INDEX));
+	}
+
 	public function testWithExprCacheHelper(): void
 	{
 		$fileHelper = self::getContainer()->getByType(FileHelper::class);