Improve code base through psalm and psalter

Overview

Since Flow 6.0+ we make use of vimeo/psalm static analysis tool to keep our code base in check for things like type hinting issues. To get this into the existing code base, we needed to set a so-called “baseline”, which just contains a list of the existing errors so the tool can ignore them on checks.

Obviously, just whitelisting all those errors is not the goal, and we should get rid of those errors, but it is a tedious and long process. Fortunately everyone can easily help with that!

Instructions to incrementally resolve psalm errors

Here are some instructions on how you can help improve the code base:

  • setup a development environment as outlined in Development setup
  • checkout master
  • create a new branch
  • run ./bin/psalm --config=Packages/Framework/psalm.xml --ignore-baseline, maybe also use --report=psalm-report.txt (or .json or .xml - see https://github.com/ErikBooijCB/psalm/blob/master/src/psalm.php#L172) if the console is flooded with too many errors
  • fix some of the errors. You can possibly use the psalter tool for this, see below
  • run bin/psalm --config=Packages/Framework/psalm.xml --update-baseline
  • commit and push to create a PR, following the guideline at Creating a pull request

False-negatives

Note though, that there are a couple of errors, that just can not be fixed sensibly, which are basically false-negatives of the tool, for example:

ERROR: PossiblyNullReference 
- Packages\Framework\Neos.Utility.Unicode\Classes\TextIterator.php:263:56
- Cannot call method getValue on possibly null value
            $allValues[] = $this->getCurrentElement()->getValue();
        $this->rewind();
        $allValues = [];
        while ($this->valid()) {
            $allValues[] = $this->getCurrentElement()->getValue();
            $this->next();
        }

This is a false negative, because as long as valid() returns true, getCurrentElement() will return a valid entry and not null. Psalm can not know this and hence shows the error.

In order to “resolve” such issues, they just need to stay in the baseline. If you find a new such issue, you can execute
./bin/psalm --config=Packages/Framework/psalm.xml --set-baseline=Packages/Framework/psalm-baseline.xml
to update the existing baseline.

Using psalter to fix errors

For some types of issues, psalm provides an automatic fixing tool.
See https://psalm.dev/docs/manipulating_code/fixing/

You can run this with e.g.
./bin/psalter --config=Packages/Framework/psalm.xml --issues=InvalidReturnType --php-version=7.2
Make sure you adjust the --issues and --php-version arguments.
Then thoroughly review the changes created by the tool and commit them (optimally with the command executed in the commit message body). Afterwards follow the remaining steps from the guideline above and run psalm with --update-baseline and create a PR

3 Likes