Repository: WordPress/WordPress-Coding-Standards (php)

Issue:
WordPress.NamingConventions.ValidHookName.NotLowercase does not distinguish string literals from function calls
<!--
PLEASE FILL OUT THE TEMPLATE COMPLETELY.
BUG REPORTS WHICH CANNOT BE REPRODUCED BASED ON THE INFORMATION PROVIDED WILL BE CLOSED.
-->

## Bug Description
<!--
Please provide a clear and concise description of what the bug is.

What did you expect to happen? What actually happened?
-->

The `WordPress.NamingConventions.ValidHookName.NotLowercase` sniff violation is triggered when the first argument of `apply_filters` is a function call containing an argument that's not entirely lowercase, instead of validating on the return value of the function call:

```
  31 | ERROR   | [ ] Hook names should be lowercase. Expected: Filters::getBlockFilterName('form', 'datatypeselector'), but found: Filters::getBlockFilterName('form', 'dataTypeSelector').
```


## Minimal Code Snippet
<!-- Please provide example code that allows us to reproduce the issue. Do NOT paste screenshots of code! -->

The issue happens when running this command:
```bash
 php ./vendor/squizlabs/php_codesniffer/bin/phpcs --standard=WordPress form.php
```

... over a file containing this code:
```php
$formDataTypeSelector = apply_filters(
	Filters::getBlockFilterName('form', 'dataTypeSelector'), 
	Components::checkAttr('formDataTypeSelector', $attributes, $manifest),
	$attributes
);
```

Important to note that the return value of the `Filters::getBlockFilterName('form', 'dataTypeSelector')` method call is `es_forms_block_form_data_type_selector`, which is entirely lowercase.

## Error Code
<!--
The error code for the sniff that is (or should be) being triggered (you
can see the sniff error codes by running `phpcs` with the `-s` flag).
e.g. `WordPress.PHP.NoSilencedErrors.Discouraged`

You can leave this section empty if you are reporting a false negative.
-->

`WordPress.NamingConventions.ValidHookName.NotLowercase`

## Environment
<!--
To find out the versions used:
* PHP: run `php -v`.
* PHPCS: run `[vendor/bin/]phpcs --version`
* WPCS: run `composer [global] info` for a Composer install.
-->

| Question               | Answer
| ------------------------| -------
| PHP version             | 7.4.29
| PHP_CodeSniffer version | 3.6.2
| WPCS version            | 2.3.0
| WPCS install type       | Composer local install as a dependency of Eightshift Coding Standards



## Additional Context (optional)
<!-- Add any other context about the problem here. -->

## Tested Against `develop` branch?
- [ ] I have verified the issue still exists in the `develop` branch of WPCS.


Pull request:
NamingConventions/ValidHookName: bug fix - ignore parameters passed to function calls
This commit changes the sniff to ignore parameters passed to function calls by skipping over the tokens containing within parentheses, but only if those parentheses appear to be for a literal function call (last non-empty token before the open parenthesis is a `T_STRING`). Text string tokens within _arbitrary_ parentheses will still be examined.

Note: this does not take variable function calls into account, but as, generally speaking, the hook name shouldn't be generated via a function call anyway, I don't think that's an issue we need to worry about (unless someone submits a bug report for it).

Includes unit tests.

Fixes #2055

Patch:
diff --git a/WordPress/Sniffs/NamingConventions/ValidHookNameSniff.php b/WordPress/Sniffs/NamingConventions/ValidHookNameSniff.php
index f0d3558c..7ce5f8f9 100644
--- a/WordPress/Sniffs/NamingConventions/ValidHookNameSniff.php
+++ b/WordPress/Sniffs/NamingConventions/ValidHookNameSniff.php
@@ -100,10 +100,11 @@ class ValidHookNameSniff extends AbstractFunctionParameterSniff {
 
 		$regex = $this->prepare_regex();
 
-		$case_errors = 0;
-		$underscores = 0;
-		$content     = array();
-		$expected    = array();
+		$case_errors    = 0;
+		$underscores    = 0;
+		$content        = array();
+		$expected       = array();
+		$last_non_empty = null;
 
 		for ( $i = $parameters[1]['start']; $i <= $parameters[1]['end']; $i++ ) {
 			// Skip past comment tokens.
@@ -129,15 +130,28 @@ class ValidHookNameSniff extends AbstractFunctionParameterSniff {
 
 				} while ( isset( $this->tokens[ $i ] ) && $i <= $parameters[1]['end'] );
 
+				$last_non_empty = $i;
+				continue;
+			}
+
+			// Skip over parameters passed to function calls.
+			if ( \T_OPEN_PARENTHESIS === $this->tokens[ $i ]['code']
+				&& \T_STRING === $this->tokens[ $last_non_empty ]['code']
+				&& isset( $this->tokens[ $i ]['parenthesis_closer'] )
+			) {
+				$i              = $this->tokens[ $i ]['parenthesis_closer'];
+				$last_non_empty = $i;
 				continue;
 			}
 
 			// Skip past non-string tokens.
 			if ( isset( Tokens::$stringTokens[ $this->tokens[ $i ]['code'] ] ) === false ) {
+				$last_non_empty = $i;
 				continue;
 			}
 
-			$string = TextStrings::stripQuotes( $this->tokens[ $i ]['content'] );
+			$last_non_empty = $i;
+			$string         = TextStrings::stripQuotes( $this->tokens[ $i ]['content'] );
 
 			/*
 			 * Here be dragons - a double quoted string can contain extrapolated variables


Test patch:
diff --git a/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.1.inc b/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.1.inc
index 039ca648..2c03bfe5 100644
--- a/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.1.inc
+++ b/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.1.inc
@@ -94,3 +94,17 @@ do_action(
 	// phpcs:ignore Stnd.Cat.Sniff -- For reasons.
 	'prefix_hook-name' /* comment */
 );
