Conversation
PHPStan has improved a lot since version 1.12 used by SimplePie until now. Take advantage of explicit PHPStan phpVersion range. Define explicit range of minimum and maximum PHP version supported. Useful e.g. for function signatures, which have changed over time https://phpstan.org/config-reference#phpversion Do not run PHPStan for PHP < 7.4 but the tests are covered by phpVersion min, so it is actually enough to run PHPStan once on any supported PHP version. Cannot support PHPUnit >= 10 at the same time as PHP 7.2.
Keep change for another PR
demo/index.php
Outdated
|
|
||
| // ... and display it. | ||
| echo '<p>' . htmlspecialchars($feed->error()) . "</p>\r\n"; | ||
| if (is_string($feed->error())) { |
There was a problem hiding this comment.
Interestingly, the demo supports multifeed: https://simplepie.org/demo/?feed[]=http://rss.cnn.com/rss/cnn_topstories.rss&feed[]=http://blogs.law.harvard.edu/home/feed/rdf/
We should probably disable that since we have deprecated it. But that can be done separately.
There was a problem hiding this comment.
Deprecating it in #948 along with some other demo cleanups. Including your that make sense outside of PHPStan bump.
Flagged by PHPStan, needed for simplepie#939
Flagged by PHPStan (strict mode). Found during #939 [PHPUnit documentation](https://docs.phpunit.de/en/12.3/assertions.html) proposes to use either `$this->assertTrue()` or `self::assertTrue()` as discussed in sebastianbergmann/phpunit#1914. A static method should be called statically (I could not even find in the official PHP documentation anything that suggests that it should be allowed otherwise, but it does still work so far): * https://phpstan.org/error-identifiers/staticMethod.dynamicCall * phpstan/phpstan#514
| - name: Run unit tests | ||
| run: composer test | ||
|
|
||
| # It would be sufficient to run PHPStan with one supported PHP version |
There was a problem hiding this comment.
Older PHP versions have quirks that sometimes need to be accounted for. IIRC, running PHPStan on those versions could sometimes point them out.
Though we cannot use PHPStan 2 on unsupported versions (and trying to support both PHPStan 1.0 and 2.0 would be too much) so the versions that would benefit from it the most will not be covered.
Even PHPStan itself runs static analysis on the complete compatible subset of the matrix: https://github.com/phpstan/phpstan-src/blob/53e98f9975b28de5d55b7b37cce3dcfc4adfca0e/.github/workflows/static-analysis.yml#L82-L87
🤷♀️
There was a problem hiding this comment.
From PHPStan 2.0, which supports a PHP version range, variations between PHP versions on the whole range are accounted for, for instance PHP functions that have changed signature over time. I am not aware of cases where it would make a difference to run PHPStan on multiple PHP versions, since the same rules will be checked (as opposed to PHPStan 1.x, where it could make a difference)
| @@ -1,65 +1,25 @@ | |||
| parameters: | |||
| phpVersion: | |||
| min: 70200 # PHP 7.2 | |||
There was a problem hiding this comment.
https://phpstan.org/config-reference#phpversion
PHPStan will automatically infer the
config.platform.phpversion from the lastcomposer.jsonfile it can find, if not configured in the PHPStan configuration file.
Looks like it uses the requires field:
| "phpunit/phpunit": "^8 || ^9 || ^10", | ||
| "phpstan/phpstan": "~2.1.25", | ||
| "phpstan/phpstan-phpunit": "~2.0.7", | ||
| "phpunit/phpunit": "^8 || ^9", |
There was a problem hiding this comment.
Cannot support PHPUnit >= 10 at the same time as PHP 7.2 for now.
Any idea why that is the case?
There was a problem hiding this comment.
Additional debugging welcome (but maybe not urgent). I am facing a symbol discovery issue when trying to support both PHPUnit >= 10 and PHP 7.2 at the same time
| # Only occurs on PHP ≤ 7.4 | ||
| reportUnmatched: false | ||
| excludePaths: | ||
| analyseAndScan: |
There was a problem hiding this comment.
Why is this needed? None of these are listed under paths.
There was a problem hiding this comment.
Right, mainly because it may be interesting to run PHPStan manually on selected parts of the code base.
Furthermore (more tests welcome), I have the impression that paths is not sufficient and files outside those paths are also scanned based on other discovery rules
There was a problem hiding this comment.
Using echo '<?php \PHPStan\dumpType(1 + 1);' | tee .git/foo.php build/foo.php vendor/foo.php and composer phpstan -- -v ., I verified that .git is not necessary while the other two are.
Co-authored-by: Jan Tojnar <jtojnar@gmail.com>
Since simplepie#939 has been waiting for a long time, here is a PR still using PHPStan 1 (so it should be easier to merge), but fixing many problems caught by PHPStan 2.
|
A subset of this PR is available at #957 to ease merging |
https://github.com/phpstan/phpstan/blob/2.0.x/UPGRADING.md
PHPStan has improved a lot since version 1.12 used by SimplePie until now.
Take advantage of explicit PHPStan phpVersion range: define explicit range of minimum and maximum PHP version supported. Useful e.g. for function signatures, which have changed over time https://phpstan.org/config-reference#phpversion
Do not run PHPStan for PHP < 8.0 but the tests are covered by phpVersion min, so it would actually be enough to run PHPStan once on any supported PHP version (keep that optimisation for another PR).
Cannot support PHPUnit >= 10 at the same time as PHP 7.2 for now.