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.

Sign up for GitHub

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

Add strict PHPStan rules for Speculative Loading #1392

Merged
merged 3 commits into from
Jul 24, 2024
Merged

Add strict PHPStan rules for Speculative Loading #1392

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7ffe2b1
Add strict PHPStan rules for Speculative Loading
westonruter Jul 23, 2024
97c7254
Refine JSON schema
westonruter Jul 23, 2024
2465855
Fix since tags
westonruter Jul 24, 2024
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
1 change: 0 additions & 1 deletion phpstan.neon.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,4 @@ parameters:
- */tests/*
- plugins/dominant-color-images
- plugins/performance-lab
- plugins/speculation-rules
- plugins/webp-uploads
16 changes: 3 additions & 13 deletions plugins/speculation-rules/helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,11 @@
*
* @since 1.0.0
*
* @return array<string, array<int, array<string, mixed>>> Associative array of speculation rules by type.
* @return non-empty-array<string, array<int, array<string, mixed>>> 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();
Expand Down
7 changes: 1 addition & 6 deletions plugins/speculation-rules/hooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,8 @@
* @since 1.0.0
*/
function plsr_print_speculation_rules(): void {
$rules = plsr_get_speculation_rules();
if ( empty( $rules ) ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This function never returns an empty array, so this condition is dead code.

return;
}

wp_print_inline_script_tag(
(string) wp_json_encode( $rules ),
(string) wp_json_encode( plsr_get_speculation_rules() ),
array( 'type' => 'speculationrules' )
);
}
Expand Down
85 changes: 45 additions & 40 deletions plugins/speculation-rules/settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*
* @since 1.0.0
*
* @return array<string, string> 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(
Expand All @@ -30,7 +30,7 @@ function plsr_get_mode_labels(): array {
*
* @since 1.0.0
*
* @return array<string, string> 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(
Expand All @@ -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
westonruter marked this conversation as resolved.
Show resolved Hide resolved
*
* @return array<string, string> {
* @return array{ mode: 'prerender', eagerness: 'moderate' } {
* Default setting value.
*
* @type string $mode Mode.
Expand All @@ -59,13 +59,29 @@ function plsr_get_setting_default(): array {
);
}

/**
* Returns the stored setting value for Speculative Loading configuration.
*
* @since 1.0.0
westonruter marked this conversation as resolved.
Show resolved Hide resolved
*
* @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' ) );
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 plsr_sanitize_setting() function could perhaps be eliminated in favor of the JSON schema.

}

/**
* Sanitizes the setting for Speculative Loading configuration.
*
* @since 1.0.0
*
* @param mixed $input Setting to sanitize.
* @return array<string, string> {
* @return array{ mode: 'prefetch'|'prerender', eagerness: 'conservative'|'moderate'|'eager' } {
* Sanitized setting.
*
* @type string $mode Mode.
Expand All @@ -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'];
}

Expand All @@ -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?
westonruter marked this conversation as resolved.
Show resolved Hide resolved
'default' => plsr_get_setting_default(),
'show_in_rest' => array(
'schema' => array(
Expand All @@ -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'],
),
),
),
Expand Down Expand Up @@ -188,7 +203,7 @@ static function (): void {
* @since 1.0.0
* @access private
*
* @param array<string, string> $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.
Expand All @@ -197,28 +212,24 @@ static function (): void {
* }
*/
function plsr_render_settings_field( array $args ): void {
if ( empty( $args['field'] ) || empty( $args['title'] ) ) { // Invalid.
return;
}
Comment on lines -200 to -202
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 field and title args are always provided in plsr_add_setting_ui().


$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'] ];
?>
<fieldset>
<legend class="screen-reader-text"><?php echo esc_html( $args['title'] ); ?></legend>
<?php
foreach ( $choices as $slug => $label ) {
?>
<?php foreach ( $choices as $slug => $label ) : ?>
<p>
<label>
<input
Expand All @@ -230,17 +241,11 @@ function plsr_render_settings_field( array $args ): void {
<?php echo esc_html( $label ); ?>
</label>
</p>
<?php
}
<?php endforeach; ?>

if ( ! empty( $args['description'] ) ) {
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 description is always provided in plsr_add_setting_ui().

?>
<p class="description" style="max-width: 800px;">
<?php echo esc_html( $args['description'] ); ?>
</p>
<?php
}
?>
<p class="description" style="max-width: 800px;">
<?php echo esc_html( $args['description'] ); ?>
</p>
</fieldset>
<?php
}
Expand Down