How I upgraded eight PHP katas
Table of Contents
Why did I upgrade #
I stated in my blog post on “Why I enjoy coding katas” I had updated six katas to be consistent, all with the same tooling and documentation. This means anyone who can try one Kata will know how to pall any of the six others.
PHP 8 has been around for over two years now. The katas have already been bumped, two years ago, with support for PHP 8. Now that PHP 7.4 is at the end of its life it was a good time to fully update the starting code to the latest PHP 8.0 standard. The most noticeable is the way Classes are constructed, and the removal of properties, which are now generated in the constructor.
If I open a Class and see properties, a constructor with parameters and the parameters being assigned to the properties, I know I’m in an ‘old’, pre-PHP 8 class.
It would therefore be good to upgrade all the starting code to be the PHP 8 style. The purpose of the Kata is to practice refactoring. The starting code should not need upgrading.
How did I upgrade #
I’ve already completed six of the eight Kata. I have a plan of attack.
- create a ‘php8’ branch
- update composer.json
- update the readme.md
- remove .editorconfig and composer.lock
- add them to the .gitignore
- upgrade the easy coding standard config file (ecs.php)
- run composer install
- require Rector
- run the tests
- check the code standard
- check the static analysis
- repeat 9-11 until I’m satisfied
- remove Rector
- push the code and create a PR
Required tooling #
All the katas have:
- PHPUnit ^9.5 - for running the tests
- PhpStan ^0.12.25 - for static analysis
- Easy Coding Standard (ECS) ^9.0 - for code standard
- PHP ^7.3|^8.0 - From PHP 7.3 to PHP 8.2+
I want all the katas to have the latest versions of these tools, ECS also needs an updated config file.
I also want to temporarily pull in Rector PHP, to try and automatically upgrade the code.
Create a branch #
I have already forked the code from emilybache/Racing-Car-Katas to my repo and cloned it to my computer.
cd Racing-Car-Katas/php
git checkout -b php8
update composer.json #
The composer.json file can be updated in three parts:
Bump the minimum PHP version to 8 #
To bump the minimum required PHP version to 8, update the php requirement:
From:
"require": {
"php": "^7.3|^8.0"
},
To:
"require": {
"php": "^8.0"
},
Update the tooling #
By this point, I had already locked in the tooling, by using composer remove
Was:
"require-dev": {
"phpunit/phpunit": "^9.5",
"phpstan/phpstan": "^0.12.25",
"phpstan/phpstan-phpunit": "^0.12.10",
"symplify/easy-coding-standard": "^9.0",
"symplify/phpstan-extensions": "^9.0"
},
I had the exact versions from the previous kata, which I had already upgraded. Now:
"require-dev": {
"phpunit/phpunit": "^9.5",
"phpstan/phpstan": "^1.9",
"phpstan/phpstan-phpunit": "^1.3",
"symplify/easy-coding-standard": "^11.1",
"symplify/phpstan-extensions": "^11.1"
},
update the scripts #
Remove the unnecessary scripts and update the ecs scripts, which get the folder from the config file.
"scripts": {
- "checkcode": "phpcs src tests --standard=PSR12",
- "fixcode": "phpcbf src tests --standard=PSR12",
- "test": "phpunit",
"tests": "phpunit",
"test-coverage": "phpunit --coverage-html build/coverage",
-"check-cs": "ecs check src tests --ansi",
+"check-cs": "ecs check --ansi",
-"fix-cs": "ecs check src tests --fix --ansi",
+"fix-cs": "ecs check --fix --ansi",
"phpstan": "phpstan analyse --ansi"
}
Update the README.md #
The PHP version of README.md needs to be consistent across all the Katas.
If anyone pulls one Kata they will find the other PHP Kata to be consistent and familiar. This means they can get on with the Kata rather than working out how each one is set up.
Remove .editorconfig and composer.lock and add them to the .gitignore #
The .editorconfig isn’t required, the code standard will take care of incorrect spaces.
composer.lock can be removed, as a composer install will regenerate a composer.lock
.gitignore can be updated to ignore these files:
/.idea
/build
/vendor
/.vscode
/.editorconfig
/.phpunit.result.cache
/composer.lock
Upgrade the ECS config file #
A new ecs.php can be copied from previous projects. A new template can be created by running vendor/bin/ecs init
which will update ECS to the latest config. The exact config can
be tailored to each project.
<?php
declare(strict_types=1);
use PhpCsFixer\Fixer\ArrayNotation\ArraySyntaxFixer;
use PhpCsFixer\Fixer\Strict\DeclareStrictTypesFixer;
use Symplify\EasyCodingStandard\Config\ECSConfig;
use Symplify\EasyCodingStandard\ValueObject\Set\SetList;
// composer require --dev symplify/easy-coding-standard
// vendor/bin/ecs init
return static function (ECSConfig $ecsConfig): void {
$ecsConfig->paths([
__DIR__ . '/src',
__DIR__ . '/tests',
__DIR__ . '/ecs.php', // check this file too!
]);
$ecsConfig->skip([
// rules to skip
]);
// run and fix, one by one
$ecsConfig->sets([
SetList::SPACES,
SetList::ARRAY,
SetList::DOCBLOCK,
SetList::NAMESPACES,
SetList::CONTROL_STRUCTURES,
SetList::CLEAN_CODE,
SetList::STRICT,
SetList::PSR_12,
SetList::PHPUNIT,
]);
// add declare(strict_types=1); to all php files:
$ecsConfig->rule(DeclareStrictTypesFixer::class);
// change $array = array(); to $array = [];
$ecsConfig->ruleWithConfiguration(ArraySyntaxFixer::class, [
'syntax' => 'short',
]);
// [default: PHP_EOL]; other options: "\n"
$ecsConfig->lineEnding("\n");
};
Remove the config file ecs.yaml. ECS used to use a yaml file format for its config setting. The php style config was introduced several years ago, which is a much better completion and has no more errors due to tabs or spaces!
Run composer install #
The Kata is ready to be started, with the required tooling.
composer install
Run the tests #
Make sure all the tests are passing, although this Kata starts with a failing test.
pu.bat
# or composer tests
composer tests
> phpunit
PHPUnit 9.5.27 by Sebastian Bergmann and contributors.
Runtime: PHP 8.0.27
Configuration: E:\laragon\cli\Racing-Car-Katas\php\phpunit.xml.dist
....IEF.F 9 / 9 (100%)
Time: 00:00.038, Memory: 4.00 MB
There was 1 error:
1) Tests\TextConverter\HtmlPagesConverterTest::testFoo
fopen(foo): Failed to open stream: No such file or directory
E:\laragon\cli\Racing-Car-Katas\php\src\TextConverter\HtmlPagesConverter.php:18
E:\laragon\cli\Racing-Car-Katas\php\tests\TextConverter\HtmlPagesConverterTest.php:14
--
There were 2 failures:
1) Tests\TextConverter\HtmlTextConverterTest::testFoo
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'fixme'
+'foo'
E:\laragon\cli\Racing-Car-Katas\php\tests\TextConverter\HtmlTextConverterTest.php:15
2) Tests\TurnTicketDispenser\TicketDispenserTest::testFoo
Failed asserting that 0 is identical to -1.
E:\laragon\cli\Racing-Car-Katas\php\tests\TurnTicketDispenser\TicketDispenserTest.php:16
--
There was 1 incomplete test:
1) Tests\TelemetrySystem\TelemetryDiagnosticControlsTest::testCheckTransmissionShouldSendAndReceiveDiagnosticMessage
E:\laragon\cli\Racing-Car-Katas\php\tests\TelemetrySystem\TelemetryDiagnosticControlsTest.php:14
ERRORS!
Tests: 9, Assertions: 10, Errors: 1, Failures: 2, Incomplete: 1.
Script phpunit handling the tests event returned with error code 2
Ideally, full test coverage is preferred, but this Kata doesn’t start that way, the upgraded code may need to be reviewed later.
Require rector PHP #
Require rector PHP and initialise it, following the documentation.
composer require rector/rector --dev
# …
# Package operations: 1 install, 0 updates, 0 removals
# - Downloading rector/rector (0.15.3)
# - Installing rector/rector (0.15.3): Extracting archive
# …
vendor/bin/rector init
# or vendor\bin\rector.bat init
# [OK] "rector.php" config file was added
The init command will automatically generate a starter rector.php config, this file can be edited to help, the nice
thing about the config is how easy it is to read. In the future, if PHP 8.1 or PHP 8.2 is the minimum then change the
LevelSetList::UP_TO_PHP_<version>.
<?php
declare(strict_types=1);
use Rector\CodeQuality\Rector\ClassMethod\ReturnTypeFromStrictScalarReturnExprRector;
use Rector\Config\RectorConfig;
use Rector\Set\ValueObject\LevelSetList;
use Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictNativeCallRector;
return static function (RectorConfig $rectorConfig): void {
$rectorConfig->paths([
__DIR__ . '/src',
__DIR__ . '/tests',
]);
// register rules to add return type when known
$rectorConfig->rules([
ReturnTypeFromStrictNativeCallRector::class,
ReturnTypeFromStrictScalarReturnExprRector::class,
]);
// define sets of rules
$rectorConfig->sets([
LevelSetList::UP_TO_PHP_80
]);
};
To see a preview of suggested changes, run the rector command with –dry-run option:
vendor\bin\rector.bat --dry-run
# or vendor/bin/rector --dry-run
The output was reviewed, and 13 files would be updated. Most of the changes where to do with updating the constructor e.g.
class Leaderboard
{
--- /**
--- * @var Race[]
--- */
--- private $races;
---
--- public function __construct(array $races)
--- {
--- $this->races = $races;
+++ /**
+++ * @param \RacingCar\Leaderboard\Race[] $races
+++ */
+++ public function __construct(private array $races)
+++ {
}
The other changes were to add the implements \Stringable when the class uses __toString e.g.
namespace RacingCar\Leaderboard;
--- class Driver
+++ class Driver implements \Stringable
{
public $name;
@@ @@
$this->country = $country;
}
--- public function __toString()
+++ public function __toString(): string
{
--- return "{$this->name} {$this->country}";
+++ return (string) "{$this->name} {$this->country}";
}
}
Another change was to inline the parameter type e.g.
10) src/TelemetrySystem/TelemetryClient.php:10
---------- begin diff ----------
@@ @@
{
public const DIAGNOSTIC_MESSAGE = 'AT#UD';
--- private $onlineStatus = false;
+++ private bool $onlineStatus = false;
--- private $diagnosticMessageJustSent = false;
+++ private bool $diagnosticMessageJustSent = false;
Although in the test cases, the full class address is used. The Driver class would be better to be a use statement:
class LeaderboardTest extends TestCase
{
--- private $driver1;
+++ private \RacingCar\Leaderboard\Driver $driver1;
An interesting one was the random_int rector:
5) src/TirePressureMonitoring/Sensor.php:20
---------- begin diff ----------
@@ @@
private static function getSamplePressure()
{
// placeholder implementation that simulates a real sensor in a real tyre
--- return 6 * mt_rand() / mt_getrandmax() * mt_rand() / mt_getrandmax();
+++ return 6 * random_int(0, mt_getrandmax()) / mt_getrandmax() * random_int(0, mt_getrandmax()) / mt_getrandmax();
}
}
----------- end diff -----------
This improvement would have been easily missed.
I wanted to double-check the output is comparable, by opening an interactive php shell:
php -a
Interactive shell
php > echo 6 * mt_rand() / mt_getrandmax() * mt_rand() / mt_getrandmax();
0.28789230076317
php > 6 * mt_rand() / mt_getrandmax() * mt_rand() / mt_getrandmax();
php > echo 6 * mt_rand() / mt_getrandmax() * mt_rand() / mt_getrandmax();
0.59193628730294
php > echo 6 * mt_rand() / mt_getrandmax() * mt_rand() / mt_getrandmax();
2.3353527303516
php > echo 6 * mt_rand() / mt_getrandmax() * mt_rand() / mt_getrandmax();
3.9111024607976
php > echo 6 * mt_rand() / mt_getrandmax() * mt_rand() / mt_getrandmax();
1.3882777102362
php > echo 6 * mt_rand() / mt_getrandmax() * mt_rand() / mt_getrandmax();
1.3381505674129
php > echo 6 * random_int(0, mt_getrandmax()) / mt_getrandmax() * random_int(0, mt_getrandmax()) / mt_getrandmax();
0.33623375077603
php > echo 6 * random_int(0, mt_getrandmax()) / mt_getrandmax() * random_int(0, mt_getrandmax()) / mt_getrandmax();
0.21711217311234
php > echo 6 * random_int(0, mt_getrandmax()) / mt_getrandmax() * random_int(0, mt_getrandmax()) / mt_getrandmax();
1.8174108426317
php > echo 6 * random_int(0, mt_getrandmax()) / mt_getrandmax() * random_int(0, mt_getrandmax()) / mt_getrandmax();
0.59516582006594
php > echo 6 * random_int(0, mt_getrandmax()) / mt_getrandmax() * random_int(0, mt_getrandmax()) / mt_getrandmax();
0.13920652881307
php > echo 6 * random_int(0, mt_getrandmax()) / mt_getrandmax() * random_int(0, mt_getrandmax()) / mt_getrandmax();
0.56419425470649
php > echo 6 * random_int(0, mt_getrandmax()) / mt_getrandmax() * random_int(0, mt_getrandmax()) / mt_getrandmax();
1.1058509887747
php > echo 6 * random_int(0, mt_getrandmax()) / mt_getrandmax() * random_int(0, mt_getrandmax()) / mt_getrandmax();
4.1742725948899
php > echo 6 * random_int(0, mt_getrandmax()) / mt_getrandmax() * random_int(0, mt_getrandmax()) / mt_getrandmax();
1.8491269459026
php > echo 6 * random_int(0, mt_getrandmax()) / mt_getrandmax() * random_int(0, mt_getrandmax()) / mt_getrandmax();
2.8255782588891
Ok, the recommended change is giving a similar ‘random’ output.
One other PHP 8 improvement was str_contrains:
--- if (strpos($line, 'PAGE_BREAK') !== false) {
+++ if (str_contains($line, 'PAGE_BREAK')) {
Much nicer to read!
I’m happy with what I have seen. I am also confident after upgrading six other Kata’s that this will not break the code!
To make changes happen, run the bare command:
vendor\bin\rector.bat
# or vendor/bin/rector
I can re-run the tests can confirm the tests are giving the same results:
pu.bat
# or composer tests
The results are exactly the same:
Runtime: PHP 8.0.27
Configuration: E:\laragon\cli\Racing-Car-Katas\php\phpunit.xml.dist
....IEF.F 9 / 9 (100%)
Time: 00:00.016, Memory: 4.00 MB
# ….
ERRORS!
Tests: 9, Assertions: 10, Errors: 1, Failures: 2, Incomplete: 1.
Script phpunit handling the tests event returned with error code 2
Check the code standard #
Run cc.bat or composer check-cs to check the code quality
Most of the fixes are for StandaloneLinePromotedPropertyFixer
E.g.
-class TurnTicket
-{
- public function __construct(private int $turnNumber)
- {
- }
+class TurnTicket
+{
+ public function __construct(
+ private int $turnNumber
+ ) {
+ }
Which makes the code easier to read, especially when there are multiple properties.
Most of the other fixes are to do with spaces, brace positions and gaps between imports, all cosmetic changes, which keep the code confident to read.
- PhpCsFixer\Fixer\Basic\CurlyBracesPositionFixer
- PhpCsFixer\Fixer\Import\SingleLineAfterImportsFixer
- PhpCsFixer\Fixer\Whitespace\NoWhitespaceInBlankLineFixer
One change I don’t want was NoUnusedImportsFixer, as this is starter code with incomplete tests I want to leave the import as a clue to what to start testing.
// This import is a clue, I want to leave it in the starting code:
---use RacingCar\TelemetrySystem\TelemetryDiagnosticControls;
use PHPUnit\Framework\TestCase;
class TelemetryDiagnosticControlsTest extends TestCase
{
public function testCheckTransmissionShouldSendAndReceiveDiagnosticMessage(): void
{
$this->markTestIncomplete();
}
}
I can add this rule to the skip rules section of ecs.php config file:
$ecsConfig->skip([
// keep unused imports until the refactor is complete
PhpCsFixer\Fixer\Import\NoUnusedImportsFixer::class
]);
The other problem was the report was too long to read, I ran the script to pipe the output to a text file:
vendor\bin\ecs.bat --no-ansi > ecsoutput.txt
I can now open ecsoutput.txt in my IDE and review the changes. It is harder to read as the colour coding has been stripped. At this stage, I’m more interested in the rules used, as I don’t want the starter code to be removed. Finding Applied checkers: and reviewing the rules that would be applied to each file.
I’m now satisfied with the results, so I can run the fix
fc.bat
# or composer fix-cs
// ….
[OK] 21 errors successfully fixed and no other errors found!
I can delete the ecsoutput.txt file, which is no longer needed.
The tests can be re-run, I’m not expecting any surprises, as the changes to code style are more cosmetic, so running the tests is a precaution.
pu.bat
# or composer tests
The results are the same:
Runtime: PHP 8.0.27
Configuration: E:\laragon\cli\Racing-Car-Katas\php\phpunit.xml.dist
....IEF.F 9 / 9 (100%)
Time: 00:00.015, Memory: 4.00 MB
# ….
ERRORS!
Tests: 9, Assertions: 10, Errors: 1, Failures: 2, Incomplete: 1.
Script phpunit handling the tests event returned with error code 2
I’m happy the tests are the same, so can move on to the next thing.
Check the static analysis #
Before the analyser can be run the config file needs to be updated:
includes:
- vendor/symplify/phpstan-extensions/config/config.neon
- vendor/phpstan/phpstan-phpunit/extension.neon
parameters:
paths:
- src
- tests
--- # Level 8 is the highest
--- level: 8
+++ # max is the highest level
+++ level: max
checkGenericClassInNonGenericObjectType: false
Two years ago 8 was the highest level, it’s now 9, and we can set it to max.
Run PhpStan static analyser:
ps.bat
# or composer phpstan
The output shows each file with the error, there are 33 error in total:
λ ps.bat
composer phpstan
> phpstan analyse --ansi
Note: Using configuration file E:\laragon\cli\Racing-Car-Katas\php\phpstan.neon.
20/20 [============================] 100%
------ ------------------------------------------------------------------------
Line src\Leaderboard\Driver.php
------ ------------------------------------------------------------------------
9 Property RacingCar\Leaderboard\Driver::$name has no type specified.
11 Property RacingCar\Leaderboard\Driver::$country has no type specified.
------ ------------------------------------------------------------------------
------ -------------------------------------------------------------------------------------------------------------------------------
Line src\Leaderboard\Leaderboard.php
------ -------------------------------------------------------------------------------------------------------------------------------
17 Method RacingCar\Leaderboard\Leaderboard::getDriverResults() has no return type specified.
32 Method RacingCar\Leaderboard\Leaderboard::getDriverRankings() return type has no value type specified in iterable type array.
� See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type
------ -------------------------------------------------------------------------------------------------------------------------------
------ -----------------------------------------------------------------------------------------------------------------
Line src\Leaderboard\Race.php
------ -----------------------------------------------------------------------------------------------------------------
9 Property RacingCar\Leaderboard\Race::$points type has no value type specified in iterable type array.
� See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type
11 Property RacingCar\Leaderboard\Race::$driverNames type has no value type specified in iterable type array.
� See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type
17 Property RacingCar\Leaderboard\Race::$name is never read, only written.
� See: https://phpstan.org/developing-extensions/always-read-written-properties
31 Method RacingCar\Leaderboard\Race::getPosition() has parameter $driver with no type specified.
33 Method RacingCar\Leaderboard\Race::getPosition() should return int but returns int|string|false.
41 Method RacingCar\Leaderboard\Race::getResults() return type has no value type specified in iterable type array.
� See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type
------ -----------------------------------------------------------------------------------------------------------------
------ -----------------------------------------------------------------------------------------
Line src\Leaderboard\SelfDrivingCar.php
------ -----------------------------------------------------------------------------------------
9 Property RacingCar\Leaderboard\SelfDrivingCar::$algorithmVersion has no type specified.
------ -----------------------------------------------------------------------------------------
------ ---------------------------------------------------------------------------------------------------
Line src\TelemetrySystem\TelemetryClient.php
------ ---------------------------------------------------------------------------------------------------
98 Method RacingCar\TelemetrySystem\TelemetryClient::getOnlineStatus() has no return type specified.
------ ---------------------------------------------------------------------------------------------------
------ --------------------------------------------------------------------------------------------------------
Line src\TelemetrySystem\TelemetryDiagnosticControls.php
------ --------------------------------------------------------------------------------------------------------
13 Property RacingCar\TelemetrySystem\TelemetryDiagnosticControls::$diagnosticInfo has no type specified.
------ --------------------------------------------------------------------------------------------------------
------ -----------------------------------------------------------------------------------------------------------------------
Line src\TextConverter\HtmlPagesConverter.php
------ -----------------------------------------------------------------------------------------------------------------------
9 Property RacingCar\TextConverter\HtmlPagesConverter::$breaks type has no value type specified in iterable type array.
� See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type
16 Parameter #1 $stream of function fgets expects resource, resource|false given.
19 Parameter #1 $stream of function ftell expects resource, resource|false given.
20 Parameter #1 $stream of function ftell expects resource, resource|false given.
23 Parameter #1 $stream of function ftell expects resource, resource|false given.
24 Parameter #1 $stream of function fclose expects resource, resource|false given.
32 Parameter #1 $stream of function fseek expects resource, resource|false given.
34 Parameter #1 $stream of function ftell expects resource, resource|false given.
35 Parameter #1 $stream of function fgets expects resource, resource|false given.
35 Parameter #1 $string of function rtrim expects string, string|false given.
42 Parameter #1 $stream of function fclose expects resource, resource|false given.
46 Method RacingCar\TextConverter\HtmlPagesConverter::getFileName() has no return type specified.
------ -----------------------------------------------------------------------------------------------------------------------
------ -----------------------------------------------------------------------------------------------
Line src\TextConverter\HtmlTextConverter.php
------ -----------------------------------------------------------------------------------------------
19 Parameter #1 $stream of function fgets expects resource, resource|false given.
27 Method RacingCar\TextConverter\HtmlTextConverter::getFileName() has no return type specified.
------ -----------------------------------------------------------------------------------------------
------ ---------------------------------------------------------------------------------------------------------
Line src\TirePressureMonitoring\Sensor.php
------ ---------------------------------------------------------------------------------------------------------
14 Method RacingCar\TirePressureMonitoring\Sensor::popNextPressurePsiValue() has no return type specified.
21 Method RacingCar\TirePressureMonitoring\Sensor::getSamplePressure() has no return type specified.
------ ---------------------------------------------------------------------------------------------------------
------ -----------------------------------------------------------------------------------------------------
Line src\TurnTicketDispenser\TicketDispenser.php
------ -----------------------------------------------------------------------------------------------------
9 Method RacingCar\TurnTicketDispenser\TicketDispenser::getTurnTicket() has no return type specified.
------ -----------------------------------------------------------------------------------------------------
------ ---------------------------------------------------------------------------------------------------
Line src\TurnTicketDispenser\TurnNumberSequence.php
------ ---------------------------------------------------------------------------------------------------
11 Method RacingCar\TurnTicketDispenser\TurnNumberSequence::nextTurn() has no return type specified.
------ ---------------------------------------------------------------------------------------------------
------ ------------------------------------------------------------------------------------------------
Line src\TurnTicketDispenser\TurnTicket.php
------ ------------------------------------------------------------------------------------------------
14 Method RacingCar\TurnTicketDispenser\TurnTicket::getTurnNumber() has no return type specified.
------ ------------------------------------------------------------------------------------------------
------ ----------------------------------------------------------------------------------------------
Line tests\Leaderboard\LeaderboardTest.php
------ ----------------------------------------------------------------------------------------------
37 Property Tests\Leaderboard\LeaderboardTest::$sampleLeaderboard2 is never read, only written.
� See: https://phpstan.org/developing-extensions/always-read-written-properties
------ ----------------------------------------------------------------------------------------------
[ERROR] Found 33 errors
Script phpstan analyse --ansi handling the phpstan event returned with error code 1
Both ECS and Rector have automatic code fixes, unfortunately, there are no PhpStan automatic fixes.
Most of the problems are no type specified (for a property) and no return type (returned from a method). It’s a case of looking at each file, one by one, and manually working out the types. I may run the test with coverage to check which test is covering the code and see the input parameters.
Example (Driver.php):
---public $name;
+++public ?string $name;
---public $country;
+++public string $country;
public function __construct(?string $name, string $country)
// ...
Oddly this wasn’t updated to php 8 standard, now the types have been added, Rector can be re-run.
vendor\bin\rector.bat
# or vendor/bin/rector
class Driver implements \Stringable
{
--- public ?string $name;
---
--- public string $country;
---
--- public function __construct(?string $name, string $country)
+++ public function __construct(public ?string $name, public string $country)
{
--- $this->name = $name;
--- $this->country = $country;
}
Nice! That class has now been updated.
ECS can be re-run to clean up any code standard issues.
Re-running ps.bat confirms we are down to 31 errors. I’ll look at the remaining no type specified:
src\Leaderboard\SelfDrivingCar.php line 9
The type was in the constructor:
---public $algorithmVersion;
+++public string $algorithmVersion;
public function __construct(string $algorithmVersion, string $company)
src\TelemetrySystem\TelemetryDiagnosticControls.php line 10
Based on the default value, the type is a string. Checking the
usage $this->diagnosticInfo = $this->telemetryClient->receive(); confirm the return type from the
method receive(): string is a string
---public $diagnosticInfo = '';
+++public string $diagnosticInfo = '';
Rinse and repeat #
Again Rector can be run, ECS and PHPUnit then re-run PhpStan.
Down to 29 errors!
Next, no return type is specified, this means each method has to be manually checked. If it has a return the type will
need to be added, otherwise, it is void.
- RacingCar\Leaderboard\Leaderboard::getDriverResults()
Is an array.
- RacingCar\TelemetrySystem\TelemetryClient::getOnlineStatus()
Is returning $this->onlineStatus with is a bool
- RacingCar\TextConverter\HtmlTextConverter::getFileName()
Is returning $this->fullFileNameWithPath, which is a string
- RacingCar\TirePressureMonitoring\Sensor::popNextPressurePsiValue() is a float.
- RacingCar\TirePressureMonitoring\Sensor::getSamplePressure() is a float.
- RacingCar\TurnTicketDispenser\TicketDispenser::getTurnTicket(): TurnTicket
- RacingCar\TurnTicketDispenser\TurnNumberSequence::nextTurn(): int
- RacingCar\TurnTicketDispenser\TurnTicket::getTurnNumber(): int
- RacingCar\TextConverter\HtmlPagesConverter::getFileName(): string
- RacingCar\Leaderboard\Race::getPosition() has parameter $driver with no type specified. Is a Driver
- public function getPoints(Driver $driver): int
Down to 20 errors! Only 5 files (Classes).
Next, we can look at the next highest item return type has no value type specified in iterable type array., this is purely a PhpStan error, it needs to know the types of the array so further static analysis can help us. We need to add some doc block information about the array, advising the structure.
- src\Leaderboard\Leaderboard.php
/**
* @return array<string, int>
*/
public function getDriverResults(): array
//…
/**
* @return int[]|string[]
*/
public function getDriverRankings(): array
- src\Leaderboard\Race.php
/**
* @var int[]
*/
private static array $points = [25, 18, 15];
/**
* @var array<string, string|null>
*/
private array $driverNames;
/**
* @return Driver[]
*/
public function getResults(): array
- src\TextConverter\HtmlPagesConverter.php
/**
* @var int[]
*/
private array $breaks;
There are now 17 errors, as PhpStan “knows more” about the code, other errors have been generated:
Line src\Leaderboard\Race.php
------ --------------------------------------------------------------------------------------------------
23 Property RacingCar\Leaderboard\Race::$name is never read, only written.
� See: https://phpstan.org/developing-extensions/always-read-written-properties
39 Method RacingCar\Leaderboard\Race::getPosition() should return int but returns int|string|false.
57 Method RacingCar\Leaderboard\Race::getDriverName() should return string but returns string|null.
------ --------------------------------------------------------------------------------------------------
------ ------------------------------------------------------------------------------------------------------------
Line src\TextConverter\HtmlPagesConverter.php
------ ------------------------------------------------------------------------------------------------------------
19 Parameter #1 $stream of function fgets expects resource, resource|false given.
22 Parameter #1 $stream of function ftell expects resource, resource|false given.
23 Parameter #1 $stream of function ftell expects resource, resource|false given.
23 Property RacingCar\TextConverter\HtmlPagesConverter::$breaks (array<int>) does not accept array<bool|int>.
26 Parameter #1 $stream of function ftell expects resource, resource|false given.
26 Property RacingCar\TextConverter\HtmlPagesConverter::$breaks (array<int>) does not accept array<bool|int>.
27 Parameter #1 $stream of function fclose expects resource, resource|false given.
40 Parameter #1 $stream of function fseek expects resource, resource|false given.
42 Parameter #1 $stream of function ftell expects resource, resource|false given.
43 Parameter #1 $stream of function fgets expects resource, resource|false given.
43 Parameter #1 $string of function rtrim expects string, string|false given.
50 Parameter #1 $stream of function fclose expects resource, resource|false given.
------ ------------------------------------------------------------------------------------------------------------
------ --------------------------------------------------------------------------------
Line src\TextConverter\HtmlTextConverter.php
------ --------------------------------------------------------------------------------
19 Parameter #1 $stream of function fgets expects resource, resource|false given.
------ --------------------------------------------------------------------------------
------ ----------------------------------------------------------------------------------------------
Line tests\Leaderboard\LeaderboardTest.php
------ ----------------------------------------------------------------------------------------------
37 Property Tests\Leaderboard\LeaderboardTest::$sampleLeaderboard2 is never read, only written.
� See: https://phpstan.org/developing-extensions/always-read-written-properties
------ ----------------------------------------------------------------------------------------------
[ERROR] Found 17 errors
Script phpstan analyse --ansi handling the phpstan event returned with error code 1
Running Rector, ECS and PHPUnit, everything is working and to the correct standard.
The starter code to a Kata, the description:
For each exercise, you should identify which SOLID principles are not being followed by the code. There is only one class you are interested in writing tests for right now. As a first step, try to get some kind of test in place before you change the class at all. If the tests are hard to write, is that because of the problems with SOLID principles?
This starter code needs to be refactored. I’m happy the class structure is now PHP 8 standard, it’s now ready for someone to start the exercise.
The remaining errors need the code to be refactored to check for errors such as the file existing.
I will, therefore, PR this code as the PHP starting point for the Kata.
Remove Rector #
Before I PR the code Rector and the rector.php config file needs to be removed:
composer remove rector/rector --dev
rm rector.php
Pull Request (PR) #
If you are interested the PR was successful, the full diff, of every change, can be seen Bump to PHP 8.0+
PHP Katas #
The eight Katas with PHP versions can be run by anyone (diff for information):
- Gilded Rose Refactoring Kata - (diff)
- Tennis Refactoring Kata - (diff)
- Parrot Refactoring Kata - (diff)
- The Supermarket Receipt Refactoring Kata - (diff)
- Theatrical Players Refactoring Kata - (diff)
- Yatzy Refactoring Kata - (diff)
- Lift Kata - (diff)
- Racing Car Katas - (diff)
Anyone new to Kata and refactoring I would recommend the Theatrical Players Refactoring Kata first. It is a code along with the first chapter of ‘Refactoring’ by Martin Fowler, 2nd Edition, using Javascript. I would even go so far as to say to do the Javascript version first, then redo the PHP version.
Then I would recommend the Parrot or Tennis or Yatzy Kata. One of the most time-consuming Kata is the Gilded Rose, as this starts with no tests, and then it needs refactoring before the new feature can be added.
I wish everyone good luck with these Kata, they helped me to level up my knowledge. The confidence tests give when refactoring code is immeasurable.
Conclusion #
In summary, I upgraded eight PHP katas to make them consistent, and easy for someone to clone and run. They are a great way to learn about refactoring code, especially when that code is well-tested, which then gives the confidence to refactor. I hope other PHP developers find these Kata’s useful, as I have.
This upgrade has given me valuable experience in how to upgrade an existing code base. I now have a better understanding of how Rector works. I use ECS, PhpStan and PHPUnit in my existing projects, thanks to exercises I did, like this, several years ago. When I have production code which requires an upgrade I will have more confidence to use Rector.
I hope this blog will help others to use the fantastic tools available to PHP developers, to help us write our code.