connection: Fix Phan issues and improve phpdocs in Client class (#37056)

* Declare the shape of returned response arrays for better static analysis. * Fix a bunch of other phpdocs to be more correct too. * Remove some unnecessary boolean checks, that were confusing Phan into marking all the keys in arrays as being possibly-missing (cf. phan/phan#4846) * At least they're unnecessary based on the declared return values of stuff and a quick look through the code. If we really need them, we should be more specific about what we're checking for and we should always return the documented WP_Error instead of passing the weird-falsey-value along. The most important fix here is getting the type of the `$body` parameter to `wpcom_json_api_request_as_blog` right, which fixes PhanTypeMismatchArgumentProbablyReal issues in a lot of callers. Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/8821834019 Upstream-Ref: Automattic/jetpack@01d6d6d
Automattic · Apr 24, 2024 · 7cee60a · 7cee60a
1 parent bbd48d5
commit 7cee60a
Show file tree

Hide file tree

Showing 8 changed files with 101 additions and 89 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,3 +21,6 @@ This is an alpha version! The changes listed here are not final.
 - General: update WordPress version requirements to WordPress 6.3.
 - General: update WordPress version requirements to WordPress 6.4.
 - Updated package dependencies.
+
+### Fixed
+- Remove unnecessary boolean check that was confusing Phan.
diff --git a/functions.php b/functions.php
@@ -33,7 +33,7 @@ function jetpack_inspect_connection_request( $url, $args = array() ) {
 
 	$signature = Client::build_signed_request( $args, $body );
 
-	if ( ! $signature || is_wp_error( $signature ) ) {
+	if ( is_wp_error( $signature ) ) {
 		return $signature;
 	}
 

diff --git a/jetpack_vendor/automattic/jetpack-connection/CHANGELOG.md b/jetpack_vendor/automattic/jetpack-connection/CHANGELOG.md
@@ -14,6 +14,7 @@ This is an alpha version! The changes listed here are not final.
 
 ### Fixed
 - Disconnect connection owner on removal.
+- Improve phpdoc comments in Client class, and remove some unnecessary boolean checks.
 
 ## [2.7.2] - 2024-04-22
 ### Added

diff --git a/jetpack_vendor/automattic/jetpack-connection/src/class-client.php b/jetpack_vendor/automattic/jetpack-connection/src/class-client.php
@@ -10,6 +10,9 @@
 use Automattic\Jetpack\Constants;
 use WP_Error;
 
+// `wp_remote_request` returns an array with a particular format.
+'@phan-type _WP_Remote_Response_Array = array{headers:\WpOrg\Requests\Utility\CaseInsensitiveDictionary,body:string,response:array{code:int,message:string},cookies:\WP_HTTP_Cookie[],filename:?string,http_response:WP_HTTP_Requests_Response}';
+
 /**
  * The Client class that is used to connect to WordPress.com Jetpack API.
  */
@@ -19,9 +22,10 @@ class Client {
 	/**
 	 * Makes an authorized remote request using Jetpack_Signature
 	 *
-	 * @param array        $args the arguments for the remote request.
-	 * @param array|String $body the request body.
+	 * @param array             $args the arguments for the remote request.
+	 * @param array|string|null $body the request body.
 	 * @return array|WP_Error WP HTTP response on success
+	 * @phan-return _WP_Remote_Response_Array|WP_Error
 	 */
 	public static function remote_request( $args, $body = null ) {
 		if ( isset( $args['url'] ) ) {
@@ -36,7 +40,7 @@ public static function remote_request( $args, $body = null ) {
 		}
 
 		$result = self::build_signed_request( $args, $body );
-		if ( ! $result || is_wp_error( $result ) ) {
+		if ( is_wp_error( $result ) ) {
 			return $result;
 		}
 
@@ -65,12 +69,12 @@ public static function remote_request( $args, $body = null ) {
 	/**
 	 * Adds authorization signature to a remote request using Jetpack_Signature
 	 *
-	 * @param array        $args the arguments for the remote request.
-	 * @param array|String $body the request body.
-	 * @return WP_Error|array {
+	 * @param array             $args the arguments for the remote request.
+	 * @param array|string|null $body the request body.
+	 * @return WP_Error|array{url:string,request:array,auth:array} {
 	 *     An array containing URL and request items.
 	 *
-	 *     @type String $url     The request URL.
+	 *     @type string $url     The request URL.
 	 *     @type array  $request Request arguments.
 	 *     @type array  $auth    Authorization data.
 	 * }
@@ -123,7 +127,7 @@ public static function build_signed_request( $args, $body = null ) {
 		$request = compact( 'method', 'body', 'timeout', 'redirection', 'stream', 'filename', 'sslverify' );
 
 		@list( $token_key, $secret ) = explode( '.', $token->secret ); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged
-		if ( empty( $token ) || empty( $secret ) ) {
+		if ( ! $secret ) {
 			return new WP_Error( 'malformed_token' );
 		}
 
@@ -142,7 +146,7 @@ public static function build_signed_request( $args, $body = null ) {
 		if ( function_exists( 'wp_generate_password' ) ) {
 			$nonce = wp_generate_password( 10, false );
 		} else {
-			$nonce = substr( sha1( wp_rand( 0, 1000000 ) ), 0, 10 );
+			$nonce = substr( sha1( (string) wp_rand( 0, 1000000 ) ), 0, 10 );
 		}
 
 		// Kind of annoying.  Maybe refactor Jetpack_Signature to handle body-hashing.
@@ -196,7 +200,7 @@ public static function build_signed_request( $args, $body = null ) {
 
 		$signature = $jetpack_signature->sign_request( $token_key, $timestamp, $nonce, $body_hash, $method, $url, $body, false );
 
-		if ( ! $signature || is_wp_error( $signature ) ) {
+		if ( is_wp_error( $signature ) ) {
 			return $signature;
 		}
 
@@ -233,10 +237,11 @@ public static function build_signed_request( $args, $body = null ) {
 	 *
 	 * @internal
 	 *
-	 * @param String  $url the request URL.
+	 * @param string  $url the request URL.
 	 * @param array   $args request arguments.
-	 * @param Boolean $set_fallback whether to allow flagging this request to use a fallback certficate override.
+	 * @param boolean $set_fallback whether to allow flagging this request to use a fallback certficate override.
 	 * @return array|WP_Error WP HTTP response on success
+	 * @phan-return _WP_Remote_Response_Array|WP_Error
 	 */
 	public static function _wp_remote_request( $url, $args, $set_fallback = false ) { // phpcs:ignore PSR2.Methods.MethodDeclaration.Underscore
 		$fallback = \Jetpack_Options::get_option( 'fallback_no_verify_ssl_certs' );
@@ -316,8 +321,9 @@ public static function _wp_remote_request( $url, $args, $set_fallback = false )
 	/**
 	 * Sets the time difference for correct signature computation.
 	 *
-	 * @param HTTP_Response $response the response object.
-	 * @param Boolean       $force_set whether to force setting the time difference.
+	 * @param array $response Response array from `wp_remote_request`.
+	 * @param bool  $force_set whether to force setting the time difference.
+	 * @phan-param _WP_Remote_Response_Array $response
 	 */
 	public static function set_time_diff( &$response, $force_set = false ) {
 		$code = wp_remote_retrieve_response_code( $response );
@@ -357,7 +363,7 @@ public static function set_time_diff( &$response, $force_set = false ) {
 	 * @param  array  $args             Arguments to {@see WP_Http}. Default is `array()`.
 	 * @param  string $base_api_path    REST API root. Default is `wpcom`.
 	 *
-	 * @return array|WP_Error $response Response data, else {@see WP_Error} on failure.
+	 * @return array Validated arguments.
 	 */
 	public static function validate_args_for_wpcom_json_api_request(
 		$path,
@@ -415,6 +421,7 @@ public static function validate_args_for_wpcom_json_api_request(
 	 * @param string            $base_api_path    REST API root. Default is `wpcom`.
 	 *
 	 * @return array|WP_Error $response Response data, else {@see WP_Error} on failure.
+	 * @phan-return _WP_Remote_Response_Array|WP_Error
 	 */
 	public static function wpcom_json_api_request_as_user(
 		$path,
@@ -440,12 +447,13 @@ public static function wpcom_json_api_request_as_user(
 	/**
 	 * Query the WordPress.com REST API using the blog token
 	 *
-	 * @param String $path The API endpoint relative path.
-	 * @param String $version The API version.
-	 * @param array  $args Request arguments.
-	 * @param String $body Request body.
-	 * @param String $base_api_path (optional) the API base path override, defaults to 'rest'.
+	 * @param string            $path The API endpoint relative path.
+	 * @param string            $version The API version.
+	 * @param array             $args Request arguments.
+	 * @param array|string|null $body Request body.
+	 * @param string            $base_api_path (optional) the API base path override, defaults to 'rest'.
 	 * @return array|WP_Error $response Data.
+	 * @phan-return _WP_Remote_Response_Array|WP_Error
 	 */
 	public static function wpcom_json_api_request_as_blog(
 		$path,
@@ -472,7 +480,7 @@ public static function wpcom_json_api_request_as_blog(
 	 * make sure that body hashes are made ith the string version, which is what will be seen after a
 	 * server pulls up the data in the $_POST array.
 	 *
-	 * @param array|Mixed $data the data that needs to be stringified.
+	 * @param mixed $data the data that needs to be stringified.
 	 *
 	 * @return array|string
 	 */

diff --git a/jetpack_vendor/i18n-map.php b/jetpack_vendor/i18n-map.php
@@ -18,7 +18,7 @@
     ),
     'jetpack-connection' => array(
       'path' => 'jetpack_vendor/automattic/jetpack-connection',
-      'ver' => '2.7.3-alpha1713964570',
+      'ver' => '2.7.3-alpha1713985059',
     ),
   ),
 );
diff --git a/vendor/composer/installed.json b/vendor/composer/installed.json
@@ -7,7 +7,7 @@
             "dist": {
                 "type": "path",
                 "url": "/tmp/jetpack-build/Automattic/jetpack-a8c-mc-stats",
-                "reference": "6c356bd1c4b34d1977e3b52cc1cb063515f4f403"
+                "reference": "84eb94b932d484ee8686f0e3c2299f18262b6b44"
             },
             "require": {
                 "php": ">=7.0"
@@ -60,7 +60,7 @@
             "dist": {
                 "type": "path",
                 "url": "/tmp/jetpack-build/Automattic/jetpack-admin-ui",
-                "reference": "df17caa07981bc41ecb23636f483b37360286eea"
+                "reference": "fa2c9ed0851240e71957d43575487d2375713c49"
             },
             "require": {
                 "php": ">=7.0"
@@ -125,7 +125,7 @@
             "dist": {
                 "type": "path",
                 "url": "/tmp/jetpack-build/Automattic/jetpack-assets",
-                "reference": "76aed17c74f2cd2827a0e112e848923789b36a31"
+                "reference": "1b3cbeb9095c4b0ec521654d157d8087699be055"
             },
             "require": {
                 "automattic/jetpack-constants": "^2.0.1",
@@ -194,7 +194,7 @@
             "dist": {
                 "type": "path",
                 "url": "/tmp/jetpack-build/Automattic/jetpack-autoloader",
-                "reference": "8d245fc0a5216ef7678d65759a10a071f2990b72"
+                "reference": "df2fd93c5babcb5d6c6febb390764fc30152af77"
             },
             "require": {
                 "composer-plugin-api": "^1.1 || ^2.0",
@@ -261,7 +261,7 @@
             "dist": {
                 "type": "path",
                 "url": "/tmp/jetpack-build/Automattic/jetpack-composer-plugin",
-                "reference": "1b6ed0a1fa25816411cd83f5ffe29446b586c4f8"
+                "reference": "a1fc5e6ce32e0e5d052959c20c0a88c50b57ec6e"
             },
             "require": {
                 "composer-plugin-api": "^2.1.0",
@@ -321,7 +321,7 @@
             "dist": {
                 "type": "path",
                 "url": "/tmp/jetpack-build/Automattic/jetpack-config",
-                "reference": "918bf6dd372899d7c13b8e53da835b77d8ae3082"
+                "reference": "2889aff032d20ad447d2f1bf6ebbe2b86f9ac1ce"
             },
             "require": {
                 "php": ">=7.0"
@@ -361,12 +361,12 @@
         },
         {
             "name": "automattic/jetpack-connection",
-            "version": "2.7.3-alpha.1713964570",
-            "version_normalized": "2.7.3.0-alpha1713964570",
+            "version": "2.7.3-alpha.1713985059",
+            "version_normalized": "2.7.3.0-alpha1713985059",
             "dist": {
                 "type": "path",
                 "url": "/tmp/jetpack-build/Automattic/jetpack-connection",
-                "reference": "6faf9a44a8f58a943643ce72cc2b2a7b3339d15a"
+                "reference": "3ba7cf3a228a7128cdb0ea825d07f6bd853b8e69"
             },
             "require": {
                 "automattic/jetpack-a8c-mc-stats": "^2.0.1",
@@ -446,7 +446,7 @@
             "dist": {
                 "type": "path",
                 "url": "/tmp/jetpack-build/Automattic/jetpack-constants",
-                "reference": "2dcbd1ec882a57aac4f2ed3a4c15eb7fcd3eb461"
+                "reference": "c2a041ed88e07fc751583e3618cca057eb31d7cd"
             },
             "require": {
                 "php": ">=7.0"
@@ -500,7 +500,7 @@
             "dist": {
                 "type": "path",
                 "url": "/tmp/jetpack-build/Automattic/jetpack-redirect",
-                "reference": "e2f122e300ca8452a0c97cb7d9b74495d2eacd30"
+                "reference": "2dced0eca4011ee57450615a6df719d5ef236bb1"
             },
             "require": {
                 "automattic/jetpack-status": "^3.0.0-alpha",
@@ -555,7 +555,7 @@
             "dist": {
                 "type": "path",
                 "url": "/tmp/jetpack-build/Automattic/jetpack-roles",
-                "reference": "4a8b5459c3c5555c5c15b3c77c2948c0d49af217"
+                "reference": "fd171453020fe398b319605892fce08f8190d011"
             },
             "require": {
                 "php": ">=7.0"
@@ -609,7 +609,7 @@
             "dist": {
                 "type": "path",
                 "url": "/tmp/jetpack-build/Automattic/jetpack-status",
-                "reference": "cf319d0f870ec6ccda88aea3b8e2a7e7a2249f69"
+                "reference": "c00c235d1748a99c08a38b5b53143ac032924291"
             },
             "require": {
                 "automattic/jetpack-constants": "^2.0.1",

diff --git a/vendor/composer/installed.php b/vendor/composer/installed.php
@@ -13,7 +13,7 @@
         'automattic/jetpack-a8c-mc-stats' => array(
             'pretty_version' => '2.0.1',
             'version' => '2.0.1.0',
-            'reference' => '6c356bd1c4b34d1977e3b52cc1cb063515f4f403',
+            'reference' => '84eb94b932d484ee8686f0e3c2299f18262b6b44',
             'type' => 'jetpack-library',
             'install_path' => __DIR__ . '/../../jetpack_vendor/automattic/jetpack-a8c-mc-stats',
             'aliases' => array(),
@@ -22,7 +22,7 @@
         'automattic/jetpack-admin-ui' => array(
             'pretty_version' => '0.4.2',
             'version' => '0.4.2.0',
-            'reference' => 'df17caa07981bc41ecb23636f483b37360286eea',
+            'reference' => 'fa2c9ed0851240e71957d43575487d2375713c49',
             'type' => 'jetpack-library',
             'install_path' => __DIR__ . '/../../jetpack_vendor/automattic/jetpack-admin-ui',
             'aliases' => array(),
@@ -31,7 +31,7 @@
         'automattic/jetpack-assets' => array(
             'pretty_version' => '2.1.8',
             'version' => '2.1.8.0',
-            'reference' => '76aed17c74f2cd2827a0e112e848923789b36a31',
+            'reference' => '1b3cbeb9095c4b0ec521654d157d8087699be055',
             'type' => 'jetpack-library',
             'install_path' => __DIR__ . '/../../jetpack_vendor/automattic/jetpack-assets',
             'aliases' => array(),
@@ -40,7 +40,7 @@
         'automattic/jetpack-autoloader' => array(
             'pretty_version' => '3.0.6',
             'version' => '3.0.6.0',
-            'reference' => '8d245fc0a5216ef7678d65759a10a071f2990b72',
+            'reference' => 'df2fd93c5babcb5d6c6febb390764fc30152af77',
             'type' => 'composer-plugin',
             'install_path' => __DIR__ . '/../automattic/jetpack-autoloader',
             'aliases' => array(),
@@ -49,7 +49,7 @@
         'automattic/jetpack-composer-plugin' => array(
             'pretty_version' => '2.0.1',
             'version' => '2.0.1.0',
-            'reference' => '1b6ed0a1fa25816411cd83f5ffe29446b586c4f8',
+            'reference' => 'a1fc5e6ce32e0e5d052959c20c0a88c50b57ec6e',
             'type' => 'composer-plugin',
             'install_path' => __DIR__ . '/../automattic/jetpack-composer-plugin',
             'aliases' => array(),
@@ -58,16 +58,16 @@
         'automattic/jetpack-config' => array(
             'pretty_version' => '2.0.1',
             'version' => '2.0.1.0',
-            'reference' => '918bf6dd372899d7c13b8e53da835b77d8ae3082',
+            'reference' => '2889aff032d20ad447d2f1bf6ebbe2b86f9ac1ce',
             'type' => 'jetpack-library',
             'install_path' => __DIR__ . '/../../jetpack_vendor/automattic/jetpack-config',
             'aliases' => array(),
             'dev_requirement' => false,
         ),
         'automattic/jetpack-connection' => array(
-            'pretty_version' => '2.7.3-alpha.1713964570',
-            'version' => '2.7.3.0-alpha1713964570',
-            'reference' => '6faf9a44a8f58a943643ce72cc2b2a7b3339d15a',
+            'pretty_version' => '2.7.3-alpha.1713985059',
+            'version' => '2.7.3.0-alpha1713985059',
+            'reference' => '3ba7cf3a228a7128cdb0ea825d07f6bd853b8e69',
             'type' => 'jetpack-library',
             'install_path' => __DIR__ . '/../../jetpack_vendor/automattic/jetpack-connection',
             'aliases' => array(),
@@ -76,7 +76,7 @@
         'automattic/jetpack-constants' => array(
             'pretty_version' => '2.0.1',
             'version' => '2.0.1.0',
-            'reference' => '2dcbd1ec882a57aac4f2ed3a4c15eb7fcd3eb461',
+            'reference' => 'c2a041ed88e07fc751583e3618cca057eb31d7cd',
             'type' => 'jetpack-library',
             'install_path' => __DIR__ . '/../../jetpack_vendor/automattic/jetpack-constants',
             'aliases' => array(),
@@ -94,7 +94,7 @@
         'automattic/jetpack-redirect' => array(
             'pretty_version' => '2.0.1',
             'version' => '2.0.1.0',
-            'reference' => 'e2f122e300ca8452a0c97cb7d9b74495d2eacd30',
+            'reference' => '2dced0eca4011ee57450615a6df719d5ef236bb1',
             'type' => 'jetpack-library',
             'install_path' => __DIR__ . '/../../jetpack_vendor/automattic/jetpack-redirect',
             'aliases' => array(),
@@ -103,7 +103,7 @@
         'automattic/jetpack-roles' => array(
             'pretty_version' => '2.0.2',
             'version' => '2.0.2.0',
-            'reference' => '4a8b5459c3c5555c5c15b3c77c2948c0d49af217',
+            'reference' => 'fd171453020fe398b319605892fce08f8190d011',
             'type' => 'jetpack-library',
             'install_path' => __DIR__ . '/../../jetpack_vendor/automattic/jetpack-roles',
             'aliases' => array(),
@@ -112,7 +112,7 @@
         'automattic/jetpack-status' => array(
             'pretty_version' => '3.0.0-alpha.1713976135',
             'version' => '3.0.0.0-alpha1713976135',
-            'reference' => 'cf319d0f870ec6ccda88aea3b8e2a7e7a2249f69',
+            'reference' => 'c00c235d1748a99c08a38b5b53143ac032924291',
             'type' => 'jetpack-library',
             'install_path' => __DIR__ . '/../../jetpack_vendor/automattic/jetpack-status',
             'aliases' => array(),