[wp-trac] [WordPress Trac] #56198: Check cache key type

[WordPress Trac]

#56198: Check cache key type
-------------------------+---------------------
 Reporter:  tillkruess   |       Owner:  (none)
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  6.1
Component:  Cache API    |     Version:  trunk
 Severity:  normal       |  Resolution:
 Keywords:  has-patch    |     Focuses:
-------------------------+---------------------
Changes (by SergeyBiryukov):

 * milestone:  Awaiting Review => 6.1


Comment:

 Thanks for the patch! I have a few notes:

 * It looks like the conditional is reversed in individual methods, causing
 unit tests to fail:
 {{{
 if ( $this->_valid_key( $key ) ) {
         return false;
 }
 }}}
  This should be:
 {{{
 if ( ! $this->_valid_key( $key ) ) {
         return false;
 }
 }}}
 * The `'empty string'` part should be translatable too, but it's not. I
 think the notice could be split into two for clarity:
  * For empty strings: `Cache key must not be empty.`
  * For other types: `Cache key must be integer or non-empty string, %s
 given.`
 * A translator comment is needed for the string, something like:
 {{{
 /* translators: %s: The type of the given cache key. */
 }}}
 * We don't use `debug_backtrace()` like that anywhere else in core, I
 think for performance reasons. We might need to pass the caller method
 name to `::_valid_key()` instead, or move the message to the calling
 methods.

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