`DB/PreparedSQL` and `DB/PreparedSQLPlaceholders`: how should namespaced function calls be handled?

[rodrigoprimo]

The WordPress.DB.PreparedSQL and WordPress.DB.PreparedSQLPlaceholders sniffs check for specific functions to determine if certain code patterns are safe. I have a question about how these sniffs should handle namespace-prefixed calls like MyNamespace\absint() or \MyNamespace\absint().

WordPress.DB.PreparedSQL looks for:

• SQL escaping functions: absint(), esc_sql(), floatval(), intval(), like_escape().
• SQL auto-escaped functions: count().
• Formatting functions: sprintf(), vsprintf(), wp_sprintf(), implode(), join(), array_fill(), antispambot(), ent2ncr(), nl2br().

WordPress.DB.PreparedSQLPlaceholders checks for:

• sprintf(), implode(), and array_fill() to detect specific variable placeholder patterns.

Currently, both sniffs don't properly differentiate between global and namespaced function calls:

• When WordPress.DB.PreparedSQL sees code like MyNamespace\absint($foo), it generates an error for MyNamespace but then incorrectly treats absint() as if it were a global function call and doesn't check its parameters.

• WordPress.DB.PreparedSQLPlaceholders doesn't differentiate at all - it accepts both sprintf() and MyNamespace\sprintf() as valid for the dynamic placeholder generation pattern.

Question for discussion

Should these sniffs treat namespace-prefixed function calls that mirror the name of global functions they check as different functions and treat them accordingly?

I'm inclined to think that they should, but given the discussion in #2619, I'm not sure. In that issue, it was decided that WordPress.DB.DirectDatabaseQuery should treat namespaced functions like MyNamespace\wp_cache_get() as valid custom cache implementations (effectively treating it the same way as wp_cache_get()). However, security sniffs like WordPress.Security.NonceVerification and WordPress.Security.ValidatedSanitizedInput use stricter global-function-only matching. I'm curious which approach makes more sense for these two DB sniffs.

[jrfnl]

Should these sniffs treat namespace-prefixed function calls that mirror the name of global functions they check as different functions and treat them accordingly?

In my opinion: yes.

Reason for this opinion to be different from my opinion regarding the DirectDatabaseQuery sniff:

1. The chances of methods/namespaced functions with a name like wp_cache_get() to do something different than getting the cach from WP are very very slim, while with much more generic names like join(), a method or namespaced function with that name could well be doing something domain specific and not mirror the PHP native functionality we allow for.
2. The DirectDatabaseQuery sniff is not a security sniff, while the PreparedSQL* sniffs are security related sniffs, so should be less tolerant.