Detecting hidden bugs in PHP code using PHP_CodeSniffer

Although PHP serves us well as a programming language, we cannot deny that some of its behavior can be very surprising. If one is not aware of these pitfalls, this can easily lead to hidden bugs in PHP code. In fact, we have ran into a fair share of these issues ourselves, but try to take measures to prevent being struck by them again. In this post we will look into two potential pitfalls in PHP, and how these can be detected and avoided using our open-sourced collection of sniffs for PHP_CodeSniffer.

Fool me once, shame on you

Quick, what does the following code do?

<?php 
$chars = str_split("The quick brown fox jumps over the lazy dog."); foreach ($chars as $char) {
    switch (strtoupper($char)) {
        case "A": 
        case "E": 
        case "I": 
        case "O": 
        case "U": 
        case "Y": 
            continue; 
    }

print $char; 
}

It’s not strange to think that this code fragment will print the input string with all vowels removed. However, it will in fact just print the original string, including the vowels (see it for yourself). This behavior is caused by the fact that PHP considers aswitch-statement a looping structure for the purpose of continue. The continue thus jumps to the end of the switch-statement (rather than to the end of the foreach-body) and all vowels are still printed. If we want to skip printing the vowels in the example above, we would have to use continue 2 to jump to the end of the foreach-body.

I recently bumped into this bug feature interesting behavior in a piece of code. Even though I had seen an example similar to the one above before, and thus should have known this intricacy, I still spent way too much time trying to find out why my code did not produce the desired results.

Fool me twice, shame on me

To prevent getting bitten by this quirk again, we decided to implement something for automated detection of the above situation and warn us about its unexpected behavior. For this purpose we chose to write a custom sniff for PHP_CodeSniffer, which we already used for enforcing coding standards. The token- and scope-based approach used by PHP_CodeSniffer makes it easy to check all switch-cases for top-level continue-statements (i.e. not within a nested looping structure) and see if they have a numeric 'level'-argument. We disallow any such continue-statements without an explicit number of looping levels to jump over:

<?php
for ($i = 1; $i < 10; $i++) {
    switch ($x) {
        case "foo": 
            continue; // NOT OK, probably a bug 
        case "bar": 
            foreach ($a as $k => $v) {
                continue; // OK, inside a nested 'foreach' 
            }
            continue 2; // OK, explicit 'level'-argument 
    } 
}

This warns us about potential hidden bugs like the one above. If we get an error from PHP_CodeSniffer but the behavior of continue is actually what we want (although I cannot imagine why one would not use break in such a case), we can always replace the continue by continue 1 to confirm that we have thought this case through and suppress the error.

Fool me three times, …

If only this was the sole hidden pitfall when working with PHP… Due to all problems that come with non-strict comparisons we already have a check in place to enforce strict comparison operators (=== etc.) over their non-strict counterparts (== etc.). However, there are still some PHP functions that will surprise you with non-strict comparison behavior by default, like in_array and array_search.

That’s why we have also implemented a PHP_CodeSniffer sniff that requires the $strict-parameter to such functions to be set explicitly:

<?php 
in_array("foo", [0]); // NOT OK, might introduce hidden bugs in_array("foo", [0], true); // OK, strict comparison
in_array("foo", [0], false); // OK, you have probably thought about this

Although we can still opt in to the non-strict behavior, this should prevent us from having to deal with the intricacies of non-strict comparison if we have not explicitly asked for them.

… release as open-source

To help the rest of the PHP community avoid these pitfalls, we have decided to open-source our PHP_CodeSniffer sniffs. They can easily be added as a development dependency into other projects using the Composer package, either as a standalone ruleset or by integrating them into your own PHP_CodeSniffer standard.

We have a backlog of more PHP pitfalls that we want to implement checks for, so stay tuned (by following us on Twitter or subscribing to our RSS feed) for more sniffs!

Originally published at www.moxio.com.