Skip to content

Commit 7a3c1e3

Browse files
jrfnljrfnl
authored
Merge pull request #1025 from PHPCSStandards/phpcs-4.0/feature/15-property-array-extend-original-sniff-property-value
Ruleset: allow for extending pre-existing array values
2 parents cfe06b9 + 5dc0297 commit 7a3c1e3

File tree

6 files changed

+242
-16
lines changed

6 files changed

+242
-16
lines changed

‎phpcs.xml.dist

+1-3
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,7 @@
125125
<!-- Ban some functions -->
126126
<rule ref="Generic.PHP.ForbiddenFunctions">
127127
<properties>
128-
<property name="forbiddenFunctions" type="array">
129-
<element key="sizeof" value="count"/>
130-
<element key="delete" value="unset"/>
128+
<property name="forbiddenFunctions" type="array" extend="true">
131129
<element key="print" value="echo"/>
132130
<element key="is_null" value="null"/>
133131
<element key="create_function" value="null"/>

‎src/Ruleset.php

+22-8
Original file line numberDiff line numberDiff line change
@@ -1267,11 +1267,14 @@ private function processRule($rule, $newSniffs, $depth=0)
12671267
}
12681268

12691269
$values = [];
1270-
if (isset($prop['extend']) === true
1271-
&& (string) $prop['extend'] === 'true'
1272-
&& isset($this->ruleset[$code]['properties'][$name]['value']) === true
1273-
) {
1274-
$values = $this->ruleset[$code]['properties'][$name]['value'];
1270+
$extend = false;
1271+
if (isset($prop['extend']) === true && (string) $prop['extend'] === 'true') {
1272+
if (isset($this->ruleset[$code]['properties'][$name]['value']) === true) {
1273+
$values = $this->ruleset[$code]['properties'][$name]['value'];
1274+
$extend = $this->ruleset[$code]['properties'][$name]['extend'];
1275+
} else {
1276+
$extend = true;
1277+
}
12751278
}
12761279

12771280
if (isset($prop->element) === true) {
@@ -1296,8 +1299,9 @@ private function processRule($rule, $newSniffs, $depth=0)
12961299
}
12971300

12981301
$this->ruleset[$code]['properties'][$name] = [
1299-
'value' => $values,
1300-
'scope' => $propertyScope,
1302+
'value' => $values,
1303+
'scope' => $propertyScope,
1304+
'extend' => $extend,
13011305
];
13021306
if (PHP_CODESNIFFER_VERBOSITY > 1) {
13031307
$statusMessage = "=> array property \"$name\" set to \"$printValue\"";
@@ -1602,6 +1606,7 @@ public function populateTokenListeners()
16021606
* @param string $sniffClass The class name of the sniff.
16031607
* @param string $name The name of the property to change.
16041608
* @param array $settings Array with the new value of the property and the scope of the property being set.
1609+
* May optionally include an `extend` key to indicate a pre-existing array value should be extended.
16051610
*
16061611
* @return void
16071612
*
@@ -1660,7 +1665,16 @@ public function setSniffProperty($sniffClass, $name, $settings)
16601665
$value = $this->getRealPropertyValue($values);
16611666
}
16621667

1663-
$sniffObject->$propertyName = $value;
1668+
if (isset($settings['extend']) === true
1669+
&& $settings['extend'] === true
1670+
&& isset($sniffObject->$propertyName) === true
1671+
&& is_array($sniffObject->$propertyName) === true
1672+
&& is_array($value) === true
1673+
) {
1674+
$sniffObject->$propertyName = array_merge($sniffObject->$propertyName, $value);
1675+
} else {
1676+
$sniffObject->$propertyName = $value;
1677+
}
16641678

16651679
}//end setSniffProperty()
16661680

‎src/Standards/Squiz/ruleset.xml

+1-3
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,7 @@
5555
<!-- Ban some functions -->
5656
<rule ref="Generic.PHP.ForbiddenFunctions">
5757
<properties>
58-
<property name="forbiddenFunctions" type="array">
59-
<element key="sizeof" value="count"/>
60-
<element key="delete" value="unset"/>
58+
<property name="forbiddenFunctions" type="array" extend="true">
6159
<element key="print" value="echo"/>
6260
<element key="is_null" value="null"/>
6361
<element key="create_function" value="null"/>

‎tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/SetProperty/PropertyTypeHandlingSniff.php

+96
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,102 @@ final class PropertyTypeHandlingSniff implements Sniff
150150
*/
151151
public $expectsEmptyArray;
152152

