From 7c5e48ff19d4414418f4ceef5592b190ffd933f6 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 21 Jan 2017 13:12:01 +0100 Subject: [PATCH 1/2] PHP forward compat: Add some unit tests related to closures and anonymous classes. --- .../Tests/CSRF/NonceVerificationUnitTest.inc | 10 +++++++++ .../Tests/CSRF/NonceVerificationUnitTest.php | 16 +++++++------- .../Tests/VIP/DirectDatabaseQueryUnitTest.inc | 12 +++++++++++ .../Tests/VIP/DirectDatabaseQueryUnitTest.php | 9 ++++---- .../VIP/ValidatedSanitizedInputUnitTest.inc | 5 +++++ .../VIP/ValidatedSanitizedInputUnitTest.php | 1 + .../Variables/GlobalVariablesUnitTest.inc | 12 +++++++++++ .../Variables/GlobalVariablesUnitTest.php | 21 ++++++++++--------- .../WhiteSpace/OperatorSpacingUnitTest.inc | 4 ++++ .../OperatorSpacingUnitTest.inc.fixed | 4 ++++ 10 files changed, 73 insertions(+), 21 deletions(-) diff --git a/WordPress/Tests/CSRF/NonceVerificationUnitTest.inc b/WordPress/Tests/CSRF/NonceVerificationUnitTest.inc index c6eb4e9c9c..42e551646f 100644 --- a/WordPress/Tests/CSRF/NonceVerificationUnitTest.inc +++ b/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. +}; diff --git a/WordPress/Tests/CSRF/NonceVerificationUnitTest.php b/WordPress/Tests/CSRF/NonceVerificationUnitTest.php index 66979da9f9..307d6e08a4 100644 --- a/WordPress/Tests/CSRF/NonceVerificationUnitTest.php +++ b/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, ); } diff --git a/WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.inc b/WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.inc index e44b14358c..15660902b5 100644 --- a/WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.inc +++ b/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; +}; diff --git a/WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.php b/WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.php index 4b8da25df5..34156f8489 100644 --- a/WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.php +++ b/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, ); } diff --git a/WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.inc b/WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.inc index 5a6fc9a2aa..f5286f18d0 100644 --- a/WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.inc +++ b/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. +}; diff --git a/WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.php b/WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.php index 9b9cc8f113..68c79512be 100644 --- a/WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.php +++ b/WordPress/Tests/VIP/ValidatedSanitizedInputUnitTest.php @@ -41,6 +41,7 @@ public function getErrorList() { 104 => 2, 105 => 1, 114 => 2, + 121 => 1, ); } diff --git a/WordPress/Tests/Variables/GlobalVariablesUnitTest.inc b/WordPress/Tests/Variables/GlobalVariablesUnitTest.inc index 3df45ddfb3..ecbba96fe4 100644 --- a/WordPress/Tests/Variables/GlobalVariablesUnitTest.inc +++ b/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 ); diff --git a/WordPress/Tests/Variables/GlobalVariablesUnitTest.php b/WordPress/Tests/Variables/GlobalVariablesUnitTest.php index 663228fffc..1c5a40e938 100644 --- a/WordPress/Tests/Variables/GlobalVariablesUnitTest.php +++ b/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, ); } diff --git a/WordPress/Tests/WhiteSpace/OperatorSpacingUnitTest.inc b/WordPress/Tests/WhiteSpace/OperatorSpacingUnitTest.inc index bee440f694..fc9dab4191 100644 --- a/WordPress/Tests/WhiteSpace/OperatorSpacingUnitTest.inc +++ b/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' ) {}; diff --git a/WordPress/Tests/WhiteSpace/OperatorSpacingUnitTest.inc.fixed b/WordPress/Tests/WhiteSpace/OperatorSpacingUnitTest.inc.fixed index 500fa65de0..377b17d5eb 100644 --- a/WordPress/Tests/WhiteSpace/OperatorSpacingUnitTest.inc.fixed +++ b/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' ) {}; From b92e8a61308a3a3b29d2227f79ee8c1d7d7e0101 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 21 Jan 2017 12:46:29 +0100 Subject: [PATCH 2/2] PHP forward compat: Improve support for closures and anonymous classes. --- WordPress/Sniff.php | 19 ++++++++++++++++--- .../Sniffs/VIP/DirectDatabaseQuerySniff.php | 6 +++++- .../Sniffs/Variables/GlobalVariablesSniff.php | 2 +- .../ControlStructureSpacingSniff.php | 4 +++- .../WhiteSpace/OperatorSpacingSniff.php | 4 +++- 5 files changed, 28 insertions(+), 7 deletions(-) diff --git a/WordPress/Sniff.php b/WordPress/Sniff.php index 1773b3e572..3e04b5feb4 100644 --- a/WordPress/Sniff.php +++ b/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,6 +1115,8 @@ 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 ); @@ -1117,7 +1124,13 @@ protected function is_validated( $stackPtr, $array_key = null, $in_condition_onl 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'; } diff --git a/WordPress/Sniffs/VIP/DirectDatabaseQuerySniff.php b/WordPress/Sniffs/VIP/DirectDatabaseQuerySniff.php index 35169dfcf0..399def6c6a 100644 --- a/WordPress/Sniffs/VIP/DirectDatabaseQuerySniff.php +++ b/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']; diff --git a/WordPress/Sniffs/Variables/GlobalVariablesSniff.php b/WordPress/Sniffs/Variables/GlobalVariablesSniff.php index 898fe02cdf..9d7c291bfc 100644 --- a/WordPress/Sniffs/Variables/GlobalVariablesSniff.php +++ b/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; diff --git a/WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php b/WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php index a531d371fd..cbcf72b470 100644 --- a/WordPress/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php +++ b/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; diff --git a/WordPress/Sniffs/WhiteSpace/OperatorSpacingSniff.php b/WordPress/Sniffs/WhiteSpace/OperatorSpacingSniff.php index b8c80f89a5..16ecba1544 100644 --- a/WordPress/Sniffs/WhiteSpace/OperatorSpacingSniff.php +++ b/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; } }