Discourage usage of legacy properties in WP_Site

[flixos90]

Working on #38597, it was figured out that the best solution for handling problems with IDE handling of WP_Networks magic ID property is to rename the actual properties to reflect our current naming conventions. This enhancement will encourage to use the new conventions while still supporting the old ones for legacy code.

Let's do the same for WP_Site:

• $blog_id (string) is replaced with $id (int)
• $site_id (string) is replaced with $network_id (int)
• both legacy names will continue to work through magic methods

[flixos90]

38630.diff​ takes care of the proposed steps.

[johnjamesjacoby]

I reviewed and approve of the proposed changes. Nice work!

[flixos90]

In 38630.2.diff​ I adjusted the to_array() method to be backward-compatible, but at the same time deprecated it.

The method is (as far as I can see) only called in wp_get_sites() which is already deprecated, and it is easily possible to achieve the same effect (only with the new properties instead) by using get_object_vars( $site ). We should probably discuss this change soon (as deprecations should happen early in the cycle), and it would definitely need to be highlighted in a multisite dev note.

If we decide deprecating it is right, let's do that in a separate ticket though.

[flixos90]

38630.3.diff​ brings a few improvements over the previous patch:

• In the to_array() method, it is ensured that the legacy properties are strings.
• The $id and $network_id properties are no longer handled in the __get() and __isset() methods since they are public anyway. They are however still part of __set() since this method is called manually in the constructor.

Looking at the unit tests, I'm wary of this change although I definitely like its approach. Most unit tests failures (there are several) are only a result of the deprecation of to_array() which we can easily adjust, but the other two failures are actual failures, so anyone using get_object_vars() on WP_Site objects or iterating over them would run into problems (see https://core.trac.wordpress.org/ticket/40180#comment:3 for background). I'm starting to think we should hold off changing the public properties, although I don't like to say it.

[flixos90]

This is a developer-centric change that could easily cause unexpected issues. We need to put more thoughts into it and merge it early in a major release. Let's come back to this later.