Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
grappler opened this issue Dec 10, 2016 · 7 comments
Labels
Focus: WP interoperability Doing things the WP way, prefer using WP functionality over userland/PHP native Status: Good first issue Type: Enhancement

Comments

@grappler
Copy link
Member

Once #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' ) );
@GaryJones
Copy link
Member

#633 has been merged ;-)

@jrfnl jrfnl added Type: Enhancement Focus: WP interoperability Doing things the WP way, prefer using WP functionality over userland/PHP native labels Apr 1, 2019
@spacedmonkey
Copy link
Member

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.

@jrfnl
Copy link
Member

jrfnl commented Feb 21, 2020

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
Copy link
Member

jrfnl commented Feb 21, 2020

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
Copy link
Member

jrfnl commented Feb 21, 2020

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.

@dingo-d
Copy link
Member

dingo-d commented Feb 21, 2020

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...

@jrfnl
Copy link
Member

jrfnl commented Dec 3, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Focus: WP interoperability Doing things the WP way, prefer using WP functionality over userland/PHP native Status: Good first issue Type: Enhancement
Projects
None yet
Development

No branches or pull requests

5 participants