[wp-trac] [WordPress Trac] #52539: callable typehint thows static analysis errors

[WordPress Trac]

#52539: callable typehint thows static analysis errors
-------------------------+-------------------------------------------------
 Reporter:  dingo_d      |      Owner:  (none)
     Type:  defect       |     Status:  new
  (bug)                  |
 Priority:  normal       |  Milestone:  Awaiting Review
Component:  General      |    Version:  trunk
 Severity:  normal       |   Keywords:  dev-feedback 2nd-opinion needs-
  Focuses:  docs         |  patch
-------------------------+-------------------------------------------------
 While working on client projects I tend to use a lot of automated tools to
 make my code better. One such tool is PHPStan.

 In a project I have this call

 {{{#!php
 <?php
 \add_menu_page(
         \esc_html__('Blocks', 'eightshift-libs'),
         \esc_html__('Blocks', 'eightshift-libs'),
         self::REUSABLE_BLOCKS_CAPABILITY,
         'edit.php?post_type=wp_block',
         '',
         'dashicons-editor-table',
         4
 );
 }}}

 Lo and behold PHPStan threw an error


 {{{
   79     Parameter #5 $function of function add_menu_page expects
 callable(): mixed, '' given.
 }}}

 I was a bit confused. I know that you can pass a callback to that
 function. But also, you can pass a string that will be the name of the
 callback. So I went to the documentation and to the trac, and I found the
 problem:

 https://core.trac.wordpress.org/browser/tags/5.6/src/wp-
 admin/includes/plugin.php#L1325

 The documentation is marked as


 {{{
 * @param callable $function   The function to be called to output the
 content for this page.
 }}}

 But that's not true. Sure, you can provide a callback here. But you can
 also provide a string. So I got to thinking. Wait, don't `add_action` and
 `add_filter` suffer from the same issue?

 Sure they do. I reckon there are a lot of such cases to be found thought
 the core. In #42509, John Blackbourn mentions around 80 such occurrences
 in the core I found 78 matches across 23 files.


 == So what can we do about it?

 Simple, just mention that the `callable` can also be a `string` denoting
 the callback that will be added in places where this can be done (like in
 the examples above). Some of those cannot be replaced with a string,
 because they have `is_callable` check.

 So in the case of adding menu page we'd have

 {{{
 * @param callable|string $function   The function to be called to output
 the content for this page or the string denoting the callback that will
 output the content for this page.
 }}}

 Since in the case of `add_menu_page` the default is already set as empty
 string, that seems like it would solve all the static analysis issues of
 this kind.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/52539>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform