Check: Generic function/class/define/option prefix names

[davidperezgar]

This check aims to detect short or common prefixes that could cause fatal errors in WordPress installations.
We consider as an error for this check.

How could develop this check?

We need to have a white list of common function starts. Actually we have in our internal scanner: __,_,-,set,get,is,save,show,update,add,wordpress,wp,woocommerce,wc,table,html,css,js,input,output,plugin,plugins,my_plugin,myplugin,prefix,my_custom,custom,as,widget,oauth2,handle,generate,post,site,remove,filter,display,init,start,check,sync,cache,phpmailer,declare,register,enable,include,search,upgrade,update,setup,create,admin,load,theme,fetch,apply,clear,verify,test,insert,acme,app,render,rest

And after, We check the list of named functions that are outside a Class, and a list o named Classes. Maybe we can go to Namespaces as well.

Our description to developers:

Generic function/class/define/namespace/option names

 
All plugins must have unique function names, namespaces, defines, class and option names. This prevents your plugin from conflicting with other plugins or themes. We need you to update your plugin to use more unique and distinct names.

A good way to do this is with a prefix. For example, if your plugin is called "Easy Custom Post Types" then you could use names like these:
 
function ecpt_save_post()
define( ‘ECPT_LICENSE’, true );
class ECPT_Admin{}
namespace EasyCustomPostTypes;
update_option( 'ecpt_settings', $settings );
 
Don't try to use two (2) or three (3) letter prefixes anymore. We host nearly 100-thousand plugins on WordPress.org alone. There are tens of thousands more outside our servers. Believe us, you’re going to run into conflicts.
 
You also need to avoid the use of __ (double underscores), wp_ , or _ (single underscore) as a prefix. Those are reserved for WordPress itself. You can use them inside your classes, but not as stand-alone function.

Please remember, if you're using _n() or __() for translation, that's fine. We're only talking about functions you've created for your plugin, not the core functions from WordPress. In fact, those core features are why you need to not use those prefixes in your own plugin! You don't want to break WordPress for your users.

Related to this, using if (!function_exists(‘NAME ‘)) { around all your functions and classes sounds like a great idea until you realize the fatal flaw. If something else has a function with the same name and their code loads first, your plugin will break. Using if-exists should be reserved for shared libraries only.

Remember: Good prefix names are unique and distinct to your plugin. This will help you and the next person in debugging, as well as prevent conflicts.
 
Example(s) from your plugin:

[swissspidy]

So this basically the WordPress.NamingConventions.PrefixAllGlobals PHPCS sniff in WPCS?

[davidperezgar]

Yes! You're right. We're asking 4 letters and WPCS has 3 letters. I'm going to make an issue for that.

[davidperezgar]

Ok, I've asked to update the 4 letters in the developer documentation and PrefixAllGlobals sniff.

[ernilambar]

I was digging more about this issue and I found out that PrefixAllGlobalsSniff has already a list of restricted prefixes in variable $prefix_blocklist. But it is not customizable as of now.

Ref: https://github.com/WordPress/WordPress-Coding-Standards/blob/develop/WordPress/Sniffs/NamingConventions/PrefixAllGlobalsSniff.php#L86-L91

protected $prefix_blocklist = array(
	'wordpress' => true,
	'wp'        => true,
	'_'         => true,
	'php'       => true, // See #1728, the 'php' prefix is reserved by PHP itself.
);

If we could make this array customizable then we can feed our own list of restricted prefixes.
But also we need to find a way to run this sniff without passing list of prefixes and only check for those restricted prefixes. Currently this sniff runs only if we give a list of allowed prefixes.

[swissspidy]

Sounds like a good upstream contribution to WPCS :)

[davidperezgar]

Yeah!

[ernilambar]

Hope you don't mind me pinging here. CC @jrfnl for the the input and suggestion whether this customization is feasible in WPCS repo.

[jrfnl]

@ernilambar Please read the linked issues before pinging people: #523 (comment)

As for:

