Namespaced cron_schedules callbacks not detected
Bug Description
The WordPress.WP.CronInterval sniff is producing a warning when a namespaced callback is used: "Detected changing of cron_schedules, but could not detect the interval value."
Minimal Code Snippet
namespace Foo {
function my_add_every_hour( $schedules ) {
$schedules['every_hour'] = [
'interval' => HOUR_IN_SECONDS,
'display' => __( 'Once every hour' )
];
return $schedules;
}
add_filter( 'cron_schedules', __NAMESPACE__ . '\my_add_every_hour'); // OK: > 10 min.
}
Environment
| Question | Answer |
|---|---|
| PHP version | 7.2.23 |
| PHP_CodeSniffer version | 3.5.4 |
| WPCS version | 2.2.1 |
| WPCS install type | Composer global |
Tested Against develop branch?
- [x] I have verified the issue still exists in the
developbranch of WPCS.
Added a (failing) test in #1866.
I'm happy to work on a patch, though I'd need some hints. So far I noticed that the value of $callback here is
array(3) {
["start"]=>
int(82)
["end"]=>
int(87)
["raw"]=>
string(36) "__NAMESPACE__ . '\my_add_every_hour'"
}
And token_name( $this->tokens[ $callbackArrayPtr ]['code'] ) around here returns T_NS_C.
$functionName is "\my_add_every_hour" and the token names here are
string(10) "T_OPEN_TAG"
string(12) "T_WHITESPACE"
string(8) "T_STRING"
string(14) "T_DOUBLE_COLON"
string(8) "T_STRING"
Since there is no T_FUNCTION it bails here.
@ocean90 Thanks for reporting this.
A lot of the WPCS sniffs at this moment are not adapted yet to handle namespaced code correctly in all situations.
Fixing this for this sniff will not be easy as, in most cases, a file based namespace will be used not a scoped (curly braces) namespace. And with a file based namespace, the function could be:
- declared in another file in the same namespace;
- could be imported via a
use functionstatement from elsewhere; - could be in another namespace all together.
While your example code is self-contained, this will not be the reality in most cases.
For this to be fixed, the sniff would need to:
- Keep track of the current namespace;
- Keep track of import
usestatements; - Check the second parameter for use of
__NAMESPACE__, but also forMy\NS\function_nameand get the correct function name stripped of the namespace, but keep the namespace separately to check it against the tracked current namespace/namespaces in the file.
In most of the cases I listed above, this would eventually still result in the warning.
And yes, it's a warning, not an error because the sniff did not reach a conclusion.
The warning can be whitelisted using a whitelist comment like this:
// phpcs:ignore WordPress.WP.CronInterval -- Verified > 10 min.
So, yes, for your specific code sample, this could be fixed.
Fixing this will be easier though once we move over to using PHPCSUtils in the near future as it contains a lot of utility functions which will be helpful for this.
Let me know if you still want to work on this already or if you are ok with whitelisting the code for now.
Thank you for the detailed feedback! The scoped namespace was just an example and something that can be used in in the test without the need for a new file (I might be wrong). I got the warning in a file based namespace.
For me, this was actually the first time something was triggered due to the use of a namespace so I wasn't aware of other issues. The whitelist comment is totally fine for now.
The bug still occurs for me in the current version 3.2.0. I also use the hook “cron_schedules” in a namespace. The only way I can prevent this is with
<exclude name="WordPress.WP.CronInterval" />
But nice is different :|