Skip to content

Ruleset: allow for extending pre-existing array values #1025

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

Merged
merged 2 commits into from
Apr 18, 2025
Merged

Ruleset: allow for extending pre-existing array values #1025

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a0b15d
Ruleset: allow for extending pre-existing array values
jrfnl Apr 3, 2025
5dc0297
Squiz + PHPCS native ruleset: start using the new ability to extend p…
jrfnl Apr 18, 2025
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
4 changes: 1 addition & 3 deletions phpcs.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,7 @@
<!-- Ban some functions -->
<rule ref="Generic.PHP.ForbiddenFunctions">
<properties>
<property name="forbiddenFunctions" type="array">
<element key="sizeof" value="count"/>
<element key="delete" value="unset"/>
<property name="forbiddenFunctions" type="array" extend="true">
<element key="print" value="echo"/>
<element key="is_null" value="null"/>
<element key="create_function" value="null"/>
Expand Down
30 changes: 22 additions & 8 deletions src/Ruleset.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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\"";
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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()

Expand Down
4 changes: 1 addition & 3 deletions src/Standards/Squiz/ruleset.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@
<!-- Ban some functions -->
<rule ref="Generic.PHP.ForbiddenFunctions">
<properties>
<property name="forbiddenFunctions" type="array">
<element key="sizeof" value="count"/>
<element key="delete" value="unset"/>
<property name="forbiddenFunctions" type="array" extend="true">
<element key="print" value="echo"/>
<element key="is_null" value="null"/>
<element key="create_function" value="null"/>
Expand Down
96 changes: 96 additions & 0 deletions tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/SetProperty/PropertyTypeHandlingSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>
*/
public $expectsNonEmptyArrayOverruledToEmpty = [
'key' => 'value',
];

/**
* Used to verify that array properties with a default value allow for (re-)setting the property value.
*
* @var array<string, string>
*/
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<mixed>
*/
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<mixed>
*/
public $expectsExtendsWillOverruleNonArrayToNewArrayValue = true;

/**
* Used to verify that array properties with a default value can get extended.
*
* @var array<string, mixed>
*/
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<string, mixed>
*/
public $expectsNonEmptyArrayKeepsValueWhenExtendedWithEmptyArray = [
'key' => 'value',
];

/**
* Used to verify that array properties with a default value can get extended multiple times.
*
* @var array<string, mixed>
*/
public $expectsNonEmptyArrayDoubleExtendedWithNonEmptyArray = [
'key' => 'value',
];

/**
* Used to verify the value for a specific array key can be overwritten from the ruleset.
*
* @var array<string, mixed>
*/
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<mixed>
*/
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<string|int, mixed>
*/
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<mixed>
*/
public $expectsStringNotArray;

public function register()
{
return [T_ECHO];
Expand Down
74 changes: 72 additions & 2 deletions tests/Core/Ruleset/PropertyTypeHandlingTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
50 changes: 50 additions & 0 deletions tests/Core/Ruleset/PropertyTypeHandlingTest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,56 @@
</property>

<property name="expectsEmptyArray" type="array"/>

<!-- Test array properties in combination with pre-existing array values. -->
<property name="expectsNonEmptyArrayOverruledToEmpty" type="array">
</property>

<property name="expectsNonEmptyArrayOverruledToNewValue" type="array">
<element key="another key" value="another value"/>
</property>

<property name="expectsExtendsWillJustSetToArrayWhenNoDefaultValuePresent" type="array" extend="true">
<element key="foo" value="bar"/>
</property>
<property name="expectsExtendsWillOverruleNonArrayToNewArrayValue" type="array" extend="true">
<element value="phpcbf"/>
</property>

<property name="expectsNonEmptyArrayExtendedWithNonEmptyArray" type="array" extend="true">
<element key="another key" value="another value"/>
</property>

<property name="expectsNonEmptyArrayKeepsValueWhenExtendedWithEmptyArray" type="array" extend="true"/>

<property name="expectsNonEmptyArrayDoubleExtendedWithNonEmptyArray" type="array" extend="true">
<element key="foo" value="bar"/>
</property>

<property name="expectsNonEmptyArrayDoubleExtendedWithNonEmptyArray" type="array" extend="true">
<element key="bar" value="baz"/>
<element key="baz" value="boo"/>
</property>

<property name="expectsValuesInNonEmptyAssociativeArrayCanBeRedefined" type="array" extend="true">
<element key="foo" value="bar"/>
<element key="bar" value="foo"/>
</property>

<property name="expectsValuesInNonEmptyNumericallyIndexedArrayAreNotOverwritten" type="array" extend="true">
<element value="valueB"/>
<element value="valueC"/>
</property>

<property name="expectsPreexistingValuesStayTheSameWhileNewValuesGetCleaned" type="array" extend="true">
<element key="newValueA" value="false"/>
<element key="newValueB" value=" null "/>
<element value="1.5"/>
<element value="true"/>
</property>

<property name="expectsStringNotArray" extend="true" value="some value"/>

</properties>
</rule>

Expand Down
Loading