Closed mundschenk-at closed 5 years ago
@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.
@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.
@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 ?
Sure, its mundschenk-at/avatar-privacy. But the error should be reproducible in vitro if you use a composer.json
like the one above.
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:
autoload
section in the composer.json
file, I see the expected errors.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 https://github.com/aristath/kirki/issues/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.
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.
- With the
autoload
section in thecomposer.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.
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.
Very weird. I can replicate the bug as well:
<rule ref="WordPress.NamingConventions.PrefixAllGlobals">
<properties>
<property name="prefixes" type="array">
<element value="avatar_privacy"/>
</property>
</properties>
</rule>
includes/avatar-privacy-functions.php
.vendor/bin/phpcs includes/avatar-privacy-functions.php -s
and shows my added whitespace violationscomposer.json
,composer dumpautoload
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.
Found it! PR upcoming.
@mundschenk-at Thank you for bearing with me and providing enough clues for me to figure it out. PR #1633 should fix this.
@mundschenk-at The PR has now been merged into develop
, so please check out that branch, and see if it fixes it for your repo.
It does.
Bug Description
I've got a legacy function named
avapr_get_avatar_checkbox
in one of my include files. When I enabled theWordPress.NamingConventions.PrefixAllGlobals
sniff, I expected this function to be flagged. (Other "incorrect" function names are!)Minimal Code Snippet
Minimal
phpcs.xml
:I'd expect both function names to be flagged, but only the later one is.
Error Code
Environment