From 7ffe2b170757b65e880cd8260d3107f8bf6ce0c7 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 23 Jul 2024 12:31:32 -0700 Subject: [PATCH 1/3] Add strict PHPStan rules for Speculative Loading --- phpstan.neon.dist | 1 - plugins/speculation-rules/helper.php | 16 +---- plugins/speculation-rules/hooks.php | 7 +-- plugins/speculation-rules/settings.php | 85 ++++++++++++++------------ 4 files changed, 49 insertions(+), 60 deletions(-) diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 7afe3a83be..bd62e8f23b 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -70,5 +70,4 @@ parameters: - */tests/* - plugins/dominant-color-images - plugins/performance-lab - - plugins/speculation-rules - plugins/webp-uploads diff --git a/plugins/speculation-rules/helper.php b/plugins/speculation-rules/helper.php index 3740a62987..298599fb63 100644 --- a/plugins/speculation-rules/helper.php +++ b/plugins/speculation-rules/helper.php @@ -19,21 +19,11 @@ * * @since 1.0.0 * - * @return array>> Associative array of speculation rules by type. + * @return non-empty-array>> Associative array of speculation rules by type. */ function plsr_get_speculation_rules(): array { - $option = get_option( 'plsr_speculation_rules' ); - - /* - * This logic is only relevant for edge-cases where the setting may not be registered, - * a.k.a. defensive coding. - */ - if ( ! is_array( $option ) ) { - $option = array(); - } - $option = array_merge( plsr_get_setting_default(), $option ); - - $mode = (string) $option['mode']; + $option = plsr_get_stored_setting_value(); + $mode = $option['mode']; $eagerness = $option['eagerness']; $prefixer = new PLSR_URL_Pattern_Prefixer(); diff --git a/plugins/speculation-rules/hooks.php b/plugins/speculation-rules/hooks.php index e467a94d43..fa478b9843 100644 --- a/plugins/speculation-rules/hooks.php +++ b/plugins/speculation-rules/hooks.php @@ -19,13 +19,8 @@ * @since 1.0.0 */ function plsr_print_speculation_rules(): void { - $rules = plsr_get_speculation_rules(); - if ( empty( $rules ) ) { - return; - } - wp_print_inline_script_tag( - (string) wp_json_encode( $rules ), + (string) wp_json_encode( plsr_get_speculation_rules() ), array( 'type' => 'speculationrules' ) ); } diff --git a/plugins/speculation-rules/settings.php b/plugins/speculation-rules/settings.php index ae0c6ae818..206bd32aac 100644 --- a/plugins/speculation-rules/settings.php +++ b/plugins/speculation-rules/settings.php @@ -16,7 +16,7 @@ * * @since 1.0.0 * - * @return array Associative array of `$mode => $label` pairs. + * @return array{ prefetch: string, prerender: string } Associative array of `$mode => $label` pairs. */ function plsr_get_mode_labels(): array { return array( @@ -30,7 +30,7 @@ function plsr_get_mode_labels(): array { * * @since 1.0.0 * - * @return array Associative array of `$eagerness => $label` pairs. + * @return array{ conservative: string, moderate: string, eager: string } Associative array of `$eagerness => $label` pairs. */ function plsr_get_eagerness_labels(): array { return array( @@ -43,9 +43,9 @@ function plsr_get_eagerness_labels(): array { /** * Returns the default setting value for Speculative Loading configuration. * - * @since 1.0.0 + * @since n.e.x.t * - * @return array { + * @return array{ mode: 'prerender', eagerness: 'moderate' } { * Default setting value. * * @type string $mode Mode. @@ -59,13 +59,29 @@ function plsr_get_setting_default(): array { ); } +/** + * Returns the stored setting value for Speculative Loading configuration. + * + * @since 1.0.0 + * + * @return array{ mode: 'prefetch'|'prerender', eagerness: 'conservative'|'moderate'|'eager' } { + * Stored setting value. + * + * @type string $mode Mode. + * @type string $eagerness Eagerness. + * } + */ +function plsr_get_stored_setting_value(): array { + return plsr_sanitize_setting( get_option( 'plsr_speculation_rules' ) ); +} + /** * Sanitizes the setting for Speculative Loading configuration. * * @since 1.0.0 * * @param mixed $input Setting to sanitize. - * @return array { + * @return array{ mode: 'prefetch'|'prerender', eagerness: 'conservative'|'moderate'|'eager' } { * Sanitized setting. * * @type string $mode Mode. @@ -79,17 +95,14 @@ function plsr_sanitize_setting( $input ): array { return $default_value; } - $mode_labels = plsr_get_mode_labels(); - $eagerness_labels = plsr_get_eagerness_labels(); - // Ensure only valid keys are present. - $value = array_intersect_key( $input, $default_value ); + $value = array_intersect_key( array_merge( $default_value, $input ), $default_value ); - // Set any missing or invalid values to their defaults. - if ( ! isset( $value['mode'] ) || ! isset( $mode_labels[ $value['mode'] ] ) ) { + // Constrain values to what is allowed. + if ( ! in_array( $value['mode'], array_keys( plsr_get_mode_labels() ), true ) ) { $value['mode'] = $default_value['mode']; } - if ( ! isset( $value['eagerness'] ) || ! isset( $eagerness_labels[ $value['eagerness'] ] ) ) { + if ( ! in_array( $value['eagerness'], array_keys( plsr_get_eagerness_labels() ), true ) ) { $value['eagerness'] = $default_value['eagerness']; } @@ -109,7 +122,7 @@ function plsr_register_setting(): void { array( 'type' => 'object', 'description' => __( 'Configuration for the Speculation Rules API.', 'speculation-rules' ), - 'sanitize_callback' => 'plsr_sanitize_setting', + 'sanitize_callback' => 'plsr_sanitize_setting', // TODO: Is this even needed here due to the schema? 'default' => plsr_get_setting_default(), 'show_in_rest' => array( 'schema' => array( @@ -118,11 +131,13 @@ function plsr_register_setting(): void { 'description' => __( 'Whether to prefetch or prerender URLs.', 'speculation-rules' ), 'type' => 'string', 'enum' => array_keys( plsr_get_mode_labels() ), + 'default' => plsr_get_setting_default()['mode'], ), 'eagerness' => array( 'description' => __( 'The eagerness setting defines the heuristics based on which the loading is triggered. "Eager" will have the minimum delay to start speculative loads, "Conservative" increases the chance that only URLs the user actually navigates to are loaded.', 'speculation-rules' ), 'type' => 'string', 'enum' => array_keys( plsr_get_eagerness_labels() ), + 'default' => plsr_get_setting_default()['eagerness'], ), ), ), @@ -188,7 +203,7 @@ static function (): void { * @since 1.0.0 * @access private * - * @param array $args { + * @param array{ field: 'mode'|'eagerness', title: non-empty-string, description: non-empty-string } $args { * Associative array of arguments. * * @type string $field The slug of the sub setting controlled by the field. @@ -197,28 +212,24 @@ static function (): void { * } */ function plsr_render_settings_field( array $args ): void { - if ( empty( $args['field'] ) || empty( $args['title'] ) ) { // Invalid. - return; - } - - $option = get_option( 'plsr_speculation_rules' ); - if ( ! isset( $option[ $args['field'] ] ) ) { // Invalid. - return; - } + $option = plsr_get_stored_setting_value(); - $value = $option[ $args['field'] ]; - $callback = "plsr_get_{$args['field']}_labels"; - if ( ! is_callable( $callback ) ) { - return; + switch ( $args['field'] ) { + case 'mode': + $choices = plsr_get_mode_labels(); + break; + case 'eagerness': + $choices = plsr_get_eagerness_labels(); + break; + default: + return; // Invalid (and this case should never occur). } - $choices = call_user_func( $callback ); + $value = $option[ $args['field'] ]; ?>
- $label ) { - ?> + $label ) : ?>

- - if ( ! empty( $args['description'] ) ) { - ?> -

- -

- +

+ +

Date: Tue, 23 Jul 2024 13:43:16 -0700 Subject: [PATCH 2/3] Refine JSON schema --- plugins/speculation-rules/settings.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/speculation-rules/settings.php b/plugins/speculation-rules/settings.php index 206bd32aac..1923bc62d9 100644 --- a/plugins/speculation-rules/settings.php +++ b/plugins/speculation-rules/settings.php @@ -79,6 +79,7 @@ function plsr_get_stored_setting_value(): array { * Sanitizes the setting for Speculative Loading configuration. * * @since 1.0.0 + * @todo Consider whether the JSON schema for the setting could be reused here. * * @param mixed $input Setting to sanitize. * @return array{ mode: 'prefetch'|'prerender', eagerness: 'conservative'|'moderate'|'eager' } { @@ -122,24 +123,24 @@ function plsr_register_setting(): void { array( 'type' => 'object', 'description' => __( 'Configuration for the Speculation Rules API.', 'speculation-rules' ), - 'sanitize_callback' => 'plsr_sanitize_setting', // TODO: Is this even needed here due to the schema? + 'sanitize_callback' => 'plsr_sanitize_setting', 'default' => plsr_get_setting_default(), 'show_in_rest' => array( 'schema' => array( - 'properties' => array( + 'type' => 'object', + 'properties' => array( 'mode' => array( 'description' => __( 'Whether to prefetch or prerender URLs.', 'speculation-rules' ), 'type' => 'string', 'enum' => array_keys( plsr_get_mode_labels() ), - 'default' => plsr_get_setting_default()['mode'], ), 'eagerness' => array( 'description' => __( 'The eagerness setting defines the heuristics based on which the loading is triggered. "Eager" will have the minimum delay to start speculative loads, "Conservative" increases the chance that only URLs the user actually navigates to are loaded.', 'speculation-rules' ), 'type' => 'string', 'enum' => array_keys( plsr_get_eagerness_labels() ), - 'default' => plsr_get_setting_default()['eagerness'], ), ), + 'additionalProperties' => false, ), ), ) From 2465855ce7ead7c005eab32a9e1d4fba4dedc98f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 24 Jul 2024 13:07:11 -0700 Subject: [PATCH 3/3] Fix since tags --- plugins/speculation-rules/settings.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/speculation-rules/settings.php b/plugins/speculation-rules/settings.php index 1923bc62d9..4304067382 100644 --- a/plugins/speculation-rules/settings.php +++ b/plugins/speculation-rules/settings.php @@ -43,7 +43,7 @@ function plsr_get_eagerness_labels(): array { /** * Returns the default setting value for Speculative Loading configuration. * - * @since n.e.x.t + * @since 1.0.0 * * @return array{ mode: 'prerender', eagerness: 'moderate' } { * Default setting value. @@ -62,7 +62,7 @@ function plsr_get_setting_default(): array { /** * Returns the stored setting value for Speculative Loading configuration. * - * @since 1.0.0 + * @since n.e.x.t * * @return array{ mode: 'prefetch'|'prerender', eagerness: 'conservative'|'moderate'|'eager' } { * Stored setting value.