Fix Psalm incompatibility in callable type

[IanDelMar]

Unlike PHPStan, Psalm does not support a callable type with parameter name.

While both callable(mixed $input=): (bool|WP_Error) and callable(mixed=): (bool|WP_Error) are supported by PHPStan (see PHPStan documentation on callables), Psalm only supports callable(mixed=): (bool|WP_Error) (see Psalm documentation on callables).

See Psalm failing with Internal Psalm error for a callable with parameter name and see Psalm passing for the same callable without parameter name. See PHPStan passing for both.

The callable type callable(mixed $input=): (bool|WP_Error) is used in the Abilities API that was introduced in WordPress 6.9.

Fixes #410

cc @johnbillion

[IanDelMar]

The incompatibility appears to be limited to cases where the parameter is optional. For example, callable(int $num) - as used by WP_Translation_File::make_plural_form_function() - does not trigger an internal Psalm error.

See https://psalm.dev/r/eeb649ffa7

[johnbillion]

Another option is that I can fix this in WordPress core, but is it possible to release a new version of wordpress-stubs based on a WordPress nightly release?

[szepeviktor]

If it is in 1 file please remove it from core.

[IanDelMar]

There are only two callable types with parameter names: callable(mixed $input=): (bool|WP_Error) and callable(int $num) - each occurs in just one file. The callable(int $num) case is not required for Psalm compatibility.

https://github.com/search?q=repo%3AWordPress%2FWordPress+%2Fcallable%5C%28%5Cs%3F.*%5C%24.*%5C%29%5Cs%3F%3A%2F&type=code.

[szepeviktor]

Please tell me guys, what to do!

[IanDelMar]

This has been a known Psalm issue since November last year (see vimeo/psalm#11588). I’d suggest:

• Merging this PR so users who have already moved to the latest release get a version where the issue is resolved. Anyone currently blocked by Psalm could also opt to use the dev version in the meantime.
• Since the parameter name is irrelevant from PHPStan’s perspective (according to the docs on callables: "parameter names are optional and insignificant"), also remove the parameter names in WordPress core wherever they occur, to reduce the chance of reintroducing the same incompatibility elsewhere.
• Once a new WordPress version is released, remove the fix.
• In parallel, we’ll need to wait/hope for a Psalm-side fix, because we can’t assume nobody will add callable types with signatures to core again in future.

@johnbillion When you say “nightly release”, do you mean something like pulling WordPress source via "johnpbloch/wordpress": "6.9.x-dev"? I can see doing that once for this fix, but I wouldn’t want to track it routinely - you’d be following changes that might never make it into an actual release. For this to work cleanly, we’d likely need something like [email protected]<prerelease> (assuming the next release really is 6.9.1 rather than 6.10). Even then, users with "minimum-stability": "stable" wouldn’t benefit. We’d have to ask them either to relax minimum-stability or to pin an explicit dev/pre-release constraint. At that point, I’m not sure it offers any real advantage over simply telling affected users to require dev-master - which is also much simpler for us. And, frankly, I’d be glad if more users did use dev-master, as it helps us catch and avoid issues before they land in a release.

Off topic:
@johnbillion do you know the current status of the broader PHPDoc type discussion in core? My (maybe outdated) understanding is that WordPress is still aligning with the inline documentation standards and that types in this documentation are largely interpreted as PHP types rather than PHPDoc types (except for arrays; and sometimes boolean; true' ad falsewere used for PHPDoc types before they were actual PHP types). However, neithernon-integer-int` nor callable types with signatures are PHP types; I also came across a couple of other non PHP types. Does it make sense opening tickets to start adopting “more modern” types in core? I would be willing to contribute to such efforts. In fact, the stubs - while probably too strict for direct use in core - already provide a broad foundation for doing so.

[kkmuffme]

Fixed the underlying psalm issue in vimeo/psalm#11647 now - should get merged before the next psalm 6.x release, which means the changes of this PR can get reverted then.
Do you want to open a reversing PR (of this PR) already now, so it can just get merged once my psalm PR is merged and psalm is released @IanDelMar ?