Which level is right for you?
Today, whilst working on Versa, I was experimenting with different PHPStan levels.
Level 1 is the least strict level, and applies the fewest rules and returns the fewest results.
As you increase the level, the stricter PHPStan is.
Levelling up to 9
Here is the code to get the values of the --extra-args and --working-dir options:
$extraArgs = $input->getOption('extra-args');
$workingDir = $input->getOption('working-dir');
This passed PHPStan level 8, but these are the errors I get when running level 9:
------ -------------------------------------------------------------------------------------------------------
Line versa
------ -------------------------------------------------------------------------------------------------------
61 Parameter $extraArgs of static method App\Process\Process::create() expects string|null, mixed given.
62 Parameter $workingDir of static method App\Process\Process::create() expects string, mixed given.
72 Parameter $extraArgs of static method App\Process\Process::create() expects string|null, mixed given.
73 Parameter $workingDir of static method App\Process\Process::create() expects string, mixed given.
84 Parameter $extraArgs of static method App\Process\Process::create() expects string|null, mixed given.
85 Parameter $workingDir of static method App\Process\Process::create() expects string, mixed given.
94 Parameter $extraArgs of static method App\Process\Process::create() expects string|null, mixed given.
95 Parameter $workingDir of static method App\Process\Process::create() expects string, mixed given.
104 Parameter $extraArgs of static method App\Process\Process::create() expects string|null, mixed given.
105 Parameter $workingDir of static method App\Process\Process::create() expects string, mixed given.
119 Parameter $extraArgs of static method App\Process\Process::create() expects string|null, mixed given.
120 Parameter $workingDir of static method App\Process\Process::create() expects string, mixed given.
------ -------------------------------------------------------------------------------------------------------
[ERROR] Found 12 errors
The issue is that $input->getOption() from Symfony's InputInterface returns mixed - even though it always returns null or a string.
If I did --working-dir 2, it would return the string "2".
An empty string throws an error, and an empty value (if there are no extra arguments) returns null.
So, I know $workingDir will always be a string and $extraArgs will either a string or null.
Passing level 8
To pass level 8, PHPStan needs to be sure the variables are what I think they are.
Here's the code I can use to get it to pass:
$extraArgs = $input->getOption('extra-args');
$workingDir = $input->getOption('working-dir');
assert(is_string($extraArgs) || is_null($extraArgs));
assert(is_string($workingDir));
By using assert() and throwing an Exception if the condition fails, PHPStan is now happy with the code and my code passes level 9.
Here's the thing
Although the extra code gets PHPStan to pass, it it worth it?
Is this extra code adding value? Does it make the code more readable or is it likely to prevent a bug?
In this case, I know the value will always be the type I expect.
I can work around this using a baseline or annotations for PHPStan to ignore these lines, or is level 8 good enough for this project?
Similar to 100% test coverage, is aiming for the highest PHPStan level an objective to be met, or is enough value being added added by the lower level?
Which level is right for you and this codebase?
- Oliver
P.S. There's less than a year until Drupal 7's end-of-life date. Plan your upgrade to Drupal 10 now!
