From f0b0822c1c8b314c319c1d4b87aa05b9d9fdd275 Mon Sep 17 00:00:00 2001
From: Nestor Soriano <nestor.soriano@automattic.com>
Date: Thu, 16 Apr 2020 15:03:15 +0200
Subject: [PATCH] Fix code sniffer errors in CodeHacker and related classes.

---
 src/Testing/CodeHacking/CodeHacker.php        | 114 +++++++++++++-----
 .../CodeHacking/CodeHackerTestHook.php        |  17 ++-
 .../CodeHacking/Hacks/BypassFinalsHack.php    |   9 ++
 src/Testing/CodeHacking/Hacks/CodeHack.php    |  12 +-
 .../CodeHacking/Hacks/FunctionsMockerHack.php |  22 +++-
 .../CodeHacking/Hacks/StaticMockerHack.php    |  16 ++-
 6 files changed, 147 insertions(+), 43 deletions(-)

diff --git a/src/Testing/CodeHacking/CodeHacker.php b/src/Testing/CodeHacking/CodeHacker.php
index d2a10c2d4f9..24f31d04f23 100644
--- a/src/Testing/CodeHacking/CodeHacker.php
+++ b/src/Testing/CodeHacking/CodeHacker.php
@@ -1,7 +1,19 @@
 <?php
+/**
+ * CodeHacker class file.
+ *
+ * @package WooCommerce/Testing
+ */
+
+//phpcs:disable Squiz.Commenting.FunctionComment.Missing, Squiz.Commenting.VariableComment.Missing
+//phpcs:disable WordPress.WP.AlternativeFunctions, WordPress.PHP.NoSilencedErrors.Discouraged
 
 namespace Automattic\WooCommerce\Testing\CodeHacking;
 
+use \ReflectionObject;
+use \ReflectionFunction;
+use \ReflectionException;
+
 /**
  * CodeHacker - allows to hack (alter on the fly) the content of PHP code files.
  *
@@ -24,19 +36,19 @@ class CodeHacker {
 
 	const PROTOCOL = 'file';
 
-	/** @var resource|null */
 	public $context;
 
-	/** @var resource|null */
 	private $handle;
 
-	/** @var array|null */
-	private static $pathWhitelist = array();
+	private static $path_white_list = array();
 
 	private static $hacks = array();
 
 	private static $enabled = false;
 
