From d8d76d72b448283fe5dad4064656afe802e0b9e9 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 8 Dec 2018 14:36:29 +0100 Subject: [PATCH] move FilesToReprtingCollector to new cycle in RectorApplication --- ecs.yml | 1 + .../AbstractScalarTypehintRector.php | 8 -- .../ParamScalarTypehintRector.php | 37 ++++---- .../ReturnScalarTypehintRector.php | 7 -- src/Application/ErrorAndDiffCollector.php | 11 +++ src/Application/FileProcessor.php | 40 ++++---- src/Application/FilesToReprintCollector.php | 31 ------ src/Application/RectorApplication.php | 95 ++++++------------- .../PHPUnit/AbstractRectorTestCase.php | 9 +- .../PseudoNamespaceToNamespaceRectorTest.php | 4 +- 10 files changed, 85 insertions(+), 158 deletions(-) delete mode 100644 src/Application/FilesToReprintCollector.php diff --git a/ecs.yml b/ecs.yml index 0353b8b4ff9..9c3b37e3d6d 100644 --- a/ecs.yml +++ b/ecs.yml @@ -71,6 +71,7 @@ parameters: - 'src/Rector/Constant/RenameClassConstantsUseToStringsRector.php' - '*/packages/NodeTypeResolver/**/PerNodeTypeResolver/**TypeResolver.php' - '*/packages/NodeTypeResolver/**/PerNodeTypeResolver/**TypeResolver/*Test.php' + - '*RectorTest.php' - 'tests/PhpParser/Node/ConstExprEvaluatorFactoryTest.php' - 'src/Rector/AbstractPHPUnitRector.php' - 'src/Rector/Class_/ParentClassToTraitsRector.php' diff --git a/packages/Php/src/Rector/FunctionLike/AbstractScalarTypehintRector.php b/packages/Php/src/Rector/FunctionLike/AbstractScalarTypehintRector.php index 669b59659bd..42e5da2a43a 100644 --- a/packages/Php/src/Rector/FunctionLike/AbstractScalarTypehintRector.php +++ b/packages/Php/src/Rector/FunctionLike/AbstractScalarTypehintRector.php @@ -12,7 +12,6 @@ use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Function_; use PhpParser\Node\Stmt\Interface_; -use Rector\Application\FilesToReprintCollector; use Rector\NodeTypeResolver\Application\ClassLikeNodeCollector; use Rector\NodeTypeResolver\Node\Attribute; use Rector\NodeTypeResolver\Php\AbstractTypeInfo; @@ -45,15 +44,9 @@ abstract class AbstractScalarTypehintRector extends AbstractRector */ protected $classLikeNodeCollector; - /** - * @var FilesToReprintCollector - */ - protected $filesToReprintCollector; - public function __construct( DocBlockAnalyzer $docBlockAnalyzer, ClassLikeNodeCollector $classLikeNodeCollector, - FilesToReprintCollector $filesToReprintCollector, bool $enableObjectType = false ) { $this->docBlockAnalyzer = $docBlockAnalyzer; @@ -62,7 +55,6 @@ abstract class AbstractScalarTypehintRector extends AbstractRector if ($enableObjectType) { PhpTypeSupport::enableType('object'); } - $this->filesToReprintCollector = $filesToReprintCollector; } /** diff --git a/packages/Php/src/Rector/FunctionLike/ParamScalarTypehintRector.php b/packages/Php/src/Rector/FunctionLike/ParamScalarTypehintRector.php index e3ebb36ebc1..dd38a5510df 100644 --- a/packages/Php/src/Rector/FunctionLike/ParamScalarTypehintRector.php +++ b/packages/Php/src/Rector/FunctionLike/ParamScalarTypehintRector.php @@ -8,7 +8,6 @@ use PhpParser\Node\Stmt\Function_; use Rector\NodeTypeResolver\Node\Attribute; use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\RectorDefinition; -use Symplify\PackageBuilder\FileSystem\SmartFileInfo; final class ParamScalarTypehintRector extends AbstractScalarTypehintRector { @@ -148,27 +147,23 @@ CODE_SAMPLE // update their methods as well foreach ($childrenClassLikes as $childClassLike) { $childClassMethod = $childClassLike->getMethod($methodName); - if ($childClassMethod) { - if (! isset($childClassMethod->params[$i])) { - continue; - } - - $childClassMethodParam = $childClassMethod->params[$i]; - if ($childClassMethodParam->type !== null) { - continue; - } - - $childClassMethodParam->type = $this->resolveChildType($paramTagInfo, $node, $paramNode); - - // let the method know it was changed now - $childClassMethodParam->type->setAttribute(self::HAS_NEW_INHERITED_TYPE, true); - - // reprint the file - /** @var SmartFileInfo $fileInfo */ - $fileInfo = $childClassMethod->getAttribute(Attribute::FILE_INFO); - - $this->filesToReprintCollector->addFileInfoAndRectorClass($fileInfo, self::class); + if ($childClassMethod === null) { + continue; } + + if (! isset($childClassMethod->params[$i])) { + continue; + } + + $childClassMethodParam = $childClassMethod->params[$i]; + if ($childClassMethodParam->type !== null) { + continue; + } + + $childClassMethodParam->type = $this->resolveChildType($paramTagInfo, $node, $paramNode); + + // let the method know it was changed now + $childClassMethodParam->type->setAttribute(self::HAS_NEW_INHERITED_TYPE, true); } } } diff --git a/packages/Php/src/Rector/FunctionLike/ReturnScalarTypehintRector.php b/packages/Php/src/Rector/FunctionLike/ReturnScalarTypehintRector.php index 56e7060afe5..e7b77207a42 100644 --- a/packages/Php/src/Rector/FunctionLike/ReturnScalarTypehintRector.php +++ b/packages/Php/src/Rector/FunctionLike/ReturnScalarTypehintRector.php @@ -8,7 +8,6 @@ use PhpParser\Node\Stmt\Function_; use Rector\NodeTypeResolver\Node\Attribute; use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\RectorDefinition; -use Symplify\PackageBuilder\FileSystem\SmartFileInfo; final class ReturnScalarTypehintRector extends AbstractScalarTypehintRector { @@ -120,12 +119,6 @@ CODE_SAMPLE // let the method now it was changed now $childClassMethod->returnType->setAttribute(self::HAS_NEW_INHERITED_TYPE, true); - - // reprint the file - /** @var SmartFileInfo $fileInfo */ - $fileInfo = $childClassMethod->getAttribute(Attribute::FILE_INFO); - - $this->filesToReprintCollector->addFileInfoAndRectorClass($fileInfo, self::class); } } diff --git a/src/Application/ErrorAndDiffCollector.php b/src/Application/ErrorAndDiffCollector.php index 7081c2af667..c8c48b3dfac 100644 --- a/src/Application/ErrorAndDiffCollector.php +++ b/src/Application/ErrorAndDiffCollector.php @@ -8,6 +8,7 @@ use Rector\Error\ExceptionCorrector; use Rector\NodeTypeResolver\FileSystem\CurrentFileInfoProvider; use Rector\Reporting\FileDiff; use Symplify\PackageBuilder\FileSystem\SmartFileInfo; +use Throwable; final class ErrorAndDiffCollector { @@ -105,4 +106,14 @@ final class ErrorAndDiffCollector $this->addError(new Error($fileInfo, $message)); } + + public function addThrowableWithFileInfo(Throwable $throwable, SmartFileInfo $fileInfo): void + { + $rectorClass = $this->exceptionCorrector->matchRectorClass($throwable); + if ($rectorClass) { + $this->addErrorWithRectorMessage($rectorClass, $throwable->getMessage()); + } else { + $this->addError(new Error($fileInfo, $throwable->getMessage(), $throwable->getCode())); + } + } } diff --git a/src/Application/FileProcessor.php b/src/Application/FileProcessor.php index 4c1ba919271..907626705b3 100644 --- a/src/Application/FileProcessor.php +++ b/src/Application/FileProcessor.php @@ -64,8 +64,13 @@ final class FileProcessor $this->currentFileInfoProvider = $currentFileInfoProvider; } - public function processFile(SmartFileInfo $smartFileInfo): string + public function parseFileInfoToLocalCache(SmartFileInfo $smartFileInfo): void { + if (isset($this->tokensByFilePath[$smartFileInfo->getRealPath()])) { + // already parsed + return; + } + $this->currentFileInfoProvider->setCurrentFileInfo($smartFileInfo); [$newStmts, $oldStmts, $oldTokens] = $this->parseAndTraverseFileInfoToNodes($smartFileInfo); @@ -73,41 +78,31 @@ final class FileProcessor // store tokens by absolute path, so we don't have to print them right now $this->tokensByFilePath[$smartFileInfo->getRealPath()] = [$newStmts, $oldStmts, $oldTokens]; - return $this->formatPerservingPrinter->printToFile($smartFileInfo, $newStmts, $oldStmts, $oldTokens); + // @todo use filesystem cache to save parsing? } - public function reprintFile(SmartFileInfo $smartFileInfo): string + public function printToFile(SmartFileInfo $smartFileInfo): string { - // restore tokens [$newStmts, $oldStmts, $oldTokens] = $this->tokensByFilePath[$smartFileInfo->getRealPath()]; - return $this->formatPerservingPrinter->printToFile($smartFileInfo, $newStmts, $oldStmts, $oldTokens); } /** * See https://github.com/nikic/PHP-Parser/issues/344#issuecomment-298162516. */ - public function processFileToString(SmartFileInfo $smartFileInfo): string + public function printToString(SmartFileInfo $smartFileInfo): string { - $this->currentFileInfoProvider->setCurrentFileInfo($smartFileInfo); + [$newStmts, $oldStmts, $oldTokens] = $this->tokensByFilePath[$smartFileInfo->getRealPath()]; + return $this->formatPerservingPrinter->printToString($newStmts, $oldStmts, $oldTokens); + } - [$newStmts, $oldStmts, $oldTokens] = $this->parseAndTraverseFileInfoToNodes($smartFileInfo); + public function refactor(SmartFileInfo $smartFileInfo): void + { + [$newStmts, $oldStmts, $oldTokens] = $this->tokensByFilePath[$smartFileInfo->getRealPath()]; + $newStmts = $this->rectorNodeTraverser->traverse($newStmts); - // store tokens by absolute path, so we don't have to print them right now + // this is needed for new tokens added in "afterTraverse()" $this->tokensByFilePath[$smartFileInfo->getRealPath()] = [$newStmts, $oldStmts, $oldTokens]; - - return $this->formatPerservingPrinter->printToString($newStmts, $oldStmts, $oldTokens); - } - - /** - * See https://github.com/nikic/PHP-Parser/issues/344#issuecomment-298162516. - */ - public function reprintToString(SmartFileInfo $smartFileInfo): string - { - // restore tokens - [$newStmts, $oldStmts, $oldTokens] = $this->tokensByFilePath[$smartFileInfo->getRealPath()]; - - return $this->formatPerservingPrinter->printToString($newStmts, $oldStmts, $oldTokens); } /** @@ -122,7 +117,6 @@ final class FileProcessor $oldStmts, $smartFileInfo->getRealPath() ); - $newStmts = $this->rectorNodeTraverser->traverse($newStmts); return [$newStmts, $oldStmts, $oldTokens]; } diff --git a/src/Application/FilesToReprintCollector.php b/src/Application/FilesToReprintCollector.php deleted file mode 100644 index 08b1d2396e1..00000000000 --- a/src/Application/FilesToReprintCollector.php +++ /dev/null @@ -1,31 +0,0 @@ -items[$smartFileInfo->getRealPath()] = [$rectorClass, $smartFileInfo]; - } - - /** - * @return SmartFileInfo[]|string[] - */ - public function getFileInfosAndRectorClasses(): array - { - return $this->items; - } - - public function reset(): void - { - $this->items = []; - } -} diff --git a/src/Application/RectorApplication.php b/src/Application/RectorApplication.php index 8ff9e126c85..bc75641793f 100644 --- a/src/Application/RectorApplication.php +++ b/src/Application/RectorApplication.php @@ -4,7 +4,6 @@ namespace Rector\Application; use PHPStan\AnalysedCodeException; use Rector\Configuration\Configuration; -use Rector\Error\ExceptionCorrector; use Rector\FileSystemRector\FileSystemFileProcessor; use Symfony\Component\Console\Style\SymfonyStyle; use Symplify\PackageBuilder\FileSystem\SmartFileInfo; @@ -31,11 +30,6 @@ final class RectorApplication */ private $fileSystemFileProcessor; - /** - * @var ExceptionCorrector - */ - private $exceptionCorrector; - /** * @var ErrorAndDiffCollector */ @@ -51,27 +45,18 @@ final class RectorApplication */ private $fileProcessor; - /** - * @var FilesToReprintCollector - */ - private $filesToReprintCollector; - public function __construct( SymfonyStyle $symfonyStyle, FileSystemFileProcessor $fileSystemFileProcessor, - ExceptionCorrector $exceptionCorrector, ErrorAndDiffCollector $errorAndDiffCollector, Configuration $configuration, - FileProcessor $fileProcessor, - FilesToReprintCollector $filesToReprintCollector + FileProcessor $fileProcessor ) { $this->symfonyStyle = $symfonyStyle; $this->fileSystemFileProcessor = $fileSystemFileProcessor; - $this->exceptionCorrector = $exceptionCorrector; $this->errorAndDiffCollector = $errorAndDiffCollector; $this->configuration = $configuration; $this->fileProcessor = $fileProcessor; - $this->filesToReprintCollector = $filesToReprintCollector; } /** @@ -80,10 +65,25 @@ final class RectorApplication public function runOnFileInfos(array $fileInfos): void { $totalFiles = count($fileInfos); + if (! $this->symfonyStyle->isVerbose()) { - $this->symfonyStyle->progressStart($totalFiles); + // why 3? one for each cycle, so user sees some activity all the time + $this->symfonyStyle->progressStart($totalFiles * 3); } + // 1. parse files to nodes + foreach ($fileInfos as $fileInfo) { + $this->advance(); + $this->fileProcessor->parseFileInfoToLocalCache($fileInfo); + } + + // 2. change nodes with Rectors + foreach ($fileInfos as $fileInfo) { + $this->advance(); + $this->fileProcessor->refactor($fileInfo); + } + + // 3. print to file or string foreach ($fileInfos as $fileInfo) { $this->processFileInfo($fileInfo); if ($this->symfonyStyle->isVerbose()) { @@ -99,7 +99,16 @@ final class RectorApplication private function processFileInfo(SmartFileInfo $fileInfo): void { try { - $this->processFile($fileInfo); + $oldContent = $fileInfo->getContents(); + + if ($this->configuration->isDryRun()) { + $newContent = $this->fileProcessor->printToString($fileInfo); + } else { + $newContent = $this->fileProcessor->printToFile($fileInfo); + } + + $this->errorAndDiffCollector->addFileDiff($fileInfo, $newContent, $oldContent); + $this->fileSystemFileProcessor->processFileInfo($fileInfo); } catch (AnalysedCodeException $analysedCodeException) { if ($this->configuration->shouldHideAutoloadErrors()) { @@ -112,56 +121,14 @@ final class RectorApplication throw $throwable; } - $rectorClass = $this->exceptionCorrector->matchRectorClass($throwable); - if ($rectorClass) { - $this->errorAndDiffCollector->addErrorWithRectorMessage($rectorClass, $throwable->getMessage()); - } else { - $this->errorAndDiffCollector->addError( - new Error($fileInfo, $throwable->getMessage(), $throwable->getCode()) - ); - } + $this->errorAndDiffCollector->addThrowableWithFileInfo($throwable, $fileInfo); } } - private function processFile(SmartFileInfo $fileInfo): void + private function advance(): void { - $oldContent = $fileInfo->getContents(); - - if ($this->configuration->isDryRun()) { - $newContent = $this->fileProcessor->processFileToString($fileInfo); - - /** @var string $rectorClass */ - foreach ($this->filesToReprintCollector->getFileInfosAndRectorClasses() as $rectorClassToFileInfo) { - [$rectorClass, $fileInfoToReprint] = $rectorClassToFileInfo; - - $reprintedOldContent = $fileInfoToReprint->getContents(); - $reprintedNewContent = $this->fileProcessor->reprintToString($fileInfoToReprint); - $this->errorAndDiffCollector->addFileDiff( - $fileInfoToReprint, - $reprintedNewContent, - $reprintedOldContent, - $rectorClass - ); - } - } else { - $newContent = $this->fileProcessor->processFile($fileInfo); - - /** @var string $rectorClass */ - foreach ($this->filesToReprintCollector->getFileInfosAndRectorClasses() as $rectorClassToFileInfo) { - [$rectorClass, $fileInfoToReprint] = $rectorClassToFileInfo; - - $reprintedOldContent = $fileInfoToReprint->getContents(); - $reprintedNewContent = $this->fileProcessor->reprintFile($fileInfoToReprint); - $this->errorAndDiffCollector->addFileDiff( - $fileInfoToReprint, - $reprintedNewContent, - $reprintedOldContent, - $rectorClass - ); - } + if ($this->symfonyStyle->isVerbose() === false) { + $this->symfonyStyle->progressAdvance(); } - - $this->errorAndDiffCollector->addFileDiff($fileInfo, $newContent, $oldContent); - $this->filesToReprintCollector->reset(); } } diff --git a/src/Testing/PHPUnit/AbstractRectorTestCase.php b/src/Testing/PHPUnit/AbstractRectorTestCase.php index 53fcdbe7815..5ee96f61ea4 100644 --- a/src/Testing/PHPUnit/AbstractRectorTestCase.php +++ b/src/Testing/PHPUnit/AbstractRectorTestCase.php @@ -173,7 +173,7 @@ abstract class AbstractRectorTestCase extends TestCase private function createTemporaryPathWithPrefix(SmartFileInfo $smartFileInfo, string $prefix): string { - $hash = Strings::substring(md5($smartFileInfo->getPathname()), 0, 5); + $hash = Strings::substring(md5($smartFileInfo->getRealPath()), 0, 5); return sprintf( sys_get_temp_dir() . '/rector_temp_tests/%s_%s_%s', @@ -187,7 +187,12 @@ abstract class AbstractRectorTestCase extends TestCase { $this->parameterProvider->changeParameter(Option::SOURCE, [$originalFile]); - $changedContent = $this->fileProcessor->processFileToString(new SmartFileInfo($originalFile)); + $smartFileInfo = new SmartFileInfo($originalFile); + + // life-cycle trio :) + $this->fileProcessor->parseFileInfoToLocalCache($smartFileInfo); + $this->fileProcessor->refactor($smartFileInfo); + $changedContent = $this->fileProcessor->printToString($smartFileInfo); $this->assertStringEqualsFile($expectedFile, $changedContent); } diff --git a/tests/Rector/Namespace_/PseudoNamespaceToNamespaceRector/PseudoNamespaceToNamespaceRectorTest.php b/tests/Rector/Namespace_/PseudoNamespaceToNamespaceRector/PseudoNamespaceToNamespaceRectorTest.php index 5f3b5fc8692..cca8c8ff9d0 100644 --- a/tests/Rector/Namespace_/PseudoNamespaceToNamespaceRector/PseudoNamespaceToNamespaceRectorTest.php +++ b/tests/Rector/Namespace_/PseudoNamespaceToNamespaceRector/PseudoNamespaceToNamespaceRectorTest.php @@ -2,7 +2,6 @@ namespace Rector\Tests\Rector\Namespace_\PseudoNamespaceToNamespaceRector; -use PHPUnit_Framework_MockObject_MockObject; use Rector\Rector\Namespace_\PseudoNamespaceToNamespaceRector; use Rector\Testing\PHPUnit\AbstractRectorTestCase; @@ -30,7 +29,8 @@ final class PseudoNamespaceToNamespaceRectorTest extends AbstractRectorTestCase protected function getRectorConfiguration(): array { return [ - 'PHPUnit_' => [PHPUnit_Framework_MockObject_MockObject::class], + // namespace prefix => excluded classes + 'PHPUnit_' => ['PHPUnit_Framework_MockObject_MockObject'], 'ChangeMe_' => ['KeepMe_'], ]; }