Static Analyzer (PHPStan) Integration

[UlrichEckhardt]

Greetings,

this PR integrates PHPStan into the Clockwork sources. The analyzer is automatically executed on every push using a Github action. As alternatives to PHPStan, there would be Psalm and perhaps SonarQube. I have worked with Psalm before and it works similarly to PHPStan. About SonarQube, I can't say much, but it seems to have a much larger scope. In particular, it provides a multi-language (not just PHP) framework and also includes support for making code reviews.

If you look at the README, there is a section on how to work with PHPStan, also the commit messages should contain some interesting context infos.

As for what drove me to develop this

• It helps find bugs (two of them just recently fixed as byproducts from this integration).
• It prevents regressions.
• It gives the devs more time to work on the important stuff. ;)

Cheers!

Uli

[itsgoingd]

Hey, I'm not sure how I feel about this, hence the delay in responding.

I can see how this can be helpful, but on the other hand also very annoying with false positives, especially in the slightly unconventional Clockwork code-base.

Anyway, before we can think about trying this, we need to solve two big problems:

• How to deal with third-party dependencies. One solution would be to require-dev everything, which I'm not excited about, but whatever. The unique issue with Clockwork is, that we sometimes support multiple versions of a dependency, and we can't install all of them at the same time. 🤔
• Obviously we can't do level 9, as demonstrated by the incredible amount of exceptions needed to make it pass. We would need to settle on a lower level and possibly even some custom configuration to get around the Clockwork code-base quirks.

Thanks for the PR tho.

[UlrichEckhardt]

About how to deal with third-party dependencies, I have no idea. I never worked on a similar codebase that had so many "optional" parts. The approach I took here was to simply restrict scanning to the core components and ignore the optional ones. I could imagine running multiple scans with different third-party dependencies installed, but I have no clear idea how to manage that.

The scanning level 9 isn't actually a problem. Yes, it finds lots of issues, but it doesn't raise them as errors. Instead, they first get matched against the known ones in the baseline. That way, you will only receive notifications for code that is touched by your own changes, and that isn't much in practice.

I just updated the PR, there were some changes to the code it scanned. Still, I'm also not fully satisfied by the use of a baseline in the code. What I prefer is the Code Quality feature of GitLab, which manages the baseline for you as part of its CI/CD. That relieves you from any manual baseline management and it integrates in the code review.

[UlrichEckhardt]

Take a look at UlrichEckhardt#6. This makes any baseline completely redundant, because that is now managed by GitHub's infrastructure and properly integrated into the PRs. I think that this could be the way to go.

What do you think?