+	/**
+	 * Enable the code hacker.
+	 */
 	public static function enable() {
 		if ( ! self::$enabled ) {
 			stream_wrapper_unregister( self::PROTOCOL );
@@ -45,6 +57,9 @@ class CodeHacker {
 		}
 	}
 
+	/**
+	 * Disable the code hacker.
+	 */
 	public static function restore() {
 		if ( self::$enabled ) {
 			stream_wrapper_restore( self::PROTOCOL );
@@ -52,26 +67,65 @@ class CodeHacker {
 		}
 	}
 
+	/**
+	 * Unregister all the registered hacks.
+	 */
 	public static function clear_hacks() {
 		self::$hacks = array();
 	}
 
+	/**
+	 * Check if the code hacker is enabled.
+	 *
+	 * @return bool True if the code hacker is enabled.
+	 */
 	public static function is_enabled() {
 		return self::$enabled;
 	}
 
+	/**
+	 * Register a new hack.
+	 *
+	 * @param mixed $hack A function with signature "hack($code, $path)" or an object containing a method with that signature.
+	 * @throws \Exception Invalid input.
+	 */
 	public static function add_hack( $hack ) {
 		if ( ! is_callable( $hack ) && ! is_object( $hack ) ) {
-			throw new Exception( "Hacks must be either functions, or objects having a 'process(\$text, \$path)' method." );
+			throw new \Exception( "Hacks must be either functions, or objects having a 'process(\$text, \$path)' method." );
 		}
 
-		// TODO: Check that callbacks have at least two parameters (if they have more, they must be optional); and that objects have a "process" method with the same condition.
+		if ( ! self::is_valid_hack_callback( $hack ) && ! self::is_valid_hack_object( $hack ) ) {
+			throw new \Exception( "CodeHacker::addhack: hacks must be either a function with a 'hack(\$code,\$path)' signature, or an object containing a public method 'hack' with that signature. " );
+		}
 
 		self::$hacks[] = $hack;
 	}
 
-	public static function setWhitelist( array $pathWhitelist ) {
-		self::$pathWhitelist = $pathWhitelist;
+	private static function is_valid_hack_callback( $callback ) {
+		return is_callable( $callback ) && 2 === ( new ReflectionFunction( $callback ) )->getNumberOfRequiredParameters();
+	}
+
+	private static function is_valid_hack_object( $callback ) {
+		if ( ! is_object( $callback ) ) {
+			return false;
+		}
+
+		$ro = new ReflectionObject( ( $callback ) );
+		try {
+			$rm = $ro->getMethod( 'hack' );
+			return $rm->isPublic() && ! $rm->isStatic() && 2 === $rm->getNumberOfRequiredParameters();
+		} catch ( ReflectionException $exception ) {
+			return false;
+		}
+	}
+
+	/**
+	 * Set the white list of files to hack. If note set, all the PHP files will be hacked.
+	 *
+	 * @param array $path_white_list Paths of the files to hack, can be relative paths.
+	 */
+	public static function set_white_list( array $path_white_list ) {
+		self::$path_white_list = $path_white_list;
 	}
 
 
@@ -89,7 +143,7 @@ class CodeHacker {
 
 
 	public function dir_readdir() {
-		 return readdir( $this->handle );
+		return readdir( $this->handle );
 	}
 
 
@@ -104,8 +158,8 @@ class CodeHacker {
 	}
 
 
-	public function rename( $pathFrom, $pathTo ) {
-		return $this->native( 'rename', $pathFrom, $pathTo, $this->context );
+	public function rename( $path_from, $path_to ) {
+		return $this->native( 'rename', $path_from, $path_to, $this->context );
 	}
 
 
@@ -114,13 +168,11 @@ class CodeHacker {
 	}
 
 
-	public function stream_cast( $castAs ) {
+	public function stream_cast( $cast_as ) {
 		return $this->handle;
 	}
 
-
 	public function stream_close() {
-		// echo "***** CLOSE HANDLE: " . $this->handle . " \n";
 		fclose( $this->handle );
 	}
 
@@ -159,11 +211,11 @@ class CodeHacker {
 	}
 
 
-	public function stream_open( $path, $mode, $options, &$openedPath ) {
-		$usePath = (bool) ( $options & STREAM_USE_PATH );
-		if ( $mode === 'rb' && self::pathInWhitelist( $path ) && pathinfo( $path, PATHINFO_EXTENSION ) === 'php' ) {
-			$content = $this->native( 'file_get_contents', $path, $usePath, $this->context );
-			if ( $content === false ) {
+	public function stream_open( $path, $mode, $options, &$opened_path ) {
+		$use_path = (bool) ( $options & STREAM_USE_PATH );
+		if ( 'rb' === $mode && self::path_in_white_list( $path ) && 'php' === pathinfo( $path, PATHINFO_EXTENSION ) ) {
+			$content = $this->native( 'file_get_contents', $path, $use_path, $this->context );
+			if ( false === $content ) {
 				return false;
 			}
 			$modified = self::hack( $content, $path );
@@ -175,8 +227,8 @@ class CodeHacker {
 			}
 		}
 		$this->handle = $this->context
-			? $this->native( 'fopen', $path, $mode, $usePath, $this->context )
-			: $this->native( 'fopen', $path, $mode, $usePath );
+			? $this->native( 'fopen', $path, $mode, $use_path, $this->context )
+			: $this->native( 'fopen', $path, $mode, $use_path );
 		return (bool) $this->handle;
 	}
 
@@ -196,17 +248,17 @@ class CodeHacker {
 
 
 	public function stream_stat() {
-		 return fstat( $this->handle );
+		return fstat( $this->handle );
 	}
 
 
 	public function stream_tell() {
-		 return ftell( $this->handle );
+		return ftell( $this->handle );
 	}
 
 
-	public function stream_truncate( $newSize ) {
-		return ftruncate( $this->handle, $newSize );
+	public function stream_truncate( $new_size ) {
+		return ftruncate( $this->handle, $new_size );
 	}
 
 
@@ -250,15 +302,19 @@ class CodeHacker {
 	}
 
 
-	private static function pathInWhitelist( $path ) {
-		if ( empty( self::$pathWhitelist ) ) {
+	private static function path_in_white_list( $path ) {
+		if ( empty( self::$path_white_list ) ) {
 			return true;
 		}
-		foreach ( self::$pathWhitelist as $whitelistItem ) {
-			if ( substr( $path, -strlen( $whitelistItem ) ) === $whitelistItem ) {
+		foreach ( self::$path_white_list as $white_list_item ) {
+			if ( substr( $path, -strlen( $white_list_item ) ) === $white_list_item ) {
 				return true;
 			}
 		}
 		return false;
 	}
 }
+
+//phpcs:enable Squiz.Commenting.FunctionComment.Missing, Squiz.Commenting.VariableComment.Missing
+//phpcs:enable WordPress.WP.AlternativeFunctions, WordPress.PHP.NoSilencedErrors.Discouraged
+
diff --git a/src/Testing/CodeHacking/CodeHackerTestHook.php b/src/Testing/CodeHacking/CodeHackerTestHook.php
index 1f380ce7844..6ff74d99a23 100644
--- a/src/Testing/CodeHacking/CodeHackerTestHook.php
+++ b/src/Testing/CodeHacking/CodeHackerTestHook.php
@@ -1,4 +1,11 @@
 <?php
+/**
+ * CodeHackerTestHook class file.
+ *
+ * @package WooCommerce/Testing
+ */
+
+// phpcs:disable Squiz.Commenting.FunctionComment.Missing, PHPCompatibility.FunctionDeclarations
 
 namespace Automattic\WooCommerce\Testing\CodeHacking;
 
@@ -91,14 +98,14 @@ final class CodeHackerTestHook implements BeforeTestHook, AfterTestHook {
 	 *
 	 * @param object $reflection_object The class or method reflection object whose doc comment will be parsed.
 	 * @return bool True if at least one valid @hack annotation was found.
-	 * @throws Exception
+	 * @throws Exception Class specified in @hack directive doesn't exist.
 	 */
 	private function add_hacks_from_annotations( $reflection_object ) {
 		$annotations = Test::parseAnnotations( $reflection_object->getDocComment() );
 		$hacks_added = false;
 
 		foreach ( $annotations as $id => $annotation_instances ) {
-			if ( $id !== 'hack' ) {
+			if ( 'hack' !== $id ) {
 				continue;
 			}
 
@@ -107,7 +114,7 @@ final class CodeHackerTestHook implements BeforeTestHook, AfterTestHook {
 				$params = $matches[0];
 
 				$hack_class = array_shift( $params );
-				if(false === strpos( $hack_class, '\\' )) {
+				if ( false === strpos( $hack_class, '\\' ) ) {
 					$hack_class = __NAMESPACE__ . '\\Hacks\\' . $hack_class;
 				}
 
@@ -132,7 +139,7 @@ final class CodeHackerTestHook implements BeforeTestHook, AfterTestHook {
 	 *
 	 * @param string $class_name Test class name.
 	 * @param string $method_name Test method name.
-	 * @throws ReflectionException
+	 * @throws ReflectionException Error when instatiating a ReflectionClass.
 	 */
 	private function execute_before_methods( $class_name, $method_name ) {
 		$methods = array( 'before_all', "before_{$method_name}" );
@@ -154,3 +161,5 @@ final class CodeHackerTestHook implements BeforeTestHook, AfterTestHook {
 		}
 	}
 }
+
+// phpcs:enable Squiz.Commenting.FunctionComment.Missing, PHPCompatibility.FunctionDeclarations
diff --git a/src/Testing/CodeHacking/Hacks/BypassFinalsHack.php b/src/Testing/CodeHacking/Hacks/BypassFinalsHack.php
index e78c22bd511..a9d8e97bc6b 100644
--- a/src/Testing/CodeHacking/Hacks/BypassFinalsHack.php
+++ b/src/Testing/CodeHacking/Hacks/BypassFinalsHack.php
@@ -1,4 +1,11 @@
 <?php
+/**
+ * BypassFinalsHack class file.
+ *
+ * @package WooCommerce/Testing
+ */
+
+// phpcs:disable Squiz.Commenting.FunctionComment.Missing
 
 namespace Automattic\WooCommerce\Testing\CodeHacking\Hacks;
 
@@ -21,3 +28,5 @@ final class BypassFinalsHack extends CodeHack {
 		return $code;
 	}
 }
+
+// phpcs:enable Squiz.Commenting.FunctionComment.Missing
diff --git a/src/Testing/CodeHacking/Hacks/CodeHack.php b/src/Testing/CodeHacking/Hacks/CodeHack.php
index 9187d929093..47e11f2a058 100644
--- a/src/Testing/CodeHacking/Hacks/CodeHack.php
+++ b/src/Testing/CodeHacking/Hacks/CodeHack.php
@@ -1,4 +1,9 @@
 <?php
+/**
+ * CodeHack class file.
+ *
+ * @package WooCommerce/Testing
+ */
 
 namespace Automattic\WooCommerce\Testing\CodeHacking\Hacks;
 
@@ -26,6 +31,7 @@ abstract class CodeHack {
 	 * @return array Tokenized code.
 	 */
 	protected function tokenize( $code ) {
+		//phpcs:ignore PHPCompatibility.FunctionUse.NewFunctionParameters, PHPCompatibility.Constants.NewConstants
 		return PHP_VERSION_ID >= 70000 ? token_get_all( $code, TOKEN_PARSE ) : token_get_all( $code );
 	}
 
@@ -33,7 +39,7 @@ abstract class CodeHack {
 	 * Check if a token is of a given type.
 	 *
 	 * @param mixed $token Token to check.
-	 * @param int   $type Type of token to check (see https://www.php.net/manual/en/tokens.php)
+	 * @param int   $type Type of token to check (see https://www.php.net/manual/en/tokens.php).
 	 * @return bool True if it's a token of the given type, false otherwise.
 	 */
 	protected function is_token_of_type( $token, $type ) {
@@ -53,7 +59,7 @@ abstract class CodeHack {
 	/**
 	 * Converts a token to its string representation.
 	 *
-	 * @param $token Token to convert.
+	 * @param mixed $token Token to convert.
 	 * @return mixed String representation of the token.
 	 */
 	protected function token_to_string( $token ) {
@@ -70,7 +76,7 @@ abstract class CodeHack {
 	 */
 	protected function string_ends_with( $haystack, $needle ) {
 		$length = strlen( $needle );
-		if ( $length == 0 ) {
+		if ( 0 === $length ) {
 			return true;
 		}
 
diff --git a/src/Testing/CodeHacking/Hacks/FunctionsMockerHack.php b/src/Testing/CodeHacking/Hacks/FunctionsMockerHack.php
index 0e8b77c86b3..a6703928bbe 100644
--- a/src/Testing/CodeHacking/Hacks/FunctionsMockerHack.php
+++ b/src/Testing/CodeHacking/Hacks/FunctionsMockerHack.php
@@ -1,4 +1,11 @@
 <?php
+/**
+ * FunctionsMockerHack class file.
+ *
+ * @package WooCommerce/Testing
+ */
+
+// phpcs:disable Squiz.Commenting.FunctionComment.Missing
 
 namespace Automattic\WooCommerce\Testing\CodeHacking\Hacks;
 
@@ -17,6 +24,11 @@ use ReflectionClass;
  */
 final class FunctionsMockerHack extends CodeHack {
 
+	/**
+	 * Tokens that precede a non-standalone-function identifier.
+	 *
+	 * @var array
+	 */
 	private static $non_global_function_tokens = array(
 		T_PAAMAYIM_NEKUDOTAYIM,
 		T_DOUBLE_COLON,
@@ -28,9 +40,9 @@ final class FunctionsMockerHack extends CodeHack {
 	 * FunctionsMockerHack constructor.
 	 *
 	 * @param string $mock_class Name of the class containing function mocks as public static methods.
-	 * @throws ReflectionException
+	 * @throws ReflectionException Error when instantiating ReflectionClass.
 	 */
-	public function __construct( string $mock_class ) {
+	public function __construct( $mock_class ) {
 		$this->mock_class = $mock_class;
 
 		$rc = new ReflectionClass( $mock_class );
@@ -53,14 +65,16 @@ final class FunctionsMockerHack extends CodeHack {
 			$token_type = $this->token_type_of( $token );
 			if ( T_WHITESPACE === $token_type ) {
 				$code .= $this->token_to_string( $token );
-			} elseif ( T_STRING === $token_type && ! $previous_token_is_non_global_function_qualifier && in_array( $token[1], $this->mocked_methods ) ) {
+			} elseif ( T_STRING === $token_type && ! $previous_token_is_non_global_function_qualifier && in_array( $token[1], $this->mocked_methods, true ) ) {
 				$code .= "{$this->mock_class}::{$token[1]}";
 			} else {
 				$code .= $this->token_to_string( $token );
-				$previous_token_is_non_global_function_qualifier = in_array( $token_type, self::$non_global_function_tokens );
+				$previous_token_is_non_global_function_qualifier = in_array( $token_type, self::$non_global_function_tokens, true );
 			}
 		}
 
 		return $code;
 	}
 }
+
+// phpcs:enable Squiz.Commenting.FunctionComment.Missing
diff --git a/src/Testing/CodeHacking/Hacks/StaticMockerHack.php b/src/Testing/CodeHacking/Hacks/StaticMockerHack.php
index d334a2fc8c3..3c066808a7c 100644
--- a/src/Testing/CodeHacking/Hacks/StaticMockerHack.php
+++ b/src/Testing/CodeHacking/Hacks/StaticMockerHack.php
@@ -1,4 +1,11 @@
 <?php
+/**
+ * StaticMockerHack class file.
+ *
+ * @package WooCommerce/Testing
+ */
+
+// phpcs:disable Squiz.Commenting.FunctionComment.Missing
 
 namespace Automattic\WooCommerce\Testing\CodeHacking\Hacks;
 
@@ -26,9 +33,9 @@ final class StaticMockerHack extends CodeHack {
 	 *
 	 * @param string $source_class Name of the original class (the one having the members to be mocked).
 	 * @param string $mock_class Name of the mock class (the one having the replacement mock members).
-	 * @throws ReflectionException
+	 * @throws ReflectionException Error when instantiating ReflectionClass.
 	 */
-	public function __construct( string $source_class, string $mock_class ) {
+	public function __construct( $source_class, $mock_class ) {
 		$this->source_class = $source_class;
 		$this->target_class = $mock_class;
 
@@ -61,12 +68,13 @@ final class StaticMockerHack extends CodeHack {
 			$code          = '';
 			$current_token = null;
 
+			// phpcs:ignore WordPress.CodeAnalysis.AssignmentInCondition.FoundInWhileCondition
 			while ( $current_token = current( $tokens ) ) {
 				if ( $this->is_token_of_type( $current_token, T_STRING ) && $this->source_class === $current_token[1] ) {
 					$next_token = next( $tokens );
 					if ( $this->is_token_of_type( $next_token, T_DOUBLE_COLON ) ) {
 						$called_member = next( $tokens )[1];
-						if ( in_array( $called_member, $this->members_implemented_in_mock ) ) {
+						if ( in_array( $called_member, $this->members_implemented_in_mock, true ) ) {
 							// Reference to source class member that exists in mock class, replace.
 							$code .= "{$this->target_class}::{$called_member}";
 						} else {
@@ -88,3 +96,5 @@ final class StaticMockerHack extends CodeHack {
 		return $code;
 	}
 }
+
+// phpcs:enable Squiz.Commenting.FunctionComment.Missing