How to Detect Dead PHP Code in Code Review in 7 Snippets

This post was updated at November 2020

Switch from deprecated --set option to rector.php config.


After few long Nette to Symfony migration series, it's time for relax.
Let's look at 7 snippets of PHP code, that happily takes your attention but is never run.

Imagine you're doing a code review of "various improvements" pull-request with 150 changed files...

Ok, not that one. Let's go only for 7 changed files. There will be 7 code snippets, each possibly with extra code that looks useful, but it doesn't do anything really.

Then click on the button to see if you were right :)

1. Duplicated Array Key

<?php

$items = [
    1 => 'A',
    1 => 'B',
    1 => 'C'
];

What is 1?

<?php

var_dump($items[1]);

"A" or "C"? Or Array with all fo them?

See Result

2. Key with Key-Hole

<?php

$items = [];
$result = [];
foreach ($items as $key => $value) {
    $result[] = $value;
}

What value is extra here?

See Result




-foreach ($items as $key => $value) {
+foreach ($items as $value) {
     $result[] = $value;
 }

3. Call for Nothing

<?php

class ParentButNoMethod extends ParentMethod
{
    public function one()
    {
        parent::one();
    }

    public function two()
    {
        parent::two();
    }
}

class ParentMethod
{
    public function one()
    {
    }
}

See Result




<?php

class ParentButNoMethod extends ParentMethod
{
    public function one()
    {
        parent::one();
    }

    public function two()
    {
-       parent::two();
    }
}

class ParentMethod
{
    public function one()
    {
    }
}

4. Reborn?

<?php

class ProductController
{
    public function actionDiscount(Product $product)
    {
        $discount = $this->getDiscount();
        $productCategory = $this->categoryRepository->findCategoriesByProduct(
            $product->getCategory()
        );
        $discount = $this->getDiscount();

        return $this->render('product/discount.twig', [
            'discount' => $discount,
            'product' => $product,
            'productCategory' => $productCategory,
        ]);
    }
}

See Result




-$discount = $this->getDiscount();
 $productCategory = $this->categoryRepository->findCategoriesByProduct(
     $product->getCategory()
 );
 $discount = $this->getDiscount();

5. Behind the Mirror

<?php

final class TimeMachine
{
    public function mirrorFunction(Quiz $quiz)
    {
        $timeLimit = $this->resolveTimeLimitForThisTest();
        if ($timeLimit >= 20) {
            return false;
        }

        $timeLimit = $timeLimit;
        if ($this->isQuizFinished($quiz)) {
            $correctQuestions = 1;
            $correctQuestions = $correctQuestions;
            $incorrectQuestions = $correctQuestions - 3;
        }
    }
}

See Result




 <?php

 final class TimeMachine
 {
     public function mirrorFunction(Quiz $quiz)
     {
         $timeLimit = $this->resolveTimeLimitForThisTest();
         if ($timeLimit >= 20) {
             return false;
         }

-        $timeLimit = $timeLimit;
         if ($this->isQuizFinished($quiz)) {
             $correctQuestions = 1;
-            $correctQuestions = $correctQuestions;
             $incorrectQuestions = $correctQuestions - 3;
         }
     }
 }

6. Rinse & Repeat

<?php

final class WhateverMethodCall
{
    public function run()
    {
        $directories = 1;
        $anotherDirectories = 1;
        $directories = 2;
        $this->store($directories);
        $anotherDirectories = 2;
        $directories = 3;
        $anotherDirectories = 3;
        $directories = 4;
        $directories = 5;
        return $directories + $anotherDirectories;
    }
    public function store(int $directories)
    {
    }
}

See Result




<?php

final class WhateverMethodCall
{
    public function run()
    {
-       $directories = 1;
-       $anotherDirectories = 1;
        $directories = 2;
        $this->store($directories);
-       $anotherDirectories = 2;
-       $directories = 3;
        $anotherDirectories = 3;
-       $directories = 4;
        $directories = 5;
        return $directories + $anotherDirectories;
    }
}

7. Privates that No-One See

<?php

final class SomeController
{
    private const MAX_LIMIT = 5;

    private const LIMIT = 5;

    private $cachedValues = [];

    private $cachedItems = [];

    public function run()
    {
        $values = $this->repeat();
        $values[] = 5;

        return $values + $this->cachedItems;
    }

    private function repeat()
    {
        $items = [];
        while ($this->fetch() && $this->fetch() < self::LIMIT) {
            $items[] = $this->fetch();
            $this->cachedItems[] = $this->fetch();
        }

        return $items;
    }

    private function fetch()
    {
        return mt_rand(1, 15);
    }

    private function clear()
    {
        $this->cachedItems = [];
    }
}

See Result




<?php

final class SomeController
{
-    private const MAX_LIMIT = 5;

     private const LIMIT = 5;

-    private $cachedValues = [];

     private $cachedItems = [];

     public function run()
     {
         $values = $this->repeat();
         $values[] = 5;

         return $values + $this->cachedItems;
     }

     private function repeat()
     {
         $items = [];
         while ($this->fetch() && $this->fetch() < self::LIMIT) {
             $items[] = $this->fetch();
             $this->cachedItems[] = $this->fetch();
         }

         return $items;
     }

     private function fetch()
     {
         return mt_rand(1, 15);
     }

-    private function clear()
-    {
-        $this->cachedItems = [];
-    }
}

You're in the Finish!

No wonder people don't do code-review (right), there is no time and it's often super boring.


What if there would be a way to automate all that checks above + 10 more with a CI tool. Something that:

Have you tried Rector?

Rector doesn't only refactor applications from one framework to another, upgrade your codebase and get you out of legacy. It can be also part of your CI:

  1. Install Rector
composer require rector/rector --dev
  1. Update rector.php with dead code set
use Rector\Core\Configuration\Option;
use Rector\Set\ValueObject\SetList;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $containerConfigurator): void {
    $parameters = $containerConfigurator->parameters();
    $parameters->set(Option::SETS, [
        SetList::DEAD_CODE,
    ]);
};
  1. Run Rector
vendor/bin/rector process src --dry-run

If Rector detects any dead code, CI will fail. You can, of course, run it without --dry-run after to actually remove the code.

See Dead Code set for more features.