From f9dc93cb1a25a70a8e3e2731f6594d1e2f935db8 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 23 Apr 2021 21:32:59 +0200 Subject: [PATCH] Fix WrapEncapsedVariableInCurlyBracesRector reporting (#6220) Co-authored-by: kaizen-ci --- .../NodeTypeResolver/Node/AttributeKey.php | 1 + .../Fixture/fixture.php.inc | 14 +---- .../CompleteDynamicPropertiesRector.php | 10 ++- ...rapEncapsedVariableInCurlyBracesRector.php | 1 - .../RemoveParentCallWithoutParentRector.php | 10 ++- .../New_/DowngradeAnonymousClassRector.php | 11 ++++ .../ChangeLocalPropertyToVariableRector.php | 10 ++- .../FinalizeClassesWithoutChildrenRector.php | 11 ++++ ...zeLocalPropertyToPrivatePropertyRector.php | 13 +++- .../Class_/ChangeSingletonToServiceRector.php | 13 +++- .../Class_/ParentClassToTraitsRector.php | 9 ++- ...viceGetterToConstructorInjectionRector.php | 9 ++- src/NodeAnalyzer/ChangedNodeAnalyzer.php | 54 ++++++++++++++++ src/Rector/AbstractRector.php | 62 ++++++------------- 14 files changed, 162 insertions(+), 66 deletions(-) create mode 100644 src/NodeAnalyzer/ChangedNodeAnalyzer.php diff --git a/packages/NodeTypeResolver/Node/AttributeKey.php b/packages/NodeTypeResolver/Node/AttributeKey.php index 7fed1a5bcae..9c1d219d238 100644 --- a/packages/NodeTypeResolver/Node/AttributeKey.php +++ b/packages/NodeTypeResolver/Node/AttributeKey.php @@ -19,6 +19,7 @@ final class AttributeKey public const SCOPE = 'scope'; /** + * @deprecated * @var string */ public const USE_NODES = 'useNodes'; diff --git a/rules-tests/CodingStyle/Rector/Encapsed/WrapEncapsedVariableInCurlyBracesRector/Fixture/fixture.php.inc b/rules-tests/CodingStyle/Rector/Encapsed/WrapEncapsedVariableInCurlyBracesRector/Fixture/fixture.php.inc index 2ccca8a0bd0..47bbd995bd1 100644 --- a/rules-tests/CodingStyle/Rector/Encapsed/WrapEncapsedVariableInCurlyBracesRector/Fixture/fixture.php.inc +++ b/rules-tests/CodingStyle/Rector/Encapsed/WrapEncapsedVariableInCurlyBracesRector/Fixture/fixture.php.inc @@ -1,21 +1,11 @@ ----- diff --git a/rules/CodeQuality/Rector/Class_/CompleteDynamicPropertiesRector.php b/rules/CodeQuality/Rector/Class_/CompleteDynamicPropertiesRector.php index f0b9ac7c2a6..53b3fd1ce6b 100644 --- a/rules/CodeQuality/Rector/Class_/CompleteDynamicPropertiesRector.php +++ b/rules/CodeQuality/Rector/Class_/CompleteDynamicPropertiesRector.php @@ -12,6 +12,7 @@ use PHPStan\Type\Type; use Rector\CodeQuality\NodeAnalyzer\ClassLikeAnalyzer; use Rector\CodeQuality\NodeAnalyzer\LocalPropertyAnalyzer; use Rector\CodeQuality\NodeFactory\MissingPropertiesFactory; +use Rector\Core\NodeAnalyzer\ClassAnalyzer; use Rector\Core\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -45,16 +46,23 @@ final class CompleteDynamicPropertiesRector extends AbstractRector */ private $reflectionProvider; + /** + * @var ClassAnalyzer + */ + private $classAnalyzer; + public function __construct( MissingPropertiesFactory $missingPropertiesFactory, LocalPropertyAnalyzer $localPropertyAnalyzer, ClassLikeAnalyzer $classLikeAnalyzer, - ReflectionProvider $reflectionProvider + ReflectionProvider $reflectionProvider, + ClassAnalyzer $classAnalyzer ) { $this->missingPropertiesFactory = $missingPropertiesFactory; $this->localPropertyAnalyzer = $localPropertyAnalyzer; $this->classLikeAnalyzer = $classLikeAnalyzer; $this->reflectionProvider = $reflectionProvider; + $this->classAnalyzer = $classAnalyzer; } public function getRuleDefinition(): RuleDefinition diff --git a/rules/CodingStyle/Rector/Encapsed/WrapEncapsedVariableInCurlyBracesRector.php b/rules/CodingStyle/Rector/Encapsed/WrapEncapsedVariableInCurlyBracesRector.php index 7c331a93937..3fa1c29c9a6 100644 --- a/rules/CodingStyle/Rector/Encapsed/WrapEncapsedVariableInCurlyBracesRector.php +++ b/rules/CodingStyle/Rector/Encapsed/WrapEncapsedVariableInCurlyBracesRector.php @@ -61,7 +61,6 @@ CODE_SAMPLE if ($previousNodeEndTokenPosition + 1 === $nodePart->getStartTokenPos()) { $hasVariableBeenWrapped = true; - $node->parts[$index] = new Variable($nodePart->name); } } diff --git a/rules/DeadCode/Rector/StaticCall/RemoveParentCallWithoutParentRector.php b/rules/DeadCode/Rector/StaticCall/RemoveParentCallWithoutParentRector.php index 72b6b12dbdc..641270b1972 100644 --- a/rules/DeadCode/Rector/StaticCall/RemoveParentCallWithoutParentRector.php +++ b/rules/DeadCode/Rector/StaticCall/RemoveParentCallWithoutParentRector.php @@ -9,6 +9,7 @@ use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Name; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; +use Rector\Core\NodeAnalyzer\ClassAnalyzer; use Rector\Core\NodeManipulator\ClassMethodManipulator; use Rector\Core\Rector\AbstractRector; use Rector\NodeCollector\ScopeResolver\ParentClassScopeResolver; @@ -31,12 +32,19 @@ final class RemoveParentCallWithoutParentRector extends AbstractRector */ private $parentClassScopeResolver; + /** + * @var ClassAnalyzer + */ + private $classAnalyzer; + public function __construct( ClassMethodManipulator $classMethodManipulator, - ParentClassScopeResolver $parentClassScopeResolver + ParentClassScopeResolver $parentClassScopeResolver, + ClassAnalyzer $classAnalyzer ) { $this->classMethodManipulator = $classMethodManipulator; $this->parentClassScopeResolver = $parentClassScopeResolver; + $this->classAnalyzer = $classAnalyzer; } public function getRuleDefinition(): RuleDefinition diff --git a/rules/DowngradePhp70/Rector/New_/DowngradeAnonymousClassRector.php b/rules/DowngradePhp70/Rector/New_/DowngradeAnonymousClassRector.php index 1d841a25cd4..ac404ab3b73 100644 --- a/rules/DowngradePhp70/Rector/New_/DowngradeAnonymousClassRector.php +++ b/rules/DowngradePhp70/Rector/New_/DowngradeAnonymousClassRector.php @@ -13,6 +13,7 @@ use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\Function_; use PhpParser\Node\Stmt\Namespace_; +use Rector\Core\NodeAnalyzer\ClassAnalyzer; use Rector\Core\Rector\AbstractRector; use Rector\NodeTypeResolver\Node\AttributeKey; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; @@ -28,6 +29,16 @@ final class DowngradeAnonymousClassRector extends AbstractRector */ private const CLASS_NAME = 'AnonymousFor_'; + /** + * @var ClassAnalyzer + */ + private $classAnalyzer; + + public function __construct(ClassAnalyzer $classAnalyzer) + { + $this->classAnalyzer = $classAnalyzer; + } + /** * @return array> */ diff --git a/rules/Privatization/Rector/Class_/ChangeLocalPropertyToVariableRector.php b/rules/Privatization/Rector/Class_/ChangeLocalPropertyToVariableRector.php index e94070c953e..7d42dae2d6b 100644 --- a/rules/Privatization/Rector/Class_/ChangeLocalPropertyToVariableRector.php +++ b/rules/Privatization/Rector/Class_/ChangeLocalPropertyToVariableRector.php @@ -6,6 +6,7 @@ namespace Rector\Privatization\Rector\Class_; use PhpParser\Node; use PhpParser\Node\Stmt\Class_; +use Rector\Core\NodeAnalyzer\ClassAnalyzer; use Rector\Core\NodeManipulator\ClassManipulator; use Rector\Core\Rector\AbstractRector; use Rector\Privatization\NodeAnalyzer\PropertyFetchByMethodAnalyzer; @@ -33,14 +34,21 @@ final class ChangeLocalPropertyToVariableRector extends AbstractRector */ private $propertyFetchByMethodAnalyzer; + /** + * @var ClassAnalyzer + */ + private $classAnalyzer; + public function __construct( ClassManipulator $classManipulator, PropertyFetchWithVariableReplacer $propertyFetchWithVariableReplacer, - PropertyFetchByMethodAnalyzer $propertyFetchByMethodAnalyzer + PropertyFetchByMethodAnalyzer $propertyFetchByMethodAnalyzer, + ClassAnalyzer $classAnalyzer ) { $this->classManipulator = $classManipulator; $this->propertyFetchWithVariableReplacer = $propertyFetchWithVariableReplacer; $this->propertyFetchByMethodAnalyzer = $propertyFetchByMethodAnalyzer; + $this->classAnalyzer = $classAnalyzer; } public function getRuleDefinition(): RuleDefinition diff --git a/rules/Privatization/Rector/Class_/FinalizeClassesWithoutChildrenRector.php b/rules/Privatization/Rector/Class_/FinalizeClassesWithoutChildrenRector.php index ec02d32ca87..9e81926edf5 100644 --- a/rules/Privatization/Rector/Class_/FinalizeClassesWithoutChildrenRector.php +++ b/rules/Privatization/Rector/Class_/FinalizeClassesWithoutChildrenRector.php @@ -6,6 +6,7 @@ namespace Rector\Privatization\Rector\Class_; use PhpParser\Node; use PhpParser\Node\Stmt\Class_; +use Rector\Core\NodeAnalyzer\ClassAnalyzer; use Rector\Core\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -15,6 +16,16 @@ use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; */ final class FinalizeClassesWithoutChildrenRector extends AbstractRector { + /** + * @var ClassAnalyzer + */ + private $classAnalyzer; + + public function __construct(ClassAnalyzer $classAnalyzer) + { + $this->classAnalyzer = $classAnalyzer; + } + public function getRuleDefinition(): RuleDefinition { return new RuleDefinition('Finalize every class that has no children', [ diff --git a/rules/Privatization/Rector/Property/PrivatizeLocalPropertyToPrivatePropertyRector.php b/rules/Privatization/Rector/Property/PrivatizeLocalPropertyToPrivatePropertyRector.php index acd89b1ecc1..c4aa8847dd7 100644 --- a/rules/Privatization/Rector/Property/PrivatizeLocalPropertyToPrivatePropertyRector.php +++ b/rules/Privatization/Rector/Property/PrivatizeLocalPropertyToPrivatePropertyRector.php @@ -9,6 +9,7 @@ use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\Property; use PHPStan\Type\ObjectType; +use Rector\Core\NodeAnalyzer\ClassAnalyzer; use Rector\Core\Rector\AbstractRector; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\VendorLocker\NodeVendorLocker\PropertyVisibilityVendorLockResolver; @@ -41,14 +42,22 @@ final class PrivatizeLocalPropertyToPrivatePropertyRector extends AbstractRector */ private $excludedObjectTypes = []; - public function __construct(PropertyVisibilityVendorLockResolver $propertyVisibilityVendorLockResolver) - { + /** + * @var ClassAnalyzer + */ + private $classAnalyzer; + + public function __construct( + PropertyVisibilityVendorLockResolver $propertyVisibilityVendorLockResolver, + ClassAnalyzer $classAnalyzer + ) { $this->propertyVisibilityVendorLockResolver = $propertyVisibilityVendorLockResolver; $this->excludedObjectTypes = [ new ObjectType('PHPUnit\Framework\TestCase'), new ObjectType('PHP_CodeSniffer\Sniffs\Sniff'), ]; + $this->classAnalyzer = $classAnalyzer; } public function getRuleDefinition(): RuleDefinition diff --git a/rules/Transform/Rector/Class_/ChangeSingletonToServiceRector.php b/rules/Transform/Rector/Class_/ChangeSingletonToServiceRector.php index e1ca8ea68d0..c9ea59615a1 100644 --- a/rules/Transform/Rector/Class_/ChangeSingletonToServiceRector.php +++ b/rules/Transform/Rector/Class_/ChangeSingletonToServiceRector.php @@ -7,6 +7,7 @@ namespace Rector\Transform\Rector\Class_; use PhpParser\Node; use PhpParser\Node\Expr\StaticPropertyFetch; use PhpParser\Node\Stmt\Class_; +use Rector\Core\NodeAnalyzer\ClassAnalyzer; use Rector\Core\Rector\AbstractRector; use Rector\Core\ValueObject\MethodName; use Rector\Transform\NodeAnalyzer\SingletonClassMethodAnalyzer; @@ -27,9 +28,17 @@ final class ChangeSingletonToServiceRector extends AbstractRector */ private $singletonClassMethodAnalyzer; - public function __construct(SingletonClassMethodAnalyzer $singletonClassMethodAnalyzer) - { + /** + * @var ClassAnalyzer + */ + private $classAnalyzer; + + public function __construct( + SingletonClassMethodAnalyzer $singletonClassMethodAnalyzer, + ClassAnalyzer $classAnalyzer + ) { $this->singletonClassMethodAnalyzer = $singletonClassMethodAnalyzer; + $this->classAnalyzer = $classAnalyzer; } public function getRuleDefinition(): RuleDefinition diff --git a/rules/Transform/Rector/Class_/ParentClassToTraitsRector.php b/rules/Transform/Rector/Class_/ParentClassToTraitsRector.php index 49eadd325f7..9a1198f6ee7 100644 --- a/rules/Transform/Rector/Class_/ParentClassToTraitsRector.php +++ b/rules/Transform/Rector/Class_/ParentClassToTraitsRector.php @@ -7,6 +7,7 @@ namespace Rector\Transform\Rector\Class_; use PhpParser\Node; use PhpParser\Node\Stmt\Class_; use Rector\Core\Contract\Rector\ConfigurableRectorInterface; +use Rector\Core\NodeAnalyzer\ClassAnalyzer; use Rector\Core\NodeManipulator\ClassInsertManipulator; use Rector\Core\Rector\AbstractRector; use Rector\Transform\ValueObject\ParentClassToTraits; @@ -38,9 +39,15 @@ final class ParentClassToTraitsRector extends AbstractRector implements Configur */ private $classInsertManipulator; - public function __construct(ClassInsertManipulator $classInsertManipulator) + /** + * @var ClassAnalyzer + */ + private $classAnalyzer; + + public function __construct(ClassInsertManipulator $classInsertManipulator, ClassAnalyzer $classAnalyzer) { $this->classInsertManipulator = $classInsertManipulator; + $this->classAnalyzer = $classAnalyzer; } public function getRuleDefinition(): RuleDefinition diff --git a/rules/Transform/Rector/MethodCall/ServiceGetterToConstructorInjectionRector.php b/rules/Transform/Rector/MethodCall/ServiceGetterToConstructorInjectionRector.php index b45b0b22164..5ae064b9446 100644 --- a/rules/Transform/Rector/MethodCall/ServiceGetterToConstructorInjectionRector.php +++ b/rules/Transform/Rector/MethodCall/ServiceGetterToConstructorInjectionRector.php @@ -13,6 +13,7 @@ use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassLike; use PHPStan\Type\ObjectType; use Rector\Core\Contract\Rector\ConfigurableRectorInterface; +use Rector\Core\NodeAnalyzer\ClassAnalyzer; use Rector\Core\Rector\AbstractRector; use Rector\Naming\Naming\PropertyNaming; use Rector\NodeTypeResolver\Node\AttributeKey; @@ -41,9 +42,15 @@ final class ServiceGetterToConstructorInjectionRector extends AbstractRector imp */ private $propertyNaming; - public function __construct(PropertyNaming $propertyNaming) + /** + * @var ClassAnalyzer + */ + private $classAnalyzer; + + public function __construct(PropertyNaming $propertyNaming, ClassAnalyzer $classAnalyzer) { $this->propertyNaming = $propertyNaming; + $this->classAnalyzer = $classAnalyzer; } public function getRuleDefinition(): RuleDefinition diff --git a/src/NodeAnalyzer/ChangedNodeAnalyzer.php b/src/NodeAnalyzer/ChangedNodeAnalyzer.php new file mode 100644 index 00000000000..795749a3b7e --- /dev/null +++ b/src/NodeAnalyzer/ChangedNodeAnalyzer.php @@ -0,0 +1,54 @@ +nodeComparator = $nodeComparator; + } + + public function hasNodeChanged(Node $originalNode, Node $node): bool + { + if ($this->isNameIdentical($node, $originalNode)) { + return false; + } + + // @see https://github.com/rectorphp/rector/issues/6169 - special check, as php-parser skips brackets + if ($node instanceof Encapsed) { + foreach ($node->parts as $encapsedPart) { + $originalEncapsedPart = $encapsedPart->getAttribute(AttributeKey::ORIGINAL_NODE); + if ($originalEncapsedPart === null) { + return true; + } + } + } + + return ! $this->nodeComparator->areNodesEqual($originalNode, $node); + } + + private function isNameIdentical(Node $node, Node $originalNode): bool + { + if (! $originalNode instanceof Name) { + return false; + } + + // names are the same + $originalName = $originalNode->getAttribute(AttributeKey::ORIGINAL_NAME); + return $this->nodeComparator->areNodesEqual($originalName, $node); + } +} diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 7cac4db6e38..60e111a7499 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -28,7 +28,7 @@ use Rector\Core\Contract\Rector\PhpRectorInterface; use Rector\Core\Exception\ShouldNotHappenException; use Rector\Core\Exclusion\ExclusionManager; use Rector\Core\Logging\CurrentRectorProvider; -use Rector\Core\NodeAnalyzer\ClassAnalyzer; +use Rector\Core\NodeAnalyzer\ChangedNodeAnalyzer; use Rector\Core\Php\PhpVersionProvider; use Rector\Core\PhpParser\Comparing\NodeComparator; use Rector\Core\PhpParser\Node\BetterNodeFinder; @@ -62,7 +62,6 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn * @var string[] */ private const ATTRIBUTES_TO_MIRROR = [ - AttributeKey::PARENT_NODE, AttributeKey::CLASS_NODE, AttributeKey::CLASS_NAME, AttributeKey::METHOD_NODE, @@ -136,11 +135,6 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn */ protected $betterNodeFinder; - /** - * @var ClassAnalyzer - */ - protected $classAnalyzer; - /** * @var NodeRemover */ @@ -216,6 +210,11 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn */ private $currentFileProvider; + /** + * @var ChangedNodeAnalyzer + */ + private $changedNodeAnalyzer; + /** * @required */ @@ -239,14 +238,14 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn StaticTypeMapper $staticTypeMapper, ParameterProvider $parameterProvider, CurrentRectorProvider $currentRectorProvider, - ClassAnalyzer $classAnalyzer, CurrentNodeProvider $currentNodeProvider, Skipper $skipper, ValueResolver $valueResolver, NodeRepository $nodeRepository, BetterNodeFinder $betterNodeFinder, NodeComparator $nodeComparator, - CurrentFileProvider $currentFileProvider + CurrentFileProvider $currentFileProvider, + ChangedNodeAnalyzer $changedNodeAnalyzer ): void { $this->nodesToRemoveCollector = $nodesToRemoveCollector; $this->nodesToAddCollector = $nodesToAddCollector; @@ -267,7 +266,6 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn $this->staticTypeMapper = $staticTypeMapper; $this->parameterProvider = $parameterProvider; $this->currentRectorProvider = $currentRectorProvider; - $this->classAnalyzer = $classAnalyzer; $this->currentNodeProvider = $currentNodeProvider; $this->skipper = $skipper; $this->valueResolver = $valueResolver; @@ -275,6 +273,7 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn $this->betterNodeFinder = $betterNodeFinder; $this->nodeComparator = $nodeComparator; $this->currentFileProvider = $currentFileProvider; + $this->changedNodeAnalyzer = $changedNodeAnalyzer; } /** @@ -327,9 +326,7 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn } // changed! - if ($this->hasNodeChanged($originalNode, $node)) { - $this->updateAttributes($node); - + if ($this->changedNodeAnalyzer->hasNodeChanged($originalNode, $node)) { $rectorWithLineChange = new RectorWithLineChange($this, $node->getLine()); $this->file->addRectorClassWithLine($rectorWithLineChange); @@ -438,17 +435,17 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn } /** - * @param Arg[] $newArgs + * @param Arg[] $currentArgs * @param Arg[] $appendingArgs * @return Arg[] */ - protected function appendArgs(array $newArgs, array $appendingArgs): array + protected function appendArgs(array $currentArgs, array $appendingArgs): array { foreach ($appendingArgs as $appendingArg) { - $newArgs[] = new Arg($appendingArg->value); + $currentArgs[] = new Arg($appendingArg->value); } - return $newArgs; + return $currentArgs; } protected function unwrapExpression(Stmt $stmt): Node @@ -560,20 +557,15 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn $this->previousAppliedClass = static::class; } - private function hasNodeChanged(Node $originalNode, Node $node): bool - { - if ($this->isNameIdentical($node, $originalNode)) { - return false; - } - - return ! $this->nodeComparator->areNodesEqual($originalNode, $node); - } - /** * @param array $originalAttributes */ private function mirrorAttributes(array $originalAttributes, Node $newNode): void { + if ($newNode instanceof Name) { + $newNode->setAttribute(AttributeKey::RESOLVED_NAME, $newNode->toString()); + } + foreach ($originalAttributes as $attributeName => $oldAttributeValue) { if (! in_array($attributeName, self::ATTRIBUTES_TO_MIRROR, true)) { continue; @@ -583,24 +575,6 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn } } - private function updateAttributes(Node $node): void - { - // update Resolved name attribute if name is changed - if ($node instanceof Name) { - $node->setAttribute(AttributeKey::RESOLVED_NAME, $node->toString()); - } - } - - private function isNameIdentical(Node $node, Node $originalNode): bool - { - if (! $originalNode instanceof Name) { - return false; - } - - // names are the same - return $this->nodeComparator->areNodesEqual($originalNode->getAttribute(AttributeKey::ORIGINAL_NAME), $node); - } - private function connectParentNodes(Node $node): void { $nodeTraverser = new NodeTraverser();