Opened 9 years ago
Closed 9 years ago
#35404 closed defect (bug) (fixed)
Incorrect @var docs for WP_Network property types
Reported by: | jdgrimes | Owned by: | jeremyfelt |
---|---|---|---|
Milestone: | 4.5 | Priority: | normal |
Severity: | normal | Version: | 4.4 |
Component: | Networks and Sites | Keywords: | |
Focuses: | docs, multisite | Cc: |
Description
Several of the properties in the WP_Network
class are documented as @var int
when they are actually strings.
The $cookie_domain
is obviously not an integer, since even its default value is a string:
public $cookie_domain = '';
The $id
and $blog_id
properties are also documented as int
although both of them are usually populated directly from the database values, and therefore will usually be integers represented as strings. To further complicate things, the $blog_id
property is declared with a default value that is an integer:
public $blog_id = 0;
Related to this is #33929.
The quandary is whether we should change the docs from int
to string
to maintain back-compat, or whether we should cast the values to integers to meet the docs and dev expectations (I discovered this because I was doing a strict comparison while expecting the network id
to be an integer).
The WP_Network
class was just introduced in 4.4.0, so at first glance it might seem like we don't need to worry about backward compatibility for code that is built expecting these values to be strings despite their docs. However, the $current_site
global has been around for quite some time, so we should be careful to maintain back-compat with the types devs are expecting for its properties. I'm assuming that prior to [34097] the values of $current_site->id
and $current_site->blog_id
were both strings. Of course, it is possible that people have been overriding the $current_site
global with their own implementations, which might be offering these values as integers instead.
Good catches.
$cookie_domain
is the easy one, we can change that tostring
with no issue.I think we're left with changing the docs for
$blog_id
and$id
tostring
. As mentioned, this object has existed in this format as$current_site
for a long time.I'm not sure that strict comparisons for these properties are a good idea in general, as custom implementations may have been doing all sorts of things in the past. It would be interesting to look into what breakage might occur if we did start casting things to integers, especially as we're getting better at clarifying some of this.
In the
WP_Post
docs, we noteA numeric string, for compatibility reasons.
for$post_author
and$comment_count
. We should probably do the same here. We're able to provide anint
$ID
property there because it was sanitized as such long beforeWP_Post
was introduced. We do still assign a default value of0
for these strings. I don't see any harm in that at the moment, asIt also looks like we have the same doc mismatches in
WP_Comment
for$comment_ID
and$comment_post_ID
.