Use access modifiers in classes, not the var keyword

[wonderboymusic]

The minimum required PHP version is 5.2.4. The var keyword is a relic of PHP 4. Let us open our hymnals to php.net:

Note: The PHP 4 method of declaring a variable with the var keyword is still supported for compatibility reasons (as a synonym for the public keyword). In PHP 5 before 5.1.3, its usage would generate an E_STRICT warning.

PHP 5 has much better support for OO features like access modifiers in classes. WP also has a history of including PHPDoc blocks with code and using the @access tag. However, the tag is meaningless if it doesn't match the supplied access modifier.

PHPDoc blocks are present 1) for inline documentation, sure but mainly 2) to allow automatic generation of documentation when used with a command-line tool like phpDocumentor or (IMO, the far superior) ApiGen.

If I specify the following:

/**
 * @access private
 */
var $prop;

$prop will appear in the documentation as public because var means public.

I have modified class properties throughout the codebase to use access modifiers instead of var.

• If no PHPDoc was present, I made the property public, which it already was.
• If the @access tag was present, I used its value for the property.
• I then ran Unit Tests which produced some immediate fatal errors do to existing core code that was trying to access properties in a public way that had @access set to private.
• I altered those properties to indeed be public and updated the PHPDoc

[scribu]

Even if it was marked with @access private, we can't uniformly make previously public properties into private ones.

We have to evaluate the potential impact on third-party code on a case by case basis.

[wonderboymusic]

This is just a conversation starter. Every property being public in every class in the codebase might actually be just fine, but then the PHPDocs are useless. @access private on public properties and public functions accomplishes nothing. If it's supposed to say to a dev "don't use me" - let's enforce that and change / move / remove those functions at will. If it is being used to actually affect visibility, it doesn't. If it's being used to accurately explain visibility in generated docs, it doesn't.

So are we trying to create documentation that is complete and accurate? We should, and let's do it with proper access modifiers AND proper PHPDocs, even if it requires changing them all to public.

[scribu]

Well, yeah, we should remove the @access private for the properties that we can't make private. We just have to figure out which they are.

[nacin]

All new classes and properties (since 3.2) get proper visibility keywords. For existing properties, I have been following this guideline:

• If the property is public, declare it as such. If the property is designed to be protected or private but cannot be, keep "var" rather than announcing that it is "public" and keep @access.

I've begin to experiment with something else for wpdb: begin to set older properties as protected or private, but then providing a magic getter/setter to handle those values. That way, the functionality is the same, but A) the declarations and docs are accurate and B) we can throw deprecated property notices (but don't yet).

The pattern wpdb uses might not be perfect for all classes, though, in that PHP's magic methods aren't uniformly used and therefore it might not be backwards compatible for all use cases. (We ran into some issues in hyperdb but got that fixed, and otherwise wpdb access is uncommon.)

[wonderboymusic]

Uploaded a new patch that uncontroversially only touches properties that were already public. It ignores all of the properties marked with var and an @access doc block of private or protected. I removed the public @access blocks.

The @access doc block is weird. It is not supposed to document an access modifier, or override it in documentation. It is supposed to be used to suppress documentation of certain elements in your code: ​http://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_tags.access.pkg.html

Protected is allowed, but does absolutely nothing.

While technically that might have been what people had in mind while using it, it seems like it is being used to control access, which access modifiers do automatically. Also, I doubt there is code that people explicitly want to not document - since the documentation generator will catch the private access modifier and mark things as such.

[wonderboymusic]

Refreshed

[dd32]

Keep in mind, that we cannot use access modifiers in a range of files which are included when we bail for old versions of PHP (ie. PHP4): #24915

[helen]

Letting it ride for now - docs-related.

[DrewAPicture]

Per the ​inline documentation standards we're sticking with using @var in this context for the time being. However, if there are missing access modifiers, patches are welcome to include them.

[nacin]

