PrefixAllGlobals not reporting for file autoloaded in composer.json

[mundschenk-at]

Bug Description

I've got a legacy function named avapr_get_avatar_checkbox in one of my include files. When I enabled the WordPress.NamingConventions.PrefixAllGlobals sniff, I expected this function to be flagged. (Other "incorrect" function names are!)

Minimal Code Snippet

<?php
/**
 *  Foobar.
 *
 *  @package Foo
 */

/**
 * Foobar.
 */
function avapr_get_avatar_checkbox() {
}

/**
 * Something else.
 */
function another_function() {
}

Minimal phpcs.xml:

<?xml version="1.0"?>
<ruleset name="Foobar">
	<config name="minimum_supported_wp_version" value="4.9"/>

	<rule ref="WordPress"/>

	<rule ref="WordPress.NamingConventions.PrefixAllGlobals">
		<properties>
			<property name="prefixes" type="array">
				<element value="foobar"/>
			</property>
		</properties>
	</rule>
</ruleset>

I'd expect both function names to be flagged, but only the later one is.

Error Code

--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 17 | ERROR | Functions declared by a theme/plugin should start with the
    |       | theme/plugin prefix. Found: "another_function".
    |       | (WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound)
--------------------------------------------------------------------------------

Environment

Question	Answer
PHP version	7.3.1
PHP_CodeSniffer version	3.4.0
WPCS version	2.0.0
WPCS install type	Composer project local

[jrfnl]

@mundschenk-at Thanks for reporting this. Unfortunately, I cannot reproduce the issue.

Using the code and the ruleset you posted above, I have tested this and am getting both errors as expected.

$ phpcs -ps ./test-1632.inc --standard=test-1632.xml

E 1 / 1 (100%)

/test-1632.inc
------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
------------------------------------------------------------------------------------------
 11 | ERROR | Functions declared by a theme/plugin should start with the theme/plugin
    |       | prefix. Found: "avapr_get_avatar_checkbox".
    |       | (WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound)
 17 | ERROR | Functions declared by a theme/plugin should start with the theme/plugin
    |       | prefix. Found: "another_function".
    |       | (WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedFunctionFound)
------------------------------------------------------------------------------------------

Tested with PHP 7.2.13, 7.3.0, PHPCS 3.4.0 and master.

Could it be that there is an unqualified (i.e. without sniff names) // phpcs:disable / // phpcs:enable combo in your code somewhere around that first function ?

If not, can you please find a piece of code with which this issue can be consistently reproduced ? Otherwise, there is nothing I can do.

[mundschenk-at]

@jrfnl OK that's really weird. Using a fresh composer project, I can't reproduce this either. Within my existing real-world project, I can (using those minimal files instead of the real ones). The only instance of phpcs:disable is in an unrelated file and qualified with sniff names.

After some experimentation, it looks like this only happens when the file in question is autoloaded by composer (foobar.php is the test file listed above):

{
    "name": "mundschenk-at/wpcs-prefix-globals-test",
    "type": "project",
    "license": "GPL-v2-or-later",
    "require": {},
    "require-dev": {
        "squizlabs/php_codesniffer": "^3.4",
        "wp-coding-standards/wpcs": "^2.0",
        "dealerdirect/phpcodesniffer-composer-installer": "^0.5.0"
    },
    "autoload": {
        "files": ["foobar.php"]
    }
}

With the autoload, the sniff fails to detect the function. When the file is not autoloader, things work as expected.

[jrfnl]

@mundschenk-at That's very weird indeed as PHPCS is Composer agnostic, i.e. there is nothing in PHPCS which particularly takes Composer into account.

Have you got a link to the project so I can have a look ? If not public, but you're willing to let me look anyhow, you can DM me the link on Slack ?

[mundschenk-at]

Sure, its mundschenk-at/avatar-privacy. But the error should be reproducible in vitro if you use a composer.json like the one above.

[jrfnl]

Well, yes and no. (It gets even weirder)

Using the above test files and the composer.json file + adding in a minor whitespace issue in the foobar.php file to be sure that it's being scanned:

• Without the autoload section in the composer.json file, I see the expected errors.
• With the autoload section in the composer.json file, I see no errors at all for the PrefixAllGlobals sniff, not for the first, nor for the second function, but I do still see the whitespace error.

This reminds me of issue themeum/kirki#2062 and needs a lot more investigation.
I honestly haven't got a clue what's going on at this time, but there is surely something going very wrong here.

[jrfnl]

Oh and to make things even weirder: if you use a git clone install of PHPCS/WPCS instead of the Composer installed one to scan the same code: the issue doesn't exist again and I see the expected errors.

[mundschenk-at]

• With the autoload section in the composer.json file, I see no errors at all for the PrefixAllGlobals sniff, not for the first, nor for the second function, but I do still see the whitespace error.

Yes, sorry, I forgot to mention that detail. With my full project, the second function gets flagged, with the minimal sample, neither one is recognized.

[mundschenk-at]

Oh and to make things even weirder: if you use a git clone install of PHPCS/WPCS instead of the Composer installed one to scan the same code: the issue doesn't exist again and I see the expected errors.

Using the composer installed one means that the file is autoloaded when running phpcs (because it's a file "auto"load instead of a class). I don't see how that would interfere as it only contains declarations (and in the real world example is even couched in if ( ! function_exists() ), but apparently it does in some way.

[GaryJones]

Very weird. I can replicate the bug as well:

• added:

<rule ref="WordPress.NamingConventions.PrefixAllGlobals">
	<properties>
		<property name="prefixes" type="array">
			<element value="avatar_privacy"/>
		</property>
	</properties>
</rule>

• added whitespace issue to includes/avatar-privacy-functions.php.
• ran vendor/bin/phpcs includes/avatar-privacy-functions.php -s and shows my added whitespace violations
• removed the autoloaded file from composer.json,
• ran composer dumpautoload
• run PHPCS again to show whitespace AND prefix violations.

[jrfnl]

In the mean time, I have run some more tests. I have confirmed that this issue is not new to WPCS 2.0.0, but already existed when using WPCS 1.2.1.