Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 50 additions & 25 deletions src/Analyser/FileAnalysisProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@
use PhpParser\Node\Stmt\Use_;
use PhpParser\Parser;
use PhpParser\ParserFactory;
use PhpParser\Token;

use function array_key_exists;
use function array_keys;
use function file_get_contents;
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;
Expand All @@ -71,13 +71,10 @@ final class FileAnalysisProvider
/** @var array<string, bool> */
private array $validAsts = [];

/** @var array<string, array<Token>> */
private array $tokens = [];

/** @var array<string, string> */
private array $contents = [];

/** @var array<string, int|null> */
/** @var array<string, int|null> Computed eagerly at parse time so token arrays are not retained by the provider. */
private array $invalidPhpTagLines = [];

/** @var list<string> */
Expand Down Expand Up @@ -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,
Expand All @@ -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'],
Expand Down Expand Up @@ -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<Node\Stmt>|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;
}
Expand All @@ -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],
);
}

Expand All @@ -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<Token> $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 `<?php` tag and contains no other `<?`
* cannot hold an invalid tag, which skips the token walk for the common case.
*/
private function invalidPhpTagLineForCode(string $code): ?int
{
foreach ($tokens as $token) {
$invalidLine = $this->invalidPhpTagLineForToken($token->id, $token->text, $token->line);
if (
str_starts_with($code, '<?php')
&& ($code === '<?php' || str_contains(" \t\n\r\v\f", $code[5]))
&& substr_count($code, '<?') === 1
) {
return null;
}

foreach ($this->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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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) {
Expand Down
51 changes: 51 additions & 0 deletions tests/Analyser/FileAnalysisProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -49,6 +53,53 @@ final class Foo {}
$this->assertSame($fileAnalysis, $fileAnalysisProvider->analyse($file));
}

public function testReusesAstParsedBeforeAnalysis(): void
{
$file = $this->source(<<<'PHP'
<?php

final class Foo {}
PHP);

$fileAnalysisProvider = new FileAnalysisProvider();
$ast = $fileAnalysisProvider->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, '<?php final class Foo {}');

try {
$fileAnalysisProvider = new FileAnalysisProvider();

$this->assertIsArray($fileAnalysisProvider->ast($file));
$this->assertNull($fileAnalysisProvider->invalidPhpTagLine($file));

$fileAnalysisProvider->releaseAst($file);
file_put_contents($file, "<? echo 'changed';");

$this->assertSame(1, $fileAnalysisProvider->invalidPhpTagLine($file));
} finally {
unlink($file);
}
}

public function testReportsInvalidTagsAndInvalidAstWithoutThrowing(): void
{
$file = $this->source("<? echo 'short';\n<?php this is invalid !!!!!");
Expand Down
Loading