In part one of this series we took a tour into my 14+ year old legacy CMS called LampLight. It's an oldie but a goodie and it needs some love because it's still in use by a couple clients and myself. We are migrating it from PHP v 5.6 to 7.3. In this part we'll use PHP CS Fixer to identify problems and clean things up.
PHP Short Tags and Closing Tags
Talk has been cheap for some time regarding the deprecation of the short opening tag <? for PHP, and its removal has had a few false starts. Finally, a PSR for this change has passed and it will officially be deprecated in v 7.4.
We also want to eliminate the closing PHP tag in files that contain only PHP. These two issues are quite straightforward, so let's use these as an easy dip-of-the-toe into using command line code analysis tools to have a look at our code. We can even use these tools to make the changes for us.
I first commit all my work to my Git repo and push it to the remote.
We'll use PHP Coding Standards Fixer aka php-cs-fixer for this. It's a powerful tool to analyze code issues and fix them for you. At the root of my CMS project I run:
php-cs-fixer fix . --rules=full_opening_tag,no_closing_tag --verbose --dry-run --using-cache=no
I rarely use the cache because I don't want it creating files in my project, and I use the --dry-run flag to see what the output would be before actually doing anything. You can also use the --diff flag to see what would be changed by the fixer in a diff format. You can see the only rules I'm running are for short tags and closing tag removal. This gives me a list of all the files which have a problem, and at the top a nice visual summary which looks like this for only one rule:
All the green 'F's represent files that will be fixed. The dots are files with no fixes. That's a lot! I'm confident this is an easy fix that I trust to be done automatically so I remove the --dry-run flag and run the command again.
Now it's fixed my files for me. So easy! I do a full test of my application then commit the changes to Git. Not bad for 5 minutes on a Wednesday. And we are now familiar with PHP CS Fixer. A handy tool for every PHP developer's belt.
PHP Version Compatibility
PHP CS Fixer has a lot of fixes available. Let's look them up. Visit the PHP CS Fixer Configurator Rules site to look up the available rules and rulesets. You may have to update the fixer and/or install additional rules.
Ah ha! There are rules for checking PHP version compatibility. I'm most interested in the migration rule SETS (preceded by an @ symbol):
- @PHP70Migration and @PHP70Migration:risky
- @PHP71Migration and @PHP71Migration:risky
- @PHP73Migration
For some reason there doesn't seem to be a ruleset for 7.2. Oh well, let's use what we have.
One by one we will run the fixer with a rule set. First with the --dry-run flag. You could technically do all the rulesets at once by comma separating them... but I don't like to have to wade through a zillion problems at once.
The sets labelled 'risky' should sometimes be fixed by hand instead of trusting the fixer to do them. You can look at the reasons why it's risky in the rule info in the Configurator and decide for yourself. Let's do the first non-risky one:
php-cs-fixer fix . --rules=@PHP70Migration --verbose --dry-run --using-cache=no
Miraculously no problems reported! The risky rule next (note the additional --allow-risky=yes flag), and I make sure it's only a dry-run:
php-cs-fixer fix . --rules=@PHP70Migration:risky --verbose --dry-run --using-cache=no --allow-risky=yes
Woah! A ton of issues, mostly for the declare_strict_types rule. Let's look that rule up in the Configurator site. Yup just like it sounds, it tries to force a strict types declaration at the top of the file. I don't want to do that, so can I add a rule exception to my ruleset? YES! Just add it to the rules list starting with a minus:
php-cs-fixer fix . --rules=@PHP70Migration:risky,-declare_strict_types --verbose --dry-run --using-cache=no --allow-risky=yes
Much better, only a few issues, exclusively from the random_api_migration rule. We need to use the proper random functions. I'll go into each file and fix them up manually. Test and then git commit.
A 'Random' Gotcha
When I first tried to manually fix the random_api_migration rule, I changed all instances of rand() to mt_rand(). PHP CS Fixer kept flagging the same errors, even though I was doing the change via the examples in the Configurator docs. I spent too much time thinking the fixer was still using a cache. Gave up and ran the fixer without the dry-run flag. Lo and behold it changed these instances to random_int() instead of mt_rand(). So the Configurator docs are wrong, and this is the recommended way to generate random integers.
It's these kinds of surprises that keep this from being a straightforward process. On to the rules for 7.1.
Property and Method Visibility
Running the non-risky rules for PHP 7.1 is up next. Same as before but the ruleset is @PHP71Migration.
This flags only one rule violation: visibility_required which tells me class properties and methods must have visibility declared. I'm going to take the easy road here and just let them all be defaulted to public because any that should be private or protected are already declared as such. So I'll let the fixer do it for me and remove the dry-run flag.
My smoke-test reveals a new deprecation error appearing in the application:
Deprecated: Methods with the same name as their class will not be constructors in a future version of PHP; cBase has a deprecated constructor. I'll go in and fix this to a proper constructor.
Onward to the risky ruleset for 7.1.
Void Return Types
php-cs-fixer fix . --rules=@PHP71Migration:risky,-declare_strict_types --verbose --dry-run --using-cache=no --allow-risky=yes
This time we get quite a few pages flagged for the rule void_return. Looking this up in the Configurator shows that it's a suggestion to add a void return type to methods and functions if not already specified.
Return types are optional in PHP, and this application is discontinued anyway. I've decided to just leave the return types out. It's interesting to note however, if you wanted to execute this fix, PHP CS Fixer will actually check into the functions and methods to see if there are any non-empty return statements before adding the void return type. So the fixer is doing some intelligent analysis before just changing things on you.
Heredoc
php-cs-fixer fix . --rules=@PHP73Migration --verbose --using-cache=no --dry-run
This next rule set again flagged only one issue. I'm getting lucky here. The heredoc_indentation rule was complaining about 3 files. This would have been an easy fix had I just let the fixer do it. But I tried to fix it manually so that I could also learn what I was doing wrong. Well that led me down a rabbit hole of trying to fathom what the fixer was complaining about.
It turns out that it didn't like it if my heredoc text was indented more than 4 spaces beyond the line that begins the string. This seems to be a style choice enforced by the fixer. And doesn't allow me to indent it under the assignment operator (=) like I prefer.
There is no 'risky' ruleset for 7.3.
Manual Look at 7.2
Since there was no ruleset for 7.2. I'll have a look at the PHP documentation for 7.2 and see if anything catches my eye. I'm looking specifically at backward incompatible or deprecation changes.
There are quite a few gotchas in these lists. Things like the create_function() and each() functions being deprecated, and a bunch of other things. Searched my codebase for a few of these, and didn't find anything. The only real concern is that the count() function no longer accepts a value that isn't countable. I use this function everywhere, and don't do an is_countable() check before hand. I assume my values I'm passing to it are always going to be set, but this is a prime example of when a testsuite for this project would be super useful. I'm going to have to just hope that my manual test covers everything (not likely). I'll monitor the error logs just in case.
Section Wrap Up
If you wish to run ALL of the rules that we covered in this section, here is a command with everything combined:
php-cs-fixer fix . --rules=full_opening_tag,no_closing_tag,@PHP70Migration,@PHP71Migration,@PHP70Migration:risky,@PHP71Migration:risky,@PHP73Migration --verbose --dry-run --using-cache=no --allow-risky:yes
If you want to fix them one at a time, you can add the --stop-on-violation flag to the command.
The migration rules for PHP CS Fixer are not 100% exhaustive. But they are a helpful tool to cover many of your bases. I still do a read through of all the backward incompatible and deprecation changes in all the versions between 5.6 and 7.3, and scan my codebase for them.
In my next post I'll talk about using the fixer for checking PSR1 and 2 compliance. We'll then look at using PHP Code Sniffer (phpcs) to do even more code analysis.
Top comments (0)