[Naming] Fix PseudoNamespaceToNamespaceRector reporting on change (#2426)

* fix PseudoNamespaceToNamespaceRector reporting on change

* remove overly abstraction with visitor factories

* [ci-review] Rector Rectify

Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
Tomas Votruba 2022-06-04 10:09:40 +02:00 committed by GitHub
parent c3581f9d33
commit 6b9984175f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 104 additions and 160 deletions

View File

@ -85,6 +85,7 @@ return static function (RectorConfig $rectorConfig): void {
__DIR__ . '/../packages/BetterPhpDocParser/PhpDoc',
__DIR__ . '/../packages/PHPStanStaticTypeMapper/Enum',
__DIR__ . '/../packages/Caching/Cache.php',
__DIR__ . '/../packages/NodeTypeResolver/PhpDocNodeVisitor/UnderscoreRenamePhpDocNodeVisitor.php',
// used in PHPStan
__DIR__ . '/../packages/NodeTypeResolver/Reflection/BetterReflection/RectorBetterReflectionSourceLocatorFactory.php',

View File

@ -1,24 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\BetterPhpDocParser\PhpDocNodeTraverser;
use Rector\BetterPhpDocParser\PhpDocNodeVisitor\ChangedPhpDocNodeVisitor;
use Symplify\Astral\PhpDocParser\PhpDocNodeTraverser;
final class ChangedPhpDocNodeTraverserFactory
{
public function __construct(
private readonly ChangedPhpDocNodeVisitor $changedPhpDocNodeVisitor
) {
}
public function create(): PhpDocNodeTraverser
{
$changedPhpDocNodeTraverser = new PhpDocNodeTraverser();
$changedPhpDocNodeTraverser->addPhpDocNodeVisitor($this->changedPhpDocNodeVisitor);
return $changedPhpDocNodeTraverser;
}
}

View File

@ -17,7 +17,6 @@ use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
use PHPStan\PhpDocParser\Lexer\Lexer;
use Rector\BetterPhpDocParser\PhpDoc\DoctrineAnnotationTagValueNode;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
use Rector\BetterPhpDocParser\PhpDocNodeTraverser\ChangedPhpDocNodeTraverserFactory;
use Rector\BetterPhpDocParser\PhpDocNodeVisitor\ChangedPhpDocNodeVisitor;
use Rector\BetterPhpDocParser\ValueObject\PhpDocAttributeKey;
use Rector\BetterPhpDocParser\ValueObject\StartAndEnd;
@ -83,9 +82,11 @@ final class PhpDocInfoPrinter
private readonly DocBlockInliner $docBlockInliner,
private readonly RemoveNodesStartAndEndResolver $removeNodesStartAndEndResolver,
private readonly ChangedPhpDocNodeVisitor $changedPhpDocNodeVisitor,
ChangedPhpDocNodeTraverserFactory $changedPhpDocNodeTraverserFactory
) {
$this->changedPhpDocNodeTraverser = $changedPhpDocNodeTraverserFactory->create();
$changedPhpDocNodeTraverser = new PhpDocNodeTraverser();
$changedPhpDocNodeTraverser->addPhpDocNodeVisitor($this->changedPhpDocNodeVisitor);
$this->changedPhpDocNodeTraverser = $changedPhpDocNodeTraverser;
}
public function printNew(PhpDocInfo $phpDocInfo): string

View File

@ -5,15 +5,14 @@ declare(strict_types=1);
namespace Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
use Rector\NodeTypeResolver\PhpDoc\PhpDocNodeTraverser\RenamingPhpDocNodeVisitorFactory;
use Rector\NodeTypeResolver\PhpDocNodeVisitor\ClassRenamePhpDocNodeVisitor;
use Rector\NodeTypeResolver\ValueObject\OldToNewType;
use Symplify\Astral\PhpDocParser\PhpDocNodeTraverser;
final class DocBlockClassRenamer
{
public function __construct(
private readonly ClassRenamePhpDocNodeVisitor $classRenamePhpDocNodeVisitor,
private readonly RenamingPhpDocNodeVisitorFactory $renamingPhpDocNodeVisitorFactory
) {
}
@ -26,7 +25,9 @@ final class DocBlockClassRenamer
return;
}
$phpDocNodeTraverser = $this->renamingPhpDocNodeVisitorFactory->create();
$phpDocNodeTraverser = new PhpDocNodeTraverser();
$phpDocNodeTraverser->addPhpDocNodeVisitor($this->classRenamePhpDocNodeVisitor);
$this->classRenamePhpDocNodeVisitor->setOldToNewTypes($oldToNewTypes);
$phpDocNodeTraverser->traverse($phpDocInfo->getPhpDocNode());

View File

@ -6,14 +6,13 @@ namespace Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer;
use PhpParser\Node;
use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocNode;
use Rector\NodeTypeResolver\PhpDoc\PhpDocNodeTraverser\ImportingPhpDocNodeTraverserFactory;
use Rector\NodeTypeResolver\PhpDocNodeVisitor\NameImportingPhpDocNodeVisitor;
use Symplify\Astral\PhpDocParser\PhpDocNodeTraverser;
final class DocBlockNameImporter
{
public function __construct(
private readonly NameImportingPhpDocNodeVisitor $nameImportingPhpDocNodeVisitor,
private readonly ImportingPhpDocNodeTraverserFactory $importingPhpDocNodeTraverserFactory
) {
}
@ -25,7 +24,8 @@ final class DocBlockNameImporter
$this->nameImportingPhpDocNodeVisitor->setCurrentNode($node);
$phpDocNodeTraverser = $this->importingPhpDocNodeTraverserFactory->create();
$phpDocNodeTraverser = new PhpDocNodeTraverser();
$phpDocNodeTraverser->addPhpDocNodeVisitor($this->nameImportingPhpDocNodeVisitor);
$phpDocNodeTraverser->traverse($phpDocNode);
}
}

