-
Notifications
You must be signed in to change notification settings - Fork 3
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
PHPStan is a good addition to our QA toolkit and with improvements PHPStan has made over the years is now a viable tool for YoastCS to use (previously it would give way too many false positives).
This commit:
* Adds a separate job to the `basics` workflow in GH Actions.
Notes:
- I've chosen **not** to add PHPStan to the Composer dependencies as it would add dependencies which could conflict/cause problems for our test runs due to those defining token constants too (PHP-Parser).
- We could potentially use [Phive](https://phar.io/) to still have a setup which can be used locally, but just running locally from a PHPStan PHAR file should work just fine.
- For CI, PHPStan will be installed as a PHAR file by `setup-php` now.
This does carry a risk _if_ PHPStan would make breaking changes or if a new release adds rules for the levels being scanned as, in that case, builds could unexpectedly start failing.
We could fix the version `setup-php` action installs to the current release `1.10.50`, but that adds an additional maintenance burden of having to keep updating the version as PHPStan releases pretty often.
So, for now, I've elected to run the risk of random failures. If and when those start happening, we can re-evaluate.
* Adds a configuration file for PHPStan.
Notes:
- PHPStan needs to know about our dependencies (PHPCS et al), so I'm (re-)using the bootstrap file we have for our tests to load the PHPCS autoloader and register the standard with the PHPCS autoloader as we can't add an `autoload` directive to our `composer.json` file as it would cause problems with the PHPCS autoloader.
* Adds the configuration file to `.gitattributes` and the typical overload file for the configuration file to `.gitignore`.
Refs:
* https://phpstan.org/
* https://phpstan.org/config-reference- Loading branch information
Showing
4 changed files
with
139 additions
and
7 deletions.
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,3 +6,4 @@ phpcs.xml | |
| phpunit.xml | ||
| .phpunit.result.cache | ||
| build/ | ||
| phpstan.neon | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| parameters: | ||
| phpVersion: 70200 | ||
| level: 9 | ||
| paths: | ||
| - Yoast | ||
| bootstrapFiles: | ||
| - phpunit-bootstrap.php | ||
| scanDirectories: | ||
| - vendor/wp-coding-standards/wpcs/WordPress | ||
| # treatPhpDocTypesAsCertain: false | ||
| # reportUnmatchedIgnoredErrors: false | ||
| dynamicConstantNames: | ||
| - YOASTCS_ABOVE_THRESHOLD | ||
|
|
||
| ignoreErrors: | ||
| # Level 3 | ||
| - | ||
| # This is an issue with the PHPCS docs. Can't be helped. | ||
| message: '`^Property PHP_CodeSniffer\\Config::\$basepath \(string\) does not accept null\.$`' | ||
| path: Yoast/Tests/Utils/PSR4PathsTraitTest.php | ||
| count: 1 | ||
| - | ||
| # This is a test specifically checking the handling when an invalid value is passed, so this is okay. | ||
| message: '`^Property YoastCS\\Yoast\\Tests\\Utils\\PSR4PathsTraitTest::\$psr4_paths \(array\<string, string\>\) does not accept array\<int, string\>\.$`' | ||
| path: Yoast/Tests/Utils/PSR4PathsTraitTest.php | ||
| count: 1 | ||
|
|
||
| # Level 4 | ||
| - | ||
| # Bug in PHPStan: PHPStan doesn't seem to like uninitialized properties... | ||
| message: '`^Property \S+Sniff::\$target_paths \(array<string, string>\) in isset\(\) is not nullable\.$`' | ||
| path: Yoast/Sniffs/Files/TestDoublesSniff.php | ||
| count: 1 | ||
| - | ||
| # Defensive coding in the PSR4PathsTrait as the property value is user provided via the ruleset. This is okay. | ||
| message: '`^Strict comparison using === between true and false will always evaluate to false\.$`' | ||
| paths: | ||
| - Yoast/Sniffs/Files/FileNameSniff.php | ||
| - Yoast/Sniffs/NamingConventions/NamespaceNameSniff.php | ||
| - Yoast/Tests/Utils/PSR4PathsTraitTest.php | ||
|
|
||
| # Level 5 | ||
| # We're not using strict types, so this will be juggled without any issues. | ||
| - '#^Parameter \#3 \$value of method \S+File::recordMetric\(\) expects string, \(?(float|int|bool)(<[^>]+>)?(\|(float|int|bool)(<[^>]+>)?)*\)? given\.$#' | ||
|
|
||
| # Level 7 | ||
| - | ||
| # False positive: the explode will never return `false` as it is never given an empty string separator. | ||
| message: '`^Parameter #2 \$array of function array_map expects array, array<int, string>\|false given\.$`' | ||
| path: Yoast/Sniffs/Commenting/CoversTagSniff.php | ||
| count: 1 | ||
| - | ||
| # False positive: the preg_replace will never return `null` as the regex is valid. | ||
| message: '`^Parameter #1 \$str of function strtolower expects string, string\|null given\.$`' | ||
| path: Yoast/Sniffs/Files/FileNameSniff.php | ||
| count: 1 | ||
| - | ||
| # False positive: the preg_replace will never return `null` as the regex is valid. | ||
| message: '`^Parameter #2 \$str of function explode expects string, string\|null given\.$`' | ||
| path: Yoast/Sniffs/NamingConventions/NamespaceNameSniff.php | ||
| count: 1 | ||
| - | ||
| # False positive: the passed value will only ever be an integer, PHPStan just doesn't know the shape of the array. | ||
| message: '`^Binary operation "\+" between int\|string and 1 results in an error\.$`' | ||
| count: 2 | ||
| path: Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php | ||
| - | ||
| # False positive: the passed value will only ever be an integer, PHPStan just doesn't know the shape of the array. | ||
| message: '`^Parameter #2 \$start of method PHP_CodeSniffer\\Files\\File::findNext\(\) expects int, int\|string given\.$`' | ||
| count: 2 | ||
| path: Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php | ||
| - | ||
| # False positive: the passed value will only ever be an integer, PHPStan just doesn't know the shape of the array. | ||
| message: '`^Parameter #1 \$stackPtr of method PHP_CodeSniffer\\Fixer::replaceToken\(\) expects int, int\|string given\.$`' | ||
| path: Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php | ||
| count: 1 | ||
| - | ||
| # This is a test specifically checking type handling, so this is okay. | ||
| message: '`^Parameter #1 \$path of static method YoastCS\\Yoast\\Utils\\PathHelper::trailingslashit\(\) expects string, bool\|string given\.$`' | ||
| path: Yoast/Tests/Utils/PathHelperTest.php | ||
| count: 1 | ||
|
|
||
| # Not a real issue (x 4), PHPstan just doesn't know the format of the array well enough. | ||
| - | ||
| message: '`^Binary operation "\+" between int\|string and 1 results in an error\.$`' | ||
| count: 2 | ||
| path: Yoast/Sniffs/Tools/BrainMonkeyRaceConditionSniff.php | ||
|
|
||
| - | ||
| message: '`^Parameter #2 \$start of method PHP_CodeSniffer\\Files\\File::findNext\(\) expects int, int\|string given\.$`' | ||
| count: 2 | ||
| path: Yoast/Sniffs/Tools/BrainMonkeyRaceConditionSniff.php | ||
|
|
||
| - | ||
| message: '`^Binary operation "\+" between int\|string and 1 results in an error\.$`' | ||
| count: 1 | ||
| path: Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php | ||
|
|
||
| - | ||
| message: '`^Parameter #2 \$start of method PHP_CodeSniffer\\Files\\File::findNext\(\) expects int, int\|string given\.$`' | ||
| count: 1 | ||
| path: Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php |