Skip to content

Commit

Permalink
Fix trait caching issue
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Jan 11, 2020
1 parent 49d8c4f commit 6f6ea7e
Show file tree
Hide file tree
Showing 7 changed files with 319 additions and 7 deletions.
83 changes: 76 additions & 7 deletions src/Type/FileTypeMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ class FileTypeMapper
/** @var array<string, ResolvedPhpDocBlock> */
private $resolvedPhpDocBlockCache = [];

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

public function __construct(
Parser $phpParser,
PhpDocStringResolver $phpDocStringResolver,
Expand Down Expand Up @@ -186,17 +189,21 @@ private function shouldPhpDocNodeBeCachedToDisk(PhpDocNode $phpDocNode): bool
private function getResolvedPhpDocMap(string $fileName): array
{
if (!isset($this->memoryCache[$fileName])) {
$modifiedTime = filemtime($fileName);
if ($modifiedTime === false) {
$modifiedTime = time();
}
$now = time();
$cacheKey = sprintf('%s-phpdocstring', $fileName);
$modifiedTimeString = sprintf('%d', $modifiedTime);
$map = $this->cache->load($cacheKey, $modifiedTimeString);
$variableCacheKey = implode(',', array_map(static function (string $fileName) use ($now): string {
$modifiedTime = filemtime($fileName);
if ($modifiedTime === false) {
$modifiedTime = $now;
}

return sprintf('%s-%d', $fileName, $modifiedTime);
}, $this->getDependentFiles($fileName)));
$map = $this->cache->load($cacheKey, $variableCacheKey);

if ($map === null) {
$map = $this->createResolvedPhpDocMap($fileName);
$this->cache->save($cacheKey, $modifiedTimeString, $map);
$this->cache->save($cacheKey, $variableCacheKey, $map);
}

$this->memoryCache[$fileName] = $map;
Expand Down Expand Up @@ -508,4 +515,66 @@ private function getPhpDocKey(
return md5(sprintf('%s-%s-%s-%s', $class, $trait, $function, $docComment));
}

/**
* @param string $fileName
* @return string[]
*/
private function getDependentFiles(string $fileName): array
{
$dependentFiles = [$fileName];

if (isset($this->alreadyProcessedDependentFiles[$fileName])) {
return $dependentFiles;
}

$this->alreadyProcessedDependentFiles[$fileName] = true;

$this->processNodes(
$this->phpParser->parseFile($fileName),
function (Node $node) use (&$dependentFiles) {
if ($node instanceof Node\Stmt\Declare_) {
return null;
}
if ($node instanceof Node\Stmt\Namespace_) {
return null;
}

if (!$node instanceof Node\Stmt\Class_ && !$node instanceof Node\Stmt\Trait_) {
return null;
}

foreach ($node->stmts as $stmt) {
if (!$stmt instanceof Node\Stmt\TraitUse) {
continue;
}

foreach ($stmt->traits as $traitName) {
$traitName = (string) $traitName;
if (!trait_exists($traitName)) {
continue;
}

$traitReflection = new \ReflectionClass($traitName);
if ($traitReflection->getFileName() === false) {
continue;
}
if (!file_exists($traitReflection->getFileName())) {
continue;
}

foreach ($this->getDependentFiles($traitReflection->getFileName()) as $traitFileName) {
$dependentFiles[] = $traitFileName;
}
}
}
},
static function (): void {
}
);

unset($this->alreadyProcessedDependentFiles[$fileName]);

return $dependentFiles;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
<?php declare(strict_types = 1);

namespace PHPStan\Analyser;

use PHPStan\File\FileReader;
use PHPStan\Testing\TestCase;
use RecursiveDirectoryIterator;
use RecursiveIteratorIterator;

class TraitsCachingIssueIntegrationTest extends TestCase
{

/** @var string|null */
private $originalTraitOneContents;

/** @var string|null */
private $originalTraitTwoContents;

public function tearDown(): void
{
$this->deleteCache();

if ($this->originalTraitOneContents !== null) {
$this->revertTrait(__DIR__ . '/data/TraitOne.php', $this->originalTraitOneContents);
}

if ($this->originalTraitTwoContents !== null) {
$this->revertTrait(__DIR__ . '/data/TraitTwo.php', $this->originalTraitTwoContents);
}
}

public function dataCachingIssue(): array
{
return [
[
false,
false,
[],
],
[
false,
true,
[
'Method class@anonymous/TestClassUsingTrait.php:20::doBar() should return stdClass but returns Exception.',
],
],
[
true,
false,
[
'Method TraitsCachingIssue\TestClassUsingTrait::doBar() should return stdClass but returns Exception.',
],
],
[
true,
true,
[
'Method TraitsCachingIssue\TestClassUsingTrait::doBar() should return stdClass but returns Exception.',
'Method class@anonymous/TestClassUsingTrait.php:20::doBar() should return stdClass but returns Exception.',
],
],
];
}

/**
* @dataProvider dataCachingIssue
* @param bool $changeOne
* @param bool $changeTwo
* @param string[] $expectedErrors
*/
public function testCachingIssue(
bool $changeOne,
bool $changeTwo,
array $expectedErrors
): void
{
$this->deleteCache();
[$statusCode, $errors] = $this->runPhpStan();
$this->assertSame([], $errors);
$this->assertSame(0, $statusCode);

if ($changeOne) {
$this->originalTraitOneContents = $this->changeTrait(__DIR__ . '/data/TraitOne.php');
}
if ($changeTwo) {
$this->originalTraitTwoContents = $this->changeTrait(__DIR__ . '/data/TraitTwo.php');
}

$errorPath = __DIR__ . '/data/TestClassUsingTrait.php';
[$statusCode, $errors] = $this->runPhpStan();

if (count($expectedErrors) === 0) {
$this->assertSame(0, $statusCode);
$this->assertArrayNotHasKey($errorPath, $errors);
return;
}

$this->assertSame(1, $statusCode);
$this->assertArrayHasKey($errorPath, $errors);
$this->assertSame(count($expectedErrors), $errors[$errorPath]['errors']);

foreach ($errors[$errorPath]['messages'] as $i => $error) {
$this->assertSame($expectedErrors[$i], $error['message']);
}
}

/**
* @return array{int, mixed[]}
*/
private function runPhpStan(): array
{
$phpstanBinPath = __DIR__ . '/../../../../bin/phpstan';
exec(
sprintf(
'%s %s analyse --no-progress --level 8 --configuration %s --error-format json %s',
escapeshellarg(PHP_BINARY),
$phpstanBinPath,
escapeshellarg(__DIR__ . '/phpstan.neon'),
escapeshellarg(__DIR__ . '/data')
),
$output,
$statusCode
);
$stringOutput = implode("\n", $output);
$json = \Nette\Utils\Json::decode($stringOutput, \Nette\Utils\Json::FORCE_ARRAY);

return [$statusCode, $json['files']];
}

private function deleteCache(): void
{
$files = new RecursiveIteratorIterator(
new RecursiveDirectoryIterator(__DIR__ . '/tmp/cache', RecursiveDirectoryIterator::SKIP_DOTS),
RecursiveIteratorIterator::CHILD_FIRST
);

foreach ($files as $fileinfo) {
if ($fileinfo->isDir()) {
rmdir($fileinfo->getRealPath());
continue;
}

unlink($fileinfo->getRealPath());
}
}

private function changeTrait(string $traitPath): string
{
$originalTraitContents = FileReader::read($traitPath);
$traitContents = str_replace('use stdClass as Foo;', 'use Exception as Foo;', $originalTraitContents);
$result = file_put_contents($traitPath, $traitContents);
if ($result === false) {
$this->fail(sprintf('Could not save file %s', $traitPath));
}

return $originalTraitContents;
}

private function revertTrait(string $traitPath, string $originalTraitContents): void
{
$result = file_put_contents($traitPath, $originalTraitContents);
if ($result === false) {
$this->fail(sprintf('Could not save file %s', $traitPath));
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace TraitsCachingIssue;

class TestClassUsingTrait
{

use TraitOne;

/**
* @return \stdClass
*/
public function doBar()
{
return $this->doFoo();
}

public function doBaz(): void
{
$class = new class() {

use TraitTwo;

/**
* @return \stdClass
*/
public function doBar()
{
return $this->doFoo();
}
};
}

}
18 changes: 18 additions & 0 deletions tests/PHPStan/Analyser/traitsCachingIssue/data/TraitOne.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace TraitsCachingIssue;

use stdClass as Foo;

trait TraitOne
{

/**
* @return Foo
*/
public function doFoo()
{
return new Foo();
}

}
18 changes: 18 additions & 0 deletions tests/PHPStan/Analyser/traitsCachingIssue/data/TraitTwo.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace TraitsCachingIssue;

use stdClass as Foo;

trait TraitTwo
{

/**
* @return Foo
*/
public function doFoo()
{
return new Foo();
}

}
4 changes: 4 additions & 0 deletions tests/PHPStan/Analyser/traitsCachingIssue/phpstan.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
parameters:
tmpDir: tmp
autoload_directories:
- data
2 changes: 2 additions & 0 deletions tests/PHPStan/Analyser/traitsCachingIssue/tmp/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
*
!.gitignore

0 comments on commit 6f6ea7e

Please sign in to comment.