Skip to content

Commit

Permalink
Squiz/ArrayDeclaration: improve handling of short lists inside a foreach
Browse files Browse the repository at this point in the history
This commit improves how the `Squiz.Arrays.ArrayDeclaration` sniff
handles short lists inside a foreach and fixes a false positive.

See #527
  • Loading branch information
rodrigoprimo authored and jrfnl committed Dec 7, 2024
1 parent 8c9f79b commit 48f1737
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 0 deletions.
20 changes: 20 additions & 0 deletions src/Standards/Squiz/Sniffs/Arrays/ArrayDeclarationSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,26 @@ public function process(File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

// Prevent acting on short lists inside a foreach (see
// https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/527).
if ($tokens[$stackPtr]['code'] === T_OPEN_SHORT_ARRAY
&& isset($tokens[$stackPtr]['nested_parenthesis']) === true
) {
$nestedParens = $tokens[$stackPtr]['nested_parenthesis'];
$lastParenthesisCloser = end($nestedParens);
$lastParenthesisOpener = key($nestedParens);

if (isset($tokens[$lastParenthesisCloser]['parenthesis_owner']) === true
&& $tokens[$tokens[$lastParenthesisCloser]['parenthesis_owner']]['code'] === T_FOREACH
) {
$asKeyword = $phpcsFile->findNext(T_AS, ($lastParenthesisOpener + 1), $lastParenthesisCloser);

if ($asKeyword !== false && $asKeyword < $stackPtr) {
return;
}
}
}

if ($tokens[$stackPtr]['code'] === T_ARRAY) {
$phpcsFile->recordMetric($stackPtr, 'Short array syntax used', 'no');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -547,3 +547,9 @@ $newlineAfterDoubleArrow = [
'',
'height' => '',
];

// Sniff should ignore short lists when inside a foreach.
// https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/527
foreach ($data as [, , $value]) {}
foreach ($array as $k => [$v1, , $v3]) {}
foreach ([$a ,$b] as $c) {} // Not a short list. Sniff should handle it.
Original file line number Diff line number Diff line change
Expand Up @@ -583,3 +583,9 @@ $newlineAfterDoubleArrow = [
'width' => '',
'height' => '',
];

// Sniff should ignore short lists when inside a foreach.
// https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/527
foreach ($data as [, , $value]) {}
foreach ($array as $k => [$v1, , $v3]) {}
foreach ([$a, $b] as $c) {} // Not a short list. Sniff should handle it.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

// Intentional parse error (missing T_AS in foreach).
// This should be the only test in this file.
// Testing that the code preventing the sniff to act on short lists inside a foreach doesn't
// interfere with the rest of sniff when the `as` keyword is missing.

foreach ([$a , $b])
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

// Intentional parse error (missing T_AS in foreach).
// This should be the only test in this file.
// Testing that the code preventing the sniff to act on short lists inside a foreach doesn't
// interfere with the rest of sniff when the `as` keyword is missing.

foreach ([$a, $b])
3 changes: 3 additions & 0 deletions src/Standards/Squiz/Tests/Arrays/ArrayDeclarationUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,10 @@ public function getErrorList($testFile='')
536 => 2,
541 => 1,
546 => 1,
555 => 2,
];
case 'ArrayDeclarationUnitTest.4.inc':
return [8 => 1];
default:
return [];
}//end switch
Expand Down

0 comments on commit 48f1737

Please sign in to comment.