Introduce Phpstan #456
Introduce Phpstan #456
Conversation
|
thanks a lot for picking this up! |
| - | ||
| message: "#^Call to static method create\\(\\) on an unknown class Symfony\\\\Component\\\\HttpClient\\\\HttpClient\\.$#" | ||
| count: 1 | ||
| path: src/ClientFactory/SymfonyFactory.php |
There was a problem hiding this comment.
i guess this means we don't test the factory, otherwise we would depend on the symfony http client.
we should probably add that, but in a separate merge request. and its no blocker for releasing 2.0
| - | ||
| message: "#^Class Http\\\\Client\\\\Plugin\\\\Vcr\\\\NamingStrategy\\\\NamingStrategyInterface not found\\.$#" | ||
| count: 1 | ||
| path: src/DependencyInjection/Configuration.php |
There was a problem hiding this comment.
does the code properly handle a missing vcr plugin? if it does its okay i guess.
There was a problem hiding this comment.
we could composer require the vcr plugin in a first step of the phpstan job, to have it validate correctly. the php tests would discover if we messed up with validation.
some nice to haves would be to have the configuration class error when configuring without vcr without having the plugin installed, and https://github.com/php-http/HttplugBundle/blob/b7ad2c1951edec36ef0bd9897e309b0038c59a54/src/DependencyInjection/HttplugExtension.php should throw an exception at the beginning of the method if vcr is not available.
but probably this still not enough to make phpstan happy, so composer require step for this could be added. that means that phpstan locally would complain about those though 🤔
we can also merge the current state and continue in separate pull requests.
|
@dbu what about raising the PHP version up for |
Co-authored-by: David Buchmann <[email protected]>
Co-authored-by: David Buchmann <[email protected]>
yes, good point. its good to support the legacy versions for legacy projects, but in the new major, we should raise to only support the maintained versions of symfony. we can potentially also drop some BC hacks when we drop old symfony versions. (but lets not do that in the same pull request ;-) ) |
What's in this PR?
Add PHPStan analyse.
Why?
Avoid issue with non typed variables.
To Do