From 330d35bd2e3f8c8960dfc1de3bfbe3cedf980340 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 6 May 2019 17:40:15 +0200 Subject: [PATCH 01/11] add null name check --- .../Rector/MethodCall/RemoveDefaultArgumentValueRector.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/DeadCode/src/Rector/MethodCall/RemoveDefaultArgumentValueRector.php b/packages/DeadCode/src/Rector/MethodCall/RemoveDefaultArgumentValueRector.php index e815a2b68dd1..fc5d67da563d 100644 --- a/packages/DeadCode/src/Rector/MethodCall/RemoveDefaultArgumentValueRector.php +++ b/packages/DeadCode/src/Rector/MethodCall/RemoveDefaultArgumentValueRector.php @@ -156,8 +156,11 @@ private function resolveKeysToRemove(Node $node, array $defaultValues): array */ private function resolveDefaultValuesFromCall(Node $node): array { - /** @var string $nodeName */ + /** @var string|null $nodeName */ $nodeName = $this->getName($node); + if ($nodeName === null) { + return []; + } if ($node instanceof FuncCall) { return $this->resolveFuncCallDefaultParamValues($nodeName); From c324a63512cde5c5c48bf5962a062cdfd6b93d5b Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 7 May 2019 13:39:14 +0200 Subject: [PATCH 02/11] [DeadCode] Fix RemoveAndTrue to work only with real true values --- .../Rector/BooleanAnd/RemoveAndTrueRector.php | 22 ++++++++++------ ...property_changed_in_another_method.php.inc | 25 +++++++++++++++++++ .../RemoveAndTrueRectorTest.php | 6 ++++- 3 files changed, 45 insertions(+), 8 deletions(-) create mode 100644 packages/DeadCode/tests/Rector/BooleanAnd/RemoveAndTrueRector/Fixture/keep_property_changed_in_another_method.php.inc diff --git a/packages/DeadCode/src/Rector/BooleanAnd/RemoveAndTrueRector.php b/packages/DeadCode/src/Rector/BooleanAnd/RemoveAndTrueRector.php index 2f9f60f994e1..1627bb1478ec 100644 --- a/packages/DeadCode/src/Rector/BooleanAnd/RemoveAndTrueRector.php +++ b/packages/DeadCode/src/Rector/BooleanAnd/RemoveAndTrueRector.php @@ -5,6 +5,7 @@ use PhpParser\Node; use PhpParser\Node\Expr\BinaryOp\BooleanAnd; use PHPStan\Type\Constant\ConstantBooleanType; +use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\Rector\AbstractRector; use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\RectorDefinition; @@ -51,24 +52,31 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if ($this->isConstantTrue($node->left)) { + if ($this->isTrueOrBooleanAndTrues($node->left)) { return $node->right; } - if ($this->isConstantTrue($node->right)) { + if ($this->isTrueOrBooleanAndTrues($node->right)) { return $node->left; } return null; } - private function isConstantTrue(Node $node): bool + + + private function isTrueOrBooleanAndTrues(Node $node): bool { - $leftStaticType = $this->getStaticType($node); - if (! $leftStaticType instanceof ConstantBooleanType) { - return false; + if ($this->isTrue($node)) { + return true; + } + + if ($node instanceof BooleanAnd) { + if ($this->isTrueOrBooleanAndTrues($node->left) && $this->isTrueOrBooleanAndTrues($node->right)) { + return true; + } } - return $leftStaticType->getValue() === true; + return false; } } diff --git a/packages/DeadCode/tests/Rector/BooleanAnd/RemoveAndTrueRector/Fixture/keep_property_changed_in_another_method.php.inc b/packages/DeadCode/tests/Rector/BooleanAnd/RemoveAndTrueRector/Fixture/keep_property_changed_in_another_method.php.inc new file mode 100644 index 000000000000..b0ba53eba8a1 --- /dev/null +++ b/packages/DeadCode/tests/Rector/BooleanAnd/RemoveAndTrueRector/Fixture/keep_property_changed_in_another_method.php.inc @@ -0,0 +1,25 @@ +isBoolAssert = false; + + $this->changeIsBoolAssert(); + + return ! $this->isBoolAssert && $expected; + } + + public function changeIsBoolAssert() + { + $this->isBoolAssert = true; + } +} diff --git a/packages/DeadCode/tests/Rector/BooleanAnd/RemoveAndTrueRector/RemoveAndTrueRectorTest.php b/packages/DeadCode/tests/Rector/BooleanAnd/RemoveAndTrueRector/RemoveAndTrueRectorTest.php index 8ec2b3c1c387..95486adc6637 100644 --- a/packages/DeadCode/tests/Rector/BooleanAnd/RemoveAndTrueRector/RemoveAndTrueRectorTest.php +++ b/packages/DeadCode/tests/Rector/BooleanAnd/RemoveAndTrueRector/RemoveAndTrueRectorTest.php @@ -9,7 +9,11 @@ final class RemoveAndTrueRectorTest extends AbstractRectorTestCase { public function test(): void { - $this->doTestFiles([__DIR__ . '/Fixture/fixture.php.inc', __DIR__ . '/Fixture/keep_something.php.inc']); + $this->doTestFiles([ + __DIR__ . '/Fixture/fixture.php.inc', + __DIR__ . '/Fixture/keep_something.php.inc', + __DIR__ . '/Fixture/keep_property_changed_in_another_method.php.inc', + ]); } protected function getRectorClass(): string From cc75a2173a85fd0770f0f1af0500b59a6e0b3c27 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 7 May 2019 13:43:26 +0200 Subject: [PATCH 03/11] travis: use laravel dev --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 364c2c95f98d..48340340a624 100644 --- a/.travis.yml +++ b/.travis.yml @@ -65,6 +65,8 @@ script: mkdir laravel-dir composer create-project laravel/framework laravel-dir + # missing in laravel for some reason + composer require doctrine/dbal -d laravel-dir composer dump-autoload --no-dev -d laravel-dir # 2. run an nother project From bf8c0c04421c3cc66a24c685288a1d31c9a708ce Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 7 May 2019 14:28:16 +0200 Subject: [PATCH 04/11] various cleanup --- .../Assign/RemoveDoubleAssignRector.php | 68 +++++++++---------- .../Rector/BooleanAnd/RemoveAndTrueRector.php | 16 ++--- .../Fixture/skip_double_assign.php.inc | 19 ++++++ .../RemoveDoubleAssignRectorTest.php | 23 ++++--- .../RemoveOverriddenValuesRectorTest.php | 2 +- 5 files changed, 72 insertions(+), 56 deletions(-) create mode 100644 packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/Fixture/skip_double_assign.php.inc diff --git a/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php b/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php index e928a9ddf0f9..cde0af0dc6ba 100644 --- a/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php +++ b/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php @@ -24,6 +24,20 @@ final class RemoveDoubleAssignRector extends AbstractRector { + /** + * @var string[] + */ + private const CONTROL_STRUCTURE_NODES = [ + Foreach_::class, + If_::class, + While_::class, + Do_::class, + Else_::class, + ElseIf_::class, + Catch_::class, + Case_::class, + ]; + public function getDefinition(): RectorDefinition { return new RectorDefinition('Simplify useless double assigns', [ @@ -79,14 +93,6 @@ public function refactor(Node $node): ?Node return null; } - // are 2 different methods - if (! $this->areNodesEqual( - $node->getAttribute(AttributeKey::METHOD_NODE), - $previousExpression->getAttribute(AttributeKey::METHOD_NODE) - )) { - return null; - } - if ($this->shouldSkipForDifferentScope($node, $previousExpression)) { return null; } @@ -123,33 +129,8 @@ private function shouldSkipDueToForeachOverride(Assign $assign, Node $node): boo private function shouldSkipForDifferenceParent(Node $firstNode, Node $secondNode): bool { - $firstNodeParent = $this->betterNodeFinder->findFirstParentInstanceOf( - $firstNode, - [ - Foreach_::class, - If_::class, - While_::class, - Do_::class, - Else_::class, - ElseIf_::class, - Catch_::class, - Case_::class, - ] - ); - - $secondNodeParent = $this->betterNodeFinder->findFirstParentInstanceOf( - $secondNode, - [ - Foreach_::class, - If_::class, - While_::class, - Do_::class, - If_::class, - ElseIf_::class, - Catch_::class, - Case_::class, - ] - ); + $firstNodeParent = $this->findParentControlStructure($firstNode); + $secondNodeParent = $this->findParentControlStructure($secondNode); if ($firstNodeParent === null || $secondNodeParent === null) { return false; @@ -160,9 +141,26 @@ private function shouldSkipForDifferenceParent(Node $firstNode, Node $secondNode private function shouldSkipForDifferentScope(Assign $assign, Node $anotherNode): bool { + if (! $this->areInSameClassMethod($assign, $anotherNode)) { + return true; + } + if ($this->shouldSkipDueToForeachOverride($assign, $anotherNode)) { return true; } return $this->shouldSkipForDifferenceParent($assign, $anotherNode); } + + private function findParentControlStructure(Node $node): ?Node + { + return $this->betterNodeFinder->findFirstParentInstanceOf($node, self::CONTROL_STRUCTURE_NODES); + } + + private function areInSameClassMethod(Node $node, Node $previousExpression): bool + { + return $this->areNodesEqual( + $node->getAttribute(AttributeKey::METHOD_NODE), + $previousExpression->getAttribute(AttributeKey::METHOD_NODE) + ); + } } diff --git a/packages/DeadCode/src/Rector/BooleanAnd/RemoveAndTrueRector.php b/packages/DeadCode/src/Rector/BooleanAnd/RemoveAndTrueRector.php index 1627bb1478ec..197f160a5111 100644 --- a/packages/DeadCode/src/Rector/BooleanAnd/RemoveAndTrueRector.php +++ b/packages/DeadCode/src/Rector/BooleanAnd/RemoveAndTrueRector.php @@ -4,8 +4,6 @@ use PhpParser\Node; use PhpParser\Node\Expr\BinaryOp\BooleanAnd; -use PHPStan\Type\Constant\ConstantBooleanType; -use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\Rector\AbstractRector; use Rector\RectorDefinition\CodeSample; use Rector\RectorDefinition\RectorDefinition; @@ -63,20 +61,20 @@ public function refactor(Node $node): ?Node return null; } - - private function isTrueOrBooleanAndTrues(Node $node): bool { if ($this->isTrue($node)) { return true; } - if ($node instanceof BooleanAnd) { - if ($this->isTrueOrBooleanAndTrues($node->left) && $this->isTrueOrBooleanAndTrues($node->right)) { - return true; - } + if (! $node instanceof BooleanAnd) { + return false; + } + + if (! $this->isTrueOrBooleanAndTrues($node->left)) { + return false; } - return false; + return $this->isTrueOrBooleanAndTrues($node->right); } } diff --git a/packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/Fixture/skip_double_assign.php.inc b/packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/Fixture/skip_double_assign.php.inc new file mode 100644 index 000000000000..8ec755abcd9b --- /dev/null +++ b/packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/Fixture/skip_double_assign.php.inc @@ -0,0 +1,19 @@ +name = $shortName; + } + + return $attributeAwareNode; + } + + $attributeAwareNode->name = $shortName; + } +} diff --git a/packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/RemoveDoubleAssignRectorTest.php b/packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/RemoveDoubleAssignRectorTest.php index 3215f9c4a060..50804d2a5f98 100644 --- a/packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/RemoveDoubleAssignRectorTest.php +++ b/packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/RemoveDoubleAssignRectorTest.php @@ -10,17 +10,18 @@ final class RemoveDoubleAssignRectorTest extends AbstractRectorTestCase public function test(): void { $this->doTestFiles([ - __DIR__ . '/Fixture/fixture.php.inc', - __DIR__ . '/Fixture/calls.php.inc', - __DIR__ . '/Fixture/keep_dim_assign.php.inc', - __DIR__ . '/Fixture/property_assign.php.inc', - __DIR__ . '/Fixture/keep_array_reset.php.inc', - __DIR__ . '/Fixture/keep_property_assign_in_different_ifs.php.inc', - __DIR__ . '/Fixture/inside_if_else.php.inc', - __DIR__ . '/Fixture/inside_the_same_if.php.inc', - // skip - __DIR__ . '/Fixture/skip_double_catch.php.inc', - __DIR__ . '/Fixture/skip_double_case.php.inc', + // __DIR__ . '/Fixture/fixture.php.inc', + // __DIR__ . '/Fixture/calls.php.inc', + // __DIR__ . '/Fixture/keep_dim_assign.php.inc', + // __DIR__ . '/Fixture/property_assign.php.inc', + // __DIR__ . '/Fixture/keep_array_reset.php.inc', + // __DIR__ . '/Fixture/keep_property_assign_in_different_ifs.php.inc', + // __DIR__ . '/Fixture/inside_if_else.php.inc', + // __DIR__ . '/Fixture/inside_the_same_if.php.inc', + // // skip + // __DIR__ . '/Fixture/skip_double_catch.php.inc', + // __DIR__ . '/Fixture/skip_double_case.php.inc', + __DIR__ . '/Fixture/skip_double_assign.php.inc', ]); } diff --git a/packages/DeadCode/tests/Rector/ClassMethod/RemoveOverriddenValuesRector/RemoveOverriddenValuesRectorTest.php b/packages/DeadCode/tests/Rector/ClassMethod/RemoveOverriddenValuesRector/RemoveOverriddenValuesRectorTest.php index 35c63bd1b77b..edf1165c1add 100644 --- a/packages/DeadCode/tests/Rector/ClassMethod/RemoveOverriddenValuesRector/RemoveOverriddenValuesRectorTest.php +++ b/packages/DeadCode/tests/Rector/ClassMethod/RemoveOverriddenValuesRector/RemoveOverriddenValuesRectorTest.php @@ -22,7 +22,7 @@ public function test(): void __DIR__ . '/Fixture/keep_re_use_2.php.inc', __DIR__ . '/Fixture/keep_same_level_but_different_condition_scope.php.inc', __DIR__ . '/Fixture/issue_1093.php.inc', - // __DIR__ . '/Fixture/issue_1286.php.inc', + __DIR__ . '/Fixture/issue_1286.php.inc', ]); } From e38feaf055c2823b3a7ce0afa40febc3fed66b7d Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 7 May 2019 14:33:26 +0200 Subject: [PATCH 05/11] decouple FlowOfControlLocator --- packages/DeadCode/config/config.yaml | 8 +++++ .../Data/VariableNodeUseInfo.php | 2 +- .../DeadCode/src/FlowOfControlLocator.php | 30 +++++++++++++++++ .../RemoveOverriddenValuesRector.php | 32 ++++++------------- .../RemoveOverriddenValuesRectorTest.php | 1 + phpstan.neon | 4 +-- 6 files changed, 52 insertions(+), 25 deletions(-) create mode 100644 packages/DeadCode/config/config.yaml rename packages/DeadCode/src/{Rector/ClassMethod => }/Data/VariableNodeUseInfo.php (97%) create mode 100644 packages/DeadCode/src/FlowOfControlLocator.php diff --git a/packages/DeadCode/config/config.yaml b/packages/DeadCode/config/config.yaml new file mode 100644 index 000000000000..03fdcc163e16 --- /dev/null +++ b/packages/DeadCode/config/config.yaml @@ -0,0 +1,8 @@ +services: + _defaults: + autowire: true + public: true + + Rector\DeadCode\: + resource: '../src' + exclude: '../src/{Rector/**/*Rector.php,Data/*}' diff --git a/packages/DeadCode/src/Rector/ClassMethod/Data/VariableNodeUseInfo.php b/packages/DeadCode/src/Data/VariableNodeUseInfo.php similarity index 97% rename from packages/DeadCode/src/Rector/ClassMethod/Data/VariableNodeUseInfo.php rename to packages/DeadCode/src/Data/VariableNodeUseInfo.php index 3a75027e3b50..480ca9c7f8c7 100644 --- a/packages/DeadCode/src/Rector/ClassMethod/Data/VariableNodeUseInfo.php +++ b/packages/DeadCode/src/Data/VariableNodeUseInfo.php @@ -1,6 +1,6 @@ getAttribute(AttributeKey::PARENT_NODE)) { + if ($parentNode instanceof Expression) { + continue; + } + + $nestingHash .= spl_object_hash($parentNode); + if ($functionLike === $parentNode) { + return $nestingHash; + } + } + + return $nestingHash; + } +} diff --git a/packages/DeadCode/src/Rector/ClassMethod/RemoveOverriddenValuesRector.php b/packages/DeadCode/src/Rector/ClassMethod/RemoveOverriddenValuesRector.php index 1bd9005332a8..1d483f06d3b3 100644 --- a/packages/DeadCode/src/Rector/ClassMethod/RemoveOverriddenValuesRector.php +++ b/packages/DeadCode/src/Rector/ClassMethod/RemoveOverriddenValuesRector.php @@ -6,9 +6,9 @@ use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\Variable; use PhpParser\Node\FunctionLike; -use PhpParser\Node\Stmt\Expression; use Rector\Context\ContextAnalyzer; -use Rector\DeadCode\Rector\ClassMethod\Data\VariableNodeUseInfo; +use Rector\DeadCode\FlowOfControlLocator; +use Rector\DeadCode\Data\VariableNodeUseInfo; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\Rector\AbstractRector; use Rector\RectorDefinition\CodeSample; @@ -21,9 +21,15 @@ final class RemoveOverriddenValuesRector extends AbstractRector */ private $contextAnalyzer; - public function __construct(ContextAnalyzer $contextAnalyzer) + /** + * @var FlowOfControlLocator + */ + private $flowOfControlLocator; + + public function __construct(ContextAnalyzer $contextAnalyzer, FlowOfControlLocator $flowOfControlLocator) { $this->contextAnalyzer = $contextAnalyzer; + $this->flowOfControlLocator = $flowOfControlLocator; } public function getDefinition(): RectorDefinition @@ -184,7 +190,7 @@ private function collectNodesByTypeAndPosition( /** @var Assign $assignNode */ $assignNode = $assignedVariable->getAttribute(AttributeKey::PARENT_NODE); - $nestingHash = $this->resolveNestingHashFromClassMethod($functionLike, $assignNode); + $nestingHash = $this->flowOfControlLocator->resolveNestingHashFromFunctionLike($functionLike, $assignNode); $nodesByTypeAndPosition[] = new VariableNodeUseInfo( $startTokenPos, @@ -286,24 +292,6 @@ private function shouldRemoveAssignNode( return ! $isVariableAssigned; } - private function resolveNestingHashFromClassMethod(FunctionLike $functionLike, Assign $assign): string - { - $nestingHash = '_'; - $parentNode = $assign; - while ($parentNode = $parentNode->getAttribute(AttributeKey::PARENT_NODE)) { - if ($parentNode instanceof Expression) { - continue; - } - - $nestingHash .= spl_object_hash($parentNode); - if ($functionLike === $parentNode) { - return $nestingHash; - } - } - - return $nestingHash; - } - private function isAssignNodeUsed( ?VariableNodeUseInfo $previousNode, VariableNodeUseInfo $nodeByTypeAndPosition diff --git a/packages/DeadCode/tests/Rector/ClassMethod/RemoveOverriddenValuesRector/RemoveOverriddenValuesRectorTest.php b/packages/DeadCode/tests/Rector/ClassMethod/RemoveOverriddenValuesRector/RemoveOverriddenValuesRectorTest.php index edf1165c1add..b64598318f89 100644 --- a/packages/DeadCode/tests/Rector/ClassMethod/RemoveOverriddenValuesRector/RemoveOverriddenValuesRectorTest.php +++ b/packages/DeadCode/tests/Rector/ClassMethod/RemoveOverriddenValuesRector/RemoveOverriddenValuesRectorTest.php @@ -15,6 +15,7 @@ public function test(): void __DIR__ . '/Fixture/multiple_assigns.php.inc', __DIR__ . '/Fixture/reference_use.php.inc', __DIR__ . '/Fixture/method_call.php.inc', + // keep __DIR__ . '/Fixture/keep.php.inc', __DIR__ . '/Fixture/keep_pre_assign.php.inc', __DIR__ . '/Fixture/keep_conditional_override.php.inc', diff --git a/phpstan.neon b/phpstan.neon index 85bdd1880b56..dcfffe3f189e 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -111,8 +111,8 @@ parameters: - '#Method Rector\\(.*?) should return array but returns array#' # known values - - '#Parameter \#2 \$variableName of class Rector\\DeadCode\\Rector\\ClassMethod\\Data\\VariableNodeUseInfo constructor expects string, string\|null given#' - - '#Cannot call method getParentNode\(\) on Rector\\DeadCode\\Rector\\ClassMethod\\Data\\VariableNodeUseInfo\|null#' + - '#Parameter \#2 \$variableName of class Rector\\DeadCode\\Data\\VariableNodeUseInfo constructor expects string, string\|null given#' + - '#Cannot call method getParentNode\(\) on Rector\\DeadCode\\Data\\VariableNodeUseInfo\|null#' # part of test - '#Class Manual\\Twig\\TwigFilter not found#' From 3d6e45e146b809255d357ec13047575ab62afa5b Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 7 May 2019 14:40:43 +0200 Subject: [PATCH 06/11] fix RemoveDoubleAssignRector in different scopes --- .../Assign/RemoveDoubleAssignRector.php | 17 ++------------ .../RemoveOverriddenValuesRector.php | 2 +- .../RemoveDoubleAssignRectorTest.php | 22 +++++++++---------- 3 files changed, 14 insertions(+), 27 deletions(-) diff --git a/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php b/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php index cde0af0dc6ba..637344c0d6cb 100644 --- a/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php +++ b/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php @@ -86,13 +86,6 @@ public function refactor(Node $node): ?Node return null; } - // skip different method expressions - if ($node->getAttribute(AttributeKey::METHOD_NAME) !== $previousExpression->getAttribute( - AttributeKey::METHOD_NAME - )) { - return null; - } - if ($this->shouldSkipForDifferentScope($node, $previousExpression)) { return null; } @@ -114,14 +107,7 @@ private function shouldSkipDueToForeachOverride(Assign $assign, Node $node): boo ); if ($nodePreviousForeach !== $previousExpressionPreviousForeach) { - if ($nodePreviousForeach instanceof Foreach_ && $assign->var instanceof Variable) { - // is value changed inside the foreach? - - $variableAssigns = $this->betterNodeFinder->findAssignsOfVariable($nodePreviousForeach, $assign->var); - - // there is probably value override - return count($variableAssigns) >= 2; - } + return true; } return false; @@ -148,6 +134,7 @@ private function shouldSkipForDifferentScope(Assign $assign, Node $anotherNode): if ($this->shouldSkipDueToForeachOverride($assign, $anotherNode)) { return true; } + return $this->shouldSkipForDifferenceParent($assign, $anotherNode); } diff --git a/packages/DeadCode/src/Rector/ClassMethod/RemoveOverriddenValuesRector.php b/packages/DeadCode/src/Rector/ClassMethod/RemoveOverriddenValuesRector.php index 1d483f06d3b3..31ffbe39ef81 100644 --- a/packages/DeadCode/src/Rector/ClassMethod/RemoveOverriddenValuesRector.php +++ b/packages/DeadCode/src/Rector/ClassMethod/RemoveOverriddenValuesRector.php @@ -7,8 +7,8 @@ use PhpParser\Node\Expr\Variable; use PhpParser\Node\FunctionLike; use Rector\Context\ContextAnalyzer; -use Rector\DeadCode\FlowOfControlLocator; use Rector\DeadCode\Data\VariableNodeUseInfo; +use Rector\DeadCode\FlowOfControlLocator; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\Rector\AbstractRector; use Rector\RectorDefinition\CodeSample; diff --git a/packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/RemoveDoubleAssignRectorTest.php b/packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/RemoveDoubleAssignRectorTest.php index 50804d2a5f98..a9149a41a7a4 100644 --- a/packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/RemoveDoubleAssignRectorTest.php +++ b/packages/DeadCode/tests/Rector/Assign/RemoveDoubleAssignRector/RemoveDoubleAssignRectorTest.php @@ -10,17 +10,17 @@ final class RemoveDoubleAssignRectorTest extends AbstractRectorTestCase public function test(): void { $this->doTestFiles([ - // __DIR__ . '/Fixture/fixture.php.inc', - // __DIR__ . '/Fixture/calls.php.inc', - // __DIR__ . '/Fixture/keep_dim_assign.php.inc', - // __DIR__ . '/Fixture/property_assign.php.inc', - // __DIR__ . '/Fixture/keep_array_reset.php.inc', - // __DIR__ . '/Fixture/keep_property_assign_in_different_ifs.php.inc', - // __DIR__ . '/Fixture/inside_if_else.php.inc', - // __DIR__ . '/Fixture/inside_the_same_if.php.inc', - // // skip - // __DIR__ . '/Fixture/skip_double_catch.php.inc', - // __DIR__ . '/Fixture/skip_double_case.php.inc', + __DIR__ . '/Fixture/fixture.php.inc', + __DIR__ . '/Fixture/calls.php.inc', + __DIR__ . '/Fixture/keep_dim_assign.php.inc', + __DIR__ . '/Fixture/property_assign.php.inc', + __DIR__ . '/Fixture/keep_array_reset.php.inc', + __DIR__ . '/Fixture/keep_property_assign_in_different_ifs.php.inc', + __DIR__ . '/Fixture/inside_if_else.php.inc', + __DIR__ . '/Fixture/inside_the_same_if.php.inc', + // skip + __DIR__ . '/Fixture/skip_double_catch.php.inc', + __DIR__ . '/Fixture/skip_double_case.php.inc', __DIR__ . '/Fixture/skip_double_assign.php.inc', ]); } From 35c3ed67644a9bd72a6febd5ffddf4fb07b64fdf Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 7 May 2019 14:43:47 +0200 Subject: [PATCH 07/11] skip anonymous class in RemoveUnusedPrivatePropertyRector --- .../RemoveUnusedPrivatePropertyRector.php | 7 +++- .../Fixture/skip_anonymous_class.php.inc | 34 +++++++++++++++++++ .../RemoveUnusedPrivatePropertyRectorTest.php | 6 +++- 3 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_anonymous_class.php.inc diff --git a/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php b/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php index 2d976ef94ad8..e68dfec674c3 100644 --- a/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php +++ b/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php @@ -62,8 +62,13 @@ public function refactor(Node $node): ?Node return null; } + /** @var Node\Stmt\ClassLike|null $classNode */ $classNode = $node->getAttribute(AttributeKey::CLASS_NODE); - if ($classNode instanceof Trait_) { + if ($classNode === null || $classNode instanceof Trait_) { + return null; + } + + if ($classNode instanceof Node\Stmt\Class_ && $classNode->isAnonymous()) { return null; } diff --git a/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_anonymous_class.php.inc b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_anonymous_class.php.inc new file mode 100644 index 000000000000..302200e930d4 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/Fixture/skip_anonymous_class.php.inc @@ -0,0 +1,34 @@ +callable = $callable; + } + + /** + * @return int|Node|null + */ + public function enterNode(Node $node) + { + $callable = $this->callable; + return $callable($node); + } + }; + } +} diff --git a/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php index a0eec93a3226..1ed362633903 100644 --- a/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php +++ b/packages/DeadCode/tests/Rector/Property/RemoveUnusedPrivatePropertyRector/RemoveUnusedPrivatePropertyRectorTest.php @@ -9,7 +9,11 @@ final class RemoveUnusedPrivatePropertyRectorTest extends AbstractRectorTestCase { public function test(): void { - $this->doTestFiles([__DIR__ . '/Fixture/fixture.php.inc', __DIR__ . '/Fixture/property_assign.php.inc']); + $this->doTestFiles([ + __DIR__ . '/Fixture/fixture.php.inc', + __DIR__ . '/Fixture/property_assign.php.inc', + __DIR__ . '/Fixture/skip_anonymous_class.php.inc', + ]); } protected function getRectorClass(): string From 2c03baf350d5a1611abfc2298e92fd2b206ee83a Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 7 May 2019 14:46:16 +0200 Subject: [PATCH 08/11] fix anonymous class for RemoveUnusedPrivateMethodRector --- .../RemoveUnusedPrivateMethodRector.php | 10 ++++ .../RemoveUnusedPrivatePropertyRector.php | 6 +- .../Fixture/skip_anonymous_class.php.inc | 56 +++++++++++++++++++ .../RemoveUnusedPrivateMethodRectorTest.php | 1 + 4 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedPrivateMethodRector/Fixture/skip_anonymous_class.php.inc diff --git a/packages/DeadCode/src/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php b/packages/DeadCode/src/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php index 39264448cea6..35add03817eb 100644 --- a/packages/DeadCode/src/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php +++ b/packages/DeadCode/src/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php @@ -3,6 +3,8 @@ namespace Rector\DeadCode\Rector\ClassMethod; use PhpParser\Node; +use PhpParser\Node\Stmt\Class_; +use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Trait_; use Rector\NodeContainer\ParsedNodesByType; @@ -68,13 +70,21 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { + /** @var ClassLike|null $classNode */ $classNode = $node->getAttribute(AttributeKey::CLASS_NODE); + if ($classNode === null) { + return null; + } // unreliable to detect trait method usage if ($classNode instanceof Trait_) { return null; } + if ($classNode instanceof Class_ && $classNode->isAnonymous()) { + return null; + } + // skips interfaces by default too if (! $node->isPrivate()) { return null; diff --git a/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php b/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php index e68dfec674c3..22076c9036f3 100644 --- a/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php +++ b/packages/DeadCode/src/Rector/Property/RemoveUnusedPrivatePropertyRector.php @@ -5,6 +5,8 @@ use PhpParser\Node; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Stmt\Class_; +use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\Property; use PhpParser\Node\Stmt\Trait_; use Rector\NodeTypeResolver\Node\AttributeKey; @@ -62,13 +64,13 @@ public function refactor(Node $node): ?Node return null; } - /** @var Node\Stmt\ClassLike|null $classNode */ + /** @var ClassLike|null $classNode */ $classNode = $node->getAttribute(AttributeKey::CLASS_NODE); if ($classNode === null || $classNode instanceof Trait_) { return null; } - if ($classNode instanceof Node\Stmt\Class_ && $classNode->isAnonymous()) { + if ($classNode instanceof Class_ && $classNode->isAnonymous()) { return null; } diff --git a/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedPrivateMethodRector/Fixture/skip_anonymous_class.php.inc b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedPrivateMethodRector/Fixture/skip_anonymous_class.php.inc new file mode 100644 index 000000000000..679a8518aaa7 --- /dev/null +++ b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedPrivateMethodRector/Fixture/skip_anonymous_class.php.inc @@ -0,0 +1,56 @@ +classManipulator, $this->propertiesByClass) extends NodeVisitorAbstract { + /** + * @var VariableInfo[][] + */ + private $propertiesByClass = []; + + /** + * @var ClassManipulator + */ + private $classManipulator; + + /** + * @param VariableInfo[][] $propertiesByClass + */ + public function __construct(ClassManipulator $classManipulator, array $propertiesByClass) + { + $this->classManipulator = $classManipulator; + $this->propertiesByClass = $propertiesByClass; + } + + public function enterNode(Node $node): ?Node + { + if (! $node instanceof Class_ || $node->isAnonymous()) { + return null; + } + + return $this->processClassNode($node); + } + + private function processClassNode(Class_ $classNode): Class_ + { + $variableInfos = $this->propertiesByClass[spl_object_hash($classNode)] ?? []; + foreach ($variableInfos as $propertyInfo) { + $this->classManipulator->addConstructorDependency($classNode, $propertyInfo); + } + + return $classNode; + } + }; + } +} diff --git a/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedPrivateMethodRector/RemoveUnusedPrivateMethodRectorTest.php b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedPrivateMethodRector/RemoveUnusedPrivateMethodRectorTest.php index 2ec6c3a89d45..9a27b9f3b3a3 100644 --- a/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedPrivateMethodRector/RemoveUnusedPrivateMethodRectorTest.php +++ b/packages/DeadCode/tests/Rector/ClassMethod/RemoveUnusedPrivateMethodRector/RemoveUnusedPrivateMethodRectorTest.php @@ -19,6 +19,7 @@ public function test(): void __DIR__ . '/Fixture/keep_in_trait.php.inc', __DIR__ . '/Fixture/keep_used_method.php.inc', __DIR__ . '/Fixture/keep_used_method_static.php.inc', + __DIR__ . '/Fixture/skip_anonymous_class.php.inc', ]); } From b8b37303dd3197c10b6bb45e8cec74dd2cf6b9ed Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 7 May 2019 15:32:27 +0200 Subject: [PATCH 09/11] [ref] apply code-quality --- .../Rector/Namespace_/ImportFullyQualifiedNamesRector.php | 2 +- .../src/Rector/Assign/RemoveDoubleAssignRector.php | 7 +------ .../Rector/MethodCall/RemoveDefaultArgumentValueRector.php | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/CodingStyle/src/Rector/Namespace_/ImportFullyQualifiedNamesRector.php b/packages/CodingStyle/src/Rector/Namespace_/ImportFullyQualifiedNamesRector.php index 0937753c2ed9..03afce5c3622 100644 --- a/packages/CodingStyle/src/Rector/Namespace_/ImportFullyQualifiedNamesRector.php +++ b/packages/CodingStyle/src/Rector/Namespace_/ImportFullyQualifiedNamesRector.php @@ -135,7 +135,7 @@ private function resolveAlreadyImportedUses(Namespace_ $namespace): void $class = $this->betterNodeFinder->findFirstInstanceOf($namespace->stmts, Class_::class); // add class itself - if ($class) { + if ($class !== null) { $className = $this->getName($class); if ($className !== null) { $this->importsInClassCollection->addImport($className); diff --git a/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php b/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php index 637344c0d6cb..8172d439e024 100644 --- a/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php +++ b/packages/DeadCode/src/Rector/Assign/RemoveDoubleAssignRector.php @@ -105,12 +105,7 @@ private function shouldSkipDueToForeachOverride(Assign $assign, Node $node): boo $node, Foreach_::class ); - - if ($nodePreviousForeach !== $previousExpressionPreviousForeach) { - return true; - } - - return false; + return $nodePreviousForeach !== $previousExpressionPreviousForeach; } private function shouldSkipForDifferenceParent(Node $firstNode, Node $secondNode): bool diff --git a/packages/DeadCode/src/Rector/MethodCall/RemoveDefaultArgumentValueRector.php b/packages/DeadCode/src/Rector/MethodCall/RemoveDefaultArgumentValueRector.php index fc5d67da563d..9eeab5b29bc8 100644 --- a/packages/DeadCode/src/Rector/MethodCall/RemoveDefaultArgumentValueRector.php +++ b/packages/DeadCode/src/Rector/MethodCall/RemoveDefaultArgumentValueRector.php @@ -203,7 +203,7 @@ private function resolveDefaultParamValuesFromFunctionLike(FunctionLike $functio private function resolveFuncCallDefaultParamValues(string $nodeName): array { $functionNode = $this->parsedNodesByType->findFunction($nodeName); - if ($functionNode) { + if ($functionNode !== null) { return $this->resolveDefaultParamValuesFromFunctionLike($functionNode); } From de331920c8adb7f5d568d7998db099bdf939224c Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Tue, 7 May 2019 15:37:50 +0200 Subject: [PATCH 10/11] fix CallableThisArrayToAnonymousFunctionRector for empty first item in array --- .../CallableThisArrayToAnonymousFunctionRector.php | 7 ++++++- ...lableThisArrayToAnonymousFunctionRectorTest.php | 1 + .../Fixture/skip_too.php.inc | 14 ++++++++++++++ phpstan.neon | 1 + 4 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_too.php.inc diff --git a/packages/CodeQuality/src/Rector/Array_/CallableThisArrayToAnonymousFunctionRector.php b/packages/CodeQuality/src/Rector/Array_/CallableThisArrayToAnonymousFunctionRector.php index 537458b735d1..13814fb1a50e 100644 --- a/packages/CodeQuality/src/Rector/Array_/CallableThisArrayToAnonymousFunctionRector.php +++ b/packages/CodeQuality/src/Rector/Array_/CallableThisArrayToAnonymousFunctionRector.php @@ -97,7 +97,12 @@ public function refactor(Node $node): ?Node return null; } - // is callable + // is callable? + // can be totally empty, e.g [, $value] + if ($node->items[0] === null) { + return null; + } + $objectVariable = $node->items[0]->value; $classMethod = $this->matchCallableMethod($objectVariable, $node->items[1]->value); diff --git a/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/CallableThisArrayToAnonymousFunctionRectorTest.php b/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/CallableThisArrayToAnonymousFunctionRectorTest.php index c5f4f05da58a..954c2fc687d2 100644 --- a/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/CallableThisArrayToAnonymousFunctionRectorTest.php +++ b/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/CallableThisArrayToAnonymousFunctionRectorTest.php @@ -13,6 +13,7 @@ public function test(): void __DIR__ . '/Fixture/fixture.php.inc', __DIR__ . '/Fixture/skip.php.inc', __DIR__ . '/Fixture/another_class.php.inc', + __DIR__ . '/Fixture/skip_too.php.inc', ]); } diff --git a/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_too.php.inc b/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_too.php.inc new file mode 100644 index 000000000000..acabb0f9f2f3 --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_too.php.inc @@ -0,0 +1,14 @@ + Date: Tue, 7 May 2019 15:49:21 +0200 Subject: [PATCH 11/11] fix CallableThisArrayToAnonymousFunctionRector for invalid array items --- ...ableThisArrayToAnonymousFunctionRector.php | 32 ++- ...ThisArrayToAnonymousFunctionRectorTest.php | 4 +- .../Fixture/skip_as_well.php.inc | 207 ++++++++++++++++++ ...php.inc => skip_empty_first_array.php.inc} | 7 +- .../ImportFullyQualifiedNamesRector.php | 2 +- 5 files changed, 238 insertions(+), 14 deletions(-) create mode 100644 packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_as_well.php.inc rename packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/{skip_too.php.inc => skip_empty_first_array.php.inc} (57%) diff --git a/packages/CodeQuality/src/Rector/Array_/CallableThisArrayToAnonymousFunctionRector.php b/packages/CodeQuality/src/Rector/Array_/CallableThisArrayToAnonymousFunctionRector.php index 13814fb1a50e..1221acf8a322 100644 --- a/packages/CodeQuality/src/Rector/Array_/CallableThisArrayToAnonymousFunctionRector.php +++ b/packages/CodeQuality/src/Rector/Array_/CallableThisArrayToAnonymousFunctionRector.php @@ -2,6 +2,8 @@ namespace Rector\CodeQuality\Rector\Array_; +use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Scalar\String_; use PhpParser\Node; use PhpParser\Node\Arg; use PhpParser\Node\Expr; @@ -93,19 +95,21 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Node { - if (count($node->items) !== 2) { + if ($this->shouldSkipArray($node)) { return null; } - // is callable? - // can be totally empty, e.g [, $value] - if ($node->items[0] === null) { + $objectVariable = $node->items[0]->value; + if (! $objectVariable instanceof Variable && ! $objectVariable instanceof PropertyFetch) { return null; } - $objectVariable = $node->items[0]->value; + $methodName = $node->items[1]->value; + if (! $methodName instanceof String_) { + return null; + } - $classMethod = $this->matchCallableMethod($objectVariable, $node->items[1]->value); + $classMethod = $this->matchCallableMethod($objectVariable, $methodName); if ($classMethod === null) { return null; } @@ -145,7 +149,10 @@ private function convertParamsToArgs(array $params): array return $args; } - private function matchCallableMethod(Expr $objectExpr, Expr $methodExpr): ?ClassMethod + /** + * @param Variable|PropertyFetch $objectExpr + */ + private function matchCallableMethod(Expr $objectExpr, String_ $methodExpr): ?ClassMethod { $methodName = $this->getValue($methodExpr); @@ -173,4 +180,15 @@ private function matchCallableMethod(Expr $objectExpr, Expr $methodExpr): ?Class return null; } + + private function shouldSkipArray(Array_ $array): bool + { + // callback is exactly "[$two, 'items']" + if (count($array->items) !== 2) { + return true; + } + + // can be totally empty in case of "[, $value]" + return $array->items[0] === null; + } } diff --git a/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/CallableThisArrayToAnonymousFunctionRectorTest.php b/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/CallableThisArrayToAnonymousFunctionRectorTest.php index 954c2fc687d2..479a44e0e153 100644 --- a/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/CallableThisArrayToAnonymousFunctionRectorTest.php +++ b/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/CallableThisArrayToAnonymousFunctionRectorTest.php @@ -13,7 +13,9 @@ public function test(): void __DIR__ . '/Fixture/fixture.php.inc', __DIR__ . '/Fixture/skip.php.inc', __DIR__ . '/Fixture/another_class.php.inc', - __DIR__ . '/Fixture/skip_too.php.inc', + __DIR__ . '/Fixture/skip_another_class.php.inc', + __DIR__ . '/Fixture/skip_empty_first_array.php.inc', + __DIR__ . '/Fixture/skip_as_well.php.inc', ]); } diff --git a/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_as_well.php.inc b/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_as_well.php.inc new file mode 100644 index 000000000000..c37b5ebcaefa --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_as_well.php.inc @@ -0,0 +1,207 @@ +classManipulator = $classManipulator; + $this->namespacePrefixesWithExcludedClasses = $namespacePrefixesWithExcludedClasses; + $this->docBlockManipulator = $docBlockManipulator; + } + + public function getDefinition(): RectorDefinition + { + return new RectorDefinition('Replaces defined Pseudo_Namespaces by Namespace\Ones.', [ + new ConfiguredCodeSample( + '$someService = new Some_Object;', + '$someService = new Some\Object;', + [ + ['Some_' => []], + ] + ), + new ConfiguredCodeSample( + <<<'CODE_SAMPLE' +/** @var Some_Object $someService */ +$someService = new Some_Object; +$someClassToKeep = new Some_Class_To_Keep; +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +/** @var Some\Object $someService */ +$someService = new Some\Object; +$someClassToKeep = new Some_Class_To_Keep; +CODE_SAMPLE + , + [ + ['Some_' => ['Some_Class_To_Keep']], + ] + ), + ]); + } + + /** + * @return string[] + */ + public function getNodeTypes(): array + { + // property, method + return [Name::class, Identifier::class, Property::class, FunctionLike::class, Expression::class]; + } + + /** + * @param Name|Identifier|Property|FunctionLike $node + */ + public function refactor(Node $node): ?Node + { + // replace on @var/@param/@return/@throws + foreach ($this->namespacePrefixesWithExcludedClasses as $namespacePrefix => $excludedClasses) { + $this->docBlockManipulator->changeUnderscoreType($node, $namespacePrefix, $excludedClasses); + } + + if ($node instanceof Name || $node instanceof Identifier) { + return $this->processNameOrIdentifier($node); + } + + return null; + } + + /** + * @param Stmt[] $nodes + * @return Node[] + */ + public function afterTraverse(array $nodes): array + { + if ($this->newNamespace === null) { + return $nodes; + } + + $namespaceNode = new Namespace_(new Name($this->newNamespace)); + foreach ($nodes as $key => $node) { + if ($node instanceof Class_) { + $nodes = $this->classManipulator->insertBeforeAndFollowWithNewline($nodes, $namespaceNode, $key); + + break; + } + } + + $this->newNamespace = null; + + return $nodes; + } + + private function processName(Name $name): Name + { + $nodeName = $this->getName($name); + + if ($nodeName !== null) { + $name->parts = explode('_', $nodeName); + } + + return $name; + } + + private function processIdentifier(Identifier $identifier): ?Identifier + { + $parentNode = $identifier->getAttribute(AttributeKey::PARENT_NODE); + if (! $parentNode instanceof Class_) { + return null; + } + + $name = $this->getName($identifier); + if ($name === null) { + return null; + } + + $newNameParts = explode('_', $name); + $lastNewNamePart = $newNameParts[count($newNameParts) - 1]; + + $namespaceParts = $newNameParts; + array_pop($namespaceParts); + + $newNamespace = implode('\\', $namespaceParts); + if ($this->newNamespace !== null && $this->newNamespace !== $newNamespace) { + throw new ShouldNotHappenException('There cannot be 2 different namespaces in one file'); + } + + $this->newNamespace = $newNamespace; + + $identifier->name = $lastNewNamePart; + + return $identifier; + } + + /** + * @param Name|Identifier $node + * @return Name|Identifier + */ + private function processNameOrIdentifier(Node $node): ?Node + { + // no name → skip + if ($node->toString() === '') { + return null; + } + + foreach ($this->namespacePrefixesWithExcludedClasses as $namespacePrefix => $excludedClasses) { + if (! $this->isName($node, $namespacePrefix . '*')) { + continue; + } + + if (is_array($excludedClasses) && $this->isNames($node, $excludedClasses)) { + return null; + } + + if ($node instanceof Name) { + return $this->processName($node); + } + + return $this->processIdentifier($node); + } + + return null; + } +} diff --git a/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_too.php.inc b/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_empty_first_array.php.inc similarity index 57% rename from packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_too.php.inc rename to packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_empty_first_array.php.inc index acabb0f9f2f3..902a8eea2c6c 100644 --- a/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_too.php.inc +++ b/packages/CodeQuality/tests/Rector/Array_/CallableThisArrayToAnonymousFunctionRector/Fixture/skip_empty_first_array.php.inc @@ -2,13 +2,10 @@ namespace Rector\CodeQuality\Tests\Rector\Array_\CallableThisArrayToAnonymousFunctionRector\Fixture; -use PhpParser\Node; -use PhpParser\Node\Stmt\Foreach_; - final class ForeachToInArrayRector { - public function refactor(Node $node) + public function refactor($node) { - [, $comparedNode] = $matchedNodes; + [, $comparedNode] = $node; } } diff --git a/packages/CodingStyle/src/Rector/Namespace_/ImportFullyQualifiedNamesRector.php b/packages/CodingStyle/src/Rector/Namespace_/ImportFullyQualifiedNamesRector.php index 03afce5c3622..b5d73de4fd91 100644 --- a/packages/CodingStyle/src/Rector/Namespace_/ImportFullyQualifiedNamesRector.php +++ b/packages/CodingStyle/src/Rector/Namespace_/ImportFullyQualifiedNamesRector.php @@ -334,7 +334,7 @@ private function resolveAlreadyUsedShortNames(Namespace_ $namespace): void private function isCurrentNamespace(string $namespaceName, string $newUseStatement): bool { $afterCurrentNamespace = Strings::after($newUseStatement, $namespaceName . '\\'); - if ($afterCurrentNamespace === false) { + if (! $afterCurrentNamespace) { return false; }