DB/DirectDatabaseQuery: add tests for namespaced names

[rodrigoprimo]

Description

In preparation for PHPCS 4.0, which changes the tokenization of namespaced names, this PR adds tests with all forms of namespaced function calls (partially qualified, fully qualified, and namespace-relative using the 'namespace' keyword) as well as fully qualified global function calls to the WordPress.DB.DirectDatabaseQuery.

Suggested changelog entry

N/A

[jrfnl]

Hmm... so as per #2615:

As suggested by Juliette, I'm including a test to document that method names that match the cache function names are deliberately not flagged by this sniff, as they will be most likely valid custom cache functions.

... we are ignoring method calls if they mirror the WP native functions, but now this PR will start flagging namespaced function calls which mirror the WP native functions ?

That sounds very inconsistent to me.

[rodrigoprimo]

That is a good point. I'm unsure what is the best approach here.

I checked other WPCS sniffs to see if they have similar behavior, and those I examined behave differently.

Similar to the list of cache functions used by DirectDatabaseQuery, the NonceVerification and ValidatedSanitizedInput sniffs have lists of WP functions that they check for usage in different circumstances. Both sniffs only match global WP functions and will not match non-global functions with the same name (whether namespaced functions or methods). All sniffs extending AbstractFunctionRestrictionsSniff and AbstractFunctionParameterSniff behave in the same way when looking for their target functions.

Do you see a reason to keep the DirectDatabaseQuery sniff behaving differently than the sniffs mentioned above? Or should we revisit its behavior and make it explicitly match only global WP cache functions (like the other sniffs do)?

[jrfnl]

The NonceVerification and ValidatedSanitizedInput sniffs are security sniffs, so those being stricter than the DirectDatabaseQuery sniff makes total sense to me.

[rodrigoprimo]

Thanks for your input, @jrfnl. I have pushed a new commit reverting the changes to the sniff code and updating the tests to safeguard against regressions for the current behavior of the sniff. Is this more or less what you had in mind? Do you prefer that I update the title and description of this PR to reflect that now it only adds DirectDatabaseQuery tests or that I close this PR and open a new one?

[rodrigoprimo]

Do you prefer that I update the title and description of this PR to reflect that now it only adds DirectDatabaseQuery tests or that I close this PR and open a new one?

@jrfnl, I ended up force pushing a new commit to this PR with just the tests safeguarding the current behavior and updated the PR title and description. It is now ready for review.

(let me know if you prefer the other approach that I suggested - close this PR and create a new one - and I will do it)