EscapingVoidReturnFunctions: Fix docs and logic

- Use the parent `$printingFunctions` list of printing functions, instead of just checking for `_e` and `_ex`. - Fix the class description. - Add ruleset tests for both standards. Fixes #387.
Automattic · Mar 6, 2019 · 402ac50 · 402ac50
1 parent c8ed37a
commit 402ac50
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 2 deletions.
diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc
@@ -322,4 +322,20 @@ update_site_option( $bar, $foo, true ); // Ok.
 // WordPressVIPMinimum.Functions.RestrictedFunctions.site_option_add_site_option
 add_site_option( 'foo', $bar ); // Ok.
 
+// WordPressVIPMinimum.Security.EscapingVoidReturnFunctions.Found
+esc_js( _deprecated_argument() ); // Error.
+esc_js( _deprecated_constructor() ); // Error.
+esc_js( _deprecated_file() ); // Error.
+esc_js( _deprecated_function() ); // Error.
+esc_js( _deprecated_hook() ); // Error.
+esc_js( _doing_it_wrong() ); // Error.
+esc_html( _e( 'foo', 'bar' ) ); // Error.
+esc_html( _ex( 'foo', 'bar' ) ); // Error.
+esc_attr( printf( 'foo', [] ) ); // Error.
+esc_attr( trigger_error( 'foo' ) ); // Error (+ warning due to trigger_error() call).
+esc_attr( user_error( 'foo', '' ) ); // Error.
+esc_attr( vprintf( 'foo', [] ) ); // Error.
+esc_attr( wp_die( 'foo' ) ); // Error.
+esc_attr( wp_dropdown_pages() ); // Error.
+
 ?> <!-- closing PHP tag should be omitted -->
diff --git a/WordPress-VIP-Go/ruleset-test.php b/WordPress-VIP-Go/ruleset-test.php
@@ -62,6 +62,20 @@
 		268 => 1,
 		270 => 1,
 		271 => 1,
+		326 => 1,
+		327 => 1,
+		328 => 1,
+		329 => 1,
+		330 => 1,
+		331 => 1,
+		332 => 1,
+		333 => 1,
+		334 => 1,
+		335 => 1,
+		336 => 1,
+		337 => 1,
+		338 => 1,
+		339 => 1,
 	],
 	'warnings' => [
 		84  => 1,
@@ -116,6 +130,7 @@
 		277 => 1,
 		281 => 1,
 		285 => 1,
+		335 => 1,
 	],
 	'messages' => [
 		4   => [

diff --git a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php
@@ -12,7 +12,9 @@
 use PHP_CodeSniffer\Util\Tokens;
 
 /**
- * Flag suspicious WP_Query and get_posts params.
+ * Flag functions that don't return anything, yet are wrapped in an escaping function call.
+ *
+ * E.g. esc_html( _e( 'foo' ) );
  *
  *  @package VIPCS\WordPressVIPMinimum
  */
@@ -57,7 +59,7 @@ public function process_token( $stackPtr ) {
 			return;
 		}
 
-		if ( 0 === strpos( $this->tokens[ $next_token ]['content'], '_e' ) ) {
+		if ( isset( $this->printingFunctions[ $this->tokens[ $next_token ]['content'] ] ) ) {
 			$message = 'Attempting to escape `%s()` which is printing its output.';
 			$data    = [ $this->tokens[ $next_token ]['content'] ];
 			$this->phpcsFile->addError( $message, $stackPtr, 'Found', $data );

diff --git a/WordPressVIPMinimum/ruleset-test.inc b/WordPressVIPMinimum/ruleset-test.inc
@@ -188,4 +188,20 @@ update_site_option( $bar, $foo, true ); // Error.
 // WordPressVIPMinimum.Functions.RestrictedFunctions.site_option_delete_site_option
 delete_site_option( $foo ); // Error.
 
+// WordPressVIPMinimum.Security.EscapingVoidReturnFunctions.Found
+esc_js( _deprecated_argument() ); // Error.
+esc_js( _deprecated_constructor() ); // Error.
+esc_js( _deprecated_file() ); // Error.
+esc_js( _deprecated_function() ); // Error.
+esc_js( _deprecated_hook() ); // Error.
+esc_js( _doing_it_wrong() ); // Error.
+esc_html( _e( 'foo', 'bar' ) ); // Error.
+esc_html( _ex( 'foo', 'bar' ) ); // Error.
+esc_attr( printf( 'foo', [] ) ); // Error.
+esc_attr( trigger_error( 'foo' ) ); // Error (+ warning due to trigger_error() call).
+esc_attr( user_error( 'foo', '' ) ); // Error.
+esc_attr( vprintf( 'foo', [] ) ); // Error.
+esc_attr( wp_die( 'foo' ) ); // Error.
+esc_attr( wp_dropdown_pages() ); // Error.
+
 ?> <!-- closing PHP tag should be omitted -->
diff --git a/WordPressVIPMinimum/ruleset-test.php b/WordPressVIPMinimum/ruleset-test.php
@@ -49,6 +49,20 @@
 		183 => 1,
 		186 => 1,
 		189 => 1,
+		192 => 1,
+		193 => 1,
+		194 => 1,
+		195 => 1,
+		196 => 1,
+		197 => 1,
+		198 => 1,
+		199 => 1,
+		200 => 1,
+		201 => 1,
+		202 => 1,
+		203 => 1,
+		204 => 1,
+		205 => 1,
 	],
 	'warnings' => [
 		18  => 1,
@@ -75,6 +89,7 @@
 		166 => 1,
 		170 => 1,
 		177 => 1,
+		201 => 1,
 	],
 	'messages' => [
 		123 => [