Removing Unused Variables Is Good

My favorite code clean is removing unused variables. This is right : variables that are hosted in the code, yet doing nothing. In fact, they are often toxic and grow bug-prone. Any old enough code source has them, and they tend to grow by the number. Have you ever tried spotting and removing those variables? Well, removing unused variables is my world and welcome to it!

What Is An Unused Variable

The simplest form and definition of unused variables are variables that are assigned, and never used. We can start with an example like this one:

<?php

$unused = 1;
$used = 2;
echo $used;

?>

The $unused variable is created, with the value 1; Then, the script ends. On the other hand, the value 2 is assigned to the variable $used, then used for display. It is not much, but it is honest work.

As usual, with code, this illustration has variations. In particular, variables are dependent on a context, so the end of the script is not necessary : the end of a method is sufficient. Also, note how $arg, being a reference, is used once, but not unused : we need to check the calling context too. This would apply to global variables and properties.

<?php

function foo(&$arg) {
    $arg = 3;
    $unused = 1;
    $used = 2;
    return $used;
}

?>

The second variation style is when the variable is prepared. This is the case when it is augmented or formatted, before usage. That way, the variable is created, then used to update itself. Such a variable is unused if it is not used in the end. This may cover situations like the following :

<?php

$unused = array();
$unused[] =  1;        // This could have been merged with the above too.

$unusedToo = 'The elephpant that roamed the world';
$unusedToo =  strtolower($unusedToo);           
                                      // Assigned, lower-cased and forgotten

$unusedThree = range(0, 3);
$unusedThree = array_merge($unusedToo, $b);
                                      // Completed with another variable. 
                                      // Once $unusedThree is removed, $b will a natural candidate too

?>

The last example is the most complex of the three : the variable is created, augmented with a function call including itself and other values, and then, never used. The nature of the function called is important here : array_merge has no side effect, so this last expression doesn’t use the $unusedToo variable, even as it merges it with $b. It could have been very different if array_merge was replaced by a methodcall, that stored the value for later.

Finding Unused Variables With Static analysis

Finding unused variable might be quite laborious, and easily automatised. So, our friends the static analysis tools will find those pesky variables for us, in no time. We’ll use Exakat to run the errands for us.

Exakat offers analysis rules for unused variables : Variables/UsedOncePerContext and Variables/WrittenOnlyVariables. We can also take a look at Classes/UsedOnceProperty, which extends the concept to class properties.

Running the audit

After the installation process, the simplest way to get the above mentioned results is to use the Diplomat report. It is the default run, so you can use the following commands in your terminal :

cd /some/folder/with/exakat

php exakat.phar init -p robo -R https://github.com/consolidation/Robo -v
php exakat.phar project -v -p robo

The result is available in HTML form, in projects\robo\report\index.html.

If you prefer a text version of the results, you can read the Diplomat report :

cd /some/folder/with/exakat

php exakat.phar report -p robo --format Text -P Variables/VariableUsedOnceByContext -O Variables/WrittenOnlyVariable -P Classes/UsedOnceProperty

You will get a text result, similar to this one :

/src/Task/Archive/Extract.php:175   Variables/VariableUsedOnceByContext Used Once Variables (In Scope)  $status
/src/Task/Assets/ImageMinify.php:192    Variables/VariableUsedOnceByContext Used Once Variables (In Scope)  $url
/src/Task/Assets/ImageMinify.php:131    Classes/UsedOnceProperty    Used Once Property  $minifiers = ['optipng', 'gifsicle', 'jpegtran', 'svgo', 'pngquant', 'advpng', 'pngout', 'zopflipng', 'pngcrush', 'jpegoptim', 'jpeg-recompress',  ]
/RoboFile.php:25    Variables/VariableUsedOnceByContext Used Once Variables (In Scope)  $taskPHPUnit
/RoboFile.php:316   Variables/VariableUsedOnceByContext Used Once Variables (In Scope)  $m
/RoboFile.php:312   Variables/VariableUsedOnceByContext Used Once Variables (In Scope)  $m
/src/Runner.php:188 Variables/VariableUsedOnceByContext Used Once Variables (In Scope)  $container
/src/Runner.php:291 Variables/VariableUsedOnceByContext Used Once Variables (In Scope)  $color
/src/Runner.php:498 Variables/VariableUsedOnceByContext Used Once Variables (In Scope)  $argv
/src/Runner.php:149 Variables/VariableUsedOnceByContext Used Once Variables (In Scope)  $argv
/src/Log/ResultPrinter.php:98   Variables/VariableUsedOnceByContext Used Once Variables (In Scope)  $task

Reviewing robo.li

We’ll use the Robo.li, a smart task runner for PHP in PHP. The code is available at github, as you can see in the initcommand. It is also available on packagist.org. It’s a very useful tool for automating development, and we like it very much. This is also why we’re using it in this article.

Removing unused variables

We’ll start with the ‘Used once variables’. Those are variable that are used only once in the entire application. All in all, there is only one mention of the variable. This is a good candidate to be an unused variable, but we need to check if it is worth removing directly. The first we can visit is Robofile.php:25

    public function test(ConsoleIO $io, array $args, $options =
        [
            'coverage-html' => false,
            'coverage' => false
        ])
    {
        $io->warning("Deprecated: use 'composer test' instead. Codeception-based tests will fail.");

        $collection = $this->collectionBuilder($io);

        $taskPHPUnit = $collection->taskPHPUnit();

        $taskCodecept = $collection->taskCodecept()
            ->args($args);

        if ($options['coverage']) {
            $taskCodecept->coverageXml('../../build/logs/clover.xml');
        }
        if ($options['coverage-html']) {
            $taskCodecept->coverageHtml('../../build/logs/coverage');
        }

        return $collection;
     }

