diff --git a/src/Analyser/FileAnalysisProvider.php b/src/Analyser/FileAnalysisProvider.php index 4e9085bf..2c1598c5 100644 --- a/src/Analyser/FileAnalysisProvider.php +++ b/src/Analyser/FileAnalysisProvider.php @@ -45,7 +45,6 @@ use PhpParser\Node\Stmt\Use_; use PhpParser\Parser; use PhpParser\ParserFactory; -use PhpParser\Token; use function array_key_exists; use function array_keys; @@ -53,6 +52,7 @@ use function is_array; use function min; use function preg_match; +use function str_contains; use function str_starts_with; use function substr; use function substr_count; @@ -71,13 +71,10 @@ final class FileAnalysisProvider /** @var array */ private array $validAsts = []; - /** @var array> */ - private array $tokens = []; - /** @var array */ private array $contents = []; - /** @var array */ + /** @var array Computed eagerly at parse time so token arrays are not retained by the provider. */ private array $invalidPhpTagLines = []; /** @var list */ @@ -143,7 +140,7 @@ public function analyse(string $file): FileAnalysis } $code = $this->contents($file); - $ast = $this->ast($file); + $ast = array_key_exists($file, $this->asts) ? $this->asts[$file] : $this->parse($file); $hasValidAst = $this->validAsts[$file]; $fileState = $hasValidAst ? $this->fileState($ast ?? []) : [ 'declaresSymbols' => false, @@ -155,7 +152,7 @@ public function analyse(string $file): FileAnalysis file: $file, hasUtf8Bom: str_starts_with($code, "\xEF\xBB\xBF"), hasValidUtf8: preg_match('//u', $code) === 1, - invalidPhpTagLine: $this->invalidPhpTagLine($file), + invalidPhpTagLine: $this->invalidPhpTagLines[$file], hasValidAst: $hasValidAst, declaresSymbols: $fileState['declaresSymbols'], hasSideEffects: $fileState['hasSideEffects'], @@ -189,18 +186,30 @@ public function ast(string $file, bool $retainForAnalysis = true): ?array return null; } + return $this->parse($file); + } + + /** + * Parses an already normalised file that has neither a cached AST nor an + * analysis, recording its AST, validity and invalid PHP tag line in one pass. + * + * @return array|null + */ + private function parse(string $file): ?array + { + $code = $this->contents($file); $ast = null; $isValid = true; try { - $ast = $this->parser->parse($this->contents($file)); + $ast = $this->parser->parse($code); } catch (Error) { $isValid = false; } - $this->asts[$file] = $ast; - $this->validAsts[$file] = $isValid; - $this->tokens[$file] = $this->parser->getTokens(); + $this->asts[$file] = $ast; + $this->validAsts[$file] = $isValid; + $this->invalidPhpTagLines[$file] = $this->invalidPhpTagLineForCode($code); return $ast; } @@ -212,8 +221,8 @@ public function releaseAst(string $file): void unset( $this->asts[$file], $this->validAsts[$file], - $this->tokens[$file], $this->contents[$file], + $this->invalidPhpTagLines[$file], ); } @@ -239,22 +248,36 @@ public function invalidPhpTagLine(string $file): ?int return $this->analyses[$file]->invalidPhpTagLine; } - if (array_key_exists($file, $this->invalidPhpTagLines)) { - return $this->invalidPhpTagLines[$file]; - } - - if (! isset($this->tokens[$file])) { - $this->ast($file); + if (! array_key_exists($file, $this->invalidPhpTagLines)) { + $this->parse($file); } - return $this->invalidPhpTagLines[$file] = $this->invalidPhpTagLineFromTokens($this->tokens[$file] ?? []); + return $this->invalidPhpTagLines[$file]; } - /** @param array $tokens */ - private function invalidPhpTagLineFromTokens(array $tokens): ?int + /** + * Must be called right after parsing $code, while the parser still holds its tokens. + * A file that opens with a well-formed `invalidPhpTagLineForToken($token->id, $token->text, $token->line); + if ( + str_starts_with($code, 'parser->getTokens() as $token) { + $id = $token->id; + + if ($id !== T_OPEN_TAG && $id !== T_INLINE_HTML) { + continue; + } + + $invalidLine = $this->invalidPhpTagLineForToken($id, $token->text, $token->line); if ($invalidLine !== null) { return $invalidLine; @@ -280,10 +303,9 @@ private function invalidPhpTagLineForToken(int $id, string $text, int $tokenLine return $tokenLine + substr_count(substr($text, 0, $tagOffset), "\n"); } + /** @param string $file An already normalised path. */ private function contents(string $file): string { - $file = Path::normalise($file, canonicalise: true); - return $this->contents[$file] ??= (string) file_get_contents($file); } @@ -415,6 +437,9 @@ private function intrinsicSideEffectLineInExpression(Expr $expr): ?int $sideEffectLine = $sideEffectLine === null ? $node->getStartLine() : min($sideEffectLine, $node->getStartLine()); + + // Descendants start on or after this node's line, so they cannot lower the minimum. + continue; } if ($node instanceof FunctionLike || $node instanceof ClassLike) { diff --git a/tests/Analyser/FileAnalysisProviderTest.php b/tests/Analyser/FileAnalysisProviderTest.php index 65163c46..d23d925f 100644 --- a/tests/Analyser/FileAnalysisProviderTest.php +++ b/tests/Analyser/FileAnalysisProviderTest.php @@ -13,6 +13,10 @@ use PHPUnit\Framework\TestCase; use function base64_encode; +use function file_put_contents; +use function sys_get_temp_dir; +use function tempnam; +use function unlink; #[CoversClass(FileAnalysis::class)] #[CoversClass(FileAnalysisProvider::class)] @@ -49,6 +53,53 @@ final class Foo {} $this->assertSame($fileAnalysis, $fileAnalysisProvider->analyse($file)); } + public function testReusesAstParsedBeforeAnalysis(): void + { + $file = $this->source(<<<'PHP' + ast($file); + + $this->assertIsArray($ast); + $this->assertSame($ast, $fileAnalysisProvider->ast($file)); + + $fileAnalysis = $fileAnalysisProvider->analyse($file); + + $this->assertTrue($fileAnalysis->hasValidAst); + $this->assertTrue($fileAnalysis->declaresSymbols); + $this->assertFalse($fileAnalysis->hasSideEffects); + $this->assertNull($fileAnalysis->invalidPhpTagLine); + + $fileAnalysisProvider->releaseAst($file); + + $this->assertNull($fileAnalysisProvider->ast($file)); + $this->assertSame($fileAnalysis, $fileAnalysisProvider->analyse($file)); + } + + public function testReleaseAstDropsInvalidPhpTagLineCache(): void + { + $file = (string) tempnam(sys_get_temp_dir(), 'structarmed'); + file_put_contents($file, 'assertIsArray($fileAnalysisProvider->ast($file)); + $this->assertNull($fileAnalysisProvider->invalidPhpTagLine($file)); + + $fileAnalysisProvider->releaseAst($file); + file_put_contents($file, "assertSame(1, $fileAnalysisProvider->invalidPhpTagLine($file)); + } finally { + unlink($file); + } + } + public function testReportsInvalidTagsAndInvalidAstWithoutThrowing(): void { $file = $this->source("