Skip to content

Commit

Permalink
jitm: Fix most Phan issues (#37075)
Browse files Browse the repository at this point in the history
The main motivation here is to handle a bit in
`tests/php/test_pre_connection_jitm.php` that's giving different issues
depending on the version of Mockery in use, but I figured I may as well
fix the rest too.

One issue remains. As far as I can tell it's technically a `mixed` from
`$_GET` that's being assumed to be an array in a few places but is
actually a string, but when it's a string it probably should be turned
into a single-element array (as implicitly happens inside the call Phan
flags). I'll leave that for someone who actually knows about this code
to deal with.
  • Loading branch information
anomiex authored Apr 25, 2024
1 parent 35b67ec commit 58bed95
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 15 deletions.
12 changes: 1 addition & 11 deletions projects/packages/jitm/.phan/baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,11 @@
*/
return [
// # Issue statistics:
// PhanTypeExpectedObjectPropAccess : 3 occurrences
// PhanTypeMismatchReturn : 2 occurrences
// PhanUndeclaredMethod : 2 occurrences
// PhanPluginSimplifyExpressionBool : 1 occurrence
// PhanTypeInvalidDimOffset : 1 occurrence
// PhanTypeMismatchArgument : 1 occurrence
// PhanTypeMismatchArgumentProbablyReal : 1 occurrence
// PhanUnreferencedUseNormal : 1 occurrence

// Currently, file_suppressions and directory_suppressions are the only supported suppressions
'file_suppressions' => [
'src/class-post-connection-jitm.php' => ['PhanTypeExpectedObjectPropAccess', 'PhanTypeMismatchArgument', 'PhanTypeMismatchReturn'],
'src/class-rest-api-endpoints.php' => ['PhanPluginSimplifyExpressionBool'],
'tests/php/test_JITM.php' => ['PhanTypeMismatchArgumentProbablyReal', 'PhanUnreferencedUseNormal'],
'tests/php/test_pre_connection_jitm.php' => ['PhanTypeInvalidDimOffset', 'PhanUndeclaredMethod'],
'src/class-post-connection-jitm.php' => ['PhanTypeMismatchArgument'],
],
// 'directory_suppressions' => ['src/directory_name' => ['PhanIssueName1', 'PhanIssueName2']] can be manually added if needed.
// (directory_suppressions will currently be ignored by subsequent calls to --save-baseline, but may be preserved in future Phan releases)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: fixed
Comment: Fix most Phan issues. One remains, I couldn't figure out what should be correct there.


4 changes: 2 additions & 2 deletions projects/packages/jitm/src/class-post-connection-jitm.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ public function __construct() {
/**
* A special filter for WooCommerce, to set a message based on local state.
*
* @param string $content The current message.
* @param object $content The current message.
*
* @return array The new message.
* @return object The new message.
*/
public static function jitm_woocommerce_services_msg( $content ) {
if ( ! function_exists( 'wc_get_base_location' ) ) {
Expand Down
2 changes: 1 addition & 1 deletion projects/packages/jitm/src/class-rest-api-endpoints.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public static function get_jitm_message( $request ) {
$query['s'] = $request['s'];
}

return $jitm->get_messages( $request['message_path'], urldecode_deep( $query ), 'true' === $request['full_jp_logo_exists'] ? true : false );
return $jitm->get_messages( $request['message_path'], urldecode_deep( $query ), 'true' === $request['full_jp_logo_exists'] );
}

/**
Expand Down
2 changes: 1 addition & 1 deletion projects/packages/jitm/tests/php/test_JITM.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
namespace Automattic\Jetpack;

use Automattic\Jetpack\JITMS\JITM;
use Automattic\Jetpack\JITMS\Pre_Connection_JITM;
use Brain\Monkey;
use Brain\Monkey\Actions;
use Brain\Monkey\Filters;
Expand Down Expand Up @@ -80,6 +79,7 @@ public function test_prepare_jitms_enqueues_assets() {

$jitm = new JITM();
$screen = (object) array( 'id' => $screen_id ); // fake screen object
// @phan-suppress-next-line PhanTypeMismatchArgumentProbablyReal -- Fake object, don't care.
$jitm->prepare_jitms( $screen );

// Set up mocks for a bunch of methods called by the hook.
Expand Down
2 changes: 2 additions & 0 deletions projects/packages/jitm/tests/php/test_pre_connection_jitm.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ public function test_get_messages_extra_key() {
->andReturn( $this->test_jitms );

$messages = $this->jitm_instance->get_messages( '/wp:plugins:admin_notices/', '', false );
// @phan-suppress-next-line PhanTypeInvalidDimOffset -- It's confused by the assignment above.
$this->assertSame( $this->test_jitms[0]['id'], $messages[0]->id );
}

Expand All @@ -201,6 +202,7 @@ public function test_message_icon_values( $message_icon_value, $expected_icon )
->andReturn( $this->test_jitms );

$jitm = \Mockery::mock( Pre_Connection_JITM::class )->makePartial();
'@phan-var \Mockery\LegacyMockInterface&\Mockery\MockInterface&Pre_Connection_JITM $jitm'; // Bad phpdoc in Mockery plus probably https://github.com/phan/phan/issues/4849 makes Phan get the type wrong.
$jitm
->expects( 'generate_icon' )
->once()
Expand Down

0 comments on commit 58bed95

Please sign in to comment.