Merge pull request #813 from WordPress-Coding-Standards/feature/closu…

…re-anon-classes

Improve sniffs to deal with anonymous functions and classes

WordPress/Sniff.php

@@ -760,8 +760,13 @@ protected function has_nonce_check( $stackPtr ) {
 
 		// If we're in a function, only look inside of it.
 		$f = $this->phpcsFile->getCondition( $stackPtr, T_FUNCTION );
-		if ( $f ) {
+		if ( false !== $f ) {
 			$start = $tokens[ $f ]['scope_opener'];
+		} else {
+			$f = $this->phpcsFile->getCondition( $stackPtr, T_CLOSURE );
+			if ( false !== $f ) {
+				$start = $tokens[ $f ]['scope_opener'];
+			}
 		}
 
 		$in_isset = $this->is_in_isset_or_empty( $stackPtr );
@@ -1110,14 +1115,22 @@ protected function is_validated( $stackPtr, $array_key = null, $in_condition_onl
 			   in the same function/file scope as it is used.
 			 */
 
+			$scope_start = 0;
+
 			// Check if we are in a function.
 			$function = $this->phpcsFile->getCondition( $stackPtr, T_FUNCTION );
 
 			// If so, we check only within the function, otherwise the whole file.
 			if ( false !== $function ) {
 				$scope_start = $this->tokens[ $function ]['scope_opener'];
 			} else {
-				$scope_start = 0;
+				// Check if we are in a closure.
+				$closure = $this->phpcsFile->getCondition( $stackPtr, T_CLOSURE );
+
+				// If so, we check only within the closure.
+				if ( false !== $closure ) {
+					$scope_start = $this->tokens[ $closure ]['scope_opener'];
+				}
 			}
 
 			$scope_end = $stackPtr;
@@ -1247,7 +1260,7 @@ protected function get_use_type( $stackPtr ) {
 		}
 
 		// USE keywords for traits.
-		if ( $this->phpcsFile->hasCondition( $stackPtr, array( T_CLASS, T_TRAIT ) ) ) {
+		if ( $this->phpcsFile->hasCondition( $stackPtr, array( T_CLASS, T_ANON_CLASS, T_TRAIT ) ) ) {
 			return 'trait';
 		}
 

WordPress/Sniffs/VIP/DirectDatabaseQuerySniff.php

@@ -203,7 +203,11 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) {
 		if ( ! $whitelisted_cache && ! empty( $tokens[ $stackPtr ]['conditions'] ) ) {
 			$scope_function = $phpcsFile->getCondition( $stackPtr, T_FUNCTION );
 
-			if ( $scope_function ) {
+			if ( false === $scope_function ) {
+				$scope_function = $phpcsFile->getCondition( $stackPtr, T_CLOSURE );
+			}
+
+			if ( false !== $scope_function ) {
 				$scopeStart = $tokens[ $scope_function ]['scope_opener'];
 				$scopeEnd   = $tokens[ $scope_function ]['scope_closer'];
 

WordPress/Sniffs/Variables/GlobalVariablesSniff.php

@@ -374,7 +374,7 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) {
 			for ( $ptr = $start; $ptr < $end; $ptr++ ) {
 
 				// If the global statement was in the global scope, skip over functions, classes and the likes.
-				if ( true === $global_scope && in_array( $this->tokens[ $ptr ]['code'], array( T_FUNCTION, T_CLOSURE, T_CLASS, T_INTERFACE, T_TRAIT ), true ) ) {
+				if ( true === $global_scope && in_array( $this->tokens[ $ptr ]['code'], array( T_FUNCTION, T_CLOSURE, T_CLASS, T_ANON_CLASS, T_INTERFACE, T_TRAIT ), true ) ) {
 					if ( ! isset( $this->tokens[ $ptr ]['scope_closer'] ) ) {
 						// Live coding, skip the rest of the file.
 						return;

WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php

@@ -437,7 +437,9 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) {
 			// We ignore spacing for some structures that tend to have their own rules.
 			$ignore = array(
 				T_FUNCTION             => true,
+				T_CLOSURE              => true,
 				T_CLASS                => true,
+				T_ANON_CLASS           => true,
 				T_INTERFACE            => true,
 				T_TRAIT                => true,
 				T_DOC_COMMENT_OPEN_TAG => true,
@@ -536,7 +538,7 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) {
 			// Another control structure's closing brace.
 			if ( isset( $this->tokens[ $trailingContent ]['scope_condition'] ) ) {
 				$owner = $this->tokens[ $trailingContent ]['scope_condition'];
-				if ( in_array( $this->tokens[ $owner ]['code'], array( T_FUNCTION, T_CLOSURE, T_CLASS, T_INTERFACE, T_TRAIT ), true ) ) {
+				if ( in_array( $this->tokens[ $owner ]['code'], array( T_FUNCTION, T_CLOSURE, T_CLASS, T_ANON_CLASS, T_INTERFACE, T_TRAIT ), true ) ) {
 					// The next content is the closing brace of a function, class, interface or trait
 					// so normal function/class rules apply and we can ignore it.
 					return;

WordPress/Sniffs/WhiteSpace/OperatorSpacingSniff.php

@@ -74,7 +74,9 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) {
 				$bracket = end( $tokens[ $stackPtr ]['nested_parenthesis'] );
 				if ( isset( $tokens[ $bracket ]['parenthesis_owner'] ) ) {
 					$function = $tokens[ $bracket ]['parenthesis_owner'];
-					if ( T_FUNCTION === $tokens[ $function ]['code'] ) {
+					if ( T_FUNCTION === $tokens[ $function ]['code']
+						|| T_CLOSURE === $tokens[ $function ]['code']
+					) {
 						return;
 					}
 				}

WordPress/Tests/CSRF/NonceVerificationUnitTest.inc

@@ -115,3 +115,13 @@ function foo_5() {
 
 	check_ajax_referer( 'something' );
 }
+
+// Test anonymous function - Bad, needs nonce check.
+check_ajax_referer( 'something' ); // Nonce check is not in function scope.
+$b = function () {
+	if ( ! isset( $_POST['abc'] ) ) { // Bad.
+		return;
+	}
+
+	do_something( $_POST['abc'] ); // Bad.
+};

WordPress/Tests/CSRF/NonceVerificationUnitTest.php

@@ -23,15 +23,17 @@ class WordPress_Tests_CSRF_NonceVerificationUnitTest extends AbstractSniffUnitTe
 	public function getErrorList() {
 
 		return array(
-			5 => 1,
-			9 => 1,
-			31 => 1,
-			44 => 1,
-			48 => 1,
-			69 => 1,
-			89 => 1,
+			5   => 1,
+			9   => 1,
+			31  => 1,
+			44  => 1,
+			48  => 1,
+			69  => 1,
+			89  => 1,
 			113 => 1,
 			114 => 1,
+			122 => 1,
+			126 => 1,
 		);
 	}
 

WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.inc

@@ -103,3 +103,15 @@ function cache_add_instead_of_set() {
 		wp_cache_add( 'baz', $baz );
 	}
 }
+
+// Database calls in a closure.
+$b = function () {
+	global $wpdb;
+
+	if ( ! ( $listofthings = wp_cache_get( $foo ) ) ) {
+		$listofthings = $wpdb->get_col( 'SELECT something FROM somewhere WHERE someotherthing = 1' ); // Warning.
+		wp_cache_set( 'foo', $listofthings );
+	}
+
+	return $listofthings;
+};

WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.php

@@ -41,10 +41,11 @@ public function getErrorList() {
 	 */
 	public function getWarningList() {
 		return array(
-			6  => 1,
-			17 => 1,
-			38 => 1,
-			50 => 1,
+			6   => 1,
+			17  => 1,
+			38  => 1,
+			50  => 1,
+			112 => 1,
 		);
 
 	}

WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.inc

@@ -115,3 +115,8 @@ output( "some string \\$_POST[some_var] $_GET[evil]" ); // Bad x2.
 
 echo esc_html( wp_strip_all_tags( wp_unslash( $_GET['a'] ) ) ); // Ok.
 
+// Test validation check vs anonymous functions.
+isset( $_POST['abc'] ); // Validation in global scope, not function scope.
+$b = function () {
+	return sanitize_text_field( wp_unslash( $_POST['abc'] ) ); // Error for no validation.
+};

WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.php

@@ -41,6 +41,7 @@ public function getErrorList() {
 			104 => 2,
 			105 => 1,
 			114 => 2,
+			121 => 1,
 		);
 
 	}

WordPress/Tests/Variables/GlobalVariablesUnitTest.inc

@@ -117,3 +117,15 @@ class Test_Class_D extends My_TestClass {
 	}
 }
 // @codingStandardsChangeSetting WordPress.Variables.GlobalVariables custom_test_class_whitelist null
+
+// Test detecting within and skipping over anonymous classes.
+global $year;
+add_filter( 'comments_open', new class {
+	public $year = 2017; // Ok.
+
+	public function __construct( $open, $post_id ) {
+		global $page;
+		$page = get_post( $post_id ); // Bad.
+		return 'page' === $page->post_type;
+	}
+}, 10, 2 );

WordPress/Tests/Variables/GlobalVariablesUnitTest.php

@@ -22,16 +22,17 @@ class WordPress_Tests_Variables_GlobalVariablesUnitTest extends AbstractSniffUni
 	 */
 	public function getErrorList() {
 		return array(
-			3  => 1,
-			6  => 1,
-			16 => 1,
-			17 => 1,
-			18 => 1,
-			25 => 1,
-			35 => 1,
-			36 => 1,
-			54 => 1,
-			95 => 1,
+			3   => 1,
+			6   => 1,
+			16  => 1,
+			17  => 1,
+			18  => 1,
+			25  => 1,
+			35  => 1,
+			36  => 1,
+			54  => 1,
+			95  => 1,
+			128 => 1,
 		);
 
 	}

WordPress/Tests/WhiteSpace/OperatorSpacingUnitTest.inc

@@ -49,3 +49,7 @@ for ( $i =  0; $i < sizeof( $posts ); $i++ ) {
 if (  ! $var ) {
 	// ...
 }
+
+// Ok: Test skipping of default values in function declarations.
+function my_test( $a=true, $b = 123, $c=  'string' ) {}
+$a = function ( $a=true, $b = 123, $c=  'string' ) {};

WordPress/Tests/WhiteSpace/OperatorSpacingUnitTest.inc.fixed

@@ -49,3 +49,7 @@ for ( $i = 0; $i < sizeof( $posts ); $i++ ) {
 if ( ! $var ) {
 	// ...
 }
+
+// Ok: Test skipping of default values in function declarations.
+function my_test( $a=true, $b = 123, $c=  'string' ) {}
+$a = function ( $a=true, $b = 123, $c=  'string' ) {};