diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 14f318645f..237034fa7d 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -125,9 +125,7 @@ - - - + diff --git a/src/Ruleset.php b/src/Ruleset.php index 5e3f378893..c9a89d0c9d 100644 --- a/src/Ruleset.php +++ b/src/Ruleset.php @@ -1267,11 +1267,14 @@ private function processRule($rule, $newSniffs, $depth=0) } $values = []; - if (isset($prop['extend']) === true - && (string) $prop['extend'] === 'true' - && isset($this->ruleset[$code]['properties'][$name]['value']) === true - ) { - $values = $this->ruleset[$code]['properties'][$name]['value']; + $extend = false; + if (isset($prop['extend']) === true && (string) $prop['extend'] === 'true') { + if (isset($this->ruleset[$code]['properties'][$name]['value']) === true) { + $values = $this->ruleset[$code]['properties'][$name]['value']; + $extend = $this->ruleset[$code]['properties'][$name]['extend']; + } else { + $extend = true; + } } if (isset($prop->element) === true) { @@ -1296,8 +1299,9 @@ private function processRule($rule, $newSniffs, $depth=0) } $this->ruleset[$code]['properties'][$name] = [ - 'value' => $values, - 'scope' => $propertyScope, + 'value' => $values, + 'scope' => $propertyScope, + 'extend' => $extend, ]; if (PHP_CODESNIFFER_VERBOSITY > 1) { $statusMessage = "=> array property \"$name\" set to \"$printValue\""; @@ -1602,6 +1606,7 @@ public function populateTokenListeners() * @param string $sniffClass The class name of the sniff. * @param string $name The name of the property to change. * @param array $settings Array with the new value of the property and the scope of the property being set. + * May optionally include an `extend` key to indicate a pre-existing array value should be extended. * * @return void * @@ -1660,7 +1665,16 @@ public function setSniffProperty($sniffClass, $name, $settings) $value = $this->getRealPropertyValue($values); } - $sniffObject->$propertyName = $value; + if (isset($settings['extend']) === true + && $settings['extend'] === true + && isset($sniffObject->$propertyName) === true + && is_array($sniffObject->$propertyName) === true + && is_array($value) === true + ) { + $sniffObject->$propertyName = array_merge($sniffObject->$propertyName, $value); + } else { + $sniffObject->$propertyName = $value; + } }//end setSniffProperty() diff --git a/src/Standards/Squiz/ruleset.xml b/src/Standards/Squiz/ruleset.xml index d1d16588e1..915586d157 100644 --- a/src/Standards/Squiz/ruleset.xml +++ b/src/Standards/Squiz/ruleset.xml @@ -55,9 +55,7 @@ - - - + diff --git a/tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/SetProperty/PropertyTypeHandlingSniff.php b/tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/SetProperty/PropertyTypeHandlingSniff.php index 5a2d4888c4..825f8ac5fe 100644 --- a/tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/SetProperty/PropertyTypeHandlingSniff.php +++ b/tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/SetProperty/PropertyTypeHandlingSniff.php @@ -150,6 +150,102 @@ final class PropertyTypeHandlingSniff implements Sniff */ public $expectsEmptyArray; + /** + * Used to verify that array properties with a default value allow for (re-)setting the property to an empty array. + * + * @var array + */ + public $expectsNonEmptyArrayOverruledToEmpty = [ + 'key' => 'value', + ]; + + /** + * Used to verify that array properties with a default value allow for (re-)setting the property value. + * + * @var array + */ + public $expectsNonEmptyArrayOverruledToNewValue = [ + 'key' => 'value', + ]; + + /** + * Used to safeguard that if `extend=true` is used on a property without pre-existing value, this will not cause errors. + * + * @var array + */ + public $expectsExtendsWillJustSetToArrayWhenNoDefaultValuePresent; + + /** + * Used to document that if `extend=true` is used on a property which doesn't have an array value, the value will be overruled. + * (= original behaviour, no change) + * + * @var array + */ + public $expectsExtendsWillOverruleNonArrayToNewArrayValue = true; + + /** + * Used to verify that array properties with a default value can get extended. + * + * @var array + */ + public $expectsNonEmptyArrayExtendedWithNonEmptyArray = [ + 'key' => 'value', + ]; + + /** + * Used to verify that array properties with a default value which are extended by an empty array do not get reset. + * + * @var array + */ + public $expectsNonEmptyArrayKeepsValueWhenExtendedWithEmptyArray = [ + 'key' => 'value', + ]; + + /** + * Used to verify that array properties with a default value can get extended multiple times. + * + * @var array + */ + public $expectsNonEmptyArrayDoubleExtendedWithNonEmptyArray = [ + 'key' => 'value', + ]; + + /** + * Used to verify the value for a specific array key can be overwritten from the ruleset. + * + * @var array + */ + public $expectsValuesInNonEmptyAssociativeArrayCanBeRedefined = [ + 'foo' => 'foo', + 'bar' => 'bar', + ]; + + /** + * Used to verify that non-keyed values are added to the array and do not overwrite existing values. + * + * @var array + */ + public $expectsValuesInNonEmptyNumericallyIndexedArrayAreNotOverwritten = [ + 'valueA', + ]; + + /** + * Used to verify that a default value for an array does not get the cleaning/type juggling treatment, while the ruleset added values do. + * + * @var array + */ + public $expectsPreexistingValuesStayTheSameWhileNewValuesGetCleaned = [ + 'predefinedA' => 'true', + 'predefinedB' => ' null ', + ]; + + /** + * Used to verify that if `extend` is used on a non-array property, the value still gets set, but not as an array. + * + * @var array + */ + public $expectsStringNotArray; + public function register() { return [T_ECHO]; diff --git a/tests/Core/Ruleset/PropertyTypeHandlingTest.php b/tests/Core/Ruleset/PropertyTypeHandlingTest.php index 447bff1963..d575729bf7 100644 --- a/tests/Core/Ruleset/PropertyTypeHandlingTest.php +++ b/tests/Core/Ruleset/PropertyTypeHandlingTest.php @@ -203,14 +203,84 @@ public static function dataArrayPropertyExtending() ]; return [ - 'Array with only values extended' => [ + 'Array with only values extended' => [ 'propertyName' => 'expectsArrayWithExtendedValues', 'expected' => $expectedArrayOnlyValuesExtended, ], - 'Array with keys and values extended' => [ + 'Array with keys and values extended' => [ 'propertyName' => 'expectsArrayWithExtendedKeysAndValues', 'expected' => $expectedArrayKeysAndValuesExtended, ], + + 'Empty array in ruleset overrules existing value' => [ + 'propertyName' => 'expectsNonEmptyArrayOverruledToEmpty', + 'expected' => [], + ], + 'Non empty array in ruleset overrules existing value' => [ + 'propertyName' => 'expectsNonEmptyArrayOverruledToNewValue', + 'expected' => ['another key' => 'another value'], + ], + + 'Extending pre-existing value when there is no value' => [ + 'propertyName' => 'expectsExtendsWillJustSetToArrayWhenNoDefaultValuePresent', + 'expected' => ['foo' => 'bar'], + ], + 'Extending pre-existing non-array value will overrule' => [ + 'propertyName' => 'expectsExtendsWillOverruleNonArrayToNewArrayValue', + 'expected' => ['phpcbf'], + ], + 'Non empty array extended by non-empty array' => [ + 'propertyName' => 'expectsNonEmptyArrayExtendedWithNonEmptyArray', + 'expected' => [ + 'key' => 'value', + 'another key' => 'another value', + ], + ], + 'Non empty array keeps value when extended by empty array' => [ + 'propertyName' => 'expectsNonEmptyArrayKeepsValueWhenExtendedWithEmptyArray', + 'expected' => ['key' => 'value'], + ], + + 'Non empty array double extended get both additions' => [ + 'propertyName' => 'expectsNonEmptyArrayDoubleExtendedWithNonEmptyArray', + 'expected' => [ + 'key' => 'value', + 'foo' => 'bar', + 'bar' => 'baz', + 'baz' => 'boo', + ], + ], + + 'Values in non empty associative array can be redefined' => [ + 'propertyName' => 'expectsValuesInNonEmptyAssociativeArrayCanBeRedefined', + 'expected' => [ + 'foo' => 'bar', + 'bar' => 'foo', + ], + ], + 'Values in non empty numerically indexed array are not overwritten' => [ + 'propertyName' => 'expectsValuesInNonEmptyNumericallyIndexedArrayAreNotOverwritten', + 'expected' => [ + 'valueA', + 'valueB', + 'valueC', + ], + ], + 'Original values are untouched, while new values get cleaned' => [ + 'propertyName' => 'expectsPreexistingValuesStayTheSameWhileNewValuesGetCleaned', + 'expected' => [ + 'predefinedA' => 'true', + 'predefinedB' => ' null ', + 'newValueA' => false, + 'newValueB' => null, + '1.5', // phpcs:ignore Squiz.Arrays.ArrayDeclaration.NoKeySpecified -- That is largely what we are testing... + true, + ], + ], + 'Invalid "extend" used on a non-array property' => [ + 'propertyName' => 'expectsStringNotArray', + 'expected' => 'some value', + ], ]; }//end dataArrayPropertyExtending() diff --git a/tests/Core/Ruleset/PropertyTypeHandlingTest.xml b/tests/Core/Ruleset/PropertyTypeHandlingTest.xml index 6ad843dea7..4e3a4e8ebf 100644 --- a/tests/Core/Ruleset/PropertyTypeHandlingTest.xml +++ b/tests/Core/Ruleset/PropertyTypeHandlingTest.xml @@ -60,6 +60,56 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +