Azuracast: Using PHPStan to discover bugs on the codebase and PRs

[Vaalyn]

In this issue I want to discuss with you if we could add PHPStan as a dev dependency to AzuraCast.

PHPStan is a static analyzer for PHP which enables us to catch a multitude of bugs that would normally go unnoticed until someone notices that something is broken. You can imagine it as a compiler that checks if you have any obvious errors in the code that would be reported by a compiler in a compiled language. PHPStan is build with varying levels of strictness (currently up to 8) that can be chosen by the user so that it is quite easy to first integrate it into an existing project and over time increase the strictness until all types of errors that this tool is able to point out are fixed without being overwhelmed by a huge amount of errors in the beginning.

As a test I have already run PHPStan on the AzuraCast codebase with the strictness level 1 and fixed most of what it found in PR #1127.

Using a static analyzer will help us move faster with more confidence in the code we write by quickly pointing out syntactical errors and errors that would have otherwise gone unnoticed and can also help us evaluate Pull Requests by running the analyzer automatically on them thus reducing the chance of accidentally breaking something.

If you think that this would be good addition to the project I'd like integrate this tool with a basic configuration and submit this as a PR and also help with fixing issues that will be discovered when we increase the strictness to higher levels.

But first of all I'd like to hear what you think about this proposal.

[SlvrEagle23]

@Vaalyn I've looked into PHPStan myself and wanted to play with it for AzuraCast, and clearly it's catching some issues that would otherwise cause us some hard-to-diagnose headaches. Considering the heavily declarative nature of our dependency injection setup, I think static analysis should run extremely well in our environment, as there's no "magic" going on a-la Laravel Facades, etc.

Happy to integrate it into the core. Feel free to submit a PR and let me know how I can run the test suites in my local dev environment.

[Vaalyn]

@SlvrEagle23 I've created the PR with some further information about how to run PHPStan and some additional infos about the setup there.

[SlvrEagle23]

@Vaalyn Provided I ignore the PHPDoc warnings, we're green-lit on strictness level 2 now after some changes to various parts of the code. It even found something that was a bonafide bug that would've prevented renaming files during media editing.

I'm leaving this open so I can trudge through the (admittedly valid) PHPDoc errors and get everything to a clean enough state that we can slowly ratchet up the strictness.

At the highest strictness levels, it seems to incorrectly think that things like constants are always going to be the value they are for its given run. While I do understand its underlying logic, it'll take some significant infrastructure changes to get rid of those legacy constants entirely.

[SlvrEagle23]

@Vaalyn We now have:

• Linting, PHPStan and Codeception on AzuraCast and azuracore, and
• Linting and PHPStan on nowplaying and azuraforms.

All four major repos have Travis CI builds that automatically run their full test suites.

[Vaalyn]

Nice work 👍

I think this is a big step for the project and will help us tremendously in the future. Happy to see this much progress. 😃