Skip to content

Commit 8e0074a

Browse files
rodrigoprimojrfnl
authored andcommitted
Squiz/ArrayDeclaration: improve handling of short lists inside a foreach
This commit improves how the `Squiz.Arrays.ArrayDeclaration` sniff handles short lists inside a foreach and fixes a false positive. See #527
1 parent 856219c commit 8e0074a

File tree

6 files changed

+51
-0
lines changed

6 files changed

+51
-0
lines changed

src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,26 @@ public function process(File $phpcsFile, $stackPtr)
4545
{
4646
$tokens = $phpcsFile->getTokens();
4747

48+
// Prevent acting on short lists inside a foreach (see
49+
// https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/527).
50+
if ($tokens[$stackPtr]['code'] === T_OPEN_SHORT_ARRAY
51+
&& isset($tokens[$stackPtr]['nested_parenthesis']) === true
52+
) {
53+
$nestedParens = $tokens[$stackPtr]['nested_parenthesis'];
54+
$lastParenthesisCloser = end($nestedParens);
55+
$lastParenthesisOpener = key($nestedParens);
56+
57+
if (isset($tokens[$lastParenthesisCloser]['parenthesis_owner']) === true
58+
&& $tokens[$tokens[$lastParenthesisCloser]['parenthesis_owner']]['code'] === T_FOREACH
59+
) {
60+
$asKeyword = $phpcsFile->findNext(T_AS, ($lastParenthesisOpener + 1), $lastParenthesisCloser);
61+
62+
if ($asKeyword !== false && $asKeyword < $stackPtr) {
63+
return;
64+
}
65+
}
66+
}
67+
4868
if ($tokens[$stackPtr]['code'] === T_ARRAY) {
4969
$phpcsFile->recordMetric($stackPtr, 'Short array syntax used', 'no');
5070

src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,3 +547,9 @@ $newlineAfterDoubleArrow = [
547547
'',
548548
'height' => '',
549549
];
550+
551+
// Sniff should ignore short lists when inside a foreach.
552+
// https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/527
553+
foreach ($data as [, , $value]) {}
554+
foreach ($array as $k => [$v1, , $v3]) {}
555+
foreach ([$a ,$b] as $c) {} // Not a short list. Sniff should handle it.

src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.2.inc.fixed

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,3 +583,9 @@ $newlineAfterDoubleArrow = [
583583
'width' => '',
584584
'height' => '',
585585
];
586+
587+
// Sniff should ignore short lists when inside a foreach.
588+
// https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/527
589+
foreach ($data as [, , $value]) {}
590+
foreach ($array as $k => [$v1, , $v3]) {}
591+
foreach ([$a, $b] as $c) {} // Not a short list. Sniff should handle it.
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
// Intentional parse error (missing T_AS in foreach).
4+
// This should be the only test in this file.
5+
// Testing that the code preventing the sniff to act on short lists inside a foreach doesn't
6+
// interfere with the rest of sniff when the `as` keyword is missing.
7+
8+
foreach ([$a , $b])
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
// Intentional parse error (missing T_AS in foreach).
4+
// This should be the only test in this file.
5+
// Testing that the code preventing the sniff to act on short lists inside a foreach doesn't
6+
// interfere with the rest of sniff when the `as` keyword is missing.
7+
8+
foreach ([$a, $b])

src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,10 @@ public function getErrorList($testFile='')
233233
536 => 2,
234234
541 => 1,
235235
546 => 1,
236+
555 => 2,
236237
];
238+
case 'ArrayDeclarationUnitTest.4.inc':
239+
return [8 => 1];
237240
default:
238241
return [];
239242
}//end switch

0 commit comments

Comments
 (0)