Opened 10 years ago
Closed 10 years ago
#35404 closed defect (bug) (fixed)
Incorrect @var docs for WP_Network property types
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| 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_domainis the easy one, we can change that tostringwith no issue.I think we're left with changing the docs for
$blog_idand$idtostring. As mentioned, this object has existed in this format as$current_sitefor 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_Postdocs, we noteA numeric string, for compatibility reasons.for$post_authorand$comment_count. We should probably do the same here. We're able to provide anint$IDproperty there because it was sanitized as such long beforeWP_Postwas introduced. We do still assign a default value of0for 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_Commentfor$comment_IDand$comment_post_ID.