Merge pull request #748 from rodrigoprimo/fix-array-declaration-short-list-bug

Squiz/ArrayDeclaration: fixes false positive when handling short lists inside `foreach`

Author: jrfnl

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

@@ -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');
 

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

@@ -19,7 +19,7 @@ class TestClass
                     'height' => '',
                    );
 
-    private $_bad = Array(
+    private $_bad = ARRAY(
                     'width' => '',
                     'height' => ''
                      );
@@ -547,3 +547,14 @@ $x = array(
       1, static::helloWorld(), $class instanceof static,
       2,
      );
+
+$noSpaceBeforeDoubleArrow = array(
+                             'width'=> '',
+                             'height' => '',
+                            );
+
+$newlineAfterDoubleArrow = array(
+                            'width'  =>
+                            '',
+                            'height' => '',
+                           );

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

@@ -586,3 +586,13 @@ $x = array(
       $class instanceof static,
       2,
      );
+
+$noSpaceBeforeDoubleArrow = array(
+                             'width'  => '',
+                             'height' => '',
+                            );
+
+$newlineAfterDoubleArrow = array(
+                            'width'  => '',
+                            'height' => '',
+                           );

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

@@ -536,3 +536,20 @@ $x = [
       1, static::helloWorld(), $class instanceof static,
       2,
      ];
+
+$noSpaceBeforeDoubleArrow = [
+                             'width'=> '',
+                             'height' => '',
+                            ];
+
+$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.

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

@@ -573,3 +573,19 @@ $x = [
       $class instanceof static,
       2,
      ];
+
+$noSpaceBeforeDoubleArrow = [
+                             'width'  => '',
+                             'height' => '',
+                            ];
+
+$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.

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

@@ -0,0 +1,7 @@
+<?php
+
+// Intentional parse error (missing closing parenthesis).
+// This should be the only test in this file.
+// Testing that the sniff is *not* triggered.
+
+$array = array(

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

@@ -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])

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

@@ -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])

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

@@ -136,6 +136,8 @@ public function getErrorList($testFile='')
                 537 => 1,
                 540 => 1,
                 547 => 2,
+                552 => 1,
+                557 => 1,
             ];
         case 'ArrayDeclarationUnitTest.2.inc':
             return [
@@ -229,7 +231,12 @@ public function getErrorList($testFile='')
                 526 => 1,
                 529 => 1,
                 536 => 2,
+                541 => 1,
+                546 => 1,
+                555 => 2,
             ];
+        case 'ArrayDeclarationUnitTest.4.inc':
+            return [8 => 1];
         default:
             return [];
         }//end switch