[wp-trac] [WordPress Trac] #31723: is_page( false ) === true ?

[WordPress Trac]

#31723: is_page( false ) === true ?
-------------------------------------------+------------------------------
 Reporter:  Compute                        |       Owner:
     Type:  defect (bug)                   |      Status:  new
 Priority:  normal                         |   Milestone:  Awaiting Review
Component:  Query                          |     Version:
 Severity:  normal                         |  Resolution:
 Keywords:  needs-unit-tests dev-feedback  |     Focuses:
-------------------------------------------+------------------------------
Changes (by jipmoors):

 * keywords:  needs-unit-tests => needs-unit-tests dev-feedback


Comment:

 While testing code: returning `true` or `false` on empty(ish) input mostly
 doesn't matter; though one of the two should be an unwanted return value.

 ''The question is: do we want to invalidate unusable values or assume
 'current' item variables apply''

 Logic dictates that we invalidate anything other than an empty call,
 because the default has been overridden.
 If this is applied to all `is_*` functions in `query.php` only one test
 breaks: entering an empty array for is_tag `is_tag(array())` which expects
 to be `true`.

 I can understand why that test has been applied; and it should be removed
 or applied to all other functions aswel.
 There are no comments on the test and situations this is created for.

 '''Feedback requested on validity of this test'''

 Writing the needed tests will be possible when the
 goal/desire/expectations are clearer.

 Suggested 'template' for functions with one parameter:
 {{{
 function is_param($param = null) {
   if ( is_null( $param ) || ! $this->is_param )
         return (bool) $this->is_param;

   $param = (array) $param;

   if ( empty( $param ) )
         return true;

   ... code ...

   return false
 }
 }}}

 '''Changes:'''
 * Changed default value of parameter from !'' to null.
 * Moved the casting to array up in the function.
 * Returning `true` on empty (array).

 This way an empty array can be entered for default behaviour, anything
 else will be made into an array value so page titles can have '0' and be
 found.

 All checks inside the function return 'true' except the first and last
 ones. Which provides much more readable code and debugging.

 If we want to invalidate the empty array the only thing to do is swap the
 array casting and empty check.

 I have a patch with this applied to all functions and they pass all
 current tests.

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