I like this but it needs a close examination to make sure we're following our guidelines. I'm going to punt this and move it into "Inline Docs" for action whenever we're able.

[GaryJ]

Any reason that 22234.2.diff can't go ahead?

[DrewAPicture]

#27167 was marked as a duplicate.

[wonderboymusic]

These files are off-limits:

wp-includes/load.php
wp-includes/version.php
wp-includes/pomo/mo.php
wp-includes/pomo/translations.php
wp-includes/pomo/streams.php
wp-includes/l10n.php
wp-includes/locale.php
wp-includes/plugin.php

Related: #27881. Edit: magic getter sounds like a good solution.

[wonderboymusic]

hackificator expects access modifiers for methods as well.

22234.3.diff​ is an example of converting classes and adding magic __get() method.

[wonderboymusic]

In 28481:

Use proper access modifiers and add a magic __get() method to Custom_Background and Custom_Image_Header.

See #27881, #22234.

[wonderboymusic]

In 28483:

Remove public keyword from some JS functions. Sorry.

Props ocean90.
See #22234.

[wonderboymusic]

In 28486:

Add access modifier (public) to members and methods in WP_Comments_List_Table and WP_Post_Comments_List_Table.

See #27881, #22234.

[wonderboymusic]

In 28487:

Add access modifiers to members and methods in WP_Filesystem_Base. Add magic __get() method for backwards compatibility.

See #27881, #22234.

[wonderboymusic]

In 28488:

Add access modifier (public) to methods in WP_Filesystem_Direct.

See #27881, #22234.

[wonderboymusic]

In 28489:

Add access modifier (public) to members and methods in WP_Filesystem_FTPext.

See #27881, #22234.

[wonderboymusic]

In 28490:

Add access modifier (public) to members and methods in WP_Filesystem_ftpsockets.

See #27881, #22234.

[wonderboymusic]

In 28491:

Add access modifier (public) to members and methods in WP_Filesystem_SSH2.

See #27881, #22234.

[wonderboymusic]

In 28492:

Add access modifier (public) to methods in WP_Importer.

See #27881, #22234.

[wonderboymusic]

In 28493:

Add access modifiers to methods and members of list table classes:

• WP_List_Table is the base class that implements __get() and __call() for BC
• Adds unit tests to confirm that subclasses properly inherit magic methods
• Add modifiers to subclasses: WP_Links_List_Table, WP_Media_List_Table, WP_MS_Sites_List_Table, WP_MS_Themes_List_Table, WP_MS_Users_List_Table, WP_Plugin_Install_List_Table, WP_Plugins_List_Table, WP_Posts_List_Table, WP_Terms_List_Table, WP_Theme_Install_List_Table, WP_Themes_List_Table

See #27881, #22234.

[wonderboymusic]

In 28494:

Add access modifiers to methods and members of WP_Users_List_Table.

See #27881, #22234.

[wonderboymusic]

In 28495:

Add access modifier (public) to methods and members of WP_Upgrader_Skin and its subclasses.

See #27881, #22234.

[wonderboymusic]

In 28496:

Add access modifier (public) to methods and members of WP_Upgrader and its subclasses.

See #27881, #22234.

[wonderboymusic]

In 28503:

Add access modifiers to methods/members in WP_Roles. Add a magic __call() method for BC.

See #27881, #22234.

[wonderboymusic]

In 28504:

Add access modifiers to methods/members in Walker_Category and Walker_CategoryDropdown.

See #27881, #22234.

[wonderboymusic]

In 28505:

Add access modifiers to methods/members in WP_Feed_Cache, WP_SimplePie_File, and WP_Feed_Cache_Transient.

See #27881, #22234.

[wonderboymusic]

In 28506:

Add access modifier to methods of HTTP classes. There are no new private or protected methods, so no need for __call().

See #27881, #22234.

[wonderboymusic]

In 28507:

