diff --git a/Sniffs/VIP/DirectDatabaseQuerySniff.php b/Sniffs/VIP/DirectDatabaseQuerySniff.php index 1c6244a257..7bd085e52e 100644 --- a/Sniffs/VIP/DirectDatabaseQuerySniff.php +++ b/Sniffs/VIP/DirectDatabaseQuerySniff.php @@ -46,23 +46,24 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) if ( false == $phpcsFile->findNext( array( T_OBJECT_OPERATOR ), $stackPtr + 1, null, null, null, true ) ) return; // This is not a call to the wpdb object - // Check for whitelisting comment - $whitelisted = false; - $whitelist_pattern = '/db call\W*(ok|pass|clear|whitelist)/i'; + // Check for whitelisting comments $endOfStatement = $phpcsFile->findNext( array( T_SEMICOLON ), $stackPtr + 1, null, null, null, true ); + $endOfLineComment = ''; for ( $i = $endOfStatement + 1; $i < count( $tokens ); $i++ ) { - if ( in_array( $tokens[$i]['code'], array( T_WHITESPACE ) ) ) { - continue; - } - - if ( $tokens[$i]['code'] != T_COMMENT ) { + if ( $tokens[$i]['line'] !== $tokens[$endOfStatement]['line'] ) { break; } - if ( preg_match( $whitelist_pattern, $tokens[$i]['content'], $matches ) > 0 ) { - $whitelisted = true; + if ( $tokens[$i]['code'] === T_COMMENT ) { + $endOfLineComment .= $tokens[$i]['content']; } + + } + + $whitelisted_db_call = false; + if ( preg_match( '/db call\W*(ok|pass|clear|whitelist)/i', $endOfLineComment, $matches ) ) { + $whitelisted_db_call = true; } // Check for Database Schema Changes @@ -75,13 +76,17 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) } // Flag instance if not whitelisted - if ( ! $whitelisted ) { + if ( ! $whitelisted_db_call ) { $message = 'Usage of a direct database call is discouraged.'; $this->add_unique_message( $phpcsFile, 'warning', $stackPtr, $tokens[$stackPtr]['line'], $message ); } + $whitelisted_cache = false; $cached = false; - if ( ! empty( $tokens[$stackPtr]['conditions'] ) ) { + if ( preg_match( '/cache\s+(ok|pass|clear|whitelist)/i', $endOfLineComment, $matches ) ) { + $whitelisted_cache = true; + } + if ( ! $whitelisted_cache && ! empty( $tokens[$stackPtr]['conditions'] ) ) { $conditions = $tokens[$stackPtr]['conditions']; $scope_function = null; foreach ( $conditions as $condPtr => $condType ) { @@ -104,7 +109,7 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) } - if ( ! $cached ) { + if ( ! $cached && ! $whitelisted_cache ) { $message = 'Usage of a direct database call without caching is prohibited. Use wp_cache_get / wp_cache_set.'; $this->add_unique_message( $phpcsFile, 'error', $stackPtr, $tokens[$stackPtr]['line'], $message ); } diff --git a/Sniffs/VIP/SuperGlobalInputUsageSniff.php b/Sniffs/VIP/SuperGlobalInputUsageSniff.php index 1cbff82da1..c33e4ca5a5 100644 --- a/Sniffs/VIP/SuperGlobalInputUsageSniff.php +++ b/Sniffs/VIP/SuperGlobalInputUsageSniff.php @@ -43,6 +43,13 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) $varName = $tokens[$stackPtr]['content']; + // If we're overriding a superglobal with an assignment, no need to test + $semicolon_position = $phpcsFile->findNext( array( T_SEMICOLON ), $stackPtr + 1, null, null, null, true ); + $assignment_position = $phpcsFile->findNext( array( T_EQUAL ), $stackPtr + 1, null, null, null, true ); + if ( $semicolon_position !== false && $assignment_position !== false && $assignment_position < $semicolon_position ) { + return; + } + // Check for whitelisting comment $currentLine = $tokens[$stackPtr]['line']; $nextPtr = $stackPtr; diff --git a/Sniffs/VIP/ValidatedSanitizedInputSniff.php b/Sniffs/VIP/ValidatedSanitizedInputSniff.php index 74d3243d49..4f9f5ceb4b 100644 --- a/Sniffs/VIP/ValidatedSanitizedInputSniff.php +++ b/Sniffs/VIP/ValidatedSanitizedInputSniff.php @@ -52,6 +52,13 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) $instance = $tokens[$stackPtr]; $varName = $instance['content']; + // If we're overriding a superglobal with an assignment, no need to test + $semicolon_position = $phpcsFile->findNext( array( T_SEMICOLON ), $stackPtr + 1, null, null, null, true ); + $assignment_position = $phpcsFile->findNext( array( T_EQUAL ), $stackPtr + 1, null, null, null, true ); + if ( $semicolon_position !== false && $assignment_position !== false && $assignment_position < $semicolon_position ) { + return; + } + if ( ! isset( $instance['nested_parenthesis'] ) ) { $phpcsFile->addError( 'Detected usage of a non-sanitized input variable: %s', $stackPtr, null, array( $tokens[$stackPtr]['content'] ) ); return; @@ -67,7 +74,6 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) $varKey = $this->getArrayIndexKey( $phpcsFile, $tokens, $stackPtr ); if ( empty( $varKey ) ) { - $phpcsFile->addWarning( 'Detected access of super global var %s without targeting a member variable.', $stackPtr, null, array( $varName ) ); return; } diff --git a/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php b/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php index 953add5f44..cf0ef50784 100644 --- a/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php +++ b/Sniffs/WhiteSpace/ControlStructureSpacingSniff.php @@ -103,13 +103,11 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr) $closer = $tokens[$openBracket]['parenthesis_closer']; if ($tokens[($closer - 1)]['code'] !== T_WHITESPACE) { - $error = 'No space before closing parenthesis is prohibited'; - $phpcsFile->addError($error, $closer); + $error = 'No space before closing parenthesis is prohibited'; + $phpcsFile->addError($error, $closer); } - $arrayLine = $tokens[$scopeOpener]['line']; - - if (isset($tokens[$arrayLine]['scope_opener']) === true && $tokens[$arrayLine]['line'] !== $tokens[$tokens[$arrayLine]['scope_opener']]['line']) { + if (isset($tokens[$openBracket]['parenthesis_owner']) && $tokens[$closer]['line'] !== $tokens[$scopeOpener]['line']) { $error = 'Opening brace should be on the same line as the declaration'; $phpcsFile->addError($error, $openBracket); return; diff --git a/Tests/VIP/DirectDatabaseQueryUnitTest.inc b/Tests/VIP/DirectDatabaseQueryUnitTest.inc index fe9a40d8c8..6a509c74fc 100644 --- a/Tests/VIP/DirectDatabaseQueryUnitTest.inc +++ b/Tests/VIP/DirectDatabaseQueryUnitTest.inc @@ -51,3 +51,8 @@ function quux() { } } + +function barzd() { + global $wpdb; + $autoload = $wpdb->get_var( $wpdb->prepare( "SELECT autoload FROM $wpdb->options WHERE option_name = %s", $option_name ) ); // db call ok; no-cache ok +} diff --git a/Tests/VIP/SuperGlobalInputUsageUnitTest.inc b/Tests/VIP/SuperGlobalInputUsageUnitTest.inc index c1646a4851..36079061e5 100644 --- a/Tests/VIP/SuperGlobalInputUsageUnitTest.inc +++ b/Tests/VIP/SuperGlobalInputUsageUnitTest.inc @@ -11,3 +11,5 @@ if ( $_GET['test'] && foo() && $bar ) { // input var okay bar( $_POST['foo'] ); // warning quux( $_REQUEST['quux'] ); // warning + +$_REQUEST['wp_customize'] = 'on'; // ok diff --git a/Tests/VIP/ValidatedSanitizedInputUnitTest.inc b/Tests/VIP/ValidatedSanitizedInputUnitTest.inc index 585e32bb11..27aebf0d27 100644 --- a/Tests/VIP/ValidatedSanitizedInputUnitTest.inc +++ b/Tests/VIP/ValidatedSanitizedInputUnitTest.inc @@ -1,6 +1,6 @@ $value ) { + // .. +} diff --git a/Tests/VIP/ValidatedSanitizedInputUnitTest.php b/Tests/VIP/ValidatedSanitizedInputUnitTest.php index ccf84103d7..18a507f120 100644 --- a/Tests/VIP/ValidatedSanitizedInputUnitTest.php +++ b/Tests/VIP/ValidatedSanitizedInputUnitTest.php @@ -45,9 +45,7 @@ public function getErrorList() */ public function getWarningList() { - return array( - 3 => 1, - ); + return array(); }//end getWarningList() diff --git a/Tests/WhiteSpace/ControlStructureSpacingUnitTest.inc b/Tests/WhiteSpace/ControlStructureSpacingUnitTest.inc index f1826ce886..596fd753d2 100644 --- a/Tests/WhiteSpace/ControlStructureSpacingUnitTest.inc +++ b/Tests/WhiteSpace/ControlStructureSpacingUnitTest.inc @@ -12,3 +12,14 @@ if ( true ) { } else { //Are we allowed to comment here? If not, message is wrong // ... } + +// bad +if ( 'update' === $option_operation['operation'] ) +{ + update_option( $option_operation['option_name'], $option_operation['old_value'] ); +} + +// good +if ( 'update' === $option_operation['operation'] ) { + update_option( $option_operation['option_name'], $option_operation['old_value'] ); +} diff --git a/Tests/WhiteSpace/ControlStructureSpacingUnitTest.php b/Tests/WhiteSpace/ControlStructureSpacingUnitTest.php index f1b6e131de..391bd1e25c 100644 --- a/Tests/WhiteSpace/ControlStructureSpacingUnitTest.php +++ b/Tests/WhiteSpace/ControlStructureSpacingUnitTest.php @@ -44,6 +44,7 @@ public function getErrorList() { return array( 4 => 2, + 17 => 1, ); }//end getErrorList() diff --git a/core.ruleset.xml b/core.ruleset.xml new file mode 100644 index 0000000000..6eae87ebe6 --- /dev/null +++ b/core.ruleset.xml @@ -0,0 +1,19 @@ + + + Non-controversial generally-agreed upon WordPress Coding Standards + + + + + + + + + + + + + + + + diff --git a/ruleset.xml b/ruleset.xml index 76e1878c78..712e05aaf7 100644 --- a/ruleset.xml +++ b/ruleset.xml @@ -1,4 +1,4 @@ - A custom coding standard. + WordPress Coding Standards