Add type checking for hook callbacks in `add_action` and `add_filter`

[johnbillion]

This implements part of #106.

I'm learning more about PHPStan internals as I work on this, so it's going to take some time 😄 . Once everything's working and covered by tests then I'll refactor it.

Applicable to all hooks:

• [X] The number used in the $accepted_args parameter should match the number of parameters that the callback accepts
• [x] The callback for add_filter must return a value
• [x] The callback for add_action must return void

~~Applicable when the hook signature is known (eg. a WordPress core hook):~~

• [ ] ~~The signature of the callback function should be checked against the parameters that get passed to the hook~~
• [ ] ~~A filter should not be used as an action, nor vice versa~~
• [ ] ~~The callback for add_filter must return a value with a type which is compatible with the type passed to it~~

I've decided to implement the WordPress core hook handling in a separate PR because it relies on the wp-hooks API package that I'm writing and it's not ready yet.

[szepeviktor]

This PR brings a fantastic feature to phpstan-wordpress.

[johnbillion]

I'm stuck on isCallable()->yes() not returning the expected value in some cases. Opened https://github.com/phpstan/phpstan/discussions/7715 to ask for help.

[szepeviktor]

I love when one finds an upstream bug during working.

[johnbillion]

We're all green ✅ but I want to add some more tests in.

[szepeviktor]

[johnbillion]

@szepeviktor @herndlm Would be great to get this in

[herndlm]

ok then, I'd say go for it! :)

[szepeviktor]

Thank you! and Thank you!

[szepeviktor]

@johnbillion @herndlm We have a bunch of problems. https://app.travis-ci.com/github/szepeviktor/phpstan-wordpress/builds/257619024 Are they coming from this PR?

[szepeviktor]

Or PHPStan, or WP stubs?

[szepeviktor]

Last successful test back in September was also with WP 6.0.1

Locking php-stubs/wordpress-stubs (v6.0.1)

[herndlm]

there are still the \FQCN problems there was another issue about. I had an easy solution in mind, but I think you didn't like it because it involved a phpstan min version bump. but I'll check that one out quickly.

and the rest are theme related type changes? could be stubs?

[szepeviktor]

there are still the \FQCN problems

Let's close our eyes for this type of problems! 🙈

[johnbillion]

Yeah might be related to #118 or #119 :(

[szepeviktor]

All right guys! Here comes a new release ... 🪨

[herndlm]

oh, this was actually one thing, the FQCN change in PHPStan 1.8.7 was causing the WP_Theme extension class to not do anything anymore (because the extension relied on the old faulty behaviour). should be fixed with https://github.com/szepeviktor/phpstan-wordpress/pull/122

[swissspidy]

@johnbillion How well is this supposed to work yet? I am getting lots of "Filter callback return statement is missing" false positives.

Example:

https://github.com/GoogleForCreators/web-stories-wp/blob/c3b5eec59b0bb79dc096260f5c9614cffc3b3778/includes/Admin/Admin.php#L75C45-L78

The top three lines are getting flagged because those methods don't have a return type (just in PHPDoc). The last one has a PHP return type and is not flagged.

Looks like it doesn't take @return doc blocks into account at all.

[johnbillion]

It's supposed to be perfect, beautiful, flawless, delightful, and kind, but it probably needs some tweaking. Can you open a new issue please?

[szepeviktor]

FYI @swissspidy There is no string|mixed type as mixed includes Everything :)

Please open that issue with the first problem 🚀

[herndlm]

thx again for this, renovatebot is just updating this in our projects, CI still green so far 🤞 🎉