Add access modifier to methods/members in WP_oEmbed. Adds a magic __call() method for BC.

See #27881, #22234.

[wonderboymusic]

In 28508:

Add access modifier to methods/members in WP_Ajax_Response. Adds a magic __get() method for BC.

See #27881, #22234.

[wonderboymusic]

In 28509:

Add missing access modifiers to methods/members in WP_Customize_*.

See #27881, #22234.

[wonderboymusic]

In 28510:

Add access modifiers to methods/members in WP_Embed.

See #27881, #22234.

[wonderboymusic]

In 28511:

Add access modifiers to methods/members in WP_Error. Add a magic __get() method for BC.

See #27881, #22234.

[wonderboymusic]

In 28512:

Add access modifiers to methods/members in WP_HTTP_IXR_Client.

See #27881, #22234.

[wonderboymusic]

In 28513:

Add missing access modifiers to methods/members in WP_Image_Editor_* classes.

See #27881, #22234.

[wonderboymusic]

In 28514:

Add missing access modifiers to methods/members in Walker and subclasses. Add a magic __get() method.

See #27881, #22234.

[wonderboymusic]

In 28515:

Add missing access modifiers to methods in wp_xmlrpc_server. Add a magic __call() method for BC.

See #27881, #22234.

[wonderboymusic]

In 28516:

Add missing access modifiers to methods in WP and WP_MatchesMapRegex. Add magic __call() and __get() methods to WP_MatchesMapRegex for BC.

See #27881, #22234.

[wonderboymusic]

In 28517:

Add missing access modifiers to methods in WP_Dependencies and _WP_Dependency.

See #27881, #22234.

[wonderboymusic]

In 28518:

Add missing access modifiers to methods in WP_Scripts and WP_Styles.

See #27881, #22234.

[wonderboymusic]

In 28519:

Add missing access modifiers to methods in WP_Comment_Query. Add a magic __call() method for BC.

See #27881, #22234.

[wonderboymusic]

In 28521:

Some classes with __get() method also need __set().

See #27881, #22234.

[wonderboymusic]

In 28522:

Add missing access modifiers to methods in WP_Meta_Query.

See #27881, #22234.

[wonderboymusic]

In 28523:

Add missing access modifiers to methods in WP_Query. Add magic methods for __get(), __set(), __isset(), __unset(), and __call().

Add unit test for magic methods.

See #27881, #22234.

[wonderboymusic]

In 28524:

Classes that have __set() also need __isset() and __unset().

See #27881, #22234.

[wonderboymusic]

In 28525:

Add access modifiers to WP_Text_Diff_Renderer_Table that are compatible with its parent class. Some of the inline docs suggest access that, if implemented, would produce fatal errors.

Add magic methods for BC: get(), set(), isset(), unset(), and call().

See #27881, #22234.

[wonderboymusic]

In 28526:

In wpdb, make some things explicitly public. Do not set anything to private. This would instantly blow up hyperdb in the wild.

See #27881, #22234.

[wonderboymusic]

In 28527:

Add public access modifier to methods/members of WP_Widget and WP_Widget_Factory.

See #27881, #22234.

[wonderboymusic]

In 28528:

Add access modifiers to WP_User_Query.

Add magic methods for BC: get(), set(), isset(), unset(), and
call().

See #27881, #22234.

[wonderboymusic]

In 28529:

In WP_Filesystem_Base, the constructor is a noop, so it shouldn't even be declared. Setting it implies that parent::__construct() should be called by its subclasses.

See #27881, #22234.

[wonderboymusic]

In 28531:

Upgrade _WP_List_Table_Compat to PHP5-style constructor.

Add public to methods/members of WP_Role.
Add public to methods/members of WP_User where appropriate. Don't set private where indicated until more study has occurred and tests have been written for compatibiliy with existing magic methods.

See #27881, #22234.

[wonderboymusic]

In 28532:

WP_Date_Query was only missing one access modifier.

Add access modifier (public) to all default widgets' class methods.

See #27881, #22234.

[wonderboymusic]

In 28533:

WP_Query was only missing one access modifier.

Add access modifier (public) to applicable class methods/members of WP_Rewrite. I am not brave enough to set some of the vars to private without more testing.

See #27881, #22234.

[wonderboymusic]

Marking this fixed - the only remaining vars are in wpdb and WP_Rewrite. Those can be attacked later, if at all.

[DrewAPicture]

In 29139:

Fill out inline documentation for magic methods added to the WP_Text_Diff_Renderer_Table class in [28525].

See #27881, #22234 and #28885.

[DrewAPicture]

In 29140:

Fill out inline documentation for magic methods added to the WP_User_Query class in [28528].

See #27881, #22234 and #28885.

[DrewAPicture]

In 29141:

Fill out inline documentation for magic methods added to the WP_Query class in [28523].

See #27881, #22234 and #28885.

[DrewAPicture]

In 29142:

Fill out inline documentation for magic methods added to the WP_MatchesMapRegex class in [28516].

See #27881, #22234 and #28885.

[DrewAPicture]

In 29143:

Fill out inline documentation for magic methods added to the Walker class in [28514].

See #27881, #22234 and #28885.

[DrewAPicture]

In 29144:

Fill out inline documentation for magic methods added to the WP_Error class in [28511].

See #27881, #22234 and #28885.

[DrewAPicture]

In 29145:

Fill out inline documentation for magic methods added to the WP_Ajax_Response class in [28524].

See #27881, #22234 and #28885.

[DrewAPicture]

In 29146:

Fill out inline documentation for magic methods added to the WP_Object_Cache class in [28502], [28521], [28524].

See #27881, #22234 and #28885.

[DrewAPicture]

In 29147:

Fill out inline documentation for magic methods added to the WP_Filesystem_Base class in [28521].

See #22234 and #28885.

[DrewAPicture]

In 29148:

Fill out inline documentation for magic methods added to the WP_List_Table class in [28493], [28521], and [28524].

See #22234 and #28885.

[DrewAPicture]

In 29149:

Fill out inline documentation for magic methods added to the Custom_Background class in [28481], [28521], and [28524].

See #22234 and #28885.

[DrewAPicture]

In 29150:

Fill out inline documentation for magic methods added to the Custom_Image_Header class in [28481], [28521], and [28524].

See #22234 and #28885.

[DrewAPicture]

In 29153:

Fill out inline documentation for the __call() magic method added to the WP_Roles class in [28503].

See #22234 and #28885.

[DrewAPicture]

In 29155:

Inline documentation cleanup for 4.0 audit.

• Fill out inline documentation for the __call() magic method added in [28507]
• Inline documentation tweaks for get_provider(), added in [28728]
• Inline documentation tweaks for _add_provider_early(), added in [28846]
• @access tag added for _remove_provider_early(), added in [28846]

See #22234 and #28885.

[DrewAPicture]

In 29161:

Fill out inline documentation for the __call() magic method added to the wp_xmlrpc_server class in [28515].

See #22234 and #28885.

[DrewAPicture]

In 29162:

Fill out inline documentation for the __call() magic method added to the WP_Comment_Query class in [28519].

See #22234 and #28885.

[DrewAPicture]

In 29165:

Add periods to short descriptions for magic methods added in [28501], [28521], and [28524].

See #22234 and #28885.

[SergeyBiryukov]

Replying to wonderboymusic:

Marking this fixed - the only remaining vars are in wpdb and WP_Rewrite. Those can be attacked later, if at all.

Follow-up: #51557

[SergeyBiryukov]

In 49184:

Code Modernization: Use explicit visibility for class property declarations.

Using var or only static to declare a class property is PHP 4 code.

This updates the codebase to use explicit visibility modifiers introduced in PHP 5.

Props jrf.
Fixes #51557. See #22234.