Namespaced cron_schedules callbacks not detected

[ocean90]

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 develop branch of WPCS.

[ocean90]

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.

[jrfnl]

@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 function statement 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 use statements;
• Check the second parameter for use of __NAMESPACE__, but also for My\NS\function_name and 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.

[ocean90]

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.