NamingConventions/PrefixAllGlobals: allow non-prefixed declarations for pluggable functions and classes #2286
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…or pluggable functions and classes
While the `PrefixAllGlobals` sniff already allowed for WP native constants which can be user declared, it did not allow for explicitly pluggable functions and classes being declared in plugins/themes.
I'm quite surprised actually that this was not reported as a bug so far.
Either way, fixed now.
In both cases, a case-insensitive name comparison is done as PHP natively treats function names and class names case-insensitively.
The lists added are based on some prelim sniffs created for issue 1803 and WP Core 6.3-beta2.
Notes:
* When generating these lists, I've explicitly excluded the `wp-includes/compat.php` file (which contains the polyfills for PHP native functionality).
* I've intentionally included the Twenty* theme related functions and classes as WPCS should also allow for being used by child themes of those themes.
* I've _excluded_ polyfills for new WP functionality from the Twenty* themes (which allow the themes to use those functions when running on older WP versions), as those functions aren't intentionally pluggable by WP itself.
* I've marked a number of functions/classes with a `// Deprecated` comment based on manual verification (and the declaration being in the `wp-includes/pluggable-deprecated.php` file).
While it would, of course, be better if those would not be "plugged", it is not for this sniff to have an opinion on that and declaring these without a prefix should be allowed.
* In the `$pluggable_functions` list, I've intentionally commented two functions out as those are declared nested within another function and do not look _intentionally_ pluggable. This may need further verification, but at least this way it is documented why these are not included in the list.
Includes tests.
jrfnl
added
Type: Bug
Type: Enhancement
Status: Review ready
Focus: WP interoperability
dingo-d
approved these changes
Jul 4, 2023
GaryJones
deleted the
feature/prefixallglobals-allow-for-pluggable-functions-classes
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While the
PrefixAllGlobalssniff already allowed for WP native constants which can be user declared, it did not allow for explicitly pluggable functions and classes being declared in plugins/themes.I'm quite surprised actually that this was not reported as a bug so far.
Either way, fixed now.
In both cases, a case-insensitive name comparison is done as PHP natively treats function names and class names case-insensitively.
The lists added are based on some prelim sniffs created for issue #1803 and WP Core 6.3-beta2.
Notes:
wp-includes/compat.phpfile (which contains the polyfills for PHP native functionality).// Deprecatedcomment based on manual verification (and the declaration being in thewp-includes/pluggable-deprecated.phpfile). While it would, of course, be better if those would not be "plugged", it is not for this sniff to have an opinion on that and declaring these without a prefix should be allowed.$pluggable_functionslist, I've intentionally commented two functions out as those are declared nested within another function and do not look intentionally pluggable. This may need further verification, but at least this way it is documented why these are not included in the list.Includes tests.