View File

@ -1,24 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\NodeTypeResolver\PhpDoc\PhpDocNodeTraverser;
use Rector\NodeTypeResolver\PhpDocNodeVisitor\NameImportingPhpDocNodeVisitor;
use Symplify\Astral\PhpDocParser\PhpDocNodeTraverser;
final class ImportingPhpDocNodeTraverserFactory
{
public function __construct(
private readonly NameImportingPhpDocNodeVisitor $nameImportingPhpDocNodeVisitor
) {
}
public function create(): PhpDocNodeTraverser
{
$phpDocNodeTraverser = new PhpDocNodeTraverser();
$phpDocNodeTraverser->addPhpDocNodeVisitor($this->nameImportingPhpDocNodeVisitor);
return $phpDocNodeTraverser;
}
}

View File

@ -1,24 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\NodeTypeResolver\PhpDoc\PhpDocNodeTraverser;
use Rector\NodeTypeResolver\PhpDocNodeVisitor\ClassRenamePhpDocNodeVisitor;
use Symplify\Astral\PhpDocParser\PhpDocNodeTraverser;
final class RenamingPhpDocNodeVisitorFactory
{
public function __construct(
private readonly ClassRenamePhpDocNodeVisitor $classRenamePhpDocNodeVisitor
) {
}
public function create(): PhpDocNodeTraverser
{
$phpDocNodeTraverser = new PhpDocNodeTraverser();
$phpDocNodeTraverser->addPhpDocNodeVisitor($this->classRenamePhpDocNodeVisitor);
return $phpDocNodeTraverser;
}
}

View File

@ -1,24 +0,0 @@
<?php
declare(strict_types=1);
namespace Rector\NodeTypeResolver\PhpDoc\PhpDocNodeTraverser;
use Rector\NodeTypeResolver\PhpDocNodeVisitor\UnderscoreRenamePhpDocNodeVisitor;
use Symplify\Astral\PhpDocParser\PhpDocNodeTraverser;
final class UnderscorePhpDocNodeTraverserFactory
{
public function __construct(
private readonly UnderscoreRenamePhpDocNodeVisitor $underscoreRenamePhpDocNodeVisitor
) {
}
public function create(): PhpDocNodeTraverser
{
$phpDocNodeTraverser = new PhpDocNodeTraverser();
$phpDocNodeTraverser->addPhpDocNodeVisitor($this->underscoreRenamePhpDocNodeVisitor);
return $phpDocNodeTraverser;
}
}

View File

