Skip to content

Commit

Permalink
[DX] Remove $nodeScopeResolver->setAnalysedFiles() usage as on parall…
Browse files Browse the repository at this point in the history
…el, it only lookup inside scheduled jobs (#4768)

Co-authored-by: GitHub Action <actions@github.com>
  • Loading branch information
samsonasik and actions-user committed Aug 11, 2023
1 parent a75341c commit 645190a
Show file tree
Hide file tree
Showing 15 changed files with 15 additions and 209 deletions.
4 changes: 0 additions & 4 deletions config/config.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
use PhpParser\NodeVisitor\CloningVisitor;
use PHPStan\Analyser\NodeScopeResolver;
use PHPStan\Analyser\ScopeFactory;
use PHPStan\Dependency\DependencyResolver;
use PHPStan\File\FileHelper;
use PHPStan\Parser\Parser;
use PHPStan\PhpDoc\TypeNodeResolver;
Expand Down Expand Up @@ -202,9 +201,6 @@
$services->set(SymfonyStyle::class)
->factory([service(SymfonyStyleFactory::class), 'create']);

// cache
$services->set(DependencyResolver::class)
->factory([service(PHPStanServicesFactory::class), 'createDependencyResolver']);
$services->set(FileHelper::class)
->factory([service(PHPStanServicesFactory::class), 'createFileHelper']);

Expand Down
37 changes: 1 addition & 36 deletions packages-tests/Caching/Detector/ChangedFilesDetectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

namespace Rector\Tests\Caching\Detector;

use Iterator;
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Caching\Detector\ChangedFilesDetector;
use Rector\Testing\PHPUnit\AbstractLazyTestCase;

Expand All @@ -30,47 +28,14 @@ public function testHasFileChanged(): void

$this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath));
$this->changedFilesDetector->addCachableFile($filePath);
$this->changedFilesDetector->cacheFileWithDependencies($filePath);
$this->changedFilesDetector->cacheFile($filePath);

$this->assertFalse($this->changedFilesDetector->hasFileChanged($filePath));
$this->changedFilesDetector->invalidateFile($filePath);

$this->assertTrue($this->changedFilesDetector->hasFileChanged($filePath));
}

/**
* @param mixed[]|string[] $dependantFiles
*/
#[DataProvider('provideData')]
public function testGetDependentFileInfos(string $filePath, array $dependantFiles): void
{
$this->changedFilesDetector->addFileDependentFiles($filePath, $dependantFiles);
$this->changedFilesDetector->addCachableFile($filePath);
$this->changedFilesDetector->cacheFileWithDependencies($filePath);

$dependantFilePaths = $this->changedFilesDetector->getDependentFilePaths($filePath);

$dependantFilesCount = count($dependantFiles);

$this->assertCount($dependantFilesCount, $dependantFilePaths);

foreach ($dependantFiles as $key => $dependantFile) {
$this->assertSame($dependantFile, $dependantFilePaths[$key]);
}
}

