Skip to content

Commit 482d9ef

Browse files
jrfnljrfnl
committed
Lists::getAssignments(): bug fix for exotic keys
If function calls or arrays would be used as the _key_ for a list item and the call or array would contain a comma, the return value for the `Lists::getAssignments()` method would be incorrect and would identify more list items than there really are. Fixed now. Includes additional tests.
1 parent 6b0c204 commit 482d9ef

File tree

3 files changed

+173
-1
lines changed

3 files changed

+173
-1
lines changed

‎PHPCSUtils/Utils/Lists.php‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ public static function getAssignments(File $phpcsFile, $stackPtr)
237237
// Partial reset.
238238
$start = null;
239239
$lastNonEmpty = null;
240+
$list = null; // Prevent confusion when short array was used as the key.
240241
$reference = null;
241242
break;
242243

@@ -245,8 +246,14 @@ public static function getAssignments(File $phpcsFile, $stackPtr)
245246
// Check if this is the end of the list or only a token with the same type as the list closer.
246247
if ($tokens[$i]['code'] === $tokens[$closer]['code']) {
247248
if ($i !== $closer) {
249+
/*
250+
* Shouldn't be possible anymore now nested brackets are being skipped over,
251+
* but keep it just in case.
252+
*/
253+
// @codeCoverageIgnoreStart
248254
$lastNonEmpty = $i;
249255
break;
256+
// @codeCoverageIgnoreEnd
250257
} elseif ($start === null && $lastComma === $opener) {
251258
// This is an empty list.
252259
break 2;
@@ -302,7 +309,7 @@ public static function getAssignments(File $phpcsFile, $stackPtr)
302309

303310
/*
304311
* As the top level list has an open/close, we know we don't have a parse error and
305-
* any nested lists will be tokenized correctly, so no need for extra checks here.
312+
* any nested (short) arrays/lists will be tokenized correctly, so no need for extra checks here.
306313
*/
307314
$nestedOpenClose = self::getOpenClose($phpcsFile, $i, true);
308315
$list = $i;
@@ -321,6 +328,25 @@ public static function getAssignments(File $phpcsFile, $stackPtr)
321328
$start = $i;
322329
}
323330

331+
// Skip over everything within all types of brackets which may be used in keys.
332+
if (isset($tokens[$i]['bracket_opener'], $tokens[$i]['bracket_closer'])
333+
&& $i === $tokens[$i]['bracket_opener']
334+
) {
335+
$i = $tokens[$i]['bracket_closer'];
336+
} elseif ($tokens[$i]['code'] === \T_OPEN_PARENTHESIS
337+
&& isset($tokens[$i]['parenthesis_closer'])
338+
) {
339+
$i = $tokens[$i]['parenthesis_closer'];
340+
} elseif (isset($tokens[$i]['scope_condition'], $tokens[$i]['scope_closer'])
341+
&& $tokens[$i]['scope_condition'] === $i
342+
) {
343+
$i = $tokens[$i]['scope_closer'];
344+
} elseif ($tokens[$i]['code'] === \T_ATTRIBUTE
345+
&& isset($tokens[$i]['attribute_closer'])
346+
) {
347+
$i = $tokens[$i]['attribute_closer'];
348+
}
349+
324350
$lastNonEmpty = $i;
325351
break;
326352
}

‎Tests/Utils/Lists/GetAssignmentsTest.inc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,22 @@ list(
8181
list("x" => $x2, "y" => $y2)
8282
) = $points;
8383

84+
/* testKeyedLongListWithCommasInKey */
85+
list(
86+
$map->getKey($type, $urls) => $x,
87+
array( $tab, $tabs ) => &$y['key'][$tab],
88+
get($year, $day) => $z[$year],
89+
#[MyAttribute]
90+
function($a, $b) { return 'key'; } => $a
91+
) = $array;
92+
93+
/* testKeyedShortListWithCommasInKeyAndTrailingComma */
94+
[
95+
$map->getKey($type, $urls) => $x,
96+
[$tab, $tabs] => &$y['key'][$tab],
97+
get($year, $day) => $z[$year],
98+
] = $array;
99+
84100
/* testLongListMixedKeyedUnkeyed */
85101
// Parse error, but not our concern.
86102
list($unkeyed, "key" => $keyed) = $array;