We can see $taskPHPUnit being assigned with the result of the task tastPHPunit(), but never used again. tastPHPunit() is the method that actually runs the tests, so, the method is important. Since the result is not use later in the method, there is no need to keep it. The next variable, $taskCodecept, on the other hand, is collected then used twice.

Congratulations! We just spotted our first unused variable. Let’s see a few of other situations.

Iffectations

The second one we’ll check is $status in the file src/Task/Archive/Extract.php:175. The code is below. It is a classic example of an ‘iffectation’ : an affectation in a if() condition. Here, $status is collected as the result of the condition, and not used later. The assignation may be removed, while keeping the !== comparison.

    protected function extractZip($extractLocation)
    {
        if (!extension_loaded('zlib')) {
            return Result::errorMissingExtension($this, 'zlib', 'zip extracting');
        }

        $zip = new \ZipArchive();
        if (($status = $zip->open($this->filename)) !== true) {
            return Result::error($this, "Could not open zip archive {$this->filename}");
        }
        if (!$zip->extractTo($extractLocation)) {
            return Result::error($this, "Could not extract zip archive {$this->filename}");
        }
        $zip->close();

        return Result::success($this);
    }

Blind variables

For the third one, we’ll switch to the ‘Used once variable (in scope)’ rule. You can select it in the Analysis column of the audit. We are looking at the file src/Collection/Collection.php:806. It is the following code, about variable $name. It is in the foreach() loop.

    public function transferTasks($builder)
    {
        foreach ($this->taskList as $name => $taskGroup) {
            // TODO: We are abandoning all of our before and after tasks here.
            // At the moment, transferTasks is only called under conditions where
            // there will be none of these, but care should be taken if that changes.
            $task = $taskGroup->getTask();
            $builder->addTaskToCollection($task);
        }
        $this->reset();
    }

$name is the key in the foreach loop. Only the value $taskGroup is used in the loop, which makes $name collected for nothing. It may be dropped, with a tiny improvement in performances. This is definitely a micro-optimisation, so you may assume it now worth your time.

Interestingly, there is another unused variable in another loop, in file src/Runner.php:291. Here, we are looking at variable $color (most of the code of the method has been omitted here. Just trust me). This is actually the opposite of the situation we just reviewed : the value is unused, while the key is used.

        if ($statusCode) {
            foreach ($this->errorConditions as $msg => $color) {
                // TODO: This was 'yell'. Add styling?
                $output->writeln($msg); // used to wrap at 40 and write in $color
            }
        }

This time, it is not possible to drop the value, as it is required in the loop. Using array_keys() on $this->errorConditions is possible, and make the intention very clear. However, the micro-optimisation we mentioned above will be void. So, at that point, the choice is yours.

Caught Exceptions

Next, we check src/Task/Base/ParallelExec.php:175, for variable $e. $e is often used for exceptions in a catch clause, and this is exactly the case here.

                try {
                    $process->checkTimeout();
                } catch (ProcessTimedOutException $e) {
                    $this->printTaskWarning("Process timed out for {command}", ['command' => $process->getCommandLine(), '_style' => ['command' => 'fg=white;bg=magenta']]);
                }

In the catch clause, $e is not mentioned. The exception is caught and processed. Thanks to its actual type, the logging doesn’t have to rely. Maybe this code will benefit from the Anonymous catches that PHP 8.0 will feature. Until then, this is actually an unused variable and a false positive that we can ignore.

Used once properties

Let’s round this up with a check on a property or two. Check the file src/Task/Development/SemVer.php:37, for the property protected $specialSeparator.

It is a protected property, so we need to check it with its parents. Since the class is not abstract, there is no need to check with potential child classes : SemVer is an autonomous class, and might be used just like that.

Here, class SemVer implements TaskInterface doesn’t extend any other class. The protected is actually a private property (since no other class can access it, except itself).

    /**
     * @param string $separator
     *
     * @return $this
     */
    public function setPrereleaseSeparator($separator)
    {
        $this->specialSeparator = $separator;
        return $this;
    }

The only usage of the property is in the method above. The property is actually updated with a new separator. The method itself is never used in the rest of the robo code, so we might be looking at a dead method at the same time. That would require a different audit.

The most important part is that specialSeparator is assigned, but never used. It is indeed a candidate for removal, along with its method.

Removing Variables Is Always Interesting

Out of the 6 examples that we reviewed, we spotted different kind of variables

one unused return value
one unused iffectation
one property
one caught exception
two blind variables

and 4 of them are good candidates for removal. This is a meager harvest, over a code that is obviously reviewed by itself. Usually, unused variables are in greater numbers than those, and they might also hold results of significant computation. Ideally, we’d like to spot those to remove the variable and its calculations, earning a nice bump in speed.

When you want to try the same review on your own code, use the auditing command that we applied on this repository, using your repository during initialisation. There are hundreds of analysis rules to checks!

Happy PHP auditing with Exakat!

Want to Keep in touch with us, subscribe to our newsletter !

Code auditing

Removing Unused Variables Is Good

Removing Unused Variables Is Good

What Is An Unused Variable

Finding Unused Variables With Static analysis

Running the audit

Reviewing robo.li

Removing unused variables

Iffectations

Blind variables

Caught Exceptions

Used once properties

Removing Variables Is Always Interesting

Login