diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md index 655fced94f7..1547838aeef 100644 --- a/build/target-repository/docs/rector_rules_overview.md +++ b/build/target-repository/docs/rector_rules_overview.md @@ -1,4 +1,4 @@ -# 404 Rules Overview +# 403 Rules Overview
@@ -12,7 +12,7 @@ - [Compatibility](#compatibility) (1) -- [DeadCode](#deadcode) (48) +- [DeadCode](#deadcode) (47) - [DependencyInjection](#dependencyinjection) (2) @@ -2956,32 +2956,6 @@ Remove duplicated key in defined arrays.
-### RemoveDuplicatedIfReturnRector - -Remove duplicated if stmt with return in function/method body - -- class: [`Rector\DeadCode\Rector\FunctionLike\RemoveDuplicatedIfReturnRector`](../rules/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector.php) - -```diff - class SomeClass - { - public function run($value) - { - if ($value) { - return true; - } - - $value2 = 100; -- -- if ($value) { -- return true; -- } - } - } -``` - -
- ### RemoveDuplicatedInstanceOfRector Remove duplicated instanceof in one call diff --git a/config/set/dead-code.php b/config/set/dead-code.php index 09af56f7a9a..90332a4330d 100644 --- a/config/set/dead-code.php +++ b/config/set/dead-code.php @@ -28,7 +28,6 @@ use Rector\DeadCode\Rector\For_\RemoveDeadLoopRector; use Rector\DeadCode\Rector\Foreach_\RemoveUnusedForeachKeyRector; use Rector\DeadCode\Rector\FunctionLike\RemoveDeadReturnRector; -use Rector\DeadCode\Rector\FunctionLike\RemoveDuplicatedIfReturnRector; use Rector\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector; use Rector\DeadCode\Rector\If_\RemoveDeadInstanceOfRector; use Rector\DeadCode\Rector\If_\RemoveUnusedNonEmptyArrayBeforeForeachRector; @@ -80,7 +79,6 @@ RemoveEmptyTestMethodRector::class, RemoveDeadTryCatchRector::class, RemoveUnusedVariableAssignRector::class, - RemoveDuplicatedIfReturnRector::class, RemoveUnusedNonEmptyArrayBeforeForeachRector::class, RemoveEmptyMethodCallRector::class, RemoveDeadConditionAboveReturnRector::class, diff --git a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/fixture.php.inc b/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/fixture.php.inc deleted file mode 100644 index c201f96735f..00000000000 --- a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/fixture.php.inc +++ /dev/null @@ -1,39 +0,0 @@ - ------ - diff --git a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_array_shifted_value.php.inc b/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_array_shifted_value.php.inc deleted file mode 100644 index 70da80f492d..00000000000 --- a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_array_shifted_value.php.inc +++ /dev/null @@ -1,23 +0,0 @@ -data)) { - return $this->data; - } - - $this->data = $data; - if (! empty($this->data)) { - return $this->data; - } - } -} diff --git a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_negation_if_cond_return_true.php.inc b/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_negation_if_cond_return_true.php.inc deleted file mode 100644 index 017bf83cc20..00000000000 --- a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_negation_if_cond_return_true.php.inc +++ /dev/null @@ -1,15 +0,0 @@ -simpleCallableNodeTraverser->traverseNodesWithCallable($stmt, function (Node $node) use ( - &$isParamUsed, - ): ?int { - if ($isParamUsed) { - return NodeTraverser::STOP_TRAVERSAL; - } - - // skip nested anonymous class - if ($node instanceof Class_ || $node instanceof Function_) { - $isParamUsed = true; - - return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN; - } - - return null; - }); - } -} diff --git a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_referenced.php.inc b/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_referenced.php.inc deleted file mode 100644 index 977b5a6a55b..00000000000 --- a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_referenced.php.inc +++ /dev/null @@ -1,21 +0,0 @@ -_render(array_shift($rows), $widths, ['style' => $config['headerStyle']]); - } - - if (empty($rows)) { - return; - } - } -} diff --git a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_this.php.inc b/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_this.php.inc deleted file mode 100644 index 93ec9010bdf..00000000000 --- a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/Fixture/skip_this.php.inc +++ /dev/null @@ -1,19 +0,0 @@ -doTestFile($filePath); - } - - public static function provideData(): Iterator - { - return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); - } - - public function provideConfigFilePath(): string - { - return __DIR__ . '/config/configured_rule.php'; - } -} diff --git a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/config/configured_rule.php b/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/config/configured_rule.php deleted file mode 100644 index fa311635e2b..00000000000 --- a/rules-tests/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector/config/configured_rule.php +++ /dev/null @@ -1,10 +0,0 @@ -rule(RemoveDuplicatedIfReturnRector::class); -}; diff --git a/rules/CodeQuality/Rector/FuncCall/SetTypeToCastRector.php b/rules/CodeQuality/Rector/FuncCall/SetTypeToCastRector.php index 199d25a70b0..312f59b09c8 100644 --- a/rules/CodeQuality/Rector/FuncCall/SetTypeToCastRector.php +++ b/rules/CodeQuality/Rector/FuncCall/SetTypeToCastRector.php @@ -4,10 +4,10 @@ namespace Rector\CodeQuality\Rector\FuncCall; -use PhpParser\Node\Expr\ArrayItem; -use PhpParser\Node\Arg; use PhpParser\Node; +use PhpParser\Node\Arg; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\ArrayItem; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\Cast; use PhpParser\Node\Expr\Cast\Array_; diff --git a/rules/CodingStyle/NodeAnalyzer/SpreadVariablesCollector.php b/rules/CodingStyle/NodeAnalyzer/SpreadVariablesCollector.php index 366ca713147..7d2b2a8d17a 100644 --- a/rules/CodingStyle/NodeAnalyzer/SpreadVariablesCollector.php +++ b/rules/CodingStyle/NodeAnalyzer/SpreadVariablesCollector.php @@ -6,10 +6,8 @@ use PhpParser\Node\Param; use PhpParser\Node\Stmt\ClassMethod; -use PHPStan\Reflection\MethodReflection; use PHPStan\Reflection\ParameterReflection; use PHPStan\Reflection\ParametersAcceptor; -use PHPStan\Reflection\ParametersAcceptorSelector; use Rector\NodeTypeResolver\Node\AttributeKey; final class SpreadVariablesCollector diff --git a/rules/CodingStyle/Rector/ClassMethod/UnSpreadOperatorRector.php b/rules/CodingStyle/Rector/ClassMethod/UnSpreadOperatorRector.php index 695adfa52ae..b9c0f1c71a8 100644 --- a/rules/CodingStyle/Rector/ClassMethod/UnSpreadOperatorRector.php +++ b/rules/CodingStyle/Rector/ClassMethod/UnSpreadOperatorRector.php @@ -16,7 +16,6 @@ use PHPStan\Reflection\ParametersAcceptorSelector; use Rector\CodingStyle\NodeAnalyzer\SpreadVariablesCollector; use Rector\CodingStyle\Reflection\VendorLocationDetector; -use Rector\Core\Rector\AbstractRector; use Rector\Core\Rector\AbstractScopeAwareRector; use Rector\Core\Reflection\ReflectionResolver; use Rector\FamilyTree\NodeAnalyzer\ClassChildAnalyzer; @@ -133,7 +132,11 @@ private function refactorMethodCall(MethodCall $methodCall, Scope $scope): ?Meth return null; } - $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs($scope, $methodCall->getArgs(), $methodReflection->getVariants()); + $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( + $scope, + $methodCall->getArgs(), + $methodReflection->getVariants() + ); $spreadParameterReflections = $this->spreadVariablesCollector->resolveFromParametersAcceptor( $parametersAcceptor ); diff --git a/rules/DeadCode/NodeCollector/ModifiedVariableNamesCollector.php b/rules/DeadCode/NodeCollector/ModifiedVariableNamesCollector.php deleted file mode 100644 index de15a28593c..00000000000 --- a/rules/DeadCode/NodeCollector/ModifiedVariableNamesCollector.php +++ /dev/null @@ -1,103 +0,0 @@ -collectFromArgs($stmt); - $assignNames = $this->collectFromAssigns($stmt); - - return array_merge($argNames, $assignNames); - } - - /** - * @return string[] - */ - private function collectFromArgs(Stmt $stmt): array - { - $variableNames = []; - - $this->simpleCallableNodeTraverser->traverseNodesWithCallable($stmt, function (Node $node) use ( - &$variableNames - ) { - if (! $node instanceof Arg) { - return null; - } - - if (! $this->isVariableChangedInReference($node)) { - return null; - } - - $variableName = $this->nodeNameResolver->getName($node->value); - if ($variableName === null) { - return null; - } - - $variableNames[] = $variableName; - }); - - return $variableNames; - } - - /** - * @return string[] - */ - private function collectFromAssigns(Stmt $stmt): array - { - $modifiedVariableNames = []; - - $this->simpleCallableNodeTraverser->traverseNodesWithCallable($stmt, function (Node $node) use ( - &$modifiedVariableNames - ) { - if (! $node instanceof Assign) { - return null; - } - - if (! $node->var instanceof Variable) { - return null; - } - - $variableName = $this->nodeNameResolver->getName($node->var); - if ($variableName === null) { - return null; - } - - $modifiedVariableNames[] = $variableName; - }); - - return $modifiedVariableNames; - } - - private function isVariableChangedInReference(Arg $arg): bool - { - $parentNode = $arg->getAttribute(AttributeKey::PARENT_NODE); - if (! $parentNode instanceof FuncCall) { - return false; - } - - return $this->nodeNameResolver->isNames($parentNode, ['array_shift', 'array_pop']); - } -} diff --git a/rules/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector.php b/rules/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector.php deleted file mode 100644 index b595fb7ef38..00000000000 --- a/rules/DeadCode/Rector/FunctionLike/RemoveDuplicatedIfReturnRector.php +++ /dev/null @@ -1,214 +0,0 @@ -nodeComparator = $nodeComparator; - } - - public function getRuleDefinition(): RuleDefinition - { - return new RuleDefinition( - 'Remove duplicated if stmt with return in function/method body', - [ - new CodeSample( - <<<'CODE_SAMPLE' -class SomeClass -{ - public function run($value) - { - if ($value) { - return true; - } - - $value2 = 100; - - if ($value) { - return true; - } - } -} -CODE_SAMPLE - , - <<<'CODE_SAMPLE' -class SomeClass -{ - public function run($value) - { - if ($value) { - return true; - } - - $value2 = 100; - } -} -CODE_SAMPLE - ), - ] - ); - } - - /** - * @return array> - */ - public function getNodeTypes(): array - { - return [FunctionLike::class]; - } - - /** - * @param FunctionLike $node - */ - public function refactor(Node $node): ?Node - { - $ifWithOnlyReturnsByHash = $this->collectDuplicatedIfWithOnlyReturnByHash($node); - if ($ifWithOnlyReturnsByHash === []) { - return null; - } - - $hasChanged = false; - - foreach ($ifWithOnlyReturnsByHash as $ifWithOnlyReturns) { - $isBool = $this->isBoolVarIfCondReturnTrueNextReturnBoolVar($ifWithOnlyReturns); - - if (count($ifWithOnlyReturns) < 2) { - continue; - } - - if (! $isBool) { - // keep first one - array_shift($ifWithOnlyReturns); - } - - foreach ($ifWithOnlyReturns as $ifWithOnlyReturn) { - $this->removeNode($ifWithOnlyReturn); - $hasChanged = true; - } - } - - if ($hasChanged) { - return $node; - } - - return null; - } - - /** - * @param If_[] $ifWithOnlyReturns - */ - private function isBoolVarIfCondReturnTrueNextReturnBoolVar(array $ifWithOnlyReturns): bool - { - if (count($ifWithOnlyReturns) > 1) { - return false; - } - - $cond = $ifWithOnlyReturns[0]->cond; - if (! in_array($cond::class, [Variable::class, PropertyFetch::class, StaticPropertyFetch::class], true)) { - return false; - } - - $type = $this->nodeTypeResolver->getType($cond); - return $type->isBoolean() - ->yes(); - } - - /** - * @return array - */ - private function collectDuplicatedIfWithOnlyReturnByHash(FunctionLike $functionLike): array - { - $ifWithOnlyReturnsByHash = []; - $modifiedVariableNames = []; - - foreach ((array) $functionLike->getStmts() as $stmt) { - if (! $this->ifManipulator->isIfWithOnly($stmt, Return_::class)) { - // variable modification - $modifiedVariableNames = array_merge( - $modifiedVariableNames, - $this->modifiedVariableNamesCollector->collectModifiedVariableNames($stmt) - ); - - continue; - } - - if ($this->containsVariableNames($stmt, $modifiedVariableNames)) { - continue; - } - - /** @var If_ $stmt */ - $isFoundPropertyFetch = (bool) $this->betterNodeFinder->findFirst( - $stmt->cond, - fn (Node $node): bool => $this->propertyFetchAnalyzer->isPropertyFetch($node) - ); - if ($isFoundPropertyFetch) { - continue; - } - - $hash = $this->nodeComparator->printWithoutComments($stmt); - $ifWithOnlyReturnsByHash[$hash][] = $stmt; - } - - return $ifWithOnlyReturnsByHash; - } - - /** - * @param string[] $modifiedVariableNames - */ - private function containsVariableNames(Stmt $stmt, array $modifiedVariableNames): bool - { - if ($modifiedVariableNames === []) { - return false; - } - - $containsVariableNames = false; - $this->traverseNodesWithCallable($stmt, function (Node $node) use ( - $modifiedVariableNames, - &$containsVariableNames - ): ?int { - if (! $node instanceof Variable) { - return null; - } - - if (! $this->isNames($node, $modifiedVariableNames)) { - return null; - } - - $containsVariableNames = true; - - return NodeTraverser::STOP_TRAVERSAL; - }); - - return $containsVariableNames; - } -} diff --git a/src/Kernel/RectorKernel.php b/src/Kernel/RectorKernel.php index cc90cfec288..079de06eaaf 100644 --- a/src/Kernel/RectorKernel.php +++ b/src/Kernel/RectorKernel.php @@ -17,7 +17,7 @@ final class RectorKernel /** * @var string */ - private const CACHE_KEY = 'v19'; + private const CACHE_KEY = 'v20'; private ContainerInterface|null $container = null; diff --git a/src/NodeManipulator/IfManipulator.php b/src/NodeManipulator/IfManipulator.php index 2cf4e39b0e5..52a4d7a3510 100644 --- a/src/NodeManipulator/IfManipulator.php +++ b/src/NodeManipulator/IfManipulator.php @@ -4,7 +4,6 @@ namespace Rector\Core\NodeManipulator; -use PhpParser\Node; use PhpParser\Node\Expr; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\BinaryOp\NotIdentical; @@ -79,7 +78,7 @@ public function collectNestedIfsWithOnlyReturn(If_ $if): array return []; } - // last node is with the return value + // last if is with the return value $ifs[] = $currentIf; return $ifs; @@ -156,7 +155,7 @@ public function collectNestedIfsWithNonBreaking(Foreach_ $foreach): array return []; } - // last node is with the expression + // last if is with the expression $ifs[] = $currentIf; @@ -166,17 +165,13 @@ public function collectNestedIfsWithNonBreaking(Foreach_ $foreach): array /** * @param class-string $stmtClass */ - public function isIfWithOnly(Node $node, string $stmtClass): bool + public function isIfWithOnly(If_ $if, string $stmtClass): bool { - if (! $node instanceof If_) { - return false; - } - - if (! $this->isIfWithoutElseAndElseIfs($node)) { + if (! $this->isIfWithoutElseAndElseIfs($if)) { return false; } - return $this->hasOnlyStmtOfType($node, $stmtClass); + return $this->hasOnlyStmtOfType($if, $stmtClass); } public function isIfWithOnlyOneStmt(If_ $if): bool diff --git a/tests/Issues/RemoveUnusedPrivateDuplicatedIf/Fixture/skip_used_in_array_map.php.inc b/tests/Issues/RemoveUnusedPrivateDuplicatedIf/Fixture/skip_used_in_array_map.php.inc deleted file mode 100644 index 755f9df3d1b..00000000000 --- a/tests/Issues/RemoveUnusedPrivateDuplicatedIf/Fixture/skip_used_in_array_map.php.inc +++ /dev/null @@ -1,19 +0,0 @@ - self::returnFoo(), - ['bar'] - ); - } - - private static function returnFoo(): string - { - return 'foo'; - } -} diff --git a/tests/Issues/RemoveUnusedPrivateDuplicatedIf/RemoveUnusedPrivateDuplicatedIfTest.php b/tests/Issues/RemoveUnusedPrivateDuplicatedIf/RemoveUnusedPrivateDuplicatedIfTest.php deleted file mode 100644 index 917d2afa98b..00000000000 --- a/tests/Issues/RemoveUnusedPrivateDuplicatedIf/RemoveUnusedPrivateDuplicatedIfTest.php +++ /dev/null @@ -1,28 +0,0 @@ -doTestFile($filePath); - } - - public static function provideData(): Iterator - { - return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); - } - - public function provideConfigFilePath(): string - { - return __DIR__ . '/config/configured_rule.php'; - } -} diff --git a/tests/Issues/RemoveUnusedPrivateDuplicatedIf/config/configured_rule.php b/tests/Issues/RemoveUnusedPrivateDuplicatedIf/config/configured_rule.php deleted file mode 100644 index a943f0214b8..00000000000 --- a/tests/Issues/RemoveUnusedPrivateDuplicatedIf/config/configured_rule.php +++ /dev/null @@ -1,11 +0,0 @@ -rules([RemoveUnusedPrivateMethodRector::class, RemoveDuplicatedIfReturnRector::class]); -};