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

Improve static analysis in Performance Lab plugin files #1188

Merged
merged 8 commits into from
May 3, 2024
2 changes: 1 addition & 1 deletion bin/phpstan/constants.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
/**
* Constants for PHPStan static analysis.
*
* @package speculation-rules
* @package performance-lab
*/

define( 'SPECULATION_RULES_VERSION', '0.0.0' );
Expand Down
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
"szepeviktor/phpstan-wordpress": "^1.3",
"wp-coding-standards/wpcs": "^3.1",
"wp-phpunit/wp-phpunit": "^6.5",
"yoast/phpunit-polyfills": "^1.0"
"yoast/phpunit-polyfills": "^1.0",
"phpstan/php-8-stubs": "^0.3.84"
},
"autoload-dev": {
"psr-4": {
Expand Down
34 changes: 33 additions & 1 deletion composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 17 additions & 13 deletions includes/admin/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +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() {
$hook_suffix = add_options_page(
Expand All @@ -32,6 +34,8 @@ function perflab_add_features_page() {

return $hook_suffix;
}

// @phpstan-ignore-next-line
Copy link
Member Author

Choose a reason for hiding this comment

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

Because PHPStan identifies correctly that this function is used as an action, and not a filter. So it shouldn't return anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

The returned value is used in testing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ideally the tests could be updated to not rely on this, but I think that is out of scope for this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

add_action( 'admin_menu', 'perflab_add_features_page' );

/**
Expand All @@ -41,7 +45,7 @@ function perflab_add_features_page() {
* @since 3.0.0 Renamed to perflab_load_features_page(), and the
* $module and $hook_suffix parameters were removed.
*/
function perflab_load_features_page() {
function perflab_load_features_page(): void {
// Handle script enqueuing for settings page.
add_action( 'admin_enqueue_scripts', 'perflab_enqueue_features_page_scripts' );

Expand All @@ -61,7 +65,7 @@ function perflab_load_features_page() {
* @since 1.0.0
* @since 3.0.0 Renamed to perflab_render_settings_page().
*/
function perflab_render_settings_page() {
function perflab_render_settings_page(): void {
?>
<div class="wrap">
<?php perflab_render_plugins_ui(); ?>
Expand All @@ -79,7 +83,7 @@ function perflab_render_settings_page() {
*
* @param string $hook_suffix The current admin page.
*/
function perflab_admin_pointer( $hook_suffix ) {
function perflab_admin_pointer( string $hook_suffix ): void {
// Do not show admin pointer in multisite Network admin or User admin UI.
if ( is_network_admin() || is_user_admin() ) {
return;
Expand Down Expand Up @@ -117,11 +121,11 @@ function perflab_admin_pointer( $hook_suffix ) {
* @since 1.0.0
* @since 2.4.0 Optional arguments were added to make the function reusable for different pointers.
*
* @param string $pointer_id Optional. ID of the pointer. Default 'perflab-admin-pointer'.
* @param array $args Optional. Pointer arguments. Supports 'heading' and 'content' entries.
* Defaults are the heading and content for the 'perflab-admin-pointer'.
* @param string $pointer_id Optional. ID of the pointer. Default 'perflab-admin-pointer'.
* @param array{heading?: string, content?: string} $args Optional. Pointer arguments. Supports 'heading' and 'content' entries.
* Defaults are the heading and content for the 'perflab-admin-pointer'.
*/
function perflab_render_pointer( $pointer_id = 'perflab-admin-pointer', $args = array() ) {
function perflab_render_pointer( string $pointer_id = 'perflab-admin-pointer', array $args = array() ): void {
if ( ! isset( $args['heading'] ) ) {
$args['heading'] = __( 'Performance Lab', 'performance-lab' );
}
Expand Down Expand Up @@ -207,7 +211,7 @@ function perflab_plugin_action_links_add_settings( $links ) {
*
* @since 2.3.0
*/
function perflab_dismiss_wp_pointer_wrapper() {
function perflab_dismiss_wp_pointer_wrapper(): void {
if ( isset( $_POST['pointer'] ) && 'perflab-admin-pointer' !== $_POST['pointer'] ) {
// Another plugin's pointer, do nothing.
return;
Expand All @@ -222,7 +226,7 @@ function perflab_dismiss_wp_pointer_wrapper() {
* @since 2.8.0
* @since 3.0.0 Renamed to perflab_enqueue_features_page_scripts().
*/
function perflab_enqueue_features_page_scripts() {
function perflab_enqueue_features_page_scripts(): void {
// These assets are needed for the "Learn more" popover.
wp_enqueue_script( 'thickbox' );
wp_enqueue_style( 'thickbox' );
Expand All @@ -237,7 +241,7 @@ function perflab_enqueue_features_page_scripts() {
*
* @since 3.0.0
*/
function perflab_install_activate_plugin_callback() {
function perflab_install_activate_plugin_callback(): void {
check_admin_referer( 'perflab_install_activate_plugin' );

require_once ABSPATH . 'wp-admin/includes/plugin.php';
Expand Down Expand Up @@ -279,9 +283,9 @@ function perflab_install_activate_plugin_callback() {
*
* @since 3.0.0
*/
function perflab_print_features_page_style() {
function perflab_print_features_page_style(): void {
?>
<style type="text/css">
Copy link
Member Author

Choose a reason for hiding this comment

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

The type attribute is obsolete.

<style>
.plugin-card .name,
.plugin-card .desc, /* For WP <6.5 versions */
.plugin-card .desc > p {
Expand All @@ -303,7 +307,7 @@ function perflab_print_features_page_style() {
*
* @since 2.8.0
*/
function perflab_plugin_admin_notices() {
function perflab_plugin_admin_notices(): void {
if ( ! current_user_can( 'install_plugins' ) ) {
$are_all_plugins_installed = true;
$installed_plugin_slugs = array_map(
Expand Down
4 changes: 2 additions & 2 deletions includes/admin/plugins.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function perflab_get_standalone_plugins(): array {
*
* @since 2.8.0
*/
function perflab_render_plugins_ui() {
function perflab_render_plugins_ui(): void {
require_once ABSPATH . 'wp-admin/includes/plugin-install.php';
require_once ABSPATH . 'wp-admin/includes/plugin.php';

Expand Down Expand Up @@ -305,7 +305,7 @@ function perflab_install_and_activate_plugin( string $plugin_slug, array &$proce
*
* @param array{name: string, slug: string, short_description: string, requires_php: string|false, requires: string|false, requires_plugins: string[], version: string, experimental: bool} $plugin_data Plugin data augmenting data from the WordPress.org API.
*/
function perflab_render_plugin_card( array $plugin_data ) {
function perflab_render_plugin_card( array $plugin_data ): void {

$name = wp_strip_all_tags( $plugin_data['name'] );
$description = wp_strip_all_tags( $plugin_data['short_description'] );
Expand Down
12 changes: 8 additions & 4 deletions includes/admin/server-timing.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +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() {
$hook_suffix = add_management_page(
Expand All @@ -35,14 +37,16 @@ function perflab_add_server_timing_page() {

return $hook_suffix;
}

// @phpstan-ignore-next-line
Copy link
Member Author

Choose a reason for hiding this comment

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

Because PHPStan identifies correctly that this function is used as an action, and not a filter. So it shouldn't return anything.

add_action( 'admin_menu', 'perflab_add_server_timing_page' );

/**
* Initializes settings sections and fields for the Server-Timing page.
*
* @since 2.6.0
*/
function perflab_load_server_timing_page() {
function perflab_load_server_timing_page(): void {
/*
* This settings section technically includes a field, however it is directly rendered as part of the section
* callback due to requiring custom markup.
Expand Down Expand Up @@ -149,7 +153,7 @@ static function () {
*
* @since 2.6.0
*/
function perflab_render_server_timing_page() {
function perflab_render_server_timing_page(): void {
?>
<div class="wrap">
<?php settings_errors(); ?>
Expand All @@ -173,7 +177,7 @@ function perflab_render_server_timing_page() {
*
* @param string $slug Slug of the field and sub-key in the Server-Timing option.
*/
function perflab_render_server_timing_page_hooks_field( $slug ) {
function perflab_render_server_timing_page_hooks_field( string $slug ): void {
$options = (array) get_option( PERFLAB_SERVER_TIMING_SETTING, array() );

// Value for the sub-key is an array of hook names.
Expand Down Expand Up @@ -205,7 +209,7 @@ class="large-text code"
*
* @since 2.6.0
*/
function perflab_render_server_timing_page_output_buffering_section() {
function perflab_render_server_timing_page_output_buffering_section(): void {
$slug = 'output_buffering';
$field_id = "server_timing_{$slug}";
$field_name = PERFLAB_SERVER_TIMING_SETTING . '[' . $slug . ']';
Expand Down
14 changes: 7 additions & 7 deletions includes/server-timing/class-perflab-server-timing-metric.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class Perflab_Server_Timing_Metric {
*
* @param string $slug The metric slug.
*/
public function __construct( $slug ) {
public function __construct( string $slug ) {
$this->slug = $slug;
}

Expand All @@ -55,7 +55,7 @@ public function __construct( $slug ) {
*
* @return string The metric slug.
*/
public function get_slug() {
public function get_slug(): string {
return $this->slug;
}

Expand All @@ -67,9 +67,9 @@ public function get_slug() {
*
* @since 1.8.0
*
* @param int|float $value The metric value to set, in milliseconds.
* @param int|float|mixed $value The metric value to set, in milliseconds.
*/
public function set_value( $value ) {
public function set_value( $value ): void {
if ( ! is_numeric( $value ) ) {
_doing_it_wrong(
__METHOD__,
Expand Down Expand Up @@ -110,14 +110,14 @@ public function get_value() {
}

/**
* Captures the current time, as a reference point to calculate the duration of a task afterwards.
* Captures the current time, as a reference point to calculate the duration of a task afterward.
*
* This should be used in combination with {@see Perflab_Server_Timing_Metric::measure_after()}. Alternatively,
* {@see Perflab_Server_Timing_Metric::set_value()} can be used to set a calculated value manually.
*
* @since 1.8.0
*/
public function measure_before() {
public function measure_before(): void {
$this->before_value = microtime( true );
}

Expand All @@ -129,7 +129,7 @@ public function measure_before() {
*
* @since 1.8.0
*/
public function measure_after() {
public function measure_after(): void {
if ( ! $this->before_value ) {
_doing_it_wrong(
__METHOD__,
Expand Down
Loading
Loading