‎Tests/Utils/Lists/GetAssignmentsTest.php‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -838,6 +838,136 @@ public function dataGetAssignments()
838838
],
839839
],
840840
],
841+
'long-list-keyed-with-commas-in-key' => [
842+
'testMarker' => '/* testKeyedLongListWithCommasInKey */',
843+
'targetToken' => \T_LIST,
844+
'expected' => [
845+
0 => [
846+
'raw' => '$map->getKey($type, $urls) => $x',
847+
'assignment' => '$x',
848+
'is_empty' => false,
849+
'is_nested_list' => false,
850+
'variable' => '$x',
851+
'assignment_token' => 16,
852+
'assignment_end_token' => 16,
853+
'assign_by_reference' => false,
854+
'reference_token' => false,
855+
'key' => '$map->getKey($type, $urls)',
856+
'key_token' => 4,
857+
'key_end_token' => 12,
858+
'double_arrow_token' => 14,
859+
],
860+
1 => [
861+
'raw' => 'array( $tab, $tabs ) => &$y[\'key\'][$tab]',
862+
'assignment' => '$y[\'key\'][$tab]',
863+
'is_empty' => false,
864+
'is_nested_list' => false,
865+
'variable' => '$y',
866+
'assignment_token' => 33,
867+
'assignment_end_token' => 39,
868+
'assign_by_reference' => true,
869+
'reference_token' => 32,
870+
'key' => 'array( $tab, $tabs )',
871+
'key_token' => 20,
872+
'key_end_token' => 28,
873+
'double_arrow_token' => 30,
874+
],
875+
2 => [
876+
'raw' => 'get($year, $day) => $z[$year]',
877+
'assignment' => '$z[$year]',
878+
'is_empty' => false,
879+
'is_nested_list' => false,
880+
'variable' => '$z',
881+
'assignment_token' => 53,
882+
'assignment_end_token' => 56,
883+
'assign_by_reference' => false,
884+
'reference_token' => false,
885+
'key' => 'get($year, $day)',
886+
'key_token' => 43,
887+
'key_end_token' => 49,
888+
'double_arrow_token' => 51,
889+
],
890+
3 => [
891+
'raw' => '#[MyAttribute]
892+
function($a, $b) { return \'key\'; } => $a',
893+
'assignment' => '$a',
894+
'is_empty' => false,
895+
'is_nested_list' => false,
896+
'variable' => '$a',
897+
'assignment_token' => 84,
898+
'assignment_end_token' => 84,
899+
'assign_by_reference' => false,
900+
'reference_token' => false,
901+
'key' => '#[MyAttribute] function($a, $b) { return \'key\'; }',
902+
'key_token' => 60,
903+
'key_end_token' => 80,
904+
'double_arrow_token' => 82,
905+
],
906+
],
907+
],
908+
'short-list-keyed-with-commas-in-key' => [
909+
'testMarker' => '/* testKeyedShortListWithCommasInKeyAndTrailingComma */',
910+
'targetToken' => \T_OPEN_SHORT_ARRAY,
911+
'expected' => [
912+
0 => [
913+
'raw' => '$map->getKey($type, $urls) => $x',
914+
'assignment' => '$x',
915+
'is_empty' => false,
916+
'is_nested_list' => false,
917+
'variable' => '$x',
918+
'assignment_token' => 15,
919+
'assignment_end_token' => 15,
920+
'assign_by_reference' => false,
921+
'reference_token' => false,
922+
'key' => '$map->getKey($type, $urls)',
923+
'key_token' => 3,
924+
'key_end_token' => 11,
925+
'double_arrow_token' => 13,
926+
],
927+
1 => [
928+
'raw' => '[$tab, $tabs] => &$y[\'key\'][$tab]',
929+
'assignment' => '$y[\'key\'][$tab]',
930+
'is_empty' => false,
931+
'is_nested_list' => false,
932+
'variable' => '$y',
933+
'assignment_token' => 29,
934+
'assignment_end_token' => 35,
935+
'assign_by_reference' => true,
936+
'reference_token' => 28,
937+
'key' => '[$tab, $tabs]',
938+
'key_token' => 19,
939+
'key_end_token' => 24,
940+
'double_arrow_token' => 26,
941+
],
942+
2 => [
943+
'raw' => 'get($year, $day) => $z[$year]',
944+
'assignment' => '$z[$year]',
945+
'is_empty' => false,
946+
'is_nested_list' => false,
947+
'variable' => '$z',
948+
'assignment_token' => 49,
949+
'assignment_end_token' => 52,
950+
'assign_by_reference' => false,
951+
'reference_token' => false,
952+
'key' => 'get($year, $day)',
953+
'key_token' => 39,
954+
'key_end_token' => 45,
955+
'double_arrow_token' => 47,
956+
],
957+
3 => [
958+
'raw' => '',
959+
'assignment' => '',
960+
'is_empty' => true,
961+
'is_nested_list' => false,
962+
'variable' => false,
963+
'assignment_token' => false,
964+
'assignment_end_token' => false,
965+
'assign_by_reference' => false,
966+
'reference_token' => false,
967+
],
968+
],
969+
],
970+
841971
'parse-error-long-list-mixed-keyed-unkeyed' => [
842972
'testMarker' => '/* testLongListMixedKeyedUnkeyed */',
843973
'targetToken' => \T_LIST,

0 commit comments

Comments
 (0)