@ -6,15 +6,15 @@ namespace Rector\NodeTypeResolver\PhpDoc;
use PhpParser\Node;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
use Rector\NodeTypeResolver\PhpDoc\PhpDocNodeTraverser\UnderscorePhpDocNodeTraverserFactory;
use Rector\NodeTypeResolver\PhpDocNodeVisitor\UnderscoreRenamePhpDocNodeVisitor;
use Rector\Renaming\ValueObject\PseudoNamespaceToNamespace;
use Rector\StaticTypeMapper\StaticTypeMapper;
use Symplify\Astral\PhpDocParser\PhpDocNodeTraverser;
final class PhpDocTypeRenamer
{
public function __construct(
private readonly UnderscorePhpDocNodeTraverserFactory $underscorePhpDocNodeTraverserFactory,
private readonly UnderscoreRenamePhpDocNodeVisitor $underscoreRenamePhpDocNodeVisitor
private readonly StaticTypeMapper $staticTypeMapper,
) {
}
@ -22,13 +22,20 @@ final class PhpDocTypeRenamer
PhpDocInfo $phpDocInfo,
Node $node,
PseudoNamespaceToNamespace $pseudoNamespaceToNamespace
): void {
): bool {
$phpDocNode = $phpDocInfo->getPhpDocNode();
$this->underscoreRenamePhpDocNodeVisitor->setPseudoNamespaceToNamespace($pseudoNamespaceToNamespace);
$this->underscoreRenamePhpDocNodeVisitor->setCurrentPhpParserNode($node);
$underscoreRenamePhpDocNodeVisitor = new UnderscoreRenamePhpDocNodeVisitor(
$this->staticTypeMapper,
$pseudoNamespaceToNamespace,
$node,
);
$phpDocNodeTraverser = $this->underscorePhpDocNodeTraverserFactory->create();
$phpDocNodeTraverser = new PhpDocNodeTraverser();
$phpDocNodeTraverser->addPhpDocNodeVisitor($underscoreRenamePhpDocNodeVisitor);
$phpDocNodeTraverser->traverse($phpDocNode);
// has changed?
return $underscoreRenamePhpDocNodeVisitor->hasChanged();
}
}

View File

