Skip to content

Style Engine: Improve PHPStan types and avoid fatal errors when storing rules #15823

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

Open
westonruter wants to merge 6 commits into
base: trunk
Choose a base branch
from
Open

Style Engine: Improve PHPStan types and avoid fatal errors when storing rules #15823

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e73cd00
Style Engine: Document raw style values as mixed
westonruter Nov 7, 2026
bf4a6e9
Style Engine: Link docblock references to BLOCK_STYLE_DEFINITIONS_MET…
westonruter Nov 7, 2026
9059592
Style Engine: Add a PHPStan type for style definitions
westonruter Nov 7, 2026
d36b032
Style Engine: Avoid a fatal error for a whitespace-only selector
westonruter Nov 7, 2026
72aa5ae
Style Engine: Only return string values as CSS declarations
westonruter Nov 7, 2026
1aae0fc
Merge branch 'trunk' into fix/style-engine-phpstan-types
westonruter Nov 9, 2026
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
89 changes: 63 additions & 26 deletions src/wp-includes/style-engine/class-wp-style-engine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@
* @since 6.7.0 Added support for typography.writingMode.
* @since 7.0.0 Added support for typography.textIndent.
* @since 7.2.0 Added support for typography.textShadow.
*
* @phpstan-type Style_Definition array{
* property_keys?: array{ default: string, individual?: string },
* path: non-empty-list<string>,
* css_vars?: array<string, string>,
* classnames?: array<string, string|true>,
* value_func?: callable( mixed, array<string, mixed>, array<mixed> ): array<string, string>,
* }
*/
#[AllowDynamicProperties]
final class WP_Style_Engine {
Expand Down Expand Up @@ -59,7 +67,8 @@ final class WP_Style_Engine {
* @since 6.1.0
* @since 7.1.0 Added `background.gradient` property.
* @since 7.2.0 Added `background.backgroundClip` property.
* @var array
* @var array<string, array<string, array>>
* @phpstan-var array<string, array<string, Style_Definition>>
*/
const BLOCK_STYLE_DEFINITIONS_METADATA = array(
'background' => array(
Expand Down Expand Up @@ -403,7 +412,7 @@ final class WP_Style_Engine {
*
* @since 6.1.0
*
* @param string $style_value A single CSS preset value.
* @param mixed $style_value A single CSS preset value.
* @param string $property_key The CSS property that is the second element of the preset string.
* Used for matching.
* @return string The slug, or empty string if not found.
Expand Down Expand Up @@ -448,7 +457,7 @@ protected static function get_css_var_value( $style_value, $css_vars ) {
*
* @since 6.1.0
*
* @param string $style_value A single CSS preset value.
* @param mixed $style_value A single raw style value.
* @return bool
*/
protected static function is_valid_style_value( $style_value ) {
Expand All @@ -473,11 +482,21 @@ protected static function is_valid_style_value( $style_value ) {
* or a CSS nested @rule, such as `@media (min-width: 80rem)`
* or `@layer module`.
*/
public static function store_css_rule( $store_name, $css_selector, $css_declarations, $rules_group = '' ) {
public static function store_css_rule( $store_name, $css_selector, $css_declarations, $rules_group = '' ): void {
if ( empty( $store_name ) || empty( $css_selector ) || empty( $css_declarations ) ) {
return;
}
static::get_store( $store_name )->add_rule( $css_selector, $rules_group )->add_declarations( $css_declarations );

$store = static::get_store( $store_name );
if ( ! $store ) {
return;
}

// The rule is null when the selector is only whitespace.
$rule = $store->add_rule( $css_selector, $rules_group );
if ( $rule ) {
$rule->add_declarations( $css_declarations );
}
}

/**
Expand All @@ -495,12 +514,12 @@ public static function get_store( $store_name ) {
/**
* Returns classnames and CSS based on the values in a styles object.
*
* Return values are parsed based on the instructions in BLOCK_STYLE_DEFINITIONS_METADATA.
* Return values are parsed based on the instructions in {@see self::BLOCK_STYLE_DEFINITIONS_METADATA}.
*
* @since 6.1.0
*
* @param array $block_styles The style object.
* @param array $options {
* @param array<string, mixed> $block_styles The style object.
* @param array $options {
* Optional. An array of options. Default empty array.
*
* @type bool $convert_vars_to_classnames Whether to skip converting incoming CSS var patterns,
Expand Down Expand Up @@ -567,10 +586,13 @@ public static function parse_block_styles( $block_styles, $options ) {
*
* @since 6.1.0
*
* @param string $style_value A single raw style value or CSS preset property
* @param mixed $style_value A single raw style value or CSS preset property
* from the `$block_styles` array.
* @param array $style_definition A single style definition from BLOCK_STYLE_DEFINITIONS_METADATA.
* @param array $style_definition A single style definition from {@see self::BLOCK_STYLE_DEFINITIONS_METADATA}.
* @return string[] An array of CSS classnames, or empty array if there are none.
*
* @phpstan-param Style_Definition $style_definition
* @phpstan-return list<string>
*/
protected static function get_classnames( $style_value, $style_definition ) {
if ( empty( $style_value ) ) {
Expand All @@ -589,7 +611,7 @@ protected static function get_classnames( $style_value, $style_definition ) {

if ( $slug ) {
/*
* Right now we expect a classname pattern to be stored in BLOCK_STYLE_DEFINITIONS_METADATA.
* Right now we expect a classname pattern to be stored in self::BLOCK_STYLE_DEFINITIONS_METADATA.
* One day, if there are no stored schemata, we could allow custom patterns or
* generate classnames based on other properties
* such as a path or a value or a prefix passed in options.
Expand All @@ -608,7 +630,7 @@ protected static function get_classnames( $style_value, $style_definition ) {
* @since 6.1.0
*
* @param mixed $style_value A single raw style value from $block_styles array.
* @param array $style_definition A single style definition from BLOCK_STYLE_DEFINITIONS_METADATA.
* @param array $style_definition A single style definition from {@see self::BLOCK_STYLE_DEFINITIONS_METADATA}.
* @param array $options {
* Optional. An array of options. Default empty array.
*
Expand All @@ -617,12 +639,18 @@ protected static function get_classnames( $style_value, $style_definition ) {
* to `var( --wp--preset--* )` values. Default false.
* }
* @return string[] An associative array of CSS definitions, e.g. `array( "$property" => "$value", "$property" => "$value" )`.
*
* @phpstan-param Style_Definition $style_definition
*/
protected static function get_css_declarations( $style_value, $style_definition, $options = array() ) {
if ( isset( $style_definition['value_func'] ) && is_callable( $style_definition['value_func'] ) ) {
return call_user_func( $style_definition['value_func'], $style_value, $style_definition, $options );
}

if ( ! isset( $style_definition['property_keys'] ) ) {
return array();
}

$css_declarations = array();
$style_property_keys = $style_definition['property_keys'];
$should_skip_css_vars = isset( $options['convert_vars_to_classnames'] ) && true === $options['convert_vars_to_classnames'];
Expand Down Expand Up @@ -659,15 +687,18 @@ protected static function get_css_declarations( $style_value, $style_definition,

$individual_property = sprintf( $style_property_keys['individual'], _wp_to_kebab_case( $key ) );

if ( $individual_property && static::is_valid_style_value( $value ) ) {
if ( $individual_property && is_string( $value ) && static::is_valid_style_value( $value ) ) {
$css_declarations[ $individual_property ] = $value;
}
}

return $css_declarations;
}

$css_declarations[ $style_property_keys['default'] ] = $style_value;
// CSS declarations only accept string values.
if ( is_string( $style_value ) ) {
$css_declarations[ $style_property_keys['default'] ] = $style_value;
}
return $css_declarations;
}

Expand All @@ -689,8 +720,8 @@ protected static function get_css_declarations( $style_value, $style_definition,
*
* @since 6.1.0
*
* @param array $style_value A single raw style value from `$block_styles` array.
* @param array $individual_property_definition A single style definition from BLOCK_STYLE_DEFINITIONS_METADATA
* @param mixed $style_value A single raw style value from `$block_styles` array.
* @param array $individual_property_definition A single style definition from {@see self::BLOCK_STYLE_DEFINITIONS_METADATA}
* representing an individual property of a CSS property,
* e.g. 'top' in 'border-top'.
* @param array $options {
Expand All @@ -701,6 +732,8 @@ protected static function get_css_declarations( $style_value, $style_definition,
* to `var( --wp--preset--* )` values. Default false.
* }
* @return string[] An associative array of CSS definitions, e.g. `array( "$property" => "$value", "$property" => "$value" )`.
*
* @phpstan-param Style_Definition $individual_property_definition
*/
protected static function get_individual_property_css_declarations( $style_value, $individual_property_definition, $options = array() ) {
if ( ! is_array( $style_value ) || empty( $style_value ) || empty( $individual_property_definition['path'] ) ) {
Expand All @@ -721,17 +754,17 @@ protected static function get_individual_property_css_declarations( $style_value
$css_declarations = array();

foreach ( $style_value as $css_property => $value ) {
if ( empty( $value ) ) {
// CSS declarations only accept string values.
if ( empty( $value ) || ! is_string( $value ) ) {
continue;
}

// Build a path to the individual rules in definitions.
$style_definition_path = array( $definition_group_key, $css_property );
$style_definition = _wp_array_get( static::BLOCK_STYLE_DEFINITIONS_METADATA, $style_definition_path, null );
// Get the corresponding style definition from the same group, e.g. `border.color`.
$style_definition = static::BLOCK_STYLE_DEFINITIONS_METADATA[ $definition_group_key ][ $css_property ] ?? null;

if ( $style_definition && isset( $style_definition['property_keys']['individual'] ) ) {
// Set a CSS var if there is a valid preset value.
if ( is_string( $value ) && str_contains( $value, 'var:' )
if ( str_contains( $value, 'var:' )
&& ! $should_skip_css_vars && ! empty( $individual_property_definition['css_vars'] )
) {
$value = static::get_css_var_value( $value, $individual_property_definition['css_vars'] );
Expand All @@ -753,9 +786,12 @@ protected static function get_individual_property_css_declarations( $style_value
*
* @since 6.4.0
*
* @param array $style_value A single raw style value from $block_styles array.
* @param array $style_definition A single style definition from BLOCK_STYLE_DEFINITIONS_METADATA.
* @param mixed $style_value A single raw style value from $block_styles array.
* @param array $style_definition A single style definition from {@see self::BLOCK_STYLE_DEFINITIONS_METADATA}.
* @return string[] An associative array of CSS definitions, e.g., array( "$property" => "$value", "$property" => "$value" ).
*
* @phpstan-param Style_Definition $style_definition
* @phpstan-return array<string, string>
*/
protected static function get_url_or_value_css_declaration( $style_value, $style_definition ) {
if ( empty( $style_value ) ) {
Expand All @@ -767,7 +803,7 @@ protected static function get_url_or_value_css_declaration( $style_value, $style
if ( isset( $style_definition['property_keys']['default'] ) ) {
$value = null;

if ( ! empty( $style_value['url'] ) ) {
if ( is_array( $style_value ) && ! empty( $style_value['url'] ) && is_string( $style_value['url'] ) ) {
$value = "url('" . $style_value['url'] . "')";
} elseif ( is_string( $style_value ) ) {
$value = $style_value;
Expand All @@ -789,10 +825,11 @@ protected static function get_url_or_value_css_declaration( $style_value, $style
*
* @since 7.2.0
*
* @param string $style_value A single raw style value from $block_styles array.
* @param array $style_definition A single style definition from BLOCK_STYLE_DEFINITIONS_METADATA.
* @param mixed $style_value A single raw style value from $block_styles array.
* @param array $style_definition A single style definition from {@see self::BLOCK_STYLE_DEFINITIONS_METADATA}.
* @return array<string, string> An associative array of CSS definitions, mapping properties to values.
*
* @phpstan-param Style_Definition $style_definition
* @phpstan-return array<non-falsy-string, non-falsy-string>
*/
protected static function get_background_clip_css_declarations( $style_value, $style_definition ) {
Expand Down
56 changes: 56 additions & 0 deletions tests/phpunit/tests/style-engine/styleEngine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -995,4 +995,60 @@ public function test_should_return_stylesheet_with_nested_rules() {

$this->assertSame( '.foo{@media (orientation: landscape){background-color:blue;}}.foo{@media (min-width > 1024px){background-color:cotton-blue;}}', $compiled_stylesheet );
}

/**
* Tests that storing a rule whose selector is only whitespace is skipped
* instead of calling {@see WP_Style_Engine_CSS_Rule::add_declarations()} on null.
*
* @covers ::wp_style_engine_get_stylesheet_from_css_rules
* @covers WP_Style_Engine::store_css_rule
*/
public function test_should_skip_storing_rule_with_whitespace_selector() {
$css_rules = array(
array(
'selector' => ' ',
'declarations' => array(
'color' => 'red',
),
),
);

wp_style_engine_get_stylesheet_from_css_rules( $css_rules, array( 'context' => 'test-whitespace-selector' ) );

$store = WP_Style_Engine::get_store( 'test-whitespace-selector' );
$this->assertInstanceOf( WP_Style_Engine_CSS_Rules_Store::class, $store );
$this->assertSame( array(), $store->get_all_rules(), 'Expected no rule to be stored.' );
}

/**
* Tests that non-string style values are left out of the returned declarations,
* since {@see WP_Style_Engine_CSS_Declarations::add_declaration()} would drop them from the CSS anyway.
*

@ramonjd ramonjd Nov 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure if this and above needs @ticket

@westonruter westonruter Nov 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Since the only ticket is the catch-all Core-65817 for PHPStan fixes, I don't think it would be helpful.

* @covers ::wp_style_engine_get_styles
*/
public function test_should_omit_non_string_values_from_declarations() {
$block_styles = array(
'typography' => array(
'textColumns' => 2,
'fontSize' => '12px',
),
'spacing' => array(
'padding' => array(
'top' => 5,
'left' => '1em',
),
),

@ramonjd ramonjd Nov 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Optional: Maybe add border.top.width: 5 to cover the individual properties for that style?

@westonruter westonruter Nov 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Could you amend the PR with what you have in mind?

);

$this->assertSame(
array(
'css' => 'padding-left:1em;font-size:12px;',
'declarations' => array(
'padding-left' => '1em',
'font-size' => '12px',
),
),
wp_style_engine_get_styles( $block_styles )
);
}
}
Loading