153+
/**
154+
* Used to verify that array properties with a default value allow for (re-)setting the property to an empty array.
155+
*
156+
* @var array<string, string>
157+
*/
158+
public $expectsNonEmptyArrayOverruledToEmpty = [
159+
'key' => 'value',
160+
];
161+
162+
/**
163+
* Used to verify that array properties with a default value allow for (re-)setting the property value.
164+
*
165+
* @var array<string, string>
166+
*/
167+
public $expectsNonEmptyArrayOverruledToNewValue = [
168+
'key' => 'value',
169+
];
170+
171+
/**
172+
* Used to safeguard that if `extend=true` is used on a property without pre-existing value, this will not cause errors.
173+
*
174+
* @var array<mixed>
175+
*/
176+
public $expectsExtendsWillJustSetToArrayWhenNoDefaultValuePresent;
177+
178+
/**
179+
* Used to document that if `extend=true` is used on a property which doesn't have an array value, the value will be overruled.
180+
* (= original behaviour, no change)
181+
*
182+
* @var array<mixed>
183+
*/
184+
public $expectsExtendsWillOverruleNonArrayToNewArrayValue = true;
185+
186+
/**
187+
* Used to verify that array properties with a default value can get extended.
188+
*
189+
* @var array<string, mixed>
190+
*/
191+
public $expectsNonEmptyArrayExtendedWithNonEmptyArray = [
192+
'key' => 'value',
193+
];
194+
195+
/**
196+
* Used to verify that array properties with a default value which are extended by an empty array do not get reset.
197+
*
198+
* @var array<string, mixed>
199+
*/
200+
public $expectsNonEmptyArrayKeepsValueWhenExtendedWithEmptyArray = [
201+
'key' => 'value',
202+
];
203+
204+
/**
205+
* Used to verify that array properties with a default value can get extended multiple times.
206+
*
207+
* @var array<string, mixed>
208+
*/
209+
public $expectsNonEmptyArrayDoubleExtendedWithNonEmptyArray = [
210+
'key' => 'value',
211+
];
212+
213+
/**
214+
* Used to verify the value for a specific array key can be overwritten from the ruleset.
215+
*
216+
* @var array<string, mixed>
217+
*/
218+
public $expectsValuesInNonEmptyAssociativeArrayCanBeRedefined = [
219+
'foo' => 'foo',
220+
'bar' => 'bar',
221+
];
222+
223+
/**
224+
* Used to verify that non-keyed values are added to the array and do not overwrite existing values.
225+
*
226+
* @var array<mixed>
227+
*/
228+
public $expectsValuesInNonEmptyNumericallyIndexedArrayAreNotOverwritten = [
229+
'valueA',
230+
];
231+
232+
/**
233+
* Used to verify that a default value for an array does not get the cleaning/type juggling treatment, while the ruleset added values do.
234+
*
235+
* @var array<string|int, mixed>
236+
*/
237+
public $expectsPreexistingValuesStayTheSameWhileNewValuesGetCleaned = [
238+
'predefinedA' => 'true',
239+
'predefinedB' => ' null ',
240+
];
241+
242+
/**
243+
* Used to verify that if `extend` is used on a non-array property, the value still gets set, but not as an array.
244+
*
245+
* @var array<mixed>
246+
*/
247+
public $expectsStringNotArray;
248+
153249
public function register()
154250
{
155251
return [T_ECHO];

‎tests/Core/Ruleset/PropertyTypeHandlingTest.php

+72-2
Original file line numberDiff line numberDiff line change
@@ -203,14 +203,84 @@ public static function dataArrayPropertyExtending()
203203
];
204204

