Skip to content

Commit

Permalink
Phan: Fix errant extra params in function calls (#41263)
Browse files Browse the repository at this point in the history 
* Remove extra param on remove_filter/remove_action

There's no arg count when removing.

* Remove extra param from rest_ensure_response

That function doesn't allow a status code to be passed. Presumably we could override the WP_REST_Response status and force 200, but passing it through has worked so far, so keeping it that way.

* Remove other stray args

In most cases, these appear to be due to refactors.

* Another stray arg

* Suppress things that confuse Phan

* Fix get_avatar_url calls

* arrayHasKey → assertArrayHasKey

* Fix misplaced parentheses

* Tighten types

We're using `WP_Option`, which allows an extra param in its `get()`.

* Add missing sprintf

* Add unused param to allow dynamic method call

* Add changelogs

* Use same behaviour as trunk

* Fix assertion

* Add param to Storage::get and use that type as before

* Inform Phan of type rather than suppressing error

* Update errant types

* Update Phan baselines

* More changelogs

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/13079055733

Upstream-Ref: Automattic/jetpack@4b2a4a9
  • Loading branch information
@tbradsha @matticbot
tbradsha authored and matticbot committed Jan 31, 2025
1 parent 2010eea commit ddc023b
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 46 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ This is an alpha version! The changes listed here are not final.
### Changed
- Updated package dependencies.

### Fixed
- Code: Remove extra params on function calls.

## [6.3.1] - 2025-01-27
### Changed
- Internal updates.
Expand Down
2 changes: 1 addition & 1 deletion composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"automattic/jetpack-assets": "^4.0.4-alpha",
"automattic/jetpack-constants": "^3.0.1",
"automattic/jetpack-roles": "^3.0.1",
"automattic/jetpack-status": "^5.0.2",
"automattic/jetpack-status": "^5.0.3-alpha",
"automattic/jetpack-redirect": "^3.0.1"
},
"require-dev": {
Expand Down
2 changes: 1 addition & 1 deletion legacy/class-jetpack-tracks-client.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public static function record_event( $event ) {
return $event;
}

$pixel = $event->build_pixel_url( $event );
$pixel = $event->build_pixel_url();

if ( ! $pixel ) {
return new WP_Error( 'invalid_pixel', 'cannot generate tracks pixel for given input', 400 );
Expand Down
2 changes: 1 addition & 1 deletion src/class-rest-connector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ public static function get_user_connection_data( $rest_response = true ) {
'id' => $current_user->ID,
'blogId' => $blog_id,
'wpcomUser' => $wpcom_user_data,
'gravatar' => get_avatar_url( $current_user->ID, 64, 'mm', '', array( 'force_display' => true ) ),
'gravatar' => get_avatar_url( $current_user->ID ),
'permissions' => array(
'connect' => current_user_can( 'jetpack_connect' ),
'connect_user' => current_user_can( 'jetpack_connect_user' ),
Expand Down
80 changes: 40 additions & 40 deletions tests/php/test_Error_Handler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,20 +72,20 @@ public function test_store_error() {

$this->assertCount( 1, $stored_errors );

$this->arrayHasKey( 'invalid_token', $stored_errors );
$this->assertArrayHasKey( 'invalid_token', $stored_errors );

$this->assertCount( 1, $stored_errors['invalid_token'] );

$this->arrayHasKey( '1', $stored_errors['invalid_token'] );
$this->assertArrayHasKey( '1', $stored_errors['invalid_token'] );

$this->arrayHasKey( 'nonce', $stored_errors['invalid_token']['1'] );
$this->arrayHasKey( 'error_code', $stored_errors['invalid_token']['1'] );
$this->arrayHasKey( 'user_id', $stored_errors['invalid_token']['1'] );
$this->arrayHasKey( 'error_message', $stored_errors['invalid_token']['1'] );
$this->arrayHasKey( 'error_data', $stored_errors['invalid_token']['1'] );
$this->arrayHasKey( 'timestamp', $stored_errors['invalid_token']['1'] );
$this->arrayHasKey( 'nonce', $stored_errors['invalid_token']['1'] );
$this->arrayHasKey( 'error_type', $stored_errors['invalid_token']['1'] );
$this->assertArrayHasKey( 'nonce', $stored_errors['invalid_token']['1'] );
$this->assertArrayHasKey( 'error_code', $stored_errors['invalid_token']['1'] );
$this->assertArrayHasKey( 'user_id', $stored_errors['invalid_token']['1'] );
$this->assertArrayHasKey( 'error_message', $stored_errors['invalid_token']['1'] );
$this->assertArrayHasKey( 'error_data', $stored_errors['invalid_token']['1'] );
$this->assertArrayHasKey( 'timestamp', $stored_errors['invalid_token']['1'] );
$this->assertArrayHasKey( 'nonce', $stored_errors['invalid_token']['1'] );
$this->assertArrayHasKey( 'error_type', $stored_errors['invalid_token']['1'] );
$this->assertEquals( 'xmlrpc', $stored_errors['invalid_token']['1']['error_type'] );
}

Expand All @@ -108,29 +108,29 @@ public function test_store_multiple_error_codes() {

$this->assertCount( 3, $stored_errors );

$this->arrayHasKey( 'invalid_token', $stored_errors );
$this->assertArrayHasKey( 'invalid_token', $stored_errors );

$this->assertCount( 1, $stored_errors['invalid_token'] );
$this->assertCount( 1, $stored_errors['unknown_user'] );
$this->assertCount( 1, $stored_errors['invalid_connection_owner'] );

$this->arrayHasKey( '1', $stored_errors['unknown_user'] );
$this->assertArrayHasKey( '1', $stored_errors['unknown_user'] );

$this->arrayHasKey( 'error_type', $stored_errors['invalid_token']['1'] );
$this->assertArrayHasKey( 'error_type', $stored_errors['invalid_token']['1'] );
$this->assertEquals( 'xmlrpc', $stored_errors['invalid_token']['1']['error_type'] );

$this->arrayHasKey( 'nonce', $stored_errors['unknown_user']['1'] );
$this->arrayHasKey( 'error_code', $stored_errors['unknown_user']['1'] );
$this->arrayHasKey( 'user_id', $stored_errors['unknown_user']['1'] );
$this->arrayHasKey( 'error_message', $stored_errors['unknown_user']['1'] );
$this->arrayHasKey( 'error_data', $stored_errors['unknown_user']['1'] );
$this->arrayHasKey( 'timestamp', $stored_errors['unknown_user']['1'] );
$this->arrayHasKey( 'nonce', $stored_errors['unknown_user']['1'] );
$this->arrayHasKey( 'error_type', $stored_errors['unknown_user']['1'] );
$this->assertArrayHasKey( 'nonce', $stored_errors['unknown_user']['1'] );
$this->assertArrayHasKey( 'error_code', $stored_errors['unknown_user']['1'] );
$this->assertArrayHasKey( 'user_id', $stored_errors['unknown_user']['1'] );
$this->assertArrayHasKey( 'error_message', $stored_errors['unknown_user']['1'] );
$this->assertArrayHasKey( 'error_data', $stored_errors['unknown_user']['1'] );
$this->assertArrayHasKey( 'timestamp', $stored_errors['unknown_user']['1'] );
$this->assertArrayHasKey( 'nonce', $stored_errors['unknown_user']['1'] );
$this->assertArrayHasKey( 'error_type', $stored_errors['unknown_user']['1'] );
$this->assertEquals( 'rest', $stored_errors['unknown_user']['1']['error_type'] );

$this->arrayHasKey( 'invalid', $stored_errors['invalid_connection_owner'] );
$this->arrayHasKey( 'error_type', $stored_errors['invalid_connection_owner']['invalid'] );
$this->assertArrayHasKey( 'invalid', $stored_errors['invalid_connection_owner'] );
$this->assertArrayHasKey( 'error_type', $stored_errors['invalid_connection_owner']['invalid'] );
$this->assertEquals( 'connection', $stored_errors['invalid_connection_owner']['invalid']['error_type'] );
}

Expand All @@ -153,21 +153,21 @@ public function test_store_multiple_error_codes_multiple_users() {

$this->assertCount( 2, $stored_errors );

$this->arrayHasKey( 'invalid_token', $stored_errors );
$this->assertArrayHasKey( 'invalid_token', $stored_errors );

$this->assertCount( 1, $stored_errors['invalid_token'] );
$this->assertCount( 2, $stored_errors['unknown_user'] );

$this->arrayHasKey( '2', $stored_errors['unknown_user'] );
$this->assertArrayHasKey( '2', $stored_errors['unknown_user'] );

$this->arrayHasKey( 'nonce', $stored_errors['unknown_user']['2'] );
$this->arrayHasKey( 'error_code', $stored_errors['unknown_user']['2'] );
$this->arrayHasKey( 'user_id', $stored_errors['unknown_user']['2'] );
$this->arrayHasKey( 'error_message', $stored_errors['unknown_user']['2'] );
$this->arrayHasKey( 'error_data', $stored_errors['unknown_user']['2'] );
$this->arrayHasKey( 'timestamp', $stored_errors['unknown_user']['2'] );
$this->arrayHasKey( 'nonce', $stored_errors['unknown_user']['2'] );
$this->arrayHasKey( 'error_type', $stored_errors['unknown_user']['2'] );
$this->assertArrayHasKey( 'nonce', $stored_errors['unknown_user']['2'] );
$this->assertArrayHasKey( 'error_code', $stored_errors['unknown_user']['2'] );
$this->assertArrayHasKey( 'user_id', $stored_errors['unknown_user']['2'] );
$this->assertArrayHasKey( 'error_message', $stored_errors['unknown_user']['2'] );
$this->assertArrayHasKey( 'error_data', $stored_errors['unknown_user']['2'] );
$this->assertArrayHasKey( 'timestamp', $stored_errors['unknown_user']['2'] );
$this->assertArrayHasKey( 'nonce', $stored_errors['unknown_user']['2'] );
$this->assertArrayHasKey( 'error_type', $stored_errors['unknown_user']['2'] );
}

/**
Expand Down Expand Up @@ -363,18 +363,18 @@ public function test_check_api_response_for_errors() {
$verified_errors = $this->error_handler->get_verified_errors();

$this->assertCount( 1, $stored_errors );
$this->arrayHasKey( 'unknown_token', $stored_errors );
$this->assertArrayHasKey( 'unknown_token', $stored_errors );
$this->assertCount( 1, $stored_errors['unknown_token'] );
$this->arrayHasKey( '1', $stored_errors['unknown_token'] );
$this->arrayHasKey( 'error_code', $stored_errors['unknown_token']['0'] );
$this->arrayHasKey( 'error_type', $stored_errors['unknown_token']['0'] );
$this->assertArrayHasKey( 0, $stored_errors['unknown_token'] );
$this->assertArrayHasKey( 'error_code', $stored_errors['unknown_token']['0'] );
$this->assertArrayHasKey( 'error_type', $stored_errors['unknown_token']['0'] );
$this->assertEquals( 'rest', $stored_errors['unknown_token']['0']['error_type'] );

$this->assertCount( 1, $verified_errors );
$this->arrayHasKey( 'unknown_token', $verified_errors );
$this->assertArrayHasKey( 'unknown_token', $verified_errors );
$this->assertCount( 1, $verified_errors['unknown_token'] );
$this->arrayHasKey( '1', $verified_errors['unknown_token'] );
$this->arrayHasKey( 'error_code', $verified_errors['unknown_token']['0'] );
$this->assertArrayHasKey( 0, $verified_errors['unknown_token'] );
$this->assertArrayHasKey( 'error_code', $verified_errors['unknown_token']['0'] );
$this->assertEquals( 'rest', $verified_errors['unknown_token']['0']['error_type'] );
}

Expand Down
3 changes: 1 addition & 2 deletions tests/php/test_Manager_integration.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,8 +633,7 @@ public function test_disconnect_user( $remote_response, $expected_user_token_cou
$this,
$callback,
),
10,
3
10
);

$this->assertCount( $expected_user_token_count, $this->manager->get_connected_users() );
Expand Down
2 changes: 1 addition & 1 deletion tests/php/test_Manager_unit.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public function test_api_url_defaults() {
$this->manager->api_url( 'another_thing/' )
);

remove_filter( 'jetpack_constant_default_value', array( $this, 'filter_api_constant' ), 10, 2 );
remove_filter( 'jetpack_constant_default_value', array( $this, 'filter_api_constant' ), 10 );
}

/**
Expand Down

0 comments on commit ddc023b

Please sign in to comment.