[wp-trac] [WordPress Trac] #63975: wp_delete_auto_drafts passes post ID as string instead of as int

[WordPress Trac]

#63975: wp_delete_auto_drafts passes post ID as string instead of as int
-----------------------------------------------+---------------------------
 Reporter:  kkmuffme                           |       Owner:  kkmuffme
     Type:  defect (bug)                       |      Status:  assigned
 Priority:  normal                             |   Milestone:  Awaiting
                                               |  Review
Component:  Posts, Post Types                  |     Version:  3.4
 Severity:  trivial                            |  Resolution:
 Keywords:  has-patch changes-requested close  |     Focuses:
-----------------------------------------------+---------------------------

Comment (by westonruter):

 @kkmuffme Thank you for providing a test case that would cause the fatal
 error.

 You suggested making this change to `wp_delete_auto_drafts()`:

 {{{#!diff
 - wp_delete_post( $delete, true );
 + wp_delete_post( (int) $delete, true );
 }}}

 But to actually fix the problem in a more robust way, any such casting
 should be done in `wp_delete_post()` instead where `do_action()` is
 called:

 {{{#!diff
 - do_action( 'before_delete_post', $post_id, $post );
 + do_action( 'before_delete_post', (int) $post_id, $post );
 }}}

 Or, even better, to coerce the `$post_id` at the top of the function so
 that all usages thereafter for all filters and actions will get the
 expected type passed:

 {{{#!diff
   $post = get_post( $post );
 + $post_id = $post->ID;
 }}}

 This will then ensure that `int` is passed to the
 `delete_post_{$post->post_type}`, `delete_post`,
 `deleted_post_{$post->post_type}`, `deleted_post`, and `after_delete_post`
 actions. Then we wouldn't have ensure the proper type being passed into
 `wp_delete_post()` every time it is called. We only update it once in
 `wp_delete_post()` itself.

 The issue here is somewhat distinct from the related issues with the data
 types being passed into filters, per #51525 and Peter's
 [https://peterwilson.cc/type-declarations-with-wordpress-filters/ blog
 post], where the ''initial'' value in the filter callback cannot be
 strictly typed because themes and plugins can do bad things and return
 arbitrary values. I similarly advocate for a
 [https://github.com/westonruter/wp-plugin-
 template/blob/main/.gemini/styleguide.md#php convention] to used `mixed`
 as the type for the initial arg passed to a filter callback, for example:

 {{{#!php
 <?php
 /**
  * Filters foo.
  *
  * @param string|mixed $foo Foo.
  * @return string Foo.
  */
 function filter_foo( mixed $foo, int $post_id ): string {
         if ( ! is_string( $foo ) ) {
                 $foo = '';
         }
         /**
          * Because plugins do bad things.
          *
          * @var string $foo
          */

          // Filtering logic goes here.

          return $foo;
 }
 add_filter( 'foo', 'filter_foo', 10, 2 );
 }}}

 But the issue here are ''actions'' not filters. Even in my example here,
 you see I have `int $post_id` because I'm expecting that the phpdoc which
 documents this hypothetical filter `foo` is correct in that an `int` is
 always passed. This then also applies to all arguments passed to actions,
 which cannot have their values manipulated by callbacks. The phpdoc for
 the action should be a contract for what value is expected when the
 arguments are passed to the callback.

 I support this change to ensure that `$post_id` gets programmatically
 coerced to `int` at the top of `wp_delete_post()`, if you want to prepare
 a patch.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/63975#comment:23>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform