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

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:

composer require rector/rector --dev
vendor/bin/rector process src --set dead-code --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-level set for more features.


Since last post you know how to find unused public methods. I'm planning to add the same feature Rector, just smarter (= context-aware) and for public constants and properties as well.


Found a typo? Fix it to join team of 73 people that improve content here

Rector Is Rector saving you time and money?
Support it at Patreon so it grows and saves you even more!