Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PHPStan to QA checks #368

Merged
merged 1 commit into from
Dec 14, 2023
Merged

Add PHPStan to QA checks #368

merged 1 commit into from
Dec 14, 2023

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Dec 14, 2023

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 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:

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
@jrfnl jrfnl added this to the 3.0 milestone Dec 14, 2023
@coveralls
Copy link

Coverage Status

coverage: 99.411%. remained the same
when pulling f6eda06 on JRF/add-phpstan
into a9f5054 on develop.

@jrfnl jrfnl merged commit e5faf47 into develop Dec 14, 2023
30 checks passed
@jrfnl jrfnl deleted the JRF/add-phpstan branch December 14, 2023 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants