False positives flagged about hook names not starting with the theme/plugin prefix

[felixarntz]

I'm getting these errors now for my AI Services plugin, all of which are false positives:

Error: Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "pre_option_{$option}".
Error: Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "pre_option".
Error: Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "ais_load_services_capabilities".
Error: Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "pre_http_request".
Error: Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "http_request_timeout".
Error: Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "http_request_redirection_count".
Error: Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "http_headers_useragent".
Error: Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "http_request_reject_unsafe_urls".
Error: Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "https_ssl_verify".

There are two underlying problems:

• ais_load_services_capabilities starts with "ais", which is a plugin-specific prefix. So that shouldn't be flagged?
• All other filters are from WordPress Core, and they have to be reused here to have the plugin behave as expected in combination with the WordPress Core logic.
  • Concretely, the plugin implements a batch HTTP request API, and for that to work correctly, it needs to manually trigger certain Core filters again.

So I think it's a false positive to flag these. Ideally, we can find a way to improve this check, or alternatively this could be downgraded to a warning, since there can be inaccuracies like the above?

[davidperezgar]

Hello Felix,
It's flagging you because the prefix is less than four characters long. We increased the number of characters some time ago, as you can see in the guidelines:
https://developer.wordpress.org/plugins/plugin-basics/best-practices/.
This number of characters was updated in the WordPress Coding Standards (WordPress/WordPress-Coding-Standards#2479) as well as in themes.

[felixarntz]

@davidperezgar Makes sense. What about the other errors though?

[ernilambar]

I was hoping WPCS would detect core hooks and exclude it from prefixing check. But looks like it is not the case.

[davilera]

Hello Felix, It's flagging you because the prefix is less than four characters long. We increased the number of characters some time ago […].

What if a plugin has been using the same three-letter prefix for over a decade, @davidperezgar? It seems reasonable to allow a way (a comment in the plugin’s main file, perhaps?) to let Plugin Check know about the prefix being used in the plugin. Forcing a full codebase refactor for a change like this feels unnecessarily disruptive and would require all plugin users to potentially update their sites as well, if they rely on those functions.

[andreiglingeanu]

What if the variable is at the top level of the file but it is not exposed as a global at all? For example files that are required from a function won't expose the variables globally, those would be kept in the scope of the function.

I understand it's hard to detect this but maybe it would make sense to allow the plugin to configure this somehow without having to explicitly disable this check in that file with a comment?

For example we're seeing a bunch of these errors but there's no need to prefix those variables, since they're not exposed globally:

FILE: framework/features/header/items/account/options.php
[{"line":3,"column":1,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$link_options\".","docs":""},{"line":10,"column":1,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$layer_settings\".","docs":""},{"line":182,"column":5,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$layer_settings\".","docs":""},{"line":198,"column":5,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$layer_settings\".","docs":""},{"line":212,"column":5,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$layer_settings\".","docs":""},{"line":228,"column":5,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$layer_settings\".","docs":""},{"line":244,"column":5,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$layer_settings\".","docs":""},{"line":260,"column":5,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$layer_settings\".","docs":""},{"line":292,"column":1,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$logout_link_options\".","docs":""},{"line":298,"column":5,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$link_options\".","docs":""},{"line":299,"column":5,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$logout_link_options\".","docs":""},{"line":302,"column":1,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$options\".","docs":""}]

FILE: framework/features/header/items/account/dynamic-styles.php
[{"line":4,"column":5,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$root_selector\".","docs":""},{"line":7,"column":1,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$forms_type\".","docs":""},{"line":10,"column":1,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$accountHeaderIconSize\".","docs":""},{"line":29,"column":1,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$accountHeaderIconSize\".","docs":""},{"line":50,"column":1,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$accountHeaderAvatarSize\".","docs":""},{"line":551,"column":1,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$close_button_type\".","docs":""},{"line":659,"column":1,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$loggedin_interaction_type\".","docs":""},{"line":663,"column":5,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$account_dropdown_top_offset\".","docs":""},{"line":678,"column":9,"type":"ERROR","code":"WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound","message":"Global variables defined by a theme\/plugin should start with the theme\/plugin prefix. Found: \"$sticky_state_account_dropdown_top_offset\".","docs":""}]

[davidperezgar]

Hello, we already covered that with the WPCLI mode update, so there won't be any problems with prefixes.
We will have to implement it on the admin page as well, so that users can use it.

Nilambar also pointed out that we need to cover WordPress hooks.

What if the variable is at the top level of the file but it is not exposed as a global at all? For example files that are required from a function won't expose the variables globally, those would be kept in the scope of the function.

Yes it's for global variables and functions.

[andreiglingeanu]

@davidperezgar could you please clarify how the system will detect that a specific file was loaded with a require from within a function? I expected this would require the plugin author to exclude that check for those files somehow.

Or maybe I'm missing something

[RiaanKnoetze]

I’m running into a similar issue with this sniff.

In my case, Plugin Check is flagging WooCommerce’s own hooks as if they were custom hooks defined by my plugin:

182 | 20 | ERROR | WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound | Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "wc_ajax_add_to_cart".

The problem is that wc_ajax_add_to_cart is a native WooCommerce hook that my plugin is extending - it’s not something I have control over, nor something I can prefix. Because of that, it feels like this is another false positive where Core or WooCommerce hooks are being treated as if they belong to the plugin.

Would it be possible for Plugin Check to recognise and exclude known Core/WooCommerce hooks from this rule?

[mahmoudsaeed]

What if the variable is at the top level of the file but it is not exposed as a global at all? For example files that are required from a function won't expose the variables globally, those would be kept in the scope of the function.

Exactly. This is a widely used pattern in view/template files.

[davidperezgar]

We definitely need to improve this check. It's a good starting point for checking prefixes. For now, we have moved to a 'Warning' result while we improve this check and gather feedback. It will be released in the next version, 1.8.0.