Security/EscapeOutput: fix false negatives when handling anonymous classes (#2559)

* Security/EscapeOutput: fix false negatives when handling anonymous classes

This commit fixes false negatives when the sniff handles readonly anonymous classes and anonymous classes with attributes that are part of a throw statement.

When stepping over tokens after `T_THROW` to find the `T_OPEN_PARENTHESIS` of the exception creation function call/class instantiation, the sniff was not considering that it might need to step over `T_READONLY` tokens or attribute declarations when dealing with anonymous classes.

Fixes #2552

---------

Co-authored-by: jrfnl <[email protected]>

Author: rodrigoprimo

.phpcs.xml.dist

@@ -68,6 +68,7 @@
 		<exclude name="PHPCompatibility.Constants.NewConstants.t_coalesceFound"/>
 		<exclude name="PHPCompatibility.Constants.NewConstants.t_coalesce_equalFound"/>
 		<exclude name="PHPCompatibility.Constants.NewConstants.t_yield_fromFound"/>
+		<exclude name="PHPCompatibility.Constants.NewConstants.t_readonlyFound"/>
 	</rule>
 
 	<!-- Enforce PSR1 compatible namespaces. -->

WordPress/Sniffs/Security/EscapeOutputSniff.php

@@ -216,22 +216,35 @@ public function process_token( $stackPtr ) {
 
 			case \T_THROW:
 				// Find the open parentheses, while stepping over the exception creation tokens.
-				$ignore  = Tokens::$emptyTokens;
-				$ignore += Collections::namespacedNameTokens();
-				$ignore += Collections::functionCallTokens();
-				$ignore += Collections::objectOperators();
-
-				$next_relevant = $this->phpcsFile->findNext( $ignore, ( $stackPtr + 1 ), null, true );
-				if ( false === $next_relevant ) {
-					return;
-				}
-
-				if ( \T_NEW === $this->tokens[ $next_relevant ]['code'] ) {
+				$ignore                = Tokens::$emptyTokens;
+				$ignore               += Collections::namespacedNameTokens();
+				$ignore               += Collections::functionCallTokens();
+				$ignore               += Collections::objectOperators();
+				$ignore[ \T_READONLY ] = \T_READONLY;
+
+				$next_relevant = $stackPtr;
+				do {
 					$next_relevant = $this->phpcsFile->findNext( $ignore, ( $next_relevant + 1 ), null, true );
 					if ( false === $next_relevant ) {
 						return;
 					}
-				}
+
+					if ( \T_NEW === $this->tokens[ $next_relevant ]['code'] ) {
+						continue;
+					}
+
+					// Skip over attribute declarations when searching for the open parenthesis.
+					if ( \T_ATTRIBUTE === $this->tokens[ $next_relevant ]['code'] ) {
+						if ( isset( $this->tokens[ $next_relevant ]['attribute_closer'] ) === false ) {
+							return;
+						}
+
+						$next_relevant = $this->tokens[ $next_relevant ]['attribute_closer'];
+						continue;
+					}
+
+					break;
+				} while ( $next_relevant < ( $this->phpcsFile->numTokens - 1 ) );
 
 				if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next_relevant ]['code']
 					|| isset( $this->tokens[ $next_relevant ]['parenthesis_closer'] ) === false

WordPress/Tests/Security/EscapeOutputUnitTest.1.inc

@@ -662,3 +662,17 @@ die( status: esc_html( $foo ) ); // Ok.
 
 exit( status: $foo ); // Bad.
 die( status: $foo ); // Bad.
+
+/*
+ * Issue https://github.com/WordPress/WordPress-Coding-Standards/issues/2552
+ * Ensure that readonly anonymous classes and anonymous classes with attributes are handled
+ * correctly when part of a throw statement.
+ */
+throw new #[MyAttribute] readonly class( esc_html( $message ) ) extends Exception {}; // Good.
+throw new readonly class( $unescaped ) {}; // Bad.
+throw new #[MyAttribute] class( $unescaped ) extends Exception {}; // Bad.
+throw new
+#[Attribute1]
+/* some comment */
+#[Attribute2('text', 10)]
+readonly class( $unescaped ) {}; // Bad.

WordPress/Tests/Security/EscapeOutputUnitTest.21.inc

@@ -0,0 +1,8 @@
+<?php
+
+/*
+ * Intentional parse error (nothing after T_ATTRIBUTE).
+ * This should be the only test in this file.
+ */
+
+throw new #[

WordPress/Tests/Security/EscapeOutputUnitTest.22.inc

@@ -0,0 +1,8 @@
+<?php
+
+/*
+ * Intentional parse error (only whitespaces after T_ATTRIBUTE_END).
+ * This should be the only test in this file.
+ */
+
+throw new #[MyAttribute]

WordPress/Tests/Security/EscapeOutputUnitTest.23.inc

@@ -0,0 +1,9 @@
+<?php
+
+/*
+ * Intentional parse error (nothing after T_ATTRIBUTE_END).
+ * There should be no whitespaces at the end of this file.
+ * This should be the only test in this file.
+ */
+
+throw new #[MyAttribute]

WordPress/Tests/Security/EscapeOutputUnitTest.php

@@ -161,6 +161,9 @@ public function getErrorList( $testFile = '' ) {
 					657 => 1,
 					663 => 1,
 					664 => 1,
+					672 => 1,
+					673 => 1,
+					678 => 1,
 				);
 
 			case 'EscapeOutputUnitTest.6.inc':