Merge pull request #402 from WordPress-Coding-Standards/develop

0.6.0

Author: JDGrimes

.travis.yml

@@ -36,5 +36,5 @@ before_script:
     - $PHPCS_BIN --config-set installed_paths $(pwd)
 
 script:
-    - find . \( -name '*.php' \) -exec php -lf {} \;
+    - if find . -name "*.php" -exec php -l {} \; | grep "^[Parse error|Fatal error]"; then exit 1; fi;
     - phpunit --filter WordPress /tmp/phpcs/tests/AllTests.php

CHANGELOG.md

@@ -11,6 +11,16 @@ This projects adheres to [Semantic Versioning](http://semver.org/) and [Keep a C
 `get_category_by_slug()`, `get_cat_ID()`, `count_user_posts()`, and `wp_old_slug_redirect()` 
 to the list of restricted functions in the `WordPress.VIP.RestrictedFunctions` sniff.
 
+## [0.6.0] - 2015-06-30
+
+### Added
+- Support for `wp_cache_add()` and `wp_cache_delete()`, as well as custom cache 
+functions,in the `WordPress.VIP.DirectDatabaseQuery` sniff.
+
+### Removed
+- `WordPress.Functions.FunctionRestrictions` and `WordPress.Variables.VariableRestrictions` 
+from the `WordPress-VIP` standard, since they are just parents for other sniffs.
+
 ## [0.5.0] - 2015-06-01
 
 ### Added

README.md

@@ -95,10 +95,13 @@ The project encompasses a super–set of the sniffs that the WordPress community
 
 You can use the following as standard names when invoking `phpcs` to select sniffs, fitting your needs:
 
- - `WordPress` — all of the sniffs in the project.
- - `WordPress-Core` — sniffs that seek to implement the [WordPress core coding standards](http://make.wordpress.org/core/handbook/coding-standards/) and go no further.
- - `WordPress-Extra` — `WordPress-Core` plus extra best practices sniffs, which are not part of core coding standards and could be controversial.
- - `WordPress-VIP` — `WordPress-Core` plus sniffs that seek to implement the [WordPress VIP coding requirements](http://vip.wordpress.com/documentation/code-review-what-we-look-for/).
+ - `WordPress` — complete set with all of the sniffs in the project
+  - `WordPress-Core` — main ruleset for [WordPress core coding standards](http://make.wordpress.org/core/handbook/coding-standards/)
+  - `WordPress-Docs` — additional ruleset for inline documentation
+  - `WordPress-Extra` — extended ruleset for optional best practices sniffs
+    - includes `WordPress-Core`
+  - `WordPress-VIP` — extended ruleset for [WordPress VIP coding requirements](http://vip.wordpress.com/documentation/code-review-what-we-look-for/)
+    - includes `WordPress-Core`
 
 
 ### Using custom ruleset

WordPress-VIP/ruleset.xml

@@ -4,10 +4,6 @@
 
 	<rule ref="WordPress-Core"/>
 
-	<!-- the following are inherited by other sniffs -->
-	<rule ref="WordPress.Functions.FunctionRestrictions"/>
-	<rule ref="WordPress.Variables.VariableRestrictions"/>
-
 	<rule ref="WordPress.VIP"/>
 
 	<rule ref="WordPress.XSS.EscapeOutput"/>

WordPress/Sniff.php

@@ -329,6 +329,40 @@ abstract class WordPress_Sniff implements PHP_CodeSniffer_Sniff {
 		'wp_die' => true,
 	);
 
+	/**
+	 * A list of functions that get data from the cache.
+	 *
+	 * @since 0.6.0
+	 *
+	 * @var array
+	 */
+	public static $cacheGetFunctions = array(
+		'wp_cache_get' => true,
+	);
+
+	/**
+	 * A list of functions that set data in the cache.
+	 *
+	 * @since 0.6.0
+	 *
+	 * @var array
+	 */
+	public static $cacheSetFunctions = array(
+		'wp_cache_set' => true,
+		'wp_cache_add' => true,
+	);
+
+	/**
+	 * A list of functions that delete data from the cache.
+	 *
+	 * @since 0.6.0
+	 *
+	 * @var array
+	 */
+	public static $cacheDeleteFunctions = array(
+		'wp_cache_delete' => true,
+	);
+
 	/**
 	 * The current file being sniffed.
 	 *

WordPress/Sniffs/Functions/FunctionRestrictionsSniff.php

@@ -93,7 +93,7 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
 		$groups = $this->getGroups();
 
 		if ( empty( $groups ) ) {
-			return;
+			return count( $tokens ) + 1;
 		}
 
 		foreach ( $groups as $groupName => $group ) {

WordPress/Sniffs/VIP/DirectDatabaseQuerySniff.php

@@ -12,6 +12,56 @@
 class WordPress_Sniffs_VIP_DirectDatabaseQuerySniff implements PHP_CodeSniffer_Sniff
 {
 
+	/**
+	 * List of custom cache get functions.
+	 *
+	 * @since 0.6.0
+	 *
+	 * @var string[]
+	 */
+	public $customCacheGetFunctions = array();
+
+	/**
+	 * List of custom cache set functions.
+	 *
+	 * @since 0.6.0
+	 *
+	 * @var string[]
+	 */
+	public $customCacheSetFunctions = array();
+
+	/**
+	 * List of custom cache delete functions.
+	 *
+	 * @since 0.6.0
+	 *
+	 * @var string[]
+	 */
+	public $customCacheDeleteFunctions = array();
+
+	/**
+	 * The lists of $wpdb methods.
+	 *
+	 * @since 0.6.0
+	 *
+	 * @var array[]
+	 */
+	protected static $methods = array(
+		'cachable' => array(
+			'delete' => true,
+			'get_var' => true,
+			'get_col' => true,
+			'get_row' => true,
+			'get_results' => true,
+			'query' => true,
+			'replace' => true,
+			'update' => true,
+		),
+		'noncachable' => array(
+			'insert' => true,
+		),
+	);
+
 	/**
 	 * Returns an array of tokens this test wants to listen for.
 	 *
@@ -33,10 +83,29 @@ public function register()
 	 * @param int                  $stackPtr  The position of the current token
 	 *                                        in the stack passed in $tokens.
 	 *
-	 * @return void
+	 * @return int|void
 	 */
 	public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
 	{
+		if ( ! isset( self::$methods['all'] ) ) {
+			self::$methods['all'] = array_merge( self::$methods['cachable'], self::$methods['noncachable'] );
+
+			WordPress_Sniff::$cacheGetFunctions = array_merge(
+				WordPress_Sniff::$cacheGetFunctions,
+				array_flip( $this->customCacheGetFunctions )
+			);
+
+			WordPress_Sniff::$cacheSetFunctions = array_merge(
+				WordPress_Sniff::$cacheSetFunctions,
+				array_flip( $this->customCacheSetFunctions )
+			);
+
+			WordPress_Sniff::$cacheDeleteFunctions = array_merge(
+				WordPress_Sniff::$cacheDeleteFunctions,
+				array_flip( $this->customCacheDeleteFunctions )
+			);
+		}
+
 		$tokens = $phpcsFile->getTokens();
 
 		// Check for $wpdb variable
@@ -50,11 +119,7 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
 		$methodPtr = $phpcsFile->findNext( array( T_WHITESPACE ), $is_object_call + 1, null, true, null, true );
 		$method = $tokens[ $methodPtr ]['content'];
 
-		$methods = array(
-			'cachable' => array( 'get_var', 'get_col', 'get_row', 'get_results', 'query' ),
-			'noncachable' => array( 'execute', 'insert', 'update', 'replace' ),
-		);
-		if ( ! in_array( $method, array_merge( $methods['cachable'], $methods['noncachable'] ) ) ) {
+		if ( ! isset( self::$methods['all'][ $method ] ) ) {
 			return;
 		}
 
@@ -81,79 +146,64 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
 		$_pos = $stackPtr;
 		while ( $_pos = $phpcsFile->findNext( array( T_CONSTANT_ENCAPSED_STRING, T_DOUBLE_QUOTED_STRING ), $_pos + 1, $endOfStatement, null, null, true ) ) {
 			if ( preg_match( '#\b(ALTER|CREATE|DROP)\b#i', $tokens[$_pos]['content'], $matches ) > 0 ) {
-				$message = 'Attempting a database schema change is highly discouraged.';
-				$this->add_unique_message( $phpcsFile, 'error', $_pos, $tokens[$_pos]['line'], $message );
+				$phpcsFile->addError( 'Attempting a database schema change is highly discouraged.', $_pos, 'SchemaChange' );
 			}
 		}
 
 		// Flag instance if not 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 );
+			$phpcsFile->addWarning( 'Usage of a direct database call is discouraged.', $stackPtr, 'DirectQuery' );
 		}
 
-		if ( ! in_array( $method, $methods['cachable'] ) ) {
-			return;
+		if ( ! isset( self::$methods['cachable'][ $method ] ) ) {
+			return $endOfStatement;
 		}
 
 		$whitelisted_cache = false;
-		$cached = false;
+		$cached = $wp_cache_get = false;
 		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 ) {
-				if ( $condType == T_FUNCTION ) {
-					$scope_function = $condPtr;
-				}
-			}
+			$scope_function = $phpcsFile->getCondition( $stackPtr, T_FUNCTION );
 
 			if ( $scope_function ) {
 				$scopeStart = $tokens[$scope_function]['scope_opener'];
 				$scopeEnd = $tokens[$scope_function]['scope_closer'];
 
-				$wpcacheget = $phpcsFile->findNext( array( T_STRING ), $scopeStart + 1, $stackPtr - 1, null, 'wp_cache_get' );
-				$wpcacheset = $phpcsFile->findNext( array( T_STRING ), $stackPtr + 1, $scopeEnd - 1, null, 'wp_cache_set' );
+				for ( $i = $scopeStart + 1; $i < $scopeEnd; $i++ ) {
+					if ( T_STRING === $tokens[ $i ]['code'] ) {
 
-				if ( $wpcacheget && $wpcacheset ) {
-					$cached = true;
-				}
-			}
+						if ( isset( WordPress_Sniff::$cacheDeleteFunctions[ $tokens[ $i ]['content'] ] ) ) {
 
-		}
+							if ( in_array( $method, array( 'query', 'update', 'replace', 'delete' ) ) ) {
+								$cached = true;
+								break;
+							}
 
-		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 );
-		}
+						} elseif ( isset( WordPress_Sniff::$cacheGetFunctions[ $tokens[ $i ]['content'] ] ) ) {
 
-	}//end process()
+							$wp_cache_get = true;
 
-	/**
-	 * Add unique message per line
-	 * @param PHP_CodeSniffer_File $phpcsFile
-	 * @param string $type      (error|warning)
-	 * @param int    $pointer
-	 * @param int    $line
-	 * @param string $message
-     * @return void
-	 */
-	function add_unique_message( PHP_CodeSniffer_File $phpcsFile, $type, $pointer, $line, $message ) {
-		$messages = call_user_func( array( $phpcsFile, 'get' . ucfirst( $type . 's' ) ) );
-		if ( isset( $messages[$line] ) ) {
-			foreach ( $messages[$line] as $idx => $events ) {
-				foreach ( $events as $arr ) {
-					if ( $arr['message'] == $message ) {
-						return false;
+						} elseif ( isset( WordPress_Sniff::$cacheSetFunctions[ $tokens[ $i ]['content'] ] ) ) {
+
+							if ( $wp_cache_get ) {
+								$cached = true;
+								break;
+							}
+						}
 					}
 				}
 			}
 		}
 
-		call_user_func( array( $phpcsFile, 'add' . ucfirst( $type ) ), $message, $pointer );
-	}
+		if ( ! $cached && ! $whitelisted_cache ) {
+			$message = 'Usage of a direct database call without caching is prohibited. Use wp_cache_get / wp_cache_set or wp_cache_delete.';
+			$phpcsFile->addError( $message, $stackPtr, 'NoCaching' );
+		}
 
+		return $endOfStatement;
+
+	}//end process()
 
 }//end class

WordPress/Sniffs/Variables/VariableRestrictionsSniff.php

@@ -86,7 +86,7 @@ public function process( PHP_CodeSniffer_File $phpcsFile, $stackPtr )
 		$groups = $this->getGroups();
 
 		if ( empty( $groups ) ) {
-			return;
+			return count( $tokens ) + 1;
 		}
 
 		// Check if it is a function not a variable

WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.inc

@@ -61,4 +61,45 @@ function taz() {
 	/* @var wpdb $wpdb */
 	global $wpdb;
 	echo $wpdb->insert_id; // Good, no actual call, and doesn't need any caching
-}
+}
+
+// Some $wpdb methods can pass with only deleting the cache.
+function cache_delete_only() {
+	global $wpdb;
+
+	$data = $where = array();
+
+	// These methods are allowed to be used with just wp_cache_delete().
+	$wpdb->update( $wpdb->users, $data, $where ); // db call ok; OK
+	$wpdb->replace( $wpdb->users, $data, $where ); // db call ok; OK
+	$wpdb->delete( $wpdb->users, $data, $where ); // db call ok; OK
+	$wpdb->query( 'SELECT X FROM Y' ); // db call ok; OK
+
+	$wpdb->get_results( 'SELECT X FROM Y' ); // db call ok; Bad
+	$wpdb->get_row( 'SELECT X FROM Y' ); // db call ok; Bad
+	$wpdb->get_col( 'SELECT X FROM Y' ); // db call ok; Bad
+
+	wp_cache_delete( 'key', 'group' );
+}
+
+// It is OK to use the wp_cache_add() function in place of wp_cache_set().
+function cache_add_instead_of_set() {
+	global $wpdb;
+
+	$baz = wp_cache_get( 'baz' );
+
+	if ( false !== $baz ) {
+
+		$data = $where = array();
+
+		$wpdb->update( $wpdb->users, $data, $where ); // db call ok; OK
+		$wpdb->replace( $wpdb->users, $data, $where ); // db call ok; OK
+		$wpdb->delete( $wpdb->users, $data, $where ); // db call ok; OK
+		$wpdb->query( 'SELECT X FROM Y' ); // db call ok; OK
+		$wpdb->get_row( 'SELECT X FROM Y' ); // db call ok; OK
+		$wpdb->get_col( 'SELECT X FROM Y' ); // db call ok; OK
+		$baz = $wpdb->get_results( $wpdb->prepare( 'SELECT X FROM Y ' ) ); // db call ok; OK
+
+		wp_cache_add( 'baz', $baz );
+	}
+}

WordPress/Tests/VIP/DirectDatabaseQueryUnitTest.php

@@ -30,6 +30,9 @@ public function getErrorList()
 			32 => 1,
 			34 => 1,
 			50 => 1,
+			78 => 1,
+			79 => 1,
+			80 => 1,
 			);
 
 	}//end getErrorList()