+
+// Ignore text strings when passed as parameters to a function call. WPCS #2055.
+$value = apply_filters(
+	get_filter_name( 'UPPERCASE', 'wrong-delimiter' ),
+	$value,
+	$attributes
+);
+
+// ... but do not ignore text strings in arbitrary parentheses.
+$value = apply_filters(
+	( $name ? 'UPPERCASE' : 'wrong-delimiter' ),
+	$value,
+	$attributes
+);
diff --git a/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.php b/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.php
index c55be6b0..b01a6262 100644
--- a/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.php
+++ b/WordPress/Tests/NamingConventions/ValidHookNameUnitTest.php
@@ -32,43 +32,44 @@ class ValidHookNameUnitTest extends AbstractSniffUnitTest {
 		switch ( $testFile ) {
 			case 'ValidHookNameUnitTest.1.inc':
 				return array(
-					14 => 1,
-					15 => 1,
-					16 => 1,
-					17 => 1,
-					28 => 1,
-					29 => 1,
-					30 => 1,
-					33 => 1,
-					53 => 1,
-					54 => 1,
-					55 => 1,
-					56 => 1,
-					57 => 1,
-					58 => 1,
-					59 => 1,
-					60 => 1,
-					61 => 1,
-					62 => 1,
-					63 => 1,
-					64 => 1,
-					65 => 1,
-					66 => 1,
-					68 => 1,
-					69 => 1,
-					70 => 1,
-					71 => 1,
-					72 => 1,
-					73 => 1,
-					74 => 1,
-					75 => 1,
-					76 => 1,
-					77 => 1,
-					78 => 1,
-					79 => 1,
-					80 => 1,
-					81 => 1,
-					89 => 1,
+					14  => 1,
+					15  => 1,
+					16  => 1,
+					17  => 1,
+					28  => 1,
+					29  => 1,
+					30  => 1,
+					33  => 1,
+					53  => 1,
+					54  => 1,
+					55  => 1,
+					56  => 1,
+					57  => 1,
+					58  => 1,
+					59  => 1,
+					60  => 1,
+					61  => 1,
+					62  => 1,
+					63  => 1,
+					64  => 1,
+					65  => 1,
+					66  => 1,
+					68  => 1,
+					69  => 1,
+					70  => 1,
+					71  => 1,
+					72  => 1,
+					73  => 1,
+					74  => 1,
+					75  => 1,
+					76  => 1,
+					77  => 1,
+					78  => 1,
+					79  => 1,
+					80  => 1,
+					81  => 1,
+					89  => 1,
+					107 => 1,
 				);
 
 			case 'ValidHookNameUnitTest.2.inc':
@@ -89,14 +90,15 @@ class ValidHookNameUnitTest extends AbstractSniffUnitTest {
 		switch ( $testFile ) {
 			case 'ValidHookNameUnitTest.1.inc':
 				return array(
-					8  => 1,
-					9  => 1,
-					10 => 1,
-					11 => 1,
-					68 => 1,
-					72 => 1,
-					77 => 1,
-					95 => 1,
+					8   => 1,
+					9   => 1,
+					10  => 1,
+					11  => 1,
+					68  => 1,
+					72  => 1,
+					77  => 1,
+					95  => 1,
+					107 => 1,
 				);
 
 			case 'ValidHookNameUnitTest.2.inc':