Part of my job at BrightMinded is developing bespoke CiviCRM extensions and functionality for our clients. When I first started writing CiviCRM extensions it quickly became clear that there were a lot of old parts to CiviCRM's core code which deserved some love. I know that CiviCRM only has a small development team, and so I started searching for quick-win code changes that I could contribute back to the CiviCRM codebase in order to improve the quality and developer experience for everyone.
Today I want to share the PHPStan configuration that I have been using to identify poor-quality (and in some cases buggy) code within the CiviCRM core codebase.
What is PHPStan?
PHPStan is a free, open-source static analysis tool for PHP projects. In other words it scans a codebase and identifies code that is likely to be buggy, poor quality or dangerous.
So how can that apply to CiviCRM?
1. PHPStan can identify use of dynamic properties. As well as making the code harder to navigate, dynamic properties are deprecated in PHP 8.2, and so this is a great example of where PHPStan can help us by identifying these PHP 8.2 incompatibilities.
2. It's common practice to document PHP functions with DocBlock comments (i.e. PHPDoc). This will be familiar to you if you've ever written a comment like this:
* Get all of the ACLs for a contact through ACL groups owned by Contact.
* @param int $contact_id
* ID of a contact to search for.
* @return array
* Array of assoc. arrays of ACL rules
* @throws \CRM_Core_Exception
These comments help provide hints to developers, documenting what parameters and return values a function handles. Unfortunately it's easy to write inaccurate DocBlock blocks, and PHPStan can help us identify cases where the comments don't match what the function is actually expecting or returning. This can significantly decrease the risk of future developers inadvertently introducing bugs when using core functions and methods.
3. More generally PHPStan can identify code which isn't necessarily buggy, but would be considered bad practice. Things like possibly undefined variables, unused variables, or passing too many arguments to a function will all be caught by PHPStan (depending on what level of strictness it is set to).
The current state of CiviCRM
PHPStan has different levels, which correspond to how strictly PHPStan checks the codebase. Using the current bleeding-edge (i.e. master) codebase each level currently produces this many errors when currently ran against CiviCRM core:
- Level 0: 2044 errors
- Level 1: 3246 errors
- Level 2: 4749 errors
- Level 3: 6374 errors
- Level 4: 7434 errors
- Level 5: 10276 errors
- Level 6: 34374 errors
- Level 7: 50180 errors
- Level 8: 53186 errors
- Level 9: 59096 errors
The errors at level 0 mainly consist of dynamic properties, and so in my opinion it looks achievable to get the lowest levels down to zero errors (or very close to that).
Running PHPStan against CiviCRM
So you've read this blog and thought, Great! I want to run PHPStan myself. If that's you, then I've uploaded my configuration to GitHub, along with full installation and setup instructions: https://github.com/braders/civicrm-phpstan. The repository is specifically aimed at testing civicrm-core, but the concepts should be transferable for people wanting to test CiviCRM extensions, or a full CiviCRM installation..
Getting setup is a pretty quick process, and I've already managed to identify and fix many quick-wins. Since starting I've been surprised at how quickly the number of errors has dropped, and one day it may even be viable to propose making PHPStan checks a pre-merge check, similar to how the current coding standard checks work.
Alternatively, if you're not interested in running PHPStan yourself, I have also uploaded the output from the level 4 run to GitHub: https://gist.github.com/braders/b004e5daee05bcd256daf2dad01b2d81
Sounds really useful, thanks for posting. Wow 8GB minimum RAM though, what a beast!
Thanks for that post @bradleyt
You have made a huge contribution on this challenging but important task & this blog takes that further - thank you!!