szepeviktor / phpstan-wordpress

WordPress extensions for PHPStan ⛏️
https://packagist.org/packages/szepeviktor/phpstan-wordpress
MIT License
269 stars 27 forks source link

add_menu_page expecting a callable but callable|callable-string should be fine too #85

Closed herndlm closed 3 years ago

herndlm commented 3 years ago

Is this a problem we can fix? Looks like it's coming from the stubs / wordpress docs, but actually it is supposed to be used with callable strings, right? E.g. as in

add_menu_page('Docs', 'Docs', 'manage_options', 'docs', 'my-docs-function', 'dashicons-editor-help');

phpstan reports

Parameter #5 $function add_menu_page expects callable(): mixed, 'my-docs-function' given.

I guess many many more functions are having the same problems?

This is more like a discussion, @szepeviktor want to enable the discussions feature in this repo? ;)

szepeviktor commented 3 years ago

Let's keep everything in issues.

:) Is callable-string a Martin-invention? PHPStan is a tool for OOP. Global function names are not supported as callable yet. There is an open issues for this.

szepeviktor commented 3 years ago

In normal OOP world 'my-docs-function' is [MyClass::class, 'myDocsFunction'] or [$this, 'myDocsFunction'].

Please see this Symfony-inspired technology https://github.com/szepeviktor/Toolkit4WP/blob/43a015f3ee1fc06290fc7b894cdb95a740287f52/src/HookAnnotation.php#L20-L25

johnbillion commented 3 years ago

my-docs-function isn't a valid function name so it's not a valid callable. Should it be my_docs_function ?

herndlm commented 3 years ago

callable-string is real :)

And yeah this was an invalid example, sorry. Was reported by a colleague and the default value of '' is also is not accepted. Not a 100% sure if it would accept valid function names, but I think it doesn't

I prefer real callables but I think that's not the WP way, right?

johnbillion commented 3 years ago

If you pass a valid callback function name to that parameter there should not be an error. What's the actual value you're passing? Is it the string name of a function that exists? my_function is a valid callable as long as that a function with that name exists. OOP has nothing to do with it.

herndlm commented 3 years ago

OK, thx. It's only about the default value of '' then. Which cannot be specified in case one of the later args needs to be set.

johnbillion commented 3 years ago

Ah yes the default value of '' is a known issue, it should be nullable instead. I think that's covered in one of the PHPStan tickets for WordPress core, maybe https://core.trac.wordpress.org/ticket/52217

herndlm commented 3 years ago

Thx for the insights! That's enough for me :)