[wp-trac] [WordPress Trac] #33929: Wrong data type for several variables holding a wpdb query result

WordPress Trac noreply at wordpress.org
Sun Nov 15 14:29:21 UTC 2015


#33929: Wrong data type for several variables holding a wpdb query result
----------------------------------------+-----------------------------
 Reporter:  tfrommen                    |       Owner:  boonebgorges
     Type:  defect (bug)                |      Status:  assigned
 Priority:  normal                      |   Milestone:  Future Release
Component:  General                     |     Version:
 Severity:  normal                      |  Resolution:
 Keywords:  has-patch needs-unit-tests  |     Focuses:
----------------------------------------+-----------------------------

Comment (by tfrommen):

 I would like to ask for how to proceed with this ticket.

 I understand
 1. handling the proposed changes individually is the way to go;
 1. almost every change ''might'' break some specific plugin (or theme)
 doing strict comparisons (with a type that shouldn't even be there,
 though!).

 But what are we going to do here?

 Would you like individual tickets? Or would individual patches in this
 ticket be good already?

 As to the possible breaking plugin/theme functionality. I guess
 @boonebgorges (or someone else) will ''slurp'' the plugins and check for
 usage of the changed action/filter parameters and function return values.
 Once again, if a function currently returns `string|null`, while its
 PHPDoc states it would return `int` instead, of course, strict comparisons
 with both `string` and `null` will break. On the other hand, no one should
 actually be doing this (in the case with `int` being included in the
 annotation).

 In the case where the annotation says `int|null` and the passed type is
 `string|null` instead, we should keep the `null` return, while casting the
 `string` to an `int`.

 What are yout thoughts? Can I be of any help here?

 Also, I don't really understand what (kind of) unit tests you would like
 to have here... My proposed changes are about data types. So would you
 want a couple of `$this->assertTrue( is_int( some_func() ) );`, or what do
 you have in mind?

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


More information about the wp-trac mailing list