public static function provideData(): Iterator
{
yield [__DIR__ . '/Source/file.php', []];

yield [__DIR__ . '/Source/file.php', [__DIR__ . '/Source/file.php']];

yield [
__DIR__ . '/Source/file.php',
[__DIR__ . '/Source/file.php', __DIR__ . '/Source/file2.php', __DIR__ . '/Source/file3.php'],
];
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config.php';
Expand Down
52 changes: 1 addition & 51 deletions packages/Caching/Detector/ChangedFilesDetector.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@
*/
final class ChangedFilesDetector
{
/**
* @var array<string, string[]>
*/
private array $dependentFiles = [];

/**
* @var array<string, true>
*/
Expand All @@ -33,16 +28,7 @@ public function __construct(
) {
}

/**
* @param string[] $dependentFiles
*/
public function addFileDependentFiles(string $filePath, array $dependentFiles): void
{
$filePathCacheKey = $this->getFilePathCacheKey($filePath);
$this->dependentFiles[$filePathCacheKey] = $dependentFiles;
}

public function cacheFileWithDependencies(string $filePath): void
public function cacheFile(string $filePath): void
{
$filePathCacheKey = $this->getFilePathCacheKey($filePath);

Expand All @@ -53,16 +39,6 @@ public function cacheFileWithDependencies(string $filePath): void
$hash = $this->hashFile($filePath);

$this->cache->save($filePathCacheKey, CacheKey::FILE_HASH_KEY, $hash);

if (! isset($this->dependentFiles[$filePathCacheKey])) {
return;
}

$this->cache->save(
$filePathCacheKey . '_files',
CacheKey::DEPENDENT_FILES_KEY,
$this->dependentFiles[$filePathCacheKey],
);
}

public function addCachableFile(string $filePath): void
Expand Down Expand Up @@ -97,32 +73,6 @@ public function clear(): void
$this->cache->clear();
}

/**
* @return string[]
*/
public function getDependentFilePaths(string $filePath): array
{
$fileInfoCacheKey = $this->getFilePathCacheKey($filePath);

$cacheValue = $this->cache->load($fileInfoCacheKey . '_files', CacheKey::DEPENDENT_FILES_KEY);
if ($cacheValue === null) {
return [];
}

$existingDependentFiles = [];

$dependentFiles = $cacheValue;
foreach ($dependentFiles as $dependentFile) {
if (! file_exists($dependentFile)) {
continue;
}

$existingDependentFiles[] = $dependentFile;
}

return $existingDependentFiles;
}

/**
* @api
*/
Expand Down
5 changes: 0 additions & 5 deletions packages/Caching/Enum/CacheKey.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,4 @@ final class CacheKey
* @var string
*/
public const FILE_HASH_KEY = 'file_hash';

/**
* @var string
*/
public const DEPENDENT_FILES_KEY = 'dependency_files_key';
}
35 changes: 0 additions & 35 deletions packages/Caching/FileSystem/DependencyResolver.php

This file was deleted.

