Yoast / yoastcs

Yoast coding standards
MIT License
21 stars 3 forks source link

ValidHookName: verify correct handling of single vs double quotes names #242

Closed jrfnl closed 3 years ago

jrfnl commented 3 years ago

Inspired by a question in Slack from @Djennez :

The NamingConventions/ValidHookName sniff needs a quick review for correct handling of double backslashes.

When single quoted strings are used, the "namespace-like" prefix for hooks should be fine. However, when double quoted strings are used, the namespace separator(s) in the hook name prefix need to be escaped as they could otherwise result in a PHP escape character in the hook name.

// Correct (single quoted).
apply_filter( 'Namespace\Like\Prefix\hookname', $var);

// Incorrect (double quoted - the `\` needs to be escaped).
apply_filter( "Namespace\Like\Prefix\hookname", $var);

// Correct (double quoted).
apply_filter( "Namespace\\Like\\Prefix\\hookname", $var);

The sniff should:

  1. Not throw false positives when the prefix is correct, but uses escaped backslashes in a double quoted string.
  2. Possibly throw an error when a double quoted string is used for the hook name, but the backslashes aren't escaped.

Implementation note:

jrfnl commented 3 years ago

@Djennez I've created PR #243 to address this. Would you feel comfortable doing a review of the PR ?