diff --git a/config/set/dead-code/dead-code.yaml b/config/set/dead-code/dead-code.yaml index fd286e754bc..0775d03300c 100644 --- a/config/set/dead-code/dead-code.yaml +++ b/config/set/dead-code/dead-code.yaml @@ -25,3 +25,4 @@ services: Rector\DeadCode\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector: ~ Rector\DeadCode\Rector\Switch_\RemoveDuplicatedCaseInSwitchRector: ~ Rector\DeadCode\Rector\Class_\RemoveUnusedDoctrineEntityMethodAndPropertyRector: ~ + Rector\DeadCode\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector: ~ diff --git a/packages/Architecture/src/Cleaner/ClassMethodCleaner.php b/packages/Architecture/src/Cleaner/ClassMethodCleaner.php deleted file mode 100644 index 3f72a01d0c4..00000000000 --- a/packages/Architecture/src/Cleaner/ClassMethodCleaner.php +++ /dev/null @@ -1,7 +0,0 @@ -<?php declare(strict_types=1); - -namespace Rector\Architecture\Cleaner; - -final class ClassMethodCleaner -{ -} diff --git a/packages/BetterPhpDocParser/src/Attributes/Attribute/Attribute.php b/packages/BetterPhpDocParser/src/Attributes/Attribute/Attribute.php index 3115a4228fc..ec5fd9b827c 100644 --- a/packages/BetterPhpDocParser/src/Attributes/Attribute/Attribute.php +++ b/packages/BetterPhpDocParser/src/Attributes/Attribute/Attribute.php @@ -9,13 +9,6 @@ final class Attribute */ public const HAS_DESCRIPTION_WITH_ORIGINAL_SPACES = 'has_description_with_restored_spaces'; - /** - * Fully-qualified name - * - * @var string - */ - public const FQN_NAME = 'fqn_name'; - /** * @var string */ diff --git a/packages/CodeQuality/src/Rector/Foreach_/ForeachToInArrayRector.php b/packages/CodeQuality/src/Rector/Foreach_/ForeachToInArrayRector.php index e38e8282a62..34d211636f8 100644 --- a/packages/CodeQuality/src/Rector/Foreach_/ForeachToInArrayRector.php +++ b/packages/CodeQuality/src/Rector/Foreach_/ForeachToInArrayRector.php @@ -219,7 +219,7 @@ CODE_SAMPLE */ private function combineCommentsToNode(Node $originalNode, Node $newNode): void { - $this->traverseNodesWithCallable([$originalNode], function (Node $node): void { + $this->traverseNodesWithCallable($originalNode, function (Node $node): void { if ($node->hasAttribute('comments')) { $this->comments = array_merge($this->comments, $node->getComments()); } diff --git a/packages/CodingStyle/src/Node/ConcatManipulator.php b/packages/CodingStyle/src/Node/ConcatManipulator.php index bb461d4dc3b..f9e9aeca5c9 100644 --- a/packages/CodingStyle/src/Node/ConcatManipulator.php +++ b/packages/CodingStyle/src/Node/ConcatManipulator.php @@ -48,7 +48,7 @@ final class ConcatManipulator $newConcat = clone $concat; $firstConcatItem = $this->getFirstConcatItem($concat); - $this->callableNodeTraverser->traverseNodesWithCallable([$newConcat], function (Node $node) use ( + $this->callableNodeTraverser->traverseNodesWithCallable($newConcat, function (Node $node) use ( $firstConcatItem ): ?Expr { if (! $node instanceof Concat) { diff --git a/packages/CodingStyle/src/Rector/String_/ManualJsonStringToJsonEncodeArrayRector.php b/packages/CodingStyle/src/Rector/String_/ManualJsonStringToJsonEncodeArrayRector.php index a9e1366a250..48342cfdf8f 100644 --- a/packages/CodingStyle/src/Rector/String_/ManualJsonStringToJsonEncodeArrayRector.php +++ b/packages/CodingStyle/src/Rector/String_/ManualJsonStringToJsonEncodeArrayRector.php @@ -22,6 +22,7 @@ use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\RectorDefinition; /** + * @sponsor Thanks https://spaceflow.io/ for sponsoring this rule - visit them on https://github.com/SpaceFlow-app * @see \Rector\CodingStyle\Tests\Rector\String_\ManualJsonStringToJsonEncodeArrayRector\ManualJsonStringToJsonEncodeArrayRectorTest */ final class ManualJsonStringToJsonEncodeArrayRector extends AbstractRector @@ -190,7 +191,7 @@ CODE_SAMPLE private function replaceNodeObjectHashPlaceholdersWithNodes(Array_ $array, array $placeholderNodes): void { // traverse and replace placeholdes by original nodes - $this->traverseNodesWithCallable([$array], function (Node $node) use ($placeholderNodes): ?Expr { + $this->traverseNodesWithCallable($array, function (Node $node) use ($placeholderNodes): ?Expr { if (! $node instanceof String_) { return null; } diff --git a/packages/CodingStyle/src/Rector/Use_/RemoveUnusedAliasRector.php b/packages/CodingStyle/src/Rector/Use_/RemoveUnusedAliasRector.php index 04cb58404b6..72e1563ea11 100644 --- a/packages/CodingStyle/src/Rector/Use_/RemoveUnusedAliasRector.php +++ b/packages/CodingStyle/src/Rector/Use_/RemoveUnusedAliasRector.php @@ -298,7 +298,7 @@ CODE_SAMPLE private function resolveDocPossibleAliases(Node $searchNode): void { - $this->traverseNodesWithCallable([$searchNode], function (Node $node): void { + $this->traverseNodesWithCallable($searchNode, function (Node $node): void { if ($node->getDocComment() === null) { return; } diff --git a/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php b/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php new file mode 100644 index 00000000000..e2f10f99865 --- /dev/null +++ b/packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php @@ -0,0 +1,211 @@ +<?php declare(strict_types=1); + +namespace Rector\DeadCode\Rector\Class_; + +use PhpParser\Node; +use PhpParser\Node\Expr\Assign; +use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Stmt\Class_; +use PhpParser\Node\Stmt\ClassMethod; +use PhpParser\Node\Stmt\Expression; +use Rector\NodeContainer\ParsedNodesByType; +use Rector\PhpParser\Node\Manipulator\ClassManipulator; +use Rector\Rector\AbstractRector; +use Rector\RectorDefinition\CodeSample; +use Rector\RectorDefinition\RectorDefinition; + +/** + * @sponsor Thanks https://spaceflow.io/ for sponsoring this rule - visit them on https://github.com/SpaceFlow-app + * @see \Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\RemoveSetterOnlyPropertyAndMethodCallRectorTest + */ +final class RemoveSetterOnlyPropertyAndMethodCallRector extends AbstractRector +{ + /** + * @var ClassManipulator + */ + private $classManipulator; + + /** + * @var ParsedNodesByType + */ + private $parsedNodesByType; + + /** + * @var string[] + */ + private $methodCallNamesToBeRemoved = []; + + public function __construct(ClassManipulator $classManipulator, ParsedNodesByType $parsedNodesByType) + { + $this->classManipulator = $classManipulator; + $this->parsedNodesByType = $parsedNodesByType; + } + + public function getDefinition(): RectorDefinition + { + return new RectorDefinition('Removes method that set values that are never used', [ + new CodeSample( + <<<'CODE_SAMPLE' +class SomeClass +{ + private $name; + + public function setName($name) + { + $this->name = $name; + } +} + +class ActiveOnlySetter +{ + public function run() + { + $someClass = new SomeClass(); + $someClass->setName('Tom'); + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +class SomeClass +{ +} + +class ActiveOnlySetter +{ + public function run() + { + $someClass = new SomeClass(); + } +} +CODE_SAMPLE + ), + ]); + } + + /** + * @return string[] + */ + public function getNodeTypes(): array + { + return [Class_::class]; + } + + /** + * @param Class_ $node + */ + public function refactor(Node $node): ?Node + { + $this->methodCallNamesToBeRemoved = []; + + // 1. get assign only private properties + $assignOnlyPrivatePropertyNames = $this->classManipulator->getAssignOnlyPrivatePropertyNames($node); + $this->classManipulator->removeProperties($node, $assignOnlyPrivatePropertyNames); + + // 2. remove assigns + class methods with only setter assign + $this->removePropertyAssigns($node, $assignOnlyPrivatePropertyNames); + + // 3. remove setter method calls + $this->removeSetterMethodCalls($node); + + return $node; + } + + /** + * @param string[] $assignOnlyPrivatePropertyNames + */ + private function removePropertyAssigns(Class_ $class, array $assignOnlyPrivatePropertyNames): void + { + $this->traverseNodesWithCallable($class, function (Node $node) use ($assignOnlyPrivatePropertyNames): void { + if ($this->isClassMethodWithSinglePropertyAssignOfNames($node, $assignOnlyPrivatePropertyNames)) { + /** @var string $classMethodName */ + $classMethodName = $this->getName($node); + $this->methodCallNamesToBeRemoved[] = $classMethodName; + + $this->removeNode($node); + return; + } + + if ($this->isPropertyAssignWithPropertyNames($node, $assignOnlyPrivatePropertyNames)) { + $this->removeNode($node); + } + }); + } + + private function removeSetterMethodCalls(Node $node): void + { + /** @var string $className */ + $className = $this->getName($node); + $methodCallsByMethodName = $this->parsedNodesByType->findMethodCallsOnClass($className); + + /** @var string $methodName */ + foreach ($methodCallsByMethodName as $methodName => $classMethodCalls) { + if (! in_array($methodName, $this->methodCallNamesToBeRemoved, true)) { + continue; + } + + foreach ($classMethodCalls as $classMethodCall) { + $this->removeNode($classMethodCall); + } + } + } + + /** + * Looks for: + * + * public function <someMethod>($value) + * { + * $this->value = $value + * } + * + * @param string[] $propertyNames + */ + private function isClassMethodWithSinglePropertyAssignOfNames(Node $node, array $propertyNames): bool + { + if (! $node instanceof ClassMethod) { + return false; + } + + if ($this->isName($node, '__construct')) { + return false; + } + + if (count((array) $node->stmts) !== 1) { + return false; + } + + if (! $node->stmts[0] instanceof Expression) { + return false; + } + + /** @var Expression $onlyExpression */ + $onlyExpression = $node->stmts[0]; + + $onlyStmt = $onlyExpression->expr; + + return $this->isPropertyAssignWithPropertyNames($onlyStmt, $propertyNames); + } + + /** + * Is: "$this->value = <$value>" + * + * @param string[] $propertyNames + */ + private function isPropertyAssignWithPropertyNames(Node $node, array $propertyNames): bool + { + if (! $node instanceof Assign) { + return false; + } + + if (! $node->var instanceof PropertyFetch) { + return false; + } + + $propertyFetch = $node->var; + if (! $this->isName($propertyFetch->var, 'this')) { + return false; + } + + return $this->isNames($propertyFetch->name, $propertyNames); + } +} diff --git a/packages/DeadCode/src/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector.php b/packages/DeadCode/src/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector.php index 3b1087c11c4..49c2f1fcbf9 100644 --- a/packages/DeadCode/src/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector.php +++ b/packages/DeadCode/src/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector.php @@ -19,6 +19,7 @@ use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\RectorDefinition; /** + * @sponsor Thanks https://spaceflow.io/ for sponsoring this rule - visit them on https://github.com/SpaceFlow-app * @see \Rector\DeadCode\Tests\Rector\Class_\RemoveUnusedDoctrineEntityMethodAndPropertyRector\RemoveUnusedDoctrineEntityMethodAndPropertyRectorTest */ final class RemoveUnusedDoctrineEntityMethodAndPropertyRector extends AbstractRector diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/fixture.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/fixture.php.inc new file mode 100644 index 00000000000..fd65f6a1520 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/fixture.php.inc @@ -0,0 +1,42 @@ +<?php + +namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\Fixture; + +class SomeClass +{ + private $name; + + public function setName($name) + { + $this->name = $name; + } +} + +class ActiveOnlySetter +{ + public function run() + { + $someClass = new SomeClass(); + $someClass->setName('Tom'); + } +} + +?> +----- +<?php + +namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\Fixture; + +class SomeClass +{ +} + +class ActiveOnlySetter +{ + public function run() + { + $someClass = new SomeClass(); + } +} + +?> diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/in_constructor.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/in_constructor.php.inc new file mode 100644 index 00000000000..a53ebdbbf0c --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/in_constructor.php.inc @@ -0,0 +1,28 @@ +<?php + +namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\Fixture; + +class InConstructor +{ + private $name; + + public function __construct($name) + { + $this->name = $name; + } +} + +?> +----- +<?php + +namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\Fixture; + +class InConstructor +{ + public function __construct($name) + { + } +} + +?> diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/keep_public_property.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/keep_public_property.php.inc new file mode 100644 index 00000000000..4b5c1f0593a --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/keep_public_property.php.inc @@ -0,0 +1,8 @@ +<?php + +namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\Fixture; + +class KeepPublicProperty +{ + public $application; +} diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/keep_serializable_object.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/keep_serializable_object.php.inc new file mode 100644 index 00000000000..9e5c1b084a4 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/keep_serializable_object.php.inc @@ -0,0 +1,19 @@ +<?php + +namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\Fixture; + +use JMS\Serializer\Annotation as Serializer; + +final class KeepSerialiazableObject +{ + /** + * @var string + * @Serializer\Type("string") + */ + private $id; + + public function __construct(string $id) + { + $this->id = $id; + } +} diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/keep_static_property.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/keep_static_property.php.inc new file mode 100644 index 00000000000..4e7dad4ede2 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/keep_static_property.php.inc @@ -0,0 +1,17 @@ +<?php + +namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector\Fixture; + +class KeepStaticProperty +{ + private static $application; + + private static function getApplication() + { + if (self::$application === null) { + self::$application = new Application(); + } + + return self::$application; + } +} diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/RemoveSetterOnlyPropertyAndMethodCallRectorTest.php b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/RemoveSetterOnlyPropertyAndMethodCallRectorTest.php new file mode 100644 index 00000000000..5227c5e45b4 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/RemoveSetterOnlyPropertyAndMethodCallRectorTest.php @@ -0,0 +1,25 @@ +<?php declare(strict_types=1); + +namespace Rector\DeadCode\Tests\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector; + +use Rector\DeadCode\Rector\Class_\RemoveSetterOnlyPropertyAndMethodCallRector; +use Rector\Testing\PHPUnit\AbstractRectorTestCase; + +final class RemoveSetterOnlyPropertyAndMethodCallRectorTest extends AbstractRectorTestCase +{ + public function test(): void + { + $this->doTestFiles([ + __DIR__ . '/Fixture/fixture.php.inc', + __DIR__ . '/Fixture/in_constructor.php.inc', + __DIR__ . '/Fixture/keep_static_property.php.inc', + __DIR__ . '/Fixture/keep_public_property.php.inc', + __DIR__ . '/Fixture/keep_serializable_object.php.inc', + ]); + } + + protected function getRectorClass(): string + { + return RemoveSetterOnlyPropertyAndMethodCallRector::class; + } +} diff --git a/packages/Php/src/Rector/FuncCall/ParseStrWithResultArgumentRector.php b/packages/Php/src/Rector/FuncCall/ParseStrWithResultArgumentRector.php index 2dae73d82ea..949502e9ff0 100644 --- a/packages/Php/src/Rector/FuncCall/ParseStrWithResultArgumentRector.php +++ b/packages/Php/src/Rector/FuncCall/ParseStrWithResultArgumentRector.php @@ -70,9 +70,7 @@ CODE_SAMPLE return null; } - $this->traverseNodesWithCallable([$nextExpression], function (Node $node) use ( - $resultVariable - ): ?Variable { + $this->traverseNodesWithCallable($nextExpression, function (Node $node) use ($resultVariable): ?Variable { if ($node instanceof FuncCall) { if ($this->isName($node, 'get_defined_vars')) { return $resultVariable; diff --git a/packages/Php/src/Rector/FuncCall/PregReplaceEModifierRector.php b/packages/Php/src/Rector/FuncCall/PregReplaceEModifierRector.php index 1e98158bd66..86d0ca13e3f 100644 --- a/packages/Php/src/Rector/FuncCall/PregReplaceEModifierRector.php +++ b/packages/Php/src/Rector/FuncCall/PregReplaceEModifierRector.php @@ -123,7 +123,7 @@ CODE_SAMPLE $stmt = $contentNodes[0]->expr; - $this->traverseNodesWithCallable([$stmt], function (Node $node): Node { + $this->traverseNodesWithCallable($stmt, function (Node $node): Node { if (! $node instanceof String_) { return $node; } diff --git a/packages/PhpSpecToPHPUnit/src/PhpSpecMockCollector.php b/packages/PhpSpecToPHPUnit/src/PhpSpecMockCollector.php index f344f6726c8..a6b39bdffc4 100644 --- a/packages/PhpSpecToPHPUnit/src/PhpSpecMockCollector.php +++ b/packages/PhpSpecToPHPUnit/src/PhpSpecMockCollector.php @@ -56,7 +56,7 @@ final class PhpSpecMockCollector return $this->mocks[$className]; } - $this->callableNodeTraverser->traverseNodesWithCallable((array) $class->stmts, function (Node $node): void { + $this->callableNodeTraverser->traverseNodesWithCallable($class, function (Node $node): void { if (! $node instanceof ClassMethod) { return; } diff --git a/packages/Twig/src/Rector/SimpleFunctionAndFilterRector.php b/packages/Twig/src/Rector/SimpleFunctionAndFilterRector.php index 9c6d0c35552..6c09bd9d721 100644 --- a/packages/Twig/src/Rector/SimpleFunctionAndFilterRector.php +++ b/packages/Twig/src/Rector/SimpleFunctionAndFilterRector.php @@ -124,7 +124,7 @@ CODE_SAMPLE return null; } - $this->traverseNodesWithCallable([$node->expr], function (Node $node): ?Node { + $this->traverseNodesWithCallable($node->expr, function (Node $node): ?Node { if (! $node instanceof ArrayItem) { return null; } diff --git a/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php b/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php index a6cc8e1190b..4db17c7357a 100644 --- a/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php +++ b/packages/TypeDeclaration/src/Rector/Property/PropertyTypeDeclarationRector.php @@ -13,6 +13,9 @@ use Rector\TypeDeclaration\Contract\PropertyTypeInfererInterface; use Rector\TypeDeclaration\Exception\ConflictingPriorityException; use Rector\TypeDeclaration\ValueObject\IdentifierValueObject; +/** + * @sponsor Thanks https://spaceflow.io/ for sponsoring this rule - visit them on https://github.com/SpaceFlow-app + */ final class PropertyTypeDeclarationRector extends AbstractRector { /** diff --git a/phpstan.neon b/phpstan.neon index f184fed4e42..067b9f00e26 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -186,3 +186,6 @@ parameters: # symfony future compatibility - '#Call to an undefined static method Symfony\\Component\\EventDispatcher\\EventDispatcher\:\:__construct\(\)#' - '#Rector\\EventDispatcher\\AutowiredEventDispatcher\:\:__construct\(\) calls parent constructor but parent does not have one#' + - '#In method "Rector\\Rector\\AbstractRector\:\:traverseNodesWithCallable", parameter \$nodes has no type\-hint and no @param annotation\. More info\: http\://bit\.ly/usetypehint#' + - '#In method "Rector\\FileSystemRector\\Rector\\AbstractFileSystemRector\:\:traverseNodesWithCallable", parameter \$nodes has no type\-hint and no @param annotation\. More info\: http\://bit\.ly/usetypehint#' + - '#Ternary operator condition is always true#' diff --git a/src/Configuration/Configuration.php b/src/Configuration/Configuration.php index b9d7e1e28d6..8cb0571e160 100644 --- a/src/Configuration/Configuration.php +++ b/src/Configuration/Configuration.php @@ -22,17 +22,6 @@ final class Configuration */ private $hideAutoloadErrors = false; - /** - * Files and directories to by analysed - * @var string[] - */ - private $source = []; - - /** - * @var string - */ - private $outputFormat; - /** * @var string|null */ @@ -54,9 +43,7 @@ final class Configuration public function resolveFromInput(InputInterface $input): void { $this->isDryRun = (bool) $input->getOption(Option::OPTION_DRY_RUN); - $this->source = (array) $input->getArgument(Option::SOURCE); $this->hideAutoloadErrors = (bool) $input->getOption(Option::HIDE_AUTOLOAD_ERRORS); - $this->outputFormat = (string) $input->getOption(Option::OPTION_OUTPUT_FORMAT); $this->showProgressBar = $this->canShowProgressBar($input); $this->setRule($input->getOption(Option::OPTION_RULE)); @@ -82,11 +69,6 @@ final class Configuration return $this->configFilePath; } - public function getOutputFormat(): string - { - return $this->outputFormat; - } - public function getPrettyVersion(): string { $version = PrettyVersions::getVersion('rector/rector'); @@ -107,14 +89,6 @@ final class Configuration return $this->isDryRun; } - /** - * @return string[] - */ - public function getSource(): array - { - return $this->source; - } - public function shouldHideAutoloadErrors(): bool { return $this->hideAutoloadErrors; diff --git a/src/Console/Command/AbstractCommand.php b/src/Console/Command/AbstractCommand.php index d785a53a3d1..185d80a499f 100644 --- a/src/Console/Command/AbstractCommand.php +++ b/src/Console/Command/AbstractCommand.php @@ -20,7 +20,7 @@ abstract class AbstractCommand extends Command /** * @required */ - public function setDescriptor(TextDescriptor $textDescriptor): void + public function autowireDescriptor(TextDescriptor $textDescriptor): void { $this->textDescriptor = $textDescriptor; } diff --git a/src/PhpParser/Node/BetterNodeFinder.php b/src/PhpParser/Node/BetterNodeFinder.php index 7d3ec163748..806da82064d 100644 --- a/src/PhpParser/Node/BetterNodeFinder.php +++ b/src/PhpParser/Node/BetterNodeFinder.php @@ -3,16 +3,12 @@ namespace Rector\PhpParser\Node; use PhpParser\Node; -use PhpParser\Node\Expr\ArrayDimFetch; -use PhpParser\Node\Expr\Assign; -use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\Expression; use PhpParser\NodeFinder; use Rector\NodeTypeResolver\Node\AttributeKey; -use Rector\PhpParser\Printer\BetterStandardPrinter; final class BetterNodeFinder { @@ -21,15 +17,9 @@ final class BetterNodeFinder */ private $nodeFinder; - /** - * @var BetterStandardPrinter - */ - private $betterStandardPrinter; - - public function __construct(NodeFinder $nodeFinder, BetterStandardPrinter $betterStandardPrinter) + public function __construct(NodeFinder $nodeFinder) { $this->nodeFinder = $nodeFinder; - $this->betterStandardPrinter = $betterStandardPrinter; } /** @@ -185,26 +175,6 @@ final class BetterNodeFinder return $this->findFirstPrevious($previousExpression, $filter); } - /** - * @return Assign[] - */ - public function findAssignsOfVariable(Node $node, Variable $variable): array - { - $assignNodes = $this->findInstanceOf($node, Assign::class); - - return array_filter($assignNodes, function (Assign $assign) use ($variable): bool { - if ($this->betterStandardPrinter->areNodesEqual($assign->var, $variable)) { - return true; - } - - if ($assign->var instanceof ArrayDimFetch) { - return $this->betterStandardPrinter->areNodesEqual($assign->var->var, $variable); - } - - return false; - }); - } - /** * @param string[] $types */ diff --git a/src/PhpParser/Node/Commander/NodeRemovingCommander.php b/src/PhpParser/Node/Commander/NodeRemovingCommander.php index a5ea02c69ce..ccf722cdb33 100644 --- a/src/PhpParser/Node/Commander/NodeRemovingCommander.php +++ b/src/PhpParser/Node/Commander/NodeRemovingCommander.php @@ -15,6 +15,7 @@ use Rector\Exception\ShouldNotHappenException; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\PhpParser\Node\NodeFactory; use Rector\PhpParser\Node\Resolver\NameResolver; +use Rector\Reporting\RemovedNodesCollector; final class NodeRemovingCommander implements CommanderInterface { @@ -33,14 +34,25 @@ final class NodeRemovingCommander implements CommanderInterface */ private $nameResolver; - public function __construct(NodeFactory $nodeFactory, NameResolver $nameResolver) - { + /** + * @var RemovedNodesCollector + */ + private $removedNodesCollector; + + public function __construct( + NodeFactory $nodeFactory, + NameResolver $nameResolver, + RemovedNodesCollector $removedNodesCollector + ) { $this->nodeFactory = $nodeFactory; $this->nameResolver = $nameResolver; + $this->removedNodesCollector = $removedNodesCollector; } public function addNode(Node $node): void { + $this->removedNodesCollector->collect($node); + // chain call: "->method()->another()" if ($node instanceof MethodCall && $node->var instanceof MethodCall) { throw new ShouldNotHappenException( diff --git a/src/PhpParser/Node/Manipulator/ClassManipulator.php b/src/PhpParser/Node/Manipulator/ClassManipulator.php index ef9587d56d7..48d99da8417 100644 --- a/src/PhpParser/Node/Manipulator/ClassManipulator.php +++ b/src/PhpParser/Node/Manipulator/ClassManipulator.php @@ -5,6 +5,7 @@ namespace Rector\PhpParser\Node\Manipulator; use PhpParser\Node; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Expr\StaticPropertyFetch; use PhpParser\Node\Name; use PhpParser\Node\Param; use PhpParser\Node\Stmt; @@ -17,10 +18,14 @@ use PhpParser\Node\Stmt\Property; use PhpParser\Node\Stmt\PropertyProperty; use PhpParser\Node\Stmt\Trait_; use PhpParser\Node\Stmt\TraitUse; +use Rector\NodeTypeResolver\Node\AttributeKey; +use Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer\DocBlockManipulator; use Rector\PhpParser\Node\BetterNodeFinder; +use Rector\PhpParser\Node\Commander\NodeRemovingCommander; use Rector\PhpParser\Node\NodeFactory; use Rector\PhpParser\Node\Resolver\NameResolver; use Rector\PhpParser\Node\VariableInfo; +use Rector\PhpParser\NodeTraverser\CallableNodeTraverser; final class ClassManipulator { @@ -44,16 +49,37 @@ final class ClassManipulator */ private $betterNodeFinder; + /** + * @var CallableNodeTraverser + */ + private $callableNodeTraverser; + + /** + * @var NodeRemovingCommander + */ + private $nodeRemovingCommander; + + /** + * @var DocBlockManipulator + */ + private $docBlockManipulator; + public function __construct( NameResolver $nameResolver, NodeFactory $nodeFactory, ChildAndParentClassManipulator $childAndParentClassManipulator, - BetterNodeFinder $betterNodeFinder + BetterNodeFinder $betterNodeFinder, + CallableNodeTraverser $callableNodeTraverser, + NodeRemovingCommander $nodeRemovingCommander, + DocBlockManipulator $docBlockManipulator ) { $this->nodeFactory = $nodeFactory; $this->nameResolver = $nameResolver; $this->childAndParentClassManipulator = $childAndParentClassManipulator; $this->betterNodeFinder = $betterNodeFinder; + $this->callableNodeTraverser = $callableNodeTraverser; + $this->nodeRemovingCommander = $nodeRemovingCommander; + $this->docBlockManipulator = $docBlockManipulator; } public function addConstructorDependency(Class_ $classNode, VariableInfo $variableInfo): void @@ -263,18 +289,7 @@ final class ClassManipulator public function removeProperty(Class_ $class, string $propertyName): void { - foreach ($class->stmts as $key => $classStmt) { - if (! $classStmt instanceof Property) { - continue; - } - - if (! $this->nameResolver->isName($classStmt, $propertyName)) { - continue; - } - - unset($class->stmts[$key]); - break; - } + $this->removeProperties($class, [$propertyName]); } public function findMethodParamByName(ClassMethod $classMethod, string $name): ?Param @@ -337,6 +352,53 @@ final class ClassManipulator return $publicMethodNames; } + /** + * @return string[] + */ + public function getAssignOnlyPrivatePropertyNames(Class_ $node): array + { + $privatePropertyNames = $this->getPrivatePropertyNames($node); + + $propertyNonAssignNames = []; + + $this->callableNodeTraverser->traverseNodesWithCallable([$node], function (Node $node) use ( + &$propertyNonAssignNames + ): void { + if (! $node instanceof PropertyFetch && ! $node instanceof StaticPropertyFetch) { + return; + } + + if (! $this->isNonAssignPropertyFetch($node)) { + return; + } + + $propertyNonAssignNames[] = $this->nameResolver->getName($node); + }); + + // skip serializable properties, because they are probably used in serialization even though assign only + $serializablePropertyNames = $this->getSerializablePropertyNames($node); + + return array_diff($privatePropertyNames, $propertyNonAssignNames, $serializablePropertyNames); + } + + /** + * @param string[] $propertyNames + */ + public function removeProperties(Class_ $class, array $propertyNames): void + { + $this->callableNodeTraverser->traverseNodesWithCallable($class, function (Node $node) use ($propertyNames) { + if (! $node instanceof Property) { + return null; + } + + if (! $this->nameResolver->isNames($node, $propertyNames)) { + return null; + } + + $this->removeNode($node); + }); + } + private function tryInsertBeforeFirstMethod(Class_ $classNode, Stmt $stmt): bool { foreach ($classNode->stmts as $key => $classElementNode) { @@ -437,4 +499,65 @@ final class ClassManipulator return false; } + + private function removeNode(Node $node): void + { + $this->nodeRemovingCommander->addNode($node); + } + + /** + * @param PropertyFetch|StaticPropertyFetch $node + */ + private function isNonAssignPropertyFetch(Node $node): bool + { + if ($node instanceof PropertyFetch) { + if (! $this->nameResolver->isName($node->var, 'this')) { + return false; + } + + // is "$this->property = x;" assign + return ! $this->isNodeLeftPartOfAssign($node); + } + + if ($node instanceof StaticPropertyFetch) { + if (! $this->nameResolver->isName($node->class, 'self')) { + return false; + } + + // is "self::$property = x;" assign + return ! $this->isNodeLeftPartOfAssign($node); + } + + return false; + } + + private function isNodeLeftPartOfAssign(Node $node): bool + { + $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE); + + return $parentNode instanceof Assign && $parentNode->var === $node; + } + + /** + * @return string[] + */ + private function getSerializablePropertyNames(Class_ $node): array + { + $serializablePropertyNames = []; + $this->callableNodeTraverser->traverseNodesWithCallable([$node], function (Node $node) use ( + &$serializablePropertyNames + ): void { + if (! $node instanceof Property) { + return; + } + + if (! $this->docBlockManipulator->hasTag($node, 'JMS\Serializer\Annotation\Type')) { + return; + } + + $serializablePropertyNames[] = $this->nameResolver->getName($node); + }); + + return $serializablePropertyNames; + } } diff --git a/src/PhpParser/NodeTraverser/CallableNodeTraverser.php b/src/PhpParser/NodeTraverser/CallableNodeTraverser.php index 34717cb5a25..15c1accfe11 100644 --- a/src/PhpParser/NodeTraverser/CallableNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/CallableNodeTraverser.php @@ -10,10 +10,14 @@ use PhpParser\NodeVisitorAbstract; final class CallableNodeTraverser { /** - * @param Node[] $nodes + * @param Node|Node[] $nodes */ - public function traverseNodesWithCallable(array $nodes, callable $callable): void + public function traverseNodesWithCallable($nodes, callable $callable): void { + if (! is_array($nodes)) { + $nodes = $nodes ? [$nodes] : []; + } + $nodeTraverser = new NodeTraverser(); $nodeTraverser->addVisitor($this->createNodeVisitor($callable)); $nodeTraverser->traverse($nodes); diff --git a/src/Rector/AbstractRector/CallableNodeTraverserTrait.php b/src/Rector/AbstractRector/CallableNodeTraverserTrait.php index bed113918f6..48e9aa7af8e 100644 --- a/src/Rector/AbstractRector/CallableNodeTraverserTrait.php +++ b/src/Rector/AbstractRector/CallableNodeTraverserTrait.php @@ -25,9 +25,9 @@ trait CallableNodeTraverserTrait } /** - * @param Node[] $nodes + * @param Node|Node[] $nodes */ - public function traverseNodesWithCallable(array $nodes, callable $callable): void + public function traverseNodesWithCallable($nodes, callable $callable): void { $this->callableNodeTraverser->traverseNodesWithCallable($nodes, $callable); } diff --git a/src/Rector/AbstractRector/NodeCommandersTrait.php b/src/Rector/AbstractRector/NodeCommandersTrait.php index d31e49f722f..62c30b75f17 100644 --- a/src/Rector/AbstractRector/NodeCommandersTrait.php +++ b/src/Rector/AbstractRector/NodeCommandersTrait.php @@ -10,7 +10,6 @@ use Rector\PhpParser\Node\Commander\NodeAddingCommander; use Rector\PhpParser\Node\Commander\NodeRemovingCommander; use Rector\PhpParser\Node\Commander\PropertyAddingCommander; use Rector\PhpParser\Node\VariableInfo; -use Rector\Reporting\RemovedNodesCollector; /** * This could be part of @see AbstractRector, but decopuling to trait @@ -40,11 +39,6 @@ trait NodeCommandersTrait */ private $useAddingCommander; - /** - * @var RemovedNodesCollector - */ - private $removedNodesCollector; - /** * @required */ @@ -52,14 +46,12 @@ trait NodeCommandersTrait NodeRemovingCommander $nodeRemovingCommander, NodeAddingCommander $nodeAddingCommander, PropertyAddingCommander $propertyAddingCommander, - UseAddingCommander $useAddingCommander, - RemovedNodesCollector $removedNodesCollector + UseAddingCommander $useAddingCommander ): void { $this->nodeRemovingCommander = $nodeRemovingCommander; $this->nodeAddingCommander = $nodeAddingCommander; $this->propertyAddingCommander = $propertyAddingCommander; $this->useAddingCommander = $useAddingCommander; - $this->removedNodesCollector = $removedNodesCollector; } protected function addNodeAfterNode(Node $newNode, Node $positionNode): void @@ -89,8 +81,6 @@ trait NodeCommandersTrait $this->nodeRemovingCommander->addNode($node); $this->notifyNodeChangeFileInfo($node); - - $this->removedNodesCollector->collect($node); } protected function isNodeRemoved(Node $node): bool @@ -98,16 +88,6 @@ trait NodeCommandersTrait return $this->nodeRemovingCommander->isNodeRemoved($node); } - protected function addUseImport(Node $node, string $useImport): void - { - $this->useAddingCommander->addUseImport($node, $useImport); - } - - protected function addFunctionUseImport(Node $node, string $functionUseImport): void - { - $this->useAddingCommander->addFunctionUseImport($node, $functionUseImport); - } - /** * @param Node[] $nodes */