Add PHPStan analysis, level 5 #195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HiddenString is in bootstrapFiles because it defines a class alias
… keeping PHPStan semi-happy
In CI on all supported versions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bugs and deprecations like the one fixed in #194 should be caught by static analysis (running in CI), but unfortunately Psalm is lagging behind and it doesn't support PHP 8.4 yet vimeo/psalm#11107
This adds PHPStan, another static analyzer that seems to be (more) actively developed, and runs it in CI for all supported versions, while Psalm can only be run on 8.3 max. The PR doesn't replace Psalm just yet.
I've tried fixing most of the bugs reported on PHPStan level 5 (out of 10), and have added the rest, which I consider "errors caused by defensive programming" to the baseline file, as I don't want to remove the defensive runtime checks, at least not now.
I haven't refactored the code in any significant way, although for example the class alias in
halite/src/HiddenString.php
Line 4 in ef9c722
could be removed and the Halite's
HiddenStringreplaced with the class from the extra package as Halite's major version has changed since the change was introduced, see #124 for details.Please consider the PHPStan "move" a WIP/PoC thing at the moment, but the PR can be merged of course. I can work on getting PHPStan checks to higher levels but wanted to check first whether adding PHPStan (or replacing Psalm with PHPStan) is something you'd like to go with.