Refactoring to PHP 7

Recently I have migrated a relatively large codebase from PHP 5.6 to PHP 7 and would like to share some of my learnings. To get the most out of this article, you should be familiar with scalar type declarations (and return type declarations). To learn about these and other features of PHP 7, I recommend the " PHP 7 Explained " eBook.

The first thing to be changed is that declare(strict_types=1); must be added at the top of every PHP file. This includes tests and even templates. Anything that passes through the PHP interpreter. The reason is to avoid accidental type juggling. And why else would you like to upgrade to PHP 7 than to resolve the 123 == '123foo' paradox?

You may be wondering whether it is possible to declare strict_types globally, for example in the php.ini configuration file. No, it is not. And even if it were, most likely we would not be able to turn it on for next few years. Even if all our dependencies would support the strict interpretation of scalar type declarations, some script being used for continuous integration surely would not and all builds would be broken.

After adding declare(strict_types=1); it will definitely make sense to run your tests. You may get some failures. This will most likely be due to type conversions which happened magically before. For example, passing an object that has a __toString() method to a native function such as json_encode() . Right, native functions interpret scalar types strict, too. So now you have to explicitly cast your object to a string. And those are just native functions. You will get more of those once you add scalar type declarations to your methods.

You may be tempted to do the entire conversion to PHP 7 test-driven. This will not work and the title of this article might give you a hint why. Right, we are not introducing any new functionality. Migrating a codebase to PHP 7 is just a refactoring and the test suite (I am sure you have one) will give you the confidence to make it.

Nevertheless, I would prefer to migrate my test suite to PHP 7 first. It will just consist of changing the signatures of utility methods, the signatures of tests with data providers, and adding the array return type to data providers themselves. After those additions, some changes to the tests might be required, since we want all tests to pass before we continue.

Now it is time to add scalar type declarations to method signatures.

The first amazing discovery is that most DocBlocks with PHPDoc annotations can be removed thanks to all method parameters being typed. I do not want to start another flame war here, but the obvious thing is that PHPDoc annotations (as any comment) are generally harmful, since comments tend to lie when code is updated over time but the PHPDoc annotations remain unchanged. They also make code less readable. With scalar type declarations and return type declarations, PHPDoc annotations for methods mostly are not required anymore. And if your argument for keeping them is consistency think about putting a traffic light on every street junction just for reasons of consistency.

Removing PHPDoc annotations is a breeze. Classes suddenly become smaller and more readable. However, one disappointment comes the first time you encounter an array as a parameter or return type. In such a case, a PHPDoc annotation is still valuable, because there the type can be documented as string[] , array[] or (in the worse case) mixed[] . The PHP 7 types are limited to just array here. PHPDoc annotations should be kept if the type of a parameter or return value can be mixed. But those might be problems of your software design while the absence of typed arrays, in my opinion, is a feature that PHP is lacking.

And regarding rotting of PHPDoc annotations, you will be surprised to discover how many of them already were hiding the truth. During the conversion you probably will just copy PHPDoc annotation values into a method signature and then "bang": TypeError .

Besides relatively harmless PHPDoc annotations you may also discover some hidden problems with your code and tests. For example, some methods were forgotten during refactoring from, let's say, string to integer and were magically converting values. Or maybe some were not checking the input type at all. Maybe even some problems with logic will be revealed.

Another thing that you may notice is that some of your mock objects are not working anymore. For example, in some cases you might have gotten away with your mock returning a string instead of an object that can be cast to a string, or even not returning anything, since stubbed methods return null by default. Maybe the return value of a stub method was not used during the test of a particular branch of the code, so the return value was never specified on the test double. Now returning null will throw an exception, forcing you to properly specify return values for those methods. This is in fact a good point to rethink your mocking strategy, especially for classes which do not implement an interface. And, of course, this is another reason to consider not using test doubles for value objects.

The next thing that I was really looking forward to was dropping some factory methods. Namely those which were just validating types. And I happily started doing so.

However, thinking about it, it became clear that tests which were checking that exceptions are thrown for invalid argument types should not be deleted. In other languages, where all variables have to be typed, such tests might be superfluous. But in PHP, where type declarations are optional, type safety tests are still required as changes to the production code might accidentally remove the type from the method signature. Those argument type tests must be converted to expect a TypeError instead of custom exceptions. More than that, before you add a type declaration to a parameter of any public method you must first write a test asserting that a TypeError is thrown if a parameter with an invalid type is passed. And yes, those custom exception classes for argument type checks as mentioned above can then be removed (if they were only used to signal the wrong parameter type).

However, after removing a vast number of named constructors, I found myself thinking that Price::fromInt(5) might be more readable than new Price(5) , even if your price can be only created from integers. Also, what will happen if one fine day I will need to create a price from string? Maybe named constructors still are valuable.

And do not forget that in PHP removing named constructor and making the __construct() method public allows to break object immutability by calling __construct() directly on existing objects.

The last two small things to mention are that @ silencers can now be removed from higher order functions. This bug of PHP 5 was fixed (not sure why only) in PHP 7 and now exceptions raised inside lambda functions will bubble up.

The second thing is that code like

can be replaced with a return $foo['bar'] ?? ''; thanks to the null coalescing operator.

The catch here is, however, that now it is easy to leave a logic branch untested and code coverage report will not show you so.

This is pretty much it. I can say that for me migrating a codebase to PHP 7 has raised quite some questions and doubts. But maybe those are just road bumps on a journey to something new. I am sure further work with PHP 7 will bring even more discoveries.