Skip to content

Commit

Permalink
[DeadCode] Allow indirect duplicated grouping on RemoveDuplicatedCase…
Browse files Browse the repository at this point in the history
…InSwitchRector (#5234)

* [DeadCode] Allow indirect duplicated grouping on RemoveDuplicatedCaseInSwitchRector

* implemented 🎉

* [ci-review] Rector Rectify

---------

Co-authored-by: GitHub Action <actions@github.com>
  • Loading branch information
samsonasik and actions-user committed Nov 9, 2023
1 parent 437f96a commit 3f2fb29
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

namespace Rector\Tests\DeadCode\Rector\Switch_\RemoveDuplicatedCaseInSwitchRector\Fixture;

class IndirectDuplicated
{
public function run()
{
switch ($name) {
case 'clearAllHeaders':
return $this->modifyHeader($node, 'replace');
case 'clearHeader':
return $this->modifyHeader($node, 'remove');
case 'clearRawHeaders':
return $this->modifyHeader($node, 'replace');
case '...':
return 5;
}
}
}

?>
-----
<?php

namespace Rector\Tests\DeadCode\Rector\Switch_\RemoveDuplicatedCaseInSwitchRector\Fixture;

class IndirectDuplicated
{
public function run()
{
switch ($name) {
case 'clearAllHeaders':
case 'clearRawHeaders':
return $this->modifyHeader($node, 'replace');
case 'clearHeader':
return $this->modifyHeader($node, 'remove');
case '...':
return 5;
}
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

namespace Rector\Tests\DeadCode\Rector\Switch_\RemoveDuplicatedCaseInSwitchRector\Fixture;

class MultiIndirectDuplicated
{
public function run()
{
switch ($name) {
case 'clearAllHeaders':
return $this->modifyHeader($node, 'replace');
case 'clearHeader':
return $this->modifyHeader($node, 'remove');
case 'clearRawHeaders':
return $this->modifyHeader($node, 'replace');
case 'clearRawHeaders2':
return $this->modifyHeader($node, 'replace');
case '...':
return 5;
}
}
}

?>
-----
<?php

namespace Rector\Tests\DeadCode\Rector\Switch_\RemoveDuplicatedCaseInSwitchRector\Fixture;

class MultiIndirectDuplicated
{
public function run()
{
switch ($name) {
case 'clearAllHeaders':
case 'clearRawHeaders':
case 'clearRawHeaders2':
return $this->modifyHeader($node, 'replace');
case 'clearHeader':
return $this->modifyHeader($node, 'remove');
case '...':
return 5;
}
}
}

?>
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/
final class RemoveDuplicatedCaseInSwitchRector extends AbstractRector
{
private bool $hasChanged = false;

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
Expand Down Expand Up @@ -82,28 +84,90 @@ public function refactor(Node $node): ?Node
return null;
}

$this->hasChanged = false;

$insertByKeys = $this->resolveInsertedByKeys($node);
$this->insertCaseByKeys($node, $insertByKeys);
$this->cleanUpEqualCaseStmts($node);

if (! $this->hasChanged) {
return null;
}

return $node;
}

/**
* @return array<int, array<int, Case_>>
*/
private function resolveInsertedByKeys(Switch_ $switch): array
{
$totalKeys = count($switch->cases);
$insertByKeys = [];

foreach ($switch->cases as $key => $case) {
if ($case->stmts === []) {
continue;
}

$nextKey = $key + 1;
for ($jumpToKey = $key + 1; $jumpToKey < $totalKeys; ++$jumpToKey) {
if (! isset($switch->cases[$jumpToKey])) {
continue;
}

if (! $this->areSwitchStmtsEqualsAndWithBreak($case, $switch->cases[$jumpToKey])) {
continue;
}

if ($nextKey === $jumpToKey) {
continue 2;
}

$nextCase = $switch->cases[$jumpToKey];

unset($switch->cases[$jumpToKey]);

$insertByKeys[$key][] = $nextCase;

$this->hasChanged = true;
}
}

return $insertByKeys;
}

/**
* @param array<int, array<int, Case_>> $insertByKeys
*/
private function insertCaseByKeys(Switch_ $switch, array $insertByKeys): void
{
foreach ($insertByKeys as $key => $insertByKey) {
$switch->cases[$key]->stmts = [];
$nextKey = $key + 1;

array_splice($switch->cases, $nextKey, 0, $insertByKey);
}
}

private function cleanUpEqualCaseStmts(Switch_ $switch): void
{
/** @var Case_|null $previousCase */
$previousCase = null;
$hasChanged = false;
foreach ($node->cases as $case) {
foreach ($switch->cases as $case) {
if ($previousCase instanceof Case_ && $this->areSwitchStmtsEqualsAndWithBreak($case, $previousCase)) {
$previousCase->stmts = [];
$hasChanged = true;

$this->hasChanged = true;
}

$previousCase = $case;
}

if (! $hasChanged) {
return null;
}

return $node;
}

private function areSwitchStmtsEqualsAndWithBreak(Case_ $currentCase, Case_ $previousCase): bool
private function areSwitchStmtsEqualsAndWithBreak(Case_ $currentCase, Case_ $nextCase): bool
{
if (! $this->nodeComparator->areNodesEqual($currentCase->stmts, $previousCase->stmts)) {
if (! $this->nodeComparator->areNodesEqual($currentCase->stmts, $nextCase->stmts)) {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Rector\Php74\Rector\Property;

use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Stmt\Class_;
Expand All @@ -21,7 +22,7 @@
final class RestoreDefaultNullToNullableTypePropertyRector extends AbstractRector implements MinPhpVersionInterface
{
public function __construct(
private readonly ConstructorAssignDetector $constructorAssignDetector
private readonly ConstructorAssignDetector $constructorAssignDetector, private readonly PhpDocInfoFactory $phpDocInfoFactory
) {
}

Expand Down

0 comments on commit 3f2fb29

Please sign in to comment.