If we could make this array customizable then we can feed our own list of restricted prefixes.

Making it customizable would allow for people to clear out the list, resulting in the opposite effect.

But also we need to find a way to run this sniff without passing list of prefixes and only check for those restricted prefixes. Currently this sniff runs only if we give a list of allowed prefixes.

I suggest opening separate new issues for each of these questions in the WPCS repo to discuss (also see the existing issue, though that's a different ask/decision point: WordPress/WordPress-Coding-Standards#2467).

[ernilambar]

Another approach could be modifying PrefixAllGlobalsSniff.php class as per PCP requirement and using composer-patches for the implementation.

• We would be able to take benefit of handling different cases for functions, classes, defines, traits, etc. This class already handle such cases efficiently. So we would not have to reinvent the wheel for the same task.
• We have a list of block list of prefixes which we need to check. Since we would not be able to make that block list customizable in the upstream, directly using WordPress.NamingConventions.PrefixAllGlobals sniff is not viable.
• Also our requirement is quite inverse of what current sniff is doing. Currently an array of allowed prefixes are checked against but we require check whether given prefix is in the block list or not.

Cons of this approach is addition of burden to maintain the patch.

This is PR which I was testing to see if we can implement the feature: https://github.com/ernilambar/WordPress-Coding-Standards/pull/1/files

In this PR:

• Block list could be updated with new items but default list would not be removed.
• Add separate function to check if token has valid prefix or not.

[swissspidy]

Instead of patching I‘d just port this sniff over to this repo and modify it to our needs

[jrfnl]

Instead of patching I‘d just port this sniff over to this repo and modify it to our needs

That would be violating both the copyright as well as the license.

Instead of trying to reinvent the wheel, you may want to try to collaborate and contribute to WPCS instead.

[swissspidy]

That‘s what we suggested above :-) I was just thinking that a fork in the meantime allows for quicker iteration.

[jrfnl]

@swissspidy Quicker iteration ? How ? I suggested 10 days ago to open an issue in the WPCS repo and nobody has even bothered.

[frantorres]

Just for context on how this is right now being handled in the internal review script.

It's using a code parser to get all the relevant strings that need to be prefixed:

• A parameter of functions like define, add_option, update_option, set_transient, register_setting, do_action, apply_filters, etc.
• The name global variables, in the shape of global $example; and $GLOBALS['example'];
• Name of namespaces.
• If there are not namespaces affecting them, name of Class, Interface, Trait and function (when not inside abstractions like a Class).

With all of this in hand, it removes some false positives, like common apply_filters that authors use because of that's an ok to use case integrating with some other plugin or even the core.

Then you have an array of all the names that we expect to be prefixed, processing it you get all possible prefixes included in them (by taking out all the possibilities of substrings separated by _ , - and ) and then you deduct which prefixes are common among them (by generating another array of coincidences prioritizing longer prefixes).

This has some kind of statistics involved as some plugins might have not-exactly the same but look-alike prefixes and that might be fine. It's considered a prefix as long it's (not considering namespaces):

• In plugins with less than 8 declared names: 2 coincidences.
• In plugins between 9 and 20 declared names: a 25% of the coincidences.
• From there: A log([number of declared names])*2 of coincidences.

Of course this has false positives.

Having this, we can check if that prefix is fine (checking the size and those cases of common not valid prefixes like set_ get_ ) or it is not, and in any case, anything else that is not under a considered common prefix is considered not prefixed.

Example of output:

# This plugin is using the prefix "klrmj" for 7 element(s).
# This plugin is using the prefix "kl" for 6 element(s).

# The prefix 'kl' is too short, we require prefixes to be over 4 characters.
plugin.php:61 function kl_init
plugin.php:71 class kl
plugin.php:81 global $kl_data;

# Cannot use "deactivate" as a prefix.
plugin.php:21 function deactivate_plugin
# Cannot use "enqueue" as a prefix.
plugin.php:279 function enqueue_styles

# Looks like there are elements not using common prefixes.
plugin.php:18 define('VERSION', '1.0.0');
plugin.php:86 update_option($option_name, $serialized_plugin_data);
              ↳ Detected name: 'plugin_data'
plugin.php:34 function standard_shortcode

[jrfnl]

Sounds similar to functionality already in the WPCS PrefixAllGlobals sniff. Has any of you ever tried running it with the --report=info option ?

[davidperezgar]

I couldn't manage to run PrefixAllGlobals with a simple php @jrfnl . Could you give me some guidance on how to run this sniff?

[jrfnl]

@davidperezgar phpcs -ps . --standard=WordPress --sniffs=WordPress.NamingConventions.PrefixAllGobals --report=full,source,info --runtime-set prefixes my_prefix,tgmpa

[davidperezgar]

Thanks! I'll check it out!!

[davidperezgar]

Hello @jrfnl Why is necessary to give prefixes? It isn't supposed to check the number of prefixes instead? If not, how could I check the number of prefixes? Thanks

[jrfnl]

Hello @jrfnl Why is necessary to give prefixes? It isn't supposed to check the number of prefixes instead? If not, how could I check the number of prefixes? Thanks

The sniff needs something to check against. Without a prefix/prefixes, the sniff can't flag anything.

[davidperezgar]

Yes, but we want to check all functions, classes and variables of a file. It should check any and tell if it's correct.

[swissspidy]

It does make sense that, if you don't pass prefixes, you don't know whether a function has the right prefix or not.

The only thing we have is a list of disallowed prefixes (__,_,-,set,get,is,save,show,update,add,...)

As per above suggestion I now created two distinct feature requests on the WPCS repo that would cover this use case:

• PrefixAllGlobalsSniff: always flag blocked prefixes WordPress-Coding-Standards#2480
• PrefixAllGlobalsSniff: extend blocklist WordPress-Coding-Standards#2481

[frantorres]

Right now in the internal script as described here it gathers all names and deduces statistically if there is something that could be considered a prefix, in that case, if it is not in the list of not allowed prefixes and it's long enough, it considers it a prefix of the plugin.

It also has some leeway, it may consider more than one prefix ok (some authors give different names to prefixes of options, functions, and/or namespaces). For example, it's quite common to use the Vendor name in the namespace and something else for prefixing option names.

This can also give false positives, it is more resource intensive, and needs to take into account all plugin files at once.

Also, I think it's quite opinionated, so I'm not sure if this approach is something ready for WPCS, but maybe for Plugin Check as a tool to flag and help authors identify possible issues?

[swissspidy]

Oh, I completely missed this.

That sounds like something that could work for Plugin Check indeed.

1. Scan the code to find likely prefixes
2. Pass those to the PrefixAllGlobals sniff
3. Surface anything flagged by the sniff to the user as a warning, explaining which prefixes were used

[davidperezgar]

Yes, that could be the best approach.

[ernilambar]

That sounds like something that could work for Plugin Check indeed.

1. Scan the code to find likely prefixes

@swissspidy What could be the best approach for scanning files and find potential prefixes? Any suggestions?

[swissspidy]

Well I'd start with the implementation you already have in your internal script, see #523 (comment)

I don't know anything about that, so you'd have to ask within your team :)

[ernilambar]

In internal script, nikic/php-parser package is used.

[swissspidy]

Then you can start with that :)

[davidperezgar]

We have to save all prefixes and detect the ones that are less than 4 letters.

[davidperezgar]

This library provides a layer of abstraction to create checks more easily.

[ernilambar]

@davidperezgar I tried to split our only the Parser from the scanner but I found it is pretty much tied to other classes in the scanner. I am consulting with Fran about just developing lean and this PHP class just to find the potential prefixes. Will update here when there is some progress.

[davidperezgar]

Ok! I think this check is tough, but let's keep coding ;)

[frantorres]

Yes, all checks are tied up to some extent to the Parser class but I think this one can be easily separated, we just need to bring with us some functions like the ones that extract names.