-
-
Notifications
You must be signed in to change notification settings - Fork 416
[dead-code] Add RemoveUnusedClosureVariableUseRector #7430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| <?php | ||
|
|
||
| namespace Rector\Tests\DeadCode\Rector\Closure\RemoveUnusedClosureVariableUseRector\Fixture; | ||
|
|
||
| class Fixture | ||
| { | ||
| public function run($value) | ||
| { | ||
| return function () use ($value) { | ||
| return 'value'; | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| ?> | ||
| ----- | ||
| <?php | ||
|
|
||
| namespace Rector\Tests\DeadCode\Rector\Closure\RemoveUnusedClosureVariableUseRector\Fixture; | ||
|
|
||
| class Fixture | ||
| { | ||
| public function run($value) | ||
| { | ||
| return function () { | ||
| return 'value'; | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| ?> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| <?php | ||
|
|
||
| namespace Rector\Tests\DeadCode\Rector\Closure\RemoveUnusedClosureVariableUseRector\Fixture; | ||
|
|
||
| final class SkipUsedVariable | ||
| { | ||
| public function run($value) | ||
| { | ||
| return function () use ($value) { | ||
| $result = $value + 100; | ||
|
|
||
| return $result; | ||
| }; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Rector\Tests\DeadCode\Rector\Closure\RemoveUnusedClosureVariableUseRector; | ||
|
|
||
| use Iterator; | ||
| use PHPUnit\Framework\Attributes\DataProvider; | ||
| use Rector\Testing\PHPUnit\AbstractRectorTestCase; | ||
|
|
||
| final class RemoveUnusedClosureVariableUseRectorTest extends AbstractRectorTestCase | ||
| { | ||
| #[DataProvider('provideData')] | ||
| public function test(string $filePath): void | ||
| { | ||
| $this->doTestFile($filePath); | ||
| } | ||
|
|
||
| public static function provideData(): Iterator | ||
| { | ||
| return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); | ||
| } | ||
|
|
||
| public function provideConfigFilePath(): string | ||
| { | ||
| return __DIR__ . '/config/configured_rule.php'; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| use Rector\Config\RectorConfig; | ||
| use Rector\DeadCode\Rector\Concat\RemoveUnusedClosureVariableUseRector; | ||
|
|
||
| return RectorConfig::configure() | ||
| ->withRules([RemoveUnusedClosureVariableUseRector::class]); |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,93 @@ | ||||||||
| <?php | ||||||||
|
|
||||||||
| declare(strict_types=1); | ||||||||
|
|
||||||||
| namespace Rector\DeadCode\Rector\Concat; | ||||||||
|
|
||||||||
| use PhpParser\Node; | ||||||||
| use PhpParser\Node\Expr\Closure; | ||||||||
| use Rector\PhpParser\Node\BetterNodeFinder; | ||||||||
| use Rector\Rector\AbstractRector; | ||||||||
| use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; | ||||||||
| use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; | ||||||||
|
|
||||||||
| /** | ||||||||
| * @see \Rector\Tests\DeadCode\Rector\Closure\RemoveUnusedClosureVariableUseRector\RemoveUnusedClosureVariableUseRectorTest | ||||||||
| */ | ||||||||
| final class RemoveUnusedClosureVariableUseRector extends AbstractRector | ||||||||
| { | ||||||||
| public function __construct( | ||||||||
| private readonly BetterNodeFinder $betterNodeFinder | ||||||||
| ) { | ||||||||
| } | ||||||||
|
|
||||||||
| public function getRuleDefinition(): RuleDefinition | ||||||||
| { | ||||||||
| return new RuleDefinition('Remove unused variable in use() of closure', [ | ||||||||
| new CodeSample( | ||||||||
| <<<'CODE_SAMPLE' | ||||||||
| $var = 1; | ||||||||
|
|
||||||||
| $closure = function() use ($var) { | ||||||||
| echo 'Hello World'; | ||||||||
| }; | ||||||||
|
|
||||||||
| CODE_SAMPLE | ||||||||
|
|
||||||||
| , | ||||||||
| <<<'CODE_SAMPLE' | ||||||||
| $var = 1; | ||||||||
| $closure = function() { | ||||||||
| echo 'Hello World'; | ||||||||
| }; | ||||||||
|
|
||||||||
| CODE_SAMPLE | ||||||||
| ), | ||||||||
|
|
||||||||
| ]); | ||||||||
| } | ||||||||
|
|
||||||||
| /** | ||||||||
| * @return array<class-string<Node>> | ||||||||
| */ | ||||||||
| public function getNodeTypes(): array | ||||||||
| { | ||||||||
| return [Closure::class]; | ||||||||
| } | ||||||||
|
|
||||||||
| /** | ||||||||
| * @param Closure $node | ||||||||
| */ | ||||||||
| public function refactor(Node $node): ?Node | ||||||||
| { | ||||||||
| if ($node->uses === []) { | ||||||||
| return null; | ||||||||
| } | ||||||||
|
|
||||||||
| $hasChanged = false; | ||||||||
|
|
||||||||
| foreach ($node->uses as $key => $useVariable) { | ||||||||
| $useVariableName = $this->getName($useVariable->var); | ||||||||
| if (! is_string($useVariableName)) { | ||||||||
| continue; | ||||||||
| } | ||||||||
|
|
||||||||
| $isUseUsed = (bool) $this->betterNodeFinder->findVariableOfName($node->stmts, $useVariableName); | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should verify
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I created new PR to skip used via |
||||||||
| if ($isUseUsed) { | ||||||||
| continue; | ||||||||
| } | ||||||||
|
|
||||||||
| unset($node->uses[$key]); | ||||||||
| $hasChanged = true; | ||||||||
|
|
||||||||
| } | ||||||||
|
|
||||||||
| if ($hasChanged) { | ||||||||
| // reset keys, to keep as expected | ||||||||
| $node->uses = array_values($node->uses); | ||||||||
|
Comment on lines
+86
to
+87
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not needed, already covered at rector-src/src/Application/NodeAttributeReIndexer.php Lines 69 to 71 in d948383
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I created new PR for it: |
||||||||
| return $node; | ||||||||
| } | ||||||||
|
|
||||||||
| return null; | ||||||||
| } | ||||||||
| } | ||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should under
Closurenamespace:I will create new PR for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I create new PR to fix namespace to
Closure