12 changes: 2 additions & 10 deletions packages/Caching/UnchangedFilesFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@ public function __construct(
* @param string[] $filePaths
* @return string[]
*/
public function filterAndJoinWithDependentFileInfos(array $filePaths): array
public function filterFileInfos(array $filePaths): array
{
$changedFileInfos = [];
$dependentFileInfos = [];

foreach ($filePaths as $filePath) {
if (! $this->changedFilesDetector->hasFileChanged($filePath)) {
Expand All @@ -29,16 +28,9 @@ public function filterAndJoinWithDependentFileInfos(array $filePaths): array

$changedFileInfos[] = $filePath;
$this->changedFilesDetector->invalidateFile($filePath);

$dependentFileInfos = array_merge(
$dependentFileInfos,
$this->changedFilesDetector->getDependentFilePaths($filePath)
);
}

// add dependent files
$dependentFileInfos = array_merge($dependentFileInfos, $changedFileInfos);

return array_unique($dependentFileInfos);
return array_unique($changedFileInfos);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use PhpParser\Lexer;
use PHPStan\Analyser\NodeScopeResolver;
use PHPStan\Analyser\ScopeFactory;
use PHPStan\Dependency\DependencyResolver;
use PHPStan\DependencyInjection\Container;
use PHPStan\DependencyInjection\ContainerFactory;
use PHPStan\File\FileHelper;
Expand Down Expand Up @@ -116,14 +115,6 @@ public function getByType(string $type): object
return $this->container->getByType($type);
}

/**
* @api
*/
public function createDependencyResolver(): DependencyResolver
{
return $this->container->getByType(DependencyResolver::class);
}

/**
* @api
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,13 @@
use PhpParser\Node\Stmt\TryCatch;
use PhpParser\Node\UnionType;
use PhpParser\NodeTraverser;
use PHPStan\AnalysedCodeException;
use PHPStan\Analyser\MutatingScope;
use PHPStan\Analyser\NodeScopeResolver;
use PHPStan\Analyser\ScopeContext;
use PHPStan\Node\UnreachableStatementNode;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Type\ObjectType;
use PHPStan\Type\TypeCombinator;
use Rector\Caching\Detector\ChangedFilesDetector;
use Rector\Caching\FileSystem\DependencyResolver;
use Rector\Core\Exception\ShouldNotHappenException;
use Rector\Core\NodeAnalyzer\ClassAnalyzer;
use Rector\Core\PhpParser\Node\CustomNode\FileWithoutNamespace;
Expand Down Expand Up @@ -83,8 +80,6 @@ final class PHPStanNodeScopeResolver
* @param ScopeResolverNodeVisitorInterface[] $nodeVisitors
*/
public function __construct(
private readonly ChangedFilesDetector $changedFilesDetector,
private readonly DependencyResolver $dependencyResolver,
private readonly NodeScopeResolver $nodeScopeResolver,
private readonly ReflectionProvider $reflectionProvider,
iterable $nodeVisitors,
Expand Down Expand Up @@ -257,7 +252,7 @@ public function processNodes(
}
};

return $this->processNodesWithDependentFiles($filePath, $stmts, $scope, $nodeCallback);
return $this->processNodesWithDependentFiles($stmts, $scope, $nodeCallback);
}

public function hasUnreachableStatementNode(): bool
Expand Down Expand Up @@ -411,13 +406,11 @@ private function processTernary(Ternary $ternary, MutatingScope $mutatingScope):
* @return Stmt[]
*/
private function processNodesWithDependentFiles(
string $filePath,
array $stmts,
MutatingScope $mutatingScope,
callable $nodeCallback
): array {
$this->nodeScopeResolver->processNodes($stmts, $mutatingScope, $nodeCallback);
$this->resolveAndSaveDependentFiles($stmts, $mutatingScope, $filePath);

$nodeTraverser = new NodeTraverser();
$nodeTraverser->addVisitor(new WrappedNodeRestoringNodeVisitor());
Expand Down Expand Up @@ -465,25 +458,4 @@ private function resolveClassName(Class_ | Interface_ | Trait_| Enum_ $classLike

return $classLike->name->toString();
}

/**
* @param Stmt[] $stmts
*/
private function resolveAndSaveDependentFiles(
array $stmts,
MutatingScope $mutatingScope,
string $filePath
): void {
$dependentFiles = [];
foreach ($stmts as $stmt) {
try {
$nodeDependentFiles = $this->dependencyResolver->resolveDependencies($stmt, $mutatingScope);
$dependentFiles = array_merge($dependentFiles, $nodeDependentFiles);
} catch (AnalysedCodeException) {
// @ignoreException
}
}

$this->changedFilesDetector->addFileDependentFiles($filePath, $dependentFiles);
}
}
8 changes: 1 addition & 7 deletions packages/Parallel/WorkerRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use Clue\React\NDJson\Decoder;
use Clue\React\NDJson\Encoder;
use PHPStan\Analyser\NodeScopeResolver;
use Rector\Core\Application\ApplicationFileProcessor;
use Rector\Core\StaticReflection\DynamicSourceLocatorDecorator;
use Rector\Core\ValueObject\Configuration;
Expand All @@ -26,8 +25,7 @@ final class WorkerRunner

public function __construct(
private readonly DynamicSourceLocatorDecorator $dynamicSourceLocatorDecorator,
private readonly ApplicationFileProcessor $applicationFileProcessor,
private readonly NodeScopeResolver $nodeScopeResolver
private readonly ApplicationFileProcessor $applicationFileProcessor
) {
}

Expand Down Expand Up @@ -61,10 +59,6 @@ public function run(Encoder $encoder, Decoder $decoder, Configuration $configura

/** @var string[] $filePaths */
$filePaths = $json[Bridge::FILES] ?? [];

// 1. allow PHPStan to work with static reflection on provided files
$this->nodeScopeResolver->setAnalysedFiles($filePaths);

$systemErrorsAndFileDiffs = $this->applicationFileProcessor->processFiles($filePaths, $configuration);

/**
Expand Down
6 changes: 0 additions & 6 deletions packages/Testing/PHPUnit/AbstractRectorTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Iterator;
use Nette\Utils\FileSystem;
use Nette\Utils\Strings;
use PHPStan\Analyser\NodeScopeResolver;
use PHPUnit\Framework\ExpectationFailedException;
use Rector\Core\Application\ApplicationFileProcessor;
use Rector\Core\Autoloading\AdditionalAutoloader;
Expand Down Expand Up @@ -163,11 +162,6 @@ private function processFilePath(string $filePath): string
{
$this->dynamicSourceLocatorProvider->setFilePath($filePath);

// needed for PHPStan, because the analyzed file is just created in /temp - need for trait and similar deps
/** @var NodeScopeResolver $nodeScopeResolver */
$nodeScopeResolver = $this->getService(NodeScopeResolver::class);
$nodeScopeResolver->setAnalysedFiles([$filePath]);

/** @var ConfigurationFactory $configurationFactory */
$configurationFactory = $this->getService(ConfigurationFactory::class);
$configuration = $configurationFactory->createForTests([$filePath]);
Expand Down

0 comments on commit 645190a

Please sign in to comment.