Merge pull request #64 from WordPress-Coding-Standards/issue/58

Add new core.ruleset.xml excluding extra sniffs, and other fixes

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 );
 		}

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;

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;
 		}
 

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;

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
+}

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

Tests/VIP/ValidatedSanitizedInputUnitTest.inc

@@ -1,6 +1,6 @@
 <?php
 
-do_something( $_POST ); // Warning, unusual usage
+do_something( $_POST ); // OK
 
 do_something_with( $_POST['hello'] ); // Error for no validation, Error for no sanitizing
 
@@ -50,3 +50,12 @@ function foo() {
 	}
 	echo esc_html( $_GET['test'] ); // Good
 }
+
+$_REQUEST['wp_customize'] = 'on'; // ok
+
+// All OK
+$keys = array_keys( $_POST );
+$values = array_values( $_POST );
+foreach( $_POST as $key => $value ) {
+	// ..
+}

Tests/VIP/ValidatedSanitizedInputUnitTest.php

@@ -45,9 +45,7 @@ public function getErrorList()
 	 */
 	public function getWarningList()
 	{
-		return array(
-			3 => 1,
-			);
+		return array();
 
 	}//end getWarningList()
 

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'] );
+}

Tests/WhiteSpace/ControlStructureSpacingUnitTest.php

@@ -44,6 +44,7 @@ public function getErrorList()
     {
         return array(
                  4 => 2,
+                 17 => 1,
                 );
 
     }//end getErrorList()

core.ruleset.xml

@@ -0,0 +1,19 @@
+<?xml version="1.0"?>
+<ruleset name="WordPress Core">
+	<description>Non-controversial generally-agreed upon WordPress Coding Standards</description>
+
+	<rule ref="WordPress.Arrays.ArrayDeclaration"/>
+	<rule ref="WordPress.Classes.ValidClassName"/>
+	<rule ref="WordPress.Files.FileName"/>
+	<rule ref="WordPress.Formatting.MultipleStatementAlignment"/>
+	<rule ref="WordPress.Functions.FunctionCallSignature"/>
+	<rule ref="WordPress.Functions.FunctionDeclarationArgumentSpacing"/>
+	<rule ref="WordPress.NamingConventions.ValidFunctionName"/>
+	<rule ref="WordPress.PHP.DiscouragedFunctions"/>
+	<rule ref="WordPress.Strings.DoubleQuoteUsage"/>
+	<rule ref="WordPress.WhiteSpace.ControlStructureSpacing"/>
+	<rule ref="WordPress.WhiteSpace.OperatorSpacing"/>
+	<rule ref="WordPress.WhiteSpace.PhpIndent"/>
+	<rule ref="WordPress.WhiteSpace.ScopeIndent"/>
+
+</ruleset>

ruleset.xml

@@ -1,4 +1,4 @@
 <?xml version="1.0"?>
 <ruleset name="WordPress">
-	<description>A custom coding standard.</description>
+	<description>WordPress Coding Standards</description>
 </ruleset>