@ -58,7 +58,7 @@ final class ClassRenamePhpDocNodeVisitor extends AbstractPhpDocNodeVisitor
}
$phpParserNode = $this->currentNodeProvider->getNode();
if (! $phpParserNode instanceof \PhpParser\Node) {
if (! $phpParserNode instanceof PhpParserNode) {
throw new ShouldNotHappenException();
}

View File

@ -8,31 +8,24 @@ use Nette\Utils\Strings;
use PHPStan\PhpDocParser\Ast\Node;
use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
use PHPStan\Type\ObjectType;
use Rector\Core\Exception\ShouldNotHappenException;
use Rector\Renaming\ValueObject\PseudoNamespaceToNamespace;
use Rector\StaticTypeMapper\StaticTypeMapper;
use Symplify\Astral\PhpDocParser\PhpDocNodeVisitor\AbstractPhpDocNodeVisitor;
final class UnderscoreRenamePhpDocNodeVisitor extends AbstractPhpDocNodeVisitor
{
private ?PseudoNamespaceToNamespace $pseudoNamespaceToNamespace = null;
private ?\PhpParser\Node $currentPhpParserNode = null;
private bool $hasChanged = false;
public function __construct(
private readonly StaticTypeMapper $staticTypeMapper
private readonly StaticTypeMapper $staticTypeMapper,
private readonly PseudoNamespaceToNamespace $pseudoNamespaceToNamespace,
private readonly \PhpParser\Node $phpNode,
) {
}
public function beforeTraverse(Node $node): void
{
if ($this->pseudoNamespaceToNamespace === null) {
throw new ShouldNotHappenException('Set PseudoNamespaceToNamespace first');
}
if (! $this->currentPhpParserNode instanceof \PhpParser\Node) {
throw new ShouldNotHappenException('Set "$currentPhpParserNode" first');
}
$this->hasChanged = false;
}
public function enterNode(Node $node): ?Node
@ -41,32 +34,26 @@ final class UnderscoreRenamePhpDocNodeVisitor extends AbstractPhpDocNodeVisitor
return null;
}
if ($this->shouldSkip($node, $this->currentPhpParserNode, $this->pseudoNamespaceToNamespace)) {
if ($this->shouldSkip($node, $this->phpNode, $this->pseudoNamespaceToNamespace)) {
return null;
}
/** @var IdentifierTypeNode $node */
$staticType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType(
$node,
$this->currentPhpParserNode
);
$staticType = $this->staticTypeMapper->mapPHPStanPhpDocTypeNodeToPHPStanType($node, $this->phpNode);
if (! $staticType instanceof ObjectType) {
return null;
}
$this->hasChanged = true;
// change underscore to \\
$slashedName = '\\' . Strings::replace($staticType->getClassName(), '#_#', '\\');
return new IdentifierTypeNode($slashedName);
}
public function setPseudoNamespaceToNamespace(PseudoNamespaceToNamespace $pseudoNamespaceToNamespace): void
public function hasChanged(): bool
{
$this->pseudoNamespaceToNamespace = $pseudoNamespaceToNamespace;
}
public function setCurrentPhpParserNode(\PhpParser\Node $node): void
{
$this->currentPhpParserNode = $node;
return $this->hasChanged;
}
private function shouldSkip(

View File

@ -719,3 +719,6 @@ parameters:
path: rules/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchForAssignRector.php
- '#Method "replaceTagByAnother\(\)" returns bool type, so the name should start with is/has/was#'
- '#Method "refactorPhpDoc\(\)" returns bool type, so the name should start with is/has/was#'
- '#Cognitive complexity for "Rector\\Renaming\\Rector\\FileWithoutNamespace\\PseudoNamespaceToNamespaceRector\:\:refactorStmts\(\)" is 11, keep it under 10#'

View File

@ -7,12 +7,11 @@ use Rector\Renaming\Rector\FileWithoutNamespace\PseudoNamespaceToNamespaceRector
use Rector\Renaming\ValueObject\PseudoNamespaceToNamespace;
return static function (RectorConfig $rectorConfig): void {
$rectorConfig
->ruleWithConfiguration(PseudoNamespaceToNamespaceRector::class, [
new PseudoNamespaceToNamespace('PHPUnit_', ['PHPUnit_Framework_MockObject_MockObject']),
new PseudoNamespaceToNamespace('ChangeMe_', ['KeepMe_']),
new PseudoNamespaceToNamespace(
'Rector_Tests_Renaming_Rector_FileWithoutNamespace_PseudoNamespaceToNamespaceRector_Fixture_'
),
]);
$rectorConfig->ruleWithConfiguration(PseudoNamespaceToNamespaceRector::class, [
new PseudoNamespaceToNamespace('PHPUnit_', ['PHPUnit_Framework_MockObject_MockObject']),
new PseudoNamespaceToNamespace('ChangeMe_', ['KeepMe_']),
new PseudoNamespaceToNamespace(
'Rector_Tests_Renaming_Rector_FileWithoutNamespace_PseudoNamespaceToNamespaceRector_Fixture_'
),
]);
};

View File

@ -85,18 +85,21 @@ CODE_SAMPLE
$this->newNamespace = null;
if ($node instanceof FileWithoutNamespace) {
$stmts = $this->refactorStmts($node->stmts);
$node->stmts = $stmts;
$changedStmts = $this->refactorStmts($node->stmts);
if ($changedStmts === null) {
return null;
}
$node->stmts = $changedStmts;
// add a new namespace?
if ($this->newNamespace !== null) {
return new Namespace_(new Name($this->newNamespace), $stmts);
return new Namespace_(new Name($this->newNamespace), $changedStmts);
}
}
if ($node instanceof Namespace_) {
$this->refactorStmts([$node]);
return $node;
return $this->refactorNamespace($node);
}
return null;
@ -114,31 +117,38 @@ CODE_SAMPLE
/**
* @param Stmt[] $stmts
* @return Stmt[]
* @return Stmt[]|null
*/
private function refactorStmts(array $stmts): array
private function refactorStmts(array $stmts): ?array
{
$this->traverseNodesWithCallable($stmts, function (Node $node): ?Node {
$hasChanged = false;
$this->traverseNodesWithCallable($stmts, function (Node $node) use (&$hasChanged): ?Node {
if (! $node instanceof Name && ! $node instanceof Identifier && ! $node instanceof Property && ! $node instanceof FunctionLike) {
return null;
}
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
// replace on @var/@param/@return/@throws
foreach ($this->pseudoNamespacesToNamespaces as $pseudoNamespaceToNamespace) {
$this->phpDocTypeRenamer->changeUnderscoreType($phpDocInfo, $node, $pseudoNamespaceToNamespace);
if ($this->refactorPhpDoc($node)) {
$hasChanged = true;
}
// @todo - update rule to allow for bool instanceof check
if ($node instanceof Name || $node instanceof Identifier) {
return $this->processNameOrIdentifier($node);
$changedNode = $this->processNameOrIdentifier($node);
if ($changedNode instanceof Node) {
$hasChanged = true;
return $changedNode;
}
}
return null;
});
return $stmts;
if ($hasChanged) {
return $stmts;
}
return null;
}
/**
@ -208,4 +218,35 @@ CODE_SAMPLE
return $identifier;
}
private function refactorNamespace(Namespace_ $namespace): ?Namespace_
{
$changedStmts = $this->refactorStmts($namespace->stmts);
if ($changedStmts === null) {
return null;
}
return $namespace;
}
private function refactorPhpDoc(Name|FunctionLike|Identifier|Property $node): bool
{
$hasChanged = false;
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
// replace on @var/@param/@return/@throws
foreach ($this->pseudoNamespacesToNamespaces as $pseudoNamespaceToNamespace) {
$hasDocTypeChanged = $this->phpDocTypeRenamer->changeUnderscoreType(
$phpDocInfo,
$node,
$pseudoNamespaceToNamespace
);
if ($hasDocTypeChanged) {
$hasChanged = true;
}
}
return $hasChanged;
}
}