Skip to content

[DeadCode] Skip key used in next stmt on RemoveUnusedForeachKeyRector #5153

Merged
samsonasik merged 4 commits intomainfrom
fix-skip-key-used
Oct 10, 2023
Merged

[DeadCode] Skip key used in next stmt on RemoveUnusedForeachKeyRector #5153
samsonasik merged 4 commits intomainfrom
fix-skip-key-used

Conversation

@samsonasik
Copy link
Copy Markdown
Member

@samsonasik
Copy link
Copy Markdown
Member Author

Fixed 🎉 /cc @ethernidee

@samsonasik
Copy link
Copy Markdown
Member Author

All checks have passed 🎉 @TomasVotruba I am merging it ;)

@samsonasik samsonasik merged commit 6d96068 into main Oct 10, 2023
@samsonasik samsonasik deleted the fix-skip-key-used branch October 10, 2023 11:22
continue;
}

if ($this->stmtsManipulator->isVariableUsedInNextStmt($node, $key + 1, (string) $this->getName($keyVar))) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean, the logic only works when the var usage is directly after the loop?

the initial problem also exists on e.g.

<?php

namespace Rector\Tests\DeadCode\Rector\Foreach_\RemoveUnusedForeachKeyRector\Fixture;

final class SkipKeyUsedInNexStmt
{
    function lastKey ($container, $defKey = null) {
        $result = $defKey;

        foreach ($container as $result => $value) {
          // next
        }

       $x = 1;
       $y = 2;

        return $result;
      }
}

Copy link
Copy Markdown
Member Author

@samsonasik samsonasik Oct 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean no, that checked with array_slice() :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is check all next stmts:

        $stmts = array_slice($stmtsAware->stmts, $jumpToKey, null, true);
        return (bool) $this->betterNodeFinder->findVariableOfName($stmts, $variableName);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Ruleset DEAD_CODE original logic violation

3 participants