From b0bbd276e0894471bdf75738be4309eeab5a80d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Sun, 28 May 2023 20:01:34 +0200 Subject: [PATCH] Document how to deal with checks This addresses question 2 and 3 from #472 --- source/contribute.rst | 71 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 5 deletions(-) diff --git a/source/contribute.rst b/source/contribute.rst index 457b412b..6f133695 100644 --- a/source/contribute.rst +++ b/source/contribute.rst @@ -410,15 +410,76 @@ constraint in composer.json and run: $ composer update -Running Tests -------------- +Dealing with checks and tools +----------------------------- + +We get lots of PRs, and each of them goes though a series of checks that +should catch obvious mistakes, so that we can focus on higher order +issues. The checks are fairly standardized across all our projects, so +here is a list of the most common ones and how to deal with them. + +Coding standard check +~~~~~~~~~~~~~~~~~~~~~ + +We use `PHP_CodeSniffer `_ +along with the `Doctrine Coding Standard +`_. + +To get a list of coding standard issues, run: + +.. code-block:: console + + $ vendor/bin/phpcs -You must have installed the library with composer and the dev -dependencies (default). To run the tests: +To automatically fix some of the issues, run: .. code-block:: console - $ ./vendor/bin/phpunit + $ vendor/bin/phpcbf + +Some issues are impossible to fix automatically, so you will have to fix +them manually. + +Static analysis +~~~~~~~~~~~~~~~ + +We use two different static analysis tools, that can be complementary: + +- `Psalm `_ +- `PHPStan `_ + +It might happen that these tools report false positives. In that case, +we try to report the false positives upstream, and then we ignore them +in `psalm.xml` or `phpstan.neon`, along with a link to the bug report. +When things get overwhelming, for instance when upgrading Psalm or +PHPStan, we use baseline files, but as a last resort: it's better to +have new code pass analysis with the latest version of the tools than to +block the ugprade until every single issue is addressed. +If you are looking for something to contribute, you can try to +reduce the baseline files in repositories that have them. +This might happen accidentally when working on code, and both tools are +configured to let you know when you should remove lines from the +baseline. + +We never rely on `@psalm-suppress` except in some Symfony bundles. We +are aware of this inconsistency, and might resolve it someday. Until +then, try to be consistent with the repository you are contributing to. + +Both tools understand most of each other annotations, and we use +`@psalm-`-prefixed annotations and let PHPStan do the translation. We +use prefixed annotations for advanced features that are not understood +by all IDEs yet. + +Tests +~~~~~ + +We use `PHPUnit `_ for our tests. You can run them +with `vendor/bin/phpunit`. We often have more than just one PHPUnit +check, because we want to run them with different versions of PHP, or +with different versions of infrastructure components (e.g. different +RDBMS), etc. All these jobs produce coverage reports, which are gathered +and sent to Codecov. If you see a coverage drop, it is likely that you +are missing a test for some code you added. Security Disclosures --------------------