205205
return [
206-
'Array with only values extended' => [
206+
'Array with only values extended' => [
207207
'propertyName' => 'expectsArrayWithExtendedValues',
208208
'expected' => $expectedArrayOnlyValuesExtended,
209209
],
210-
'Array with keys and values extended' => [
210+
'Array with keys and values extended' => [
211211
'propertyName' => 'expectsArrayWithExtendedKeysAndValues',
212212
'expected' => $expectedArrayKeysAndValuesExtended,
213213
],
214+
215+
'Empty array in ruleset overrules existing value' => [
216+
'propertyName' => 'expectsNonEmptyArrayOverruledToEmpty',
217+
'expected' => [],
218+
],
219+
'Non empty array in ruleset overrules existing value' => [
220+
'propertyName' => 'expectsNonEmptyArrayOverruledToNewValue',
221+
'expected' => ['another key' => 'another value'],
222+
],
223+
224+
'Extending pre-existing value when there is no value' => [
225+
'propertyName' => 'expectsExtendsWillJustSetToArrayWhenNoDefaultValuePresent',
226+
'expected' => ['foo' => 'bar'],
227+
],
228+
'Extending pre-existing non-array value will overrule' => [
229+
'propertyName' => 'expectsExtendsWillOverruleNonArrayToNewArrayValue',
230+
'expected' => ['phpcbf'],
231+
],
232+
'Non empty array extended by non-empty array' => [
233+
'propertyName' => 'expectsNonEmptyArrayExtendedWithNonEmptyArray',
234+
'expected' => [
235+
'key' => 'value',
236+
'another key' => 'another value',
237+
],
238+
],
239+
'Non empty array keeps value when extended by empty array' => [
240+
'propertyName' => 'expectsNonEmptyArrayKeepsValueWhenExtendedWithEmptyArray',
241+
'expected' => ['key' => 'value'],
242+
],
243+
244+
'Non empty array double extended get both additions' => [
245+
'propertyName' => 'expectsNonEmptyArrayDoubleExtendedWithNonEmptyArray',
246+
'expected' => [
247+
'key' => 'value',
248+
'foo' => 'bar',
249+
'bar' => 'baz',
250+
'baz' => 'boo',
251+
],
252+
],
253+
254+
'Values in non empty associative array can be redefined' => [
255+
'propertyName' => 'expectsValuesInNonEmptyAssociativeArrayCanBeRedefined',
256+
'expected' => [
257+
'foo' => 'bar',
258+
'bar' => 'foo',
259+
],
260+
],
261+
'Values in non empty numerically indexed array are not overwritten' => [
262+
'propertyName' => 'expectsValuesInNonEmptyNumericallyIndexedArrayAreNotOverwritten',
263+
'expected' => [
264+
'valueA',
265+
'valueB',
266+
'valueC',
267+
],
268+
],
269+
'Original values are untouched, while new values get cleaned' => [
270+
'propertyName' => 'expectsPreexistingValuesStayTheSameWhileNewValuesGetCleaned',
271+
'expected' => [
272+
'predefinedA' => 'true',
273+
'predefinedB' => ' null ',
274+
'newValueA' => false,
275+
'newValueB' => null,
276+
'1.5', // phpcs:ignore Squiz.Arrays.ArrayDeclaration.NoKeySpecified -- That is largely what we are testing...
277+
true,
278+
],
279+
],
280+
'Invalid "extend" used on a non-array property' => [
281+
'propertyName' => 'expectsStringNotArray',
282+
'expected' => 'some value',
283+
],
214284
];
215285

216286
}//end dataArrayPropertyExtending()

‎tests/Core/Ruleset/PropertyTypeHandlingTest.xml

+50
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,56 @@
6060
</property>
6161

6262
<property name="expectsEmptyArray" type="array"/>
63+
64+
<!-- Test array properties in combination with pre-existing array values. -->
65+
<property name="expectsNonEmptyArrayOverruledToEmpty" type="array">
66+
</property>
67+
68+
<property name="expectsNonEmptyArrayOverruledToNewValue" type="array">
69+
<element key="another key" value="another value"/>
70+
</property>
71+
72+
<property name="expectsExtendsWillJustSetToArrayWhenNoDefaultValuePresent" type="array" extend="true">
73+
<element key="foo" value="bar"/>
74+
</property>
75+
<property name="expectsExtendsWillOverruleNonArrayToNewArrayValue" type="array" extend="true">
76+
<element value="phpcbf"/>
77+
</property>
78+
79+
<property name="expectsNonEmptyArrayExtendedWithNonEmptyArray" type="array" extend="true">
80+
<element key="another key" value="another value"/>
81+
</property>
82+
83+
<property name="expectsNonEmptyArrayKeepsValueWhenExtendedWithEmptyArray" type="array" extend="true"/>
84+
85+
<property name="expectsNonEmptyArrayDoubleExtendedWithNonEmptyArray" type="array" extend="true">
86+
<element key="foo" value="bar"/>
87+
</property>
88+
89+
<property name="expectsNonEmptyArrayDoubleExtendedWithNonEmptyArray" type="array" extend="true">
90+
<element key="bar" value="baz"/>
91+
<element key="baz" value="boo"/>
92+
</property>
93+
94+
<property name="expectsValuesInNonEmptyAssociativeArrayCanBeRedefined" type="array" extend="true">
95+
<element key="foo" value="bar"/>
96+
<element key="bar" value="foo"/>
97+
</property>
98+
99+
<property name="expectsValuesInNonEmptyNumericallyIndexedArrayAreNotOverwritten" type="array" extend="true">
100+
<element value="valueB"/>
101+
<element value="valueC"/>
102+
</property>
103+
104+
<property name="expectsPreexistingValuesStayTheSameWhileNewValuesGetCleaned" type="array" extend="true">
105+
<element key="newValueA" value="false"/>
106+
<element key="newValueB" value=" null "/>
107+
<element value="1.5"/>
108+
<element value="true"/>
109+
</property>
110+
111+
<property name="expectsStringNotArray" extend="true" value="some value"/>
112+
63113
</properties>
64114
</rule>
65115

0 commit comments

Comments
 (0)