Skip to content

Commit

Permalink
Implement const auto-removal
Browse files Browse the repository at this point in the history
  • Loading branch information
janedbal committed Nov 14, 2024
1 parent ed49b7b commit 8358d25
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 86 deletions.
25 changes: 16 additions & 9 deletions src/Formatter/RemoveDeadCodeFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use PHPStan\Command\Output;
use ShipMonk\PHPStan\DeadCode\Rule\DeadMethodRule;
use ShipMonk\PHPStan\DeadCode\Transformer\FileSystem;
use ShipMonk\PHPStan\DeadCode\Transformer\RemoveMethodCodeTransformer;
use ShipMonk\PHPStan\DeadCode\Transformer\RemoveDeadCodeTransformer;
use function count;

class RemoveDeadCodeFormatter implements ErrorFormatter
Expand Down Expand Up @@ -37,33 +37,40 @@ public function formatErrors(
return 1;
}

$deadMethodKeysByFiles = [];
$deadMembersByFiles = [];

foreach ($analysisResult->getFileSpecificErrors() as $fileSpecificError) {
if ($fileSpecificError->getIdentifier() !== DeadMethodRule::IDENTIFIER_METHOD) {
if (
$fileSpecificError->getIdentifier() !== DeadMethodRule::IDENTIFIER_METHOD
&& $fileSpecificError->getIdentifier() !== DeadMethodRule::IDENTIFIER_CONSTANT
) {
continue;
}

/** @var array<string, array{file: string, line: string}> $metadata */
/** @var array<string, array{file: string}> $metadata */
$metadata = $fileSpecificError->getMetadata();
$type = $fileSpecificError->getIdentifier();

foreach ($metadata as $key => $data) {
$deadMethodKeysByFiles[$data['file']][] = $key;
$deadMembersByFiles[$data['file']][$type][] = $key;
}
}

$count = 0;

foreach ($deadMethodKeysByFiles as $file => $blackMethodKeys) {
$count += count($blackMethodKeys);
foreach ($deadMembersByFiles as $file => $deadMembersByType) {
$deadConstants = $deadMembersByType[DeadMethodRule::IDENTIFIER_CONSTANT] ?? [];
$deadMethods = $deadMembersByType[DeadMethodRule::IDENTIFIER_METHOD] ?? [];

$transformer = new RemoveMethodCodeTransformer($blackMethodKeys);
$count += count($deadConstants) + count($deadMethods);

$transformer = new RemoveDeadCodeTransformer($deadMethods, $deadConstants);
$oldCode = $this->fileSystem->read($file);
$newCode = $transformer->transformCode($oldCode);
$this->fileSystem->write($file, $newCode);
}

$output->writeLineFormatted('Removed ' . $count . ' dead methods in ' . count($deadMethodKeysByFiles) . ' files.');
$output->writeLineFormatted('Removed ' . $count . ' dead methods in ' . count($deadMembersByFiles) . ' files.');

return 0;
}
Expand Down
103 changes: 103 additions & 0 deletions src/Transformer/RemoveClassMemberVisitor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
<?php declare(strict_types = 1);

namespace ShipMonk\PHPStan\DeadCode\Transformer;

use PhpParser\Node;
use PhpParser\Node\Const_;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\Stmt\ClassConst;
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Namespace_;
use PhpParser\NodeTraverser;
use PhpParser\NodeVisitorAbstract;
use function array_fill_keys;
use function ltrim;

class RemoveClassMemberVisitor extends NodeVisitorAbstract
{

private string $currentNamespace = '';

private string $currentClass = '';

/**
* @var array<string, true>
*/
private array $deadMethods;

/**
* @var array<string, true>
*/
private array $deadConstants;

/**
* @param list<string> $deadMethods
* @param list<string> $deadConstants
*/
public function __construct(array $deadMethods, array $deadConstants)
{
$this->deadMethods = array_fill_keys($deadMethods, true);
$this->deadConstants = array_fill_keys($deadConstants, true);
}

public function enterNode(Node $node): ?Node
{
if ($node instanceof Namespace_ && $node->name !== null) {
$this->currentNamespace = $node->name->toString();

} elseif ($node instanceof ClassLike && $node->name !== null) {
$this->currentClass = $node->name->name;
}

return null;
}

public function leaveNode(Node $node): ?int
{
if ($node instanceof ClassMethod) {
$methodKey = $this->getNamespacedName($node->name);

if (isset($this->deadMethods[$methodKey])) {
return NodeTraverser::REMOVE_NODE;
}
}

if ($node instanceof ClassConst) {
$allDead = true;

foreach ($node->consts as $const) {
$constKey = $this->getNamespacedName($const->name);

if (!isset($this->deadConstants[$constKey])) {
$allDead = false;
break;
}
}

if ($allDead) {
return NodeTraverser::REMOVE_NODE;
}
}

if ($node instanceof Const_) {
$constKey = $this->getNamespacedName($node->name);

if (isset($this->deadConstants[$constKey])) {
return NodeTraverser::REMOVE_NODE;
}
}

return null;
}

/**
* @param Name|Identifier $name
*/
private function getNamespacedName(Node $name): string
{
return ltrim($this->currentNamespace . '\\' . $this->currentClass, '\\') . '::' . $name->name;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use PhpParser\Parser\Php8;
use PhpParser\PrettyPrinter\Standard as PhpPrinter;

class RemoveMethodCodeTransformer
class RemoveDeadCodeTransformer
{

private Lexer $phpLexer;
Expand All @@ -24,9 +24,10 @@ class RemoveMethodCodeTransformer
private PhpPrinter $phpPrinter;

/**
* @param list<string> $deadMethodKeys
* @param list<string> $deadMethods
* @param list<string> $deadConstants
*/
public function __construct(array $deadMethodKeys)
public function __construct(array $deadMethods, array $deadConstants)
{
$this->phpLexer = new Lexer();
$this->phpParser = new Php8($this->phpLexer);
Expand All @@ -35,7 +36,7 @@ public function __construct(array $deadMethodKeys)
$this->cloningTraverser->addVisitor(new CloningVisitor());

$this->removingTraverser = new PhpTraverser();
$this->removingTraverser->addVisitor(new RemoveMethodVisitor($deadMethodKeys));
$this->removingTraverser->addVisitor(new RemoveClassMemberVisitor($deadMethods, $deadConstants));

$this->phpPrinter = new PhpPrinter();
}
Expand Down
59 changes: 0 additions & 59 deletions src/Transformer/RemoveMethodVisitor.php

This file was deleted.

8 changes: 4 additions & 4 deletions tests/AllServicesInConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
use ShipMonk\PHPStan\DeadCode\Crate\Visibility;
use ShipMonk\PHPStan\DeadCode\Provider\MethodEntrypointProvider;
use ShipMonk\PHPStan\DeadCode\Provider\SimpleMethodEntrypointProvider;
use ShipMonk\PHPStan\DeadCode\Transformer\RemoveMethodCodeTransformer;
use ShipMonk\PHPStan\DeadCode\Transformer\RemoveMethodVisitor;
use ShipMonk\PHPStan\DeadCode\Transformer\RemoveClassMemberVisitor;
use ShipMonk\PHPStan\DeadCode\Transformer\RemoveDeadCodeTransformer;
use function array_keys;
use function array_merge;
use function class_exists;
Expand Down Expand Up @@ -59,8 +59,8 @@ public function test(): void
Visibility::class,
MethodEntrypointProvider::class,
SimpleMethodEntrypointProvider::class,
RemoveMethodCodeTransformer::class,
RemoveMethodVisitor::class,
RemoveDeadCodeTransformer::class,
RemoveClassMemberVisitor::class,
];

/** @var DirectoryIterator $file */
Expand Down
4 changes: 2 additions & 2 deletions tests/Rule/data/DeadMethodRule/removing/no-namespace.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

interface Remove
{
public function dead(): void // error: Unused Remove::dead
;
const DEAD = 1;
public function dead(): void;
}
20 changes: 13 additions & 7 deletions tests/Rule/data/DeadMethodRule/removing/sample.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,38 @@

interface Interface1
{
public function foo(): void // error: Unused Removal\Interface1::foo
;
const DEAD_IFACE_CONST1 = 1;
const DEAD_IFACE_CONST2 = 2, DEAD_IFACE_CONST3 = 3;

public function foo(): void;
}

interface Interface2
{
public function foo(): void // error: Unused Removal\Interface2::foo
;
public function foo(): void;
}

abstract class AbstractClass implements Interface1, Interface2
{
public abstract function foo(): void // error: Unused Removal\AbstractClass::foo
public abstract function foo(): void
;
}

class Child1 extends AbstractClass
{
public function foo(): void // error: Unused Removal\Child1::foo
public function foo(): void
{
}
}

class Child2 extends AbstractClass
{
public function foo(): void {}
const USED_CONST = 1, DEAD_CONST = 2;

public function foo(): void
{
echo self::USED_CONST;
}
}

function testIt(Child2 $child2): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ class Child1 extends AbstractClass

class Child2 extends AbstractClass
{
public function foo(): void {}
const USED_CONST = 1;

public function foo(): void
{
echo self::USED_CONST;
}
}

function testIt(Child2 $child2): void
Expand Down

0 comments on commit 8358d25

Please sign in to comment.