WordPress-Coding-Standards icon indicating copy to clipboard operation
WordPress-Coding-Standards copied to clipboard

Restrict the use of private functions or classes that are not meant to be extended

Open grappler opened this issue 8 years ago • 7 comments

Once https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/pull/633 has been merged I will work on a list of private WordPress functions that should not be used by themes and plugins.

https://codex.wordpress.org/Category:Private_Functions

and WP_Internal_Pointers which is not meant to be extended.

The following are exceptions that would still need to pass.

remove_action( 'admin_enqueue_scripts', array( 'WP_Internal_Pointers', 'enqueue_scripts' ) );
remove_action( 'admin_print_footer_scripts', array( 'WP_Internal_Pointers', 'pointer_wp390_widgets' ) );

grappler avatar Dec 10 '16 14:12 grappler

#633 has been merged ;-)

GaryJones avatar Jul 23 '18 09:07 GaryJones

I was discuss this with @GaryJones and I agree we should try and stop developers using private functions. It is pretty simple to find private functions / classes in core, as they all marked as @access private.

I have generated a list of private functions and classes that can be found here.

spacedmonkey avatar Feb 11 '20 11:02 spacedmonkey

If anyone wants to work on this, this wouldn't be too hard a sniff to create.

For the functions, it would be a case of combining the list created by @spacedmonkey with the AbstractFunctionRestrictionsSniff which already contains all the logic.

IMO that sniff should only look at global functions marked as private via the docblock. Class methods should not be addressed by the sniff. Moreover: those should be fixed in WP Core to have private set for visibility.

For the classes, it would be a case of combining the list created by @spacedmonkey with the AbstractClassRestrictionsSniff which, again, already contains all the logic.

For both sniffs, there are implementations available within the code base already which can be used as an example to guide you.

jrfnl avatar Feb 21 '20 06:02 jrfnl

Note: regarding the remark by @grappler about certain callbacks which shouldn't be reported: at this time, the abstract sniffs do not look at callbacks, so that is not an issue which (at this time) would need special handling.

jrfnl avatar Feb 21 '20 06:02 jrfnl

Open question: How should the "private classes" sniff handle it when a private class is being extended ? For some of the classes I see on the list - like WP_List_Table -, that would be reported via the abstract sniff, but should be allowed AFAICS. Or rather: that is the intended use.

I also think a critical look at the function list would be a good idea.

Until recently, functions like _deprecated_function() would have been on that list too, while it is common - and a good idea - for plugins to use those too (which is why for the _deprecated_....() functions this has been changed now).

I imagine there may be some more functions like that on the list, in which case I would encourage changing their designation in the docblock in WP core, rather than forbidding their use.

jrfnl avatar Feb 21 '20 06:02 jrfnl

Are 'private classes' meant to be final? If that is the case, would there be any harm in just patching the core to actually add the final keyword which will prevent any extending of the class?

I know that we should just look at the standards but I kinda feel like core could do with some architectural changes...

dingo-d avatar Feb 21 '20 14:02 dingo-d

Any sniff like this would greatly benefit from tooling to create the initial lists and allow for updating the lists with ease. See #1803

jrfnl avatar Dec 03 '22 15:12 jrfnl