plugin-check icon indicating copy to clipboard operation
plugin-check copied to clipboard

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

Open felixarntz opened this issue 5 months ago • 8 comments

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?

felixarntz avatar Nov 10 '25 16:11 felixarntz

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 (https://github.com/WordPress/WordPress-Coding-Standards/pull/2479) as well as in themes.

davidperezgar avatar Nov 10 '25 17:11 davidperezgar

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

felixarntz avatar Nov 10 '25 18:11 felixarntz

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

ernilambar avatar Nov 11 '25 03:11 ernilambar

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.

davilera avatar Nov 11 '25 10:11 davilera

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":""}]

andreiglingeanu avatar Nov 11 '25 10:11 andreiglingeanu

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.

davidperezgar avatar Nov 12 '25 16:11 davidperezgar

@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

andreiglingeanu avatar Nov 12 '25 16:11 andreiglingeanu

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?

RiaanKnoetze avatar Nov 13 '25 22:11 RiaanKnoetze

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.

mahmoudsaeed avatar Nov 18 '25 15:11 mahmoudsaeed

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.

davidperezgar avatar Nov 27 '25 17:11 davidperezgar