Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update PHPStan level to 1 #1198

Merged
merged 7 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions bin/phpstan/constants.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
* @package performance-lab
*/

define( 'TESTS_PLUGIN_DIR', './' );
define( 'WPINC', 'wp-includes' );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@szepeviktor szepeviktor Aug 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know there is no core function to get the value of WPINC but I do not want to encourage the users of my package to use WPINC.


define( 'SPECULATION_RULES_VERSION', '0.0.0' );
define( 'SPECULATION_RULES_MAIN_FILE', 'plugins/speculation-rules/load.php' );

Expand Down
7 changes: 1 addition & 6 deletions includes/admin/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@
*
* @since 1.0.0
* @since 3.0.0 Renamed to perflab_add_features_page().
*
* @return string|false Hook suffix.
*/
function perflab_add_features_page() {
function perflab_add_features_page(): void {
$hook_suffix = add_options_page(
__( 'Performance Features', 'performance-lab' ),
__( 'Performance', 'performance-lab' ),
Expand All @@ -31,11 +29,8 @@ function perflab_add_features_page() {
add_action( "load-{$hook_suffix}", 'perflab_load_features_page', 10, 0 );
add_filter( 'plugin_action_links_' . plugin_basename( PERFLAB_MAIN_FILE ), 'perflab_plugin_action_links_add_settings' );
}

return $hook_suffix;
}

// @phpstan-ignore-next-line
Comment on lines -34 to -38
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted in #1188 (comment) this returning of the $hook_suffix was only being used in tests. This PR eliminates the need for that return value by instead computing the $hook_suffix in the test, and then seeing if the expected action is added.

add_action( 'admin_menu', 'perflab_add_features_page' );

/**
Expand Down
7 changes: 1 addition & 6 deletions includes/admin/server-timing.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@
* Adds the Server-Timing page to the Tools menu.
*
* @since 2.6.0
*
* @return string|false Hook suffix.
*/
function perflab_add_server_timing_page() {
function perflab_add_server_timing_page(): void {
$hook_suffix = add_management_page(
__( 'Server-Timing', 'performance-lab' ),
__( 'Server-Timing', 'performance-lab' ),
Expand All @@ -34,11 +32,8 @@ function perflab_add_server_timing_page() {
if ( false !== $hook_suffix ) {
add_action( "load-{$hook_suffix}", 'perflab_load_server_timing_page' );
}

return $hook_suffix;
}

// @phpstan-ignore-next-line
add_action( 'admin_menu', 'perflab_add_server_timing_page' );

/**
Expand Down
2 changes: 1 addition & 1 deletion phpstan.neon.dist
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
includes:
- phar://phpstan.phar/conf/bleedingEdge.neon
parameters:
level: 0
level: 1
paths:
- load.php
- includes/
Expand Down
4 changes: 2 additions & 2 deletions plugins/dominant-color-images/hooks.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,11 @@ function dominant_color_img_tag_add_dominant_color( $filtered_image, $context, $
$extra_class = $image_meta['has_transparency'] ? 'has-transparency' : 'not-transparent';
}

if ( ! empty( $data ) ) {
Copy link
Member Author

@westonruter westonruter May 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHPStan was complaining here:

Variable $data in empty() always exists and is always falsy.

For some reason switching to a simple boolean check fixed the issue. In any case, using empty() is discouraged per #1091 (comment)

Same goes for the change to $extra_class below.

if ( $data ) {
$filtered_image = str_replace( '<img ', '<img ' . $data, $filtered_image );
}

if ( ! empty( $extra_class ) ) {
if ( $extra_class ) {
$filtered_image = str_replace( ' class="', ' class="' . $extra_class . ' ', $filtered_image );
}

Expand Down
34 changes: 21 additions & 13 deletions plugins/webp-uploads/helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ function webp_uploads_get_upload_image_mime_transforms() {
* @since 1.0.0
* @access private
*
* @param int $attachment_id The ID of the attachment from where this image would be created.
* @param string $image_size The size name that would be used to create the image source, out of the registered subsizes.
* @param array $size_data An array with the dimensions of the image: height, width and crop.
* @param string $mime The target mime in which the image should be created.
* @param string|null $destination_file_name The path where the file would be stored, including the extension. If null, `generate_filename` is used to create the destination file name.
* @param int $attachment_id The ID of the attachment from where this image would be created.
* @param string $image_size The size name that would be used to create the image source, out of the registered subsizes.
* @param array{ width: int, height: int, crop: bool } $size_data An array with the dimensions of the image: height, width and crop.
* @param string $mime The target mime in which the image should be created.
* @param string|null $destination_file_name The path where the file would be stored, including the extension. If null, `generate_filename` is used to create the destination file name.
*
* @return array|WP_Error An array with the file and filesize if the image was created correctly, otherwise a WP_Error.
* @return array{ file: string, filesize: int }|WP_Error An array with the file and filesize if the image was created correctly, otherwise a WP_Error.
*/
function webp_uploads_generate_additional_image_source( $attachment_id, $image_size, array $size_data, $mime, $destination_file_name = null ) {
/**
Expand All @@ -88,11 +88,19 @@ function webp_uploads_generate_additional_image_source( $attachment_id, $image_s
*
* @since 1.1.0
*
* @param array|null|WP_Error $image Image data {'path'=>string, 'file'=>string, 'width'=>int, 'height'=>int, 'mime-type'=>string} or null or WP_Error.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that width, height, and mime-type are actually not used here. The only keys used are file and either path or filesize.

* @param int $attachment_id The ID of the attachment from where this image would be created.
* @param string $image_size The size name that would be used to create this image, out of the registered subsizes.
* @param array $size_data An array with the dimensions of the image: height, width and crop {'height'=>int, 'width'=>int, 'crop'}.
* @param string $mime The target mime in which the image should be created.
* @param array{
* file: string,
* path?: string,
* filesize?: int
* }|null|WP_Error $image Image data, null, or WP_Error.
* @param int $attachment_id The ID of the attachment from where this image would be created.
* @param string $image_size The size name that would be used to create this image, out of the registered subsizes.
* @param array{
* width: int,
* height: int,
* crop: bool
* } $size_data An array with the dimensions of the image.
* @param string $mime The target mime in which the image should be created.
*/
$image = apply_filters( 'webp_uploads_pre_generate_additional_image_source', null, $attachment_id, $image_size, $size_data, $mime );
if ( is_wp_error( $image ) ) {
Expand Down Expand Up @@ -157,7 +165,7 @@ function webp_uploads_generate_additional_image_source( $attachment_id, $image_s
$destination_file_name = $editor->generate_filename( $suffix, null, $extension[0] );
}

remove_filter( 'image_editor_output_format', 'webp_uploads_filter_image_editor_output_format', 10, 3 );
remove_filter( 'image_editor_output_format', 'webp_uploads_filter_image_editor_output_format', 10 );
$image = $editor->save( $destination_file_name, $mime );
add_filter( 'image_editor_output_format', 'webp_uploads_filter_image_editor_output_format', 10, 3 );

Expand Down Expand Up @@ -188,7 +196,7 @@ function webp_uploads_generate_additional_image_source( $attachment_id, $image_s
* @param string $size The size name that would be used to create this image, out of the registered subsizes.
* @param string $mime A mime type we are looking to use to create this image.
*
* @return array|WP_Error
* @return array{ file: string, filesize: int }|WP_Error
*/
function webp_uploads_generate_image_size( $attachment_id, $size, $mime ) {
$sizes = wp_get_registered_image_subsizes();
Expand Down
35 changes: 33 additions & 2 deletions plugins/webp-uploads/hooks.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,18 @@
*
* @param array $metadata An array with the metadata from this attachment.
* @param int $attachment_id The ID of the attachment where the hook was dispatched.
* @return array An array with the updated structure for the metadata before is stored in the database.
* @return array{
* width: int,
* height: int,
* file: non-falsy-string,
* sizes: array,
* image_meta: array,
* filesize: int,
* sources?: array<string, array{
* file: string,
* filesize: int
* }>
* } An array with the updated structure for the metadata before is stored in the database.
*/
function webp_uploads_create_sources_property( array $metadata, $attachment_id ) {
// This should take place only on the JPEG image.
Expand Down Expand Up @@ -538,7 +549,27 @@ function webp_uploads_update_image_references( $content ) {
* @return string The updated img tag.
*/
function webp_uploads_img_tag_update_mime_type( $original_image, $context, $attachment_id ) {
$image = $original_image;
$image = $original_image;

/**
* Metadata potentially amended by webp_uploads_create_sources_property().
*
* Note the sources key is not normally present in the response for wp_get_attachment_metadata(). The sources
* key here, however, is being injected via the 'wp_generate_attachment_metadata' filter via the
* webp_uploads_create_sources_property() function.
*
* @see webp_uploads_create_sources_property()
*
* @var array{
* width: int,
* height: int,
* file: non-falsy-string,
* sizes: array,
* image_meta: array,
* filesize: int,
* sources?: array<string, array{ file: string, filesize: int }>
* } $metadata
*/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this type definition, the isset( $metadata['sources'][ $target_mime ]['file'] ) check below fails PHPStan because wp_get_attachment_metadata() doesn't normally return a sources key.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I revisited this when bumping PHPStan to level 3. See #1200.

$metadata = wp_get_attachment_metadata( $attachment_id );

if ( empty( $metadata['file'] ) ) {
Expand Down
4 changes: 2 additions & 2 deletions plugins/webp-uploads/image-edit.php
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ static function ( $metadata, $post_meta_id ) use ( $post_id, $file_path, $mime_t
$target_file_name = preg_replace( "/\.$current_extension$/", ".$extension", $thumbnail_file );
$target_file_location = path_join( $original_directory, $target_file_name );

remove_filter( 'image_editor_output_format', 'webp_uploads_filter_image_editor_output_format', 10, 3 );
remove_filter( 'image_editor_output_format', 'webp_uploads_filter_image_editor_output_format', 10 );
$result = $editor->save( $target_file_location, $targeted_mime );
add_filter( 'image_editor_output_format', 'webp_uploads_filter_image_editor_output_format', 10, 3 );

Expand All @@ -208,7 +208,7 @@ static function ( $metadata, $post_meta_id ) use ( $post_id, $file_path, $mime_t
} else {
$destination = trailingslashit( $original_directory ) . "{$filename}.{$extension}";

remove_filter( 'image_editor_output_format', 'webp_uploads_filter_image_editor_output_format', 10, 3 );
remove_filter( 'image_editor_output_format', 'webp_uploads_filter_image_editor_output_format', 10 );
$result = $editor->save( $destination, $targeted_mime );
add_filter( 'image_editor_output_format', 'webp_uploads_filter_image_editor_output_format', 10, 3 );

Expand Down
19 changes: 14 additions & 5 deletions tests/includes/admin/load-tests.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,23 @@
*/
class Admin_Load_Tests extends WP_UnitTestCase {

/**
* @covers ::perflab_add_features_page
*/
public function test_perflab_add_features_page() {
global $_wp_submenu_nopriv;

// Reset relevant globals and filters.
$_wp_submenu_nopriv = array();
remove_all_filters( 'plugin_action_links_' . plugin_basename( PERFLAB_MAIN_FILE ) );

$hook_suffix = get_plugin_page_hookname( PERFLAB_SCREEN, 'tools.php' );

// The default user does not have the 'manage_options' capability.
$hook_suffix = perflab_add_features_page();
$this->assertFalse( $hook_suffix );
$this->assertTrue( isset( $_wp_submenu_nopriv['options-general.php'][ PERFLAB_SCREEN ] ) );
perflab_add_features_page();
$this->assertFalse( has_action( "load-{$hook_suffix}", 'perflab_load_features_page' ) );
$this->assertArrayHasKey( 'options-general.php', $_wp_submenu_nopriv );
$this->assertArrayHasKey( PERFLAB_SCREEN, $_wp_submenu_nopriv['options-general.php'] );
// Ensure plugin action link is not added.
$this->assertFalse( (bool) has_action( 'plugin_action_links_' . plugin_basename( PERFLAB_MAIN_FILE ), 'perflab_plugin_action_links_add_settings' ) );

Expand All @@ -31,9 +37,12 @@ public function test_perflab_add_features_page() {
// Rely on current user to be an administrator (with 'manage_options' capability).
$user_id = self::factory()->user->create( array( 'role' => 'administrator' ) );
wp_set_current_user( $user_id );
$hook_suffix = perflab_add_features_page();
$this->assertFalse( has_action( "load-{$hook_suffix}", 'perflab_load_features_page' ) );
perflab_add_features_page();
$this->assertSame( 10, has_action( "load-{$hook_suffix}", 'perflab_load_features_page' ) );

$this->assertSame( get_plugin_page_hookname( PERFLAB_SCREEN, 'options-general.php' ), $hook_suffix );
$this->assertFalse( isset( $_wp_submenu_nopriv['options-general.php'][ PERFLAB_SCREEN ] ) );
$this->assertArrayNotHasKey( 'options-general.php', $_wp_submenu_nopriv );
// Ensure plugin action link is added.
$this->assertTrue( (bool) has_action( 'plugin_action_links_' . plugin_basename( PERFLAB_MAIN_FILE ), 'perflab_plugin_action_links_add_settings' ) );

Expand Down
23 changes: 17 additions & 6 deletions tests/includes/admin/server-timing-tests.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
*/
class Admin_Server_Timing_Tests extends WP_UnitTestCase {

/**
* @covers ::perflab_add_server_timing_page
*/
public function test_perflab_add_server_timing_page() {
global $_wp_submenu_nopriv;

Expand All @@ -21,11 +24,16 @@ public function test_perflab_add_server_timing_page() {
// Rely on current user to be an administrator (with 'manage_options' capability).
$user_id = self::factory()->user->create( array( 'role' => 'administrator' ) );
wp_set_current_user( $user_id );
$hook_suffix = perflab_add_server_timing_page();
$this->assertSame( get_plugin_page_hookname( PERFLAB_SERVER_TIMING_SCREEN, 'tools.php' ), $hook_suffix );
$this->assertFalse( isset( $_wp_submenu_nopriv['tools.php'][ PERFLAB_SERVER_TIMING_SCREEN ] ) );
$hook_suffix = get_plugin_page_hookname( PERFLAB_SERVER_TIMING_SCREEN, 'tools.php' );
$this->assertFalse( has_action( "load-{$hook_suffix}", 'perflab_load_server_timing_page' ) );
perflab_add_server_timing_page();
$this->assertSame( 10, has_action( "load-{$hook_suffix}", 'perflab_load_server_timing_page' ) );
$this->assertArrayNotHasKey( 'tools.php', $_wp_submenu_nopriv );
}

/**
* @covers ::perflab_add_server_timing_page
*/
public function test_perflab_add_server_timing_page_missing_caps() {
global $_wp_submenu_nopriv;

Expand All @@ -34,9 +42,12 @@ public function test_perflab_add_server_timing_page_missing_caps() {
remove_all_filters( 'plugin_action_links_' . plugin_basename( PERFLAB_MAIN_FILE ) );

// The default user does not have the 'manage_options' capability.
$hook_suffix = perflab_add_server_timing_page();
$this->assertFalse( $hook_suffix );
$this->assertTrue( isset( $_wp_submenu_nopriv['tools.php'][ PERFLAB_SERVER_TIMING_SCREEN ] ) );
$hook_suffix = get_plugin_page_hookname( PERFLAB_SERVER_TIMING_SCREEN, 'tools.php' );
$this->assertFalse( has_action( "load-{$hook_suffix}", 'perflab_load_server_timing_page' ) );
perflab_add_server_timing_page();
$this->assertFalse( has_action( "load-{$hook_suffix}", 'perflab_load_server_timing_page' ) );
$this->assertArrayHasKey( 'tools.php', $_wp_submenu_nopriv );
$this->assertArrayHasKey( PERFLAB_SERVER_TIMING_SCREEN, $_wp_submenu_nopriv['tools.php'] );
}

public function test_perflab_load_server_timing_page() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ public function set_up() {
add_filter(
'template_directory_uri',
static function () {
return trailingslashit( WP_CONTENT_URL ) . 'themes/template';
return content_url( 'themes/template' );
}
);

add_filter(
'stylesheet_directory_uri',
static function () {
return trailingslashit( WP_CONTENT_URL ) . 'themes/stylesheet';
return content_url( 'themes/stylesheet' );
}
);
}
Expand Down
Loading