From e38b254f939e6d8608db2a348877477f8fddf37e Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 6 Aug 2025 14:04:24 -0300 Subject: [PATCH 1/2] Ruleset::setSniffProperty(): add extra test with empty string as array value Add tests for a scenario that I found on the WPCS codebase that I believe was not documented in a test in PHPCS, and I'm not sure if it is intentional or not. Also, not sure if it should be documented explicitly in the upgrade guide. https://github.com/WordPress/WordPress-Coding-Standards/blob/4d0160f32f8537f3cdf2e301433cb15641083963/WordPress/Tests/NamingConventions/PrefixAllGlobalsUnitTest.1.inc#L513 Co-authored-by: jrfnl --- .../Core/Ruleset/Fixtures/PropertyTypeHandlingInline.inc | 8 ++++---- tests/Core/Ruleset/PropertyTypeHandlingTest.php | 2 ++ tests/Core/Ruleset/PropertyTypeHandlingTest.xml | 6 ++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/Core/Ruleset/Fixtures/PropertyTypeHandlingInline.inc b/tests/Core/Ruleset/Fixtures/PropertyTypeHandlingInline.inc index 31caeb6198..e40ecf3c4c 100644 --- a/tests/Core/Ruleset/Fixtures/PropertyTypeHandlingInline.inc +++ b/tests/Core/Ruleset/Fixtures/PropertyTypeHandlingInline.inc @@ -20,12 +20,12 @@ // phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsBooleanFalseCase fALSe // phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsBooleanFalseTrimmed false -// phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsArrayWithOnlyValues[] string, 10, 1.5, null, true, false -// phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsArrayWithKeysAndValues[] string=>string,10=>10,float=>1.5,null=>null,true=>true,false=>false +// phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsArrayWithOnlyValues[] string, 10, 1.5, , null, true, false +// phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsArrayWithKeysAndValues[] string=>string,10=>10,float=>1.5,=>,null=>null,true=>true,false=>false // phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsEmptyArray[] -// phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsOldSchoolArrayWithOnlyValues[] string, 10, 1.5, null, true, false -// phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsOldSchoolArrayWithKeysAndValues[] string=>string,10=>10,float=>1.5,null=>null,true=>true,false=>false +// phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsOldSchoolArrayWithOnlyValues[] string, 10, 1.5, , null, true, false +// phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsOldSchoolArrayWithKeysAndValues[] string=>string,10=>10,float=>1.5,=>,null=>null,true=>true,false=>false // phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsOldSchoolEmptyArray[] echo 'hello!'; diff --git a/tests/Core/Ruleset/PropertyTypeHandlingTest.php b/tests/Core/Ruleset/PropertyTypeHandlingTest.php index 1db15aa5d2..de71ba3980 100644 --- a/tests/Core/Ruleset/PropertyTypeHandlingTest.php +++ b/tests/Core/Ruleset/PropertyTypeHandlingTest.php @@ -120,6 +120,7 @@ public static function dataTypeHandling() 'string', '10', '1.5', + '', 'null', 'true', 'false', @@ -128,6 +129,7 @@ public static function dataTypeHandling() 'string' => 'string', 10 => '10', 'float' => '1.5', + 11 => '', 'null' => 'null', 'true' => 'true', 'false' => 'false', diff --git a/tests/Core/Ruleset/PropertyTypeHandlingTest.xml b/tests/Core/Ruleset/PropertyTypeHandlingTest.xml index 2e89a37b55..eaca667532 100644 --- a/tests/Core/Ruleset/PropertyTypeHandlingTest.xml +++ b/tests/Core/Ruleset/PropertyTypeHandlingTest.xml @@ -25,6 +25,7 @@ + @@ -34,6 +35,7 @@ + @@ -60,9 +62,9 @@ - + - + From 0fc21ddb83726b2a6d5bdf3eb75ccd38d4eaa1fb Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 16 Aug 2025 15:10:00 +0200 Subject: [PATCH 2/2] Ruleset::processRule(): fix edge case bug - inconsistent handling of empty string array key Unlikely scenario for real-life, but still a bug. When an array property is set via the old comma-separated string ruleset format or via an inline `phpcs:set` annotation, and the key for an array element would be explicitly set to an empty string, the resulting array item in the property on the sniff class would be added without a key, resulting in a numeric key. However, if the (not so) "new" ruleset format using `` nodes is used and an array element key is an empty string, this would result in the array item being added with an empty string for the array key. This is inconsistent behaviour and is now fixed. As 2 out of 3 scenarios would result in a numeric key for PHPCS 3.x, the behaviour has now been standardized as such. Note: for PHPCS 4.x, an empty string array key will remain an empty string and will not become a numeric key. This is in-line with the type handling proposal in 708, which explicitly states: > I do NOT intend to change the handling of array keys. These will remain strings at all times. --- src/Ruleset.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Ruleset.php b/src/Ruleset.php index d1ef2a4d75..7614998171 100644 --- a/src/Ruleset.php +++ b/src/Ruleset.php @@ -1215,7 +1215,7 @@ private function processRule($rule, $newSniffs, $depth=0) } $value = (string) $element['value']; - if (isset($element['key']) === true) { + if (isset($element['key']) === true && trim($element['key']) !== '') { $key = (string) $element['key']; $values[$key] = $value; $printValue .= $key.'=>'.$value.',';