Skip to content

Commit

Permalink
[DeadCode] Add RemoveUnusedParamInRequiredAutowireRector (#1301)
Browse files Browse the repository at this point in the history
* [DeadCode] Add RemoveUnusedParamInRequiredAutowireRector

* [ci-review] Rector Rectify

* update example

* skip if no required annotation or Required attribute

* update example

* implemented

* add test

* [ci-review] Rector Rectify

* more fixture

* more fixture

* add see test

* final touch: collect variables first, then loop collection

Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
samsonasik and actions-user authored Nov 25, 2021
1 parent cfae725 commit 9ba0f63
Show file tree
Hide file tree
Showing 8 changed files with 242 additions and 2 deletions.
2 changes: 2 additions & 0 deletions config/set/dead-code.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Rector\DeadCode\Rector\ClassMethod\RemoveEmptyClassMethodRector;
use Rector\DeadCode\Rector\ClassMethod\RemoveLastReturnRector;
use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedConstructorParamRector;
use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedParamInRequiredAutowireRector;
use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodParameterRector;
use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector;
use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPromotedPropertyRector;
Expand Down Expand Up @@ -91,6 +92,7 @@
$services->set(RemoveDeadInstanceOfRector::class);
$services->set(RemoveDeadLoopRector::class);
$services->set(RemoveUnusedPrivateMethodParameterRector::class);
$services->set(RemoveUnusedParamInRequiredAutowireRector::class);

// docblock
$services->set(RemoveUselessParamTagRector::class);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedParamInRequiredAutowireRector\Fixture;

class SkipNoRequiredAttributeOrAnnotation
{
private $visibilityManipulator;

public function something(VisibilityManipulator $visibilityManipulator)
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedParamInRequiredAutowireRector\Fixture;

class WithRequiredAnnotation
{
private $visibilityManipulator;

/** @required */
public function autowireSomeService(VisibilityManipulator $visibilityManipulator)
{
}
}

?>
-----
<?php

namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedParamInRequiredAutowireRector\Fixture;

class WithRequiredAnnotation
{
private $visibilityManipulator;

/** @required */
public function autowireSomeService()
{
}
}

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

namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedParamInRequiredAutowireRector\Fixture;

use Symfony\Contracts\Service\Attribute\Required;

class WithRequiredAttribute
{
private $visibilityManipulator;

#[Required]
public function autowireSomeService(VisibilityManipulator $visibilityManipulator)
{
}
}

?>
-----
<?php

namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedParamInRequiredAutowireRector\Fixture;

use Symfony\Contracts\Service\Attribute\Required;

class WithRequiredAttribute
{
private $visibilityManipulator;

#[Required]
public function autowireSomeService()
{
}
}

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

declare(strict_types=1);

namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedParamInRequiredAutowireRector;

use Iterator;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
use SplFileInfo;
use Symplify\SmartFileSystem\SmartFileInfo;

final class RemoveUnusedParamInRequiredAutowireRectorTest extends AbstractRectorTestCase
{
/**
* @dataProvider provideData()
*/
public function test(SmartFileInfo $fileInfo): void
{
$this->doTestFileInfo($fileInfo);
}

/**
* @return Iterator<SplFileInfo>
*/
public function provideData(): Iterator
{
return $this->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,11 @@
<?php

declare(strict_types=1);

use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedParamInRequiredAutowireRector;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $containerConfigurator): void {
$services = $containerConfigurator->services();
$services->set(RemoveUnusedParamInRequiredAutowireRector::class);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
<?php

declare(strict_types=1);

namespace Rector\DeadCode\Rector\ClassMethod;

use PhpParser\Node;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt\ClassMethod;
use Rector\Core\Rector\AbstractRector;
use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @see \Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUnusedParamInRequiredAutowireRector\RemoveUnusedParamInRequiredAutowireRectorTest
*/
final class RemoveUnusedParamInRequiredAutowireRector extends AbstractRector
{
public function __construct(
private PhpAttributeAnalyzer $phpAttributeAnalyzer
) {
}

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Remove unused parameter in required autowire method', [
new CodeSample(
<<<'CODE_SAMPLE'
use Symfony\Contracts\Service\Attribute\Required;
final class SomeService
{
private $visibilityManipulator;
#[Required]
public function autowireSomeService(VisibilityManipulator $visibilityManipulator)
{
}
}
CODE_SAMPLE

,
<<<'CODE_SAMPLE'
use Symfony\Contracts\Service\Attribute\Required;
final class SomeService
{
private $visibilityManipulator;
#[Required]
public function autowireSomeService()
{
}
}
CODE_SAMPLE
),
]);
}

/**
* @return array<class-string<Node>>
*/
public function getNodeTypes(): array
{
return [ClassMethod::class];
}

/**
* @param ClassMethod $node
*/
public function refactor(Node $node): ?Node
{
if ($this->shouldSkip($node)) {
return null;
}

$params = $node->params;
if ($params === []) {
return null;
}

/** @var Variable[] $variables */
$variables = $this->betterNodeFinder->findInstanceOf((array) $node->getStmts(), Variable::class);
$hasRemovedParam = false;

foreach ($params as $param) {
$paramVar = $param->var;
foreach ($variables as $variable) {
if ($this->nodeComparator->areNodesEqual($variable, $paramVar)) {
continue 2;
}
}

$this->removeNode($param);
$hasRemovedParam = true;
}

if (! $hasRemovedParam) {
return null;
}

return $node;
}

private function shouldSkip(ClassMethod $classMethod): bool
{
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod);
$hasRequiredAnnotation = $phpDocInfo->hasByName('required');
$hasRequiredAttribute = $this->phpAttributeAnalyzer->hasPhpAttribute(
$classMethod,
'Symfony\Contracts\Service\Attribute\Required'
);

return ! $hasRequiredAnnotation && ! $hasRequiredAttribute;
}
}
2 changes: 0 additions & 2 deletions src/Rector/AbstractRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
use Rector\NodeTypeResolver\NodeTypeResolver;
use Rector\PostRector\Collector\NodesToAddCollector;
use Rector\PostRector\Collector\NodesToRemoveCollector;
use Rector\Privatization\NodeManipulator\VisibilityManipulator;
use Rector\StaticTypeMapper\StaticTypeMapper;
use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Contracts\Service\Attribute\Required;
Expand Down Expand Up @@ -142,7 +141,6 @@ public function autowireAbstractRector(
NodeNameResolver $nodeNameResolver,
NodeTypeResolver $nodeTypeResolver,
SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
VisibilityManipulator $visibilityManipulator,
NodeFactory $nodeFactory,
PhpDocInfoFactory $phpDocInfoFactory,
SymfonyStyle $symfonyStyle,
Expand Down

0 comments on commit 9ba0f63

Please sign in to comment.