Generic.ControlStructures.DisallowYodaConditions now returns less false positives (ref #2653)

It now ignores empty parenthesis, and considers array definitions containing only variables to not be static.

Author: gsherwood

package.xml

@@ -26,6 +26,8 @@ http://pear.php.net/dtd/package-2.0.xsd">
  </stability>
  <license uri="https://github.com/squizlabs/PHP_CodeSniffer/blob/master/licence.txt">BSD 3-Clause License</license>
  <notes>
+  - Generic.ControlStructures.DisallowYodaConditions now returns less false positives
+    -- False positives were being returned for array comparisions, or when performing some function calls
   - Squiz.WhiteSpace.SemicolonSpacing.Incorrect error message now escapes newlines and tabs
     -- Provides a clearer error message as whitespace is now visible
     -- Also allows for better output for report types such as CSV and XML

src/Standards/Generic/Sniffs/ControlStructures/DisallowYodaConditionsSniff.php

@@ -62,6 +62,9 @@ public function process(File $phpcsFile, $stackPtr)
 
         if ($tokens[$previousIndex]['code'] === T_CLOSE_SHORT_ARRAY) {
             $previousIndex = $tokens[$previousIndex]['bracket_opener'];
+            if ($this->isArrayStatic($phpcsFile, $previousIndex) === false) {
+                return;
+            }
         }
 
         $prevIndex = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($previousIndex - 1), null, true);
@@ -99,11 +102,22 @@ public function process(File $phpcsFile, $stackPtr)
                     $tokens[$previousIndex]['parenthesis_opener']
                 );
 
-                // If a variable exists it is not Yoda.
+                // If a variable exists, it is not Yoda.
                 if ($found !== false) {
                     return;
                 }
-            }
+
+                // If there is nothing inside the parenthesis, it it not a Yoda.
+                $opener = $tokens[$previousIndex]['parenthesis_opener'];
+                $prev   = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($previousIndex - 1), ($opener + 1), true);
+                if ($prev === false) {
+                    return;
+                }
+            } else if ($tokens[$closeParenthesisIndex]['code'] === T_ARRAY
+                && $this->isArrayStatic($phpcsFile, $closeParenthesisIndex) === false
+            ) {
+                return;
+            }//end if
         }//end if
 
         $phpcsFile->addError(
@@ -115,4 +129,60 @@ public function process(File $phpcsFile, $stackPtr)
     }//end process()
 
 
+    /**
+     * Determines if an array is a static definition.
+     *
+     * @param \PHP_CodeSniffer\Files\File $phpcsFile  The file being scanned.
+     * @param int                         $arrayToken The position of the array token.
+     *
+     * @return bool
+     */
+    public function isArrayStatic(File $phpcsFile, $arrayToken)
+    {
+        $tokens = $phpcsFile->getTokens();
+
+        $arrayEnd = null;
+        if ($tokens[$arrayToken]['code'] === T_OPEN_SHORT_ARRAY) {
+            $start = $arrayToken;
+            $end   = $tokens[$arrayToken]['bracket_closer'];
+        } else if ($tokens[$arrayToken]['code'] === T_ARRAY) {
+            $start = $tokens[$arrayToken]['parenthesis_opener'];
+            $end   = $tokens[$arrayToken]['parenthesis_closer'];
+        } else {
+            return true;
+        }
+
+        $staticTokens  = Tokens::$emptyTokens;
+        $staticTokens += Tokens::$textStringTokens;
+        $staticTokens += Tokens::$assignmentTokens;
+        $staticTokens += Tokens::$equalityTokens;
+        $staticTokens += Tokens::$comparisonTokens;
+        $staticTokens += Tokens::$arithmeticTokens;
+        $staticTokens += Tokens::$operators;
+        $staticTokens += Tokens::$booleanOperators;
+        $staticTokens += Tokens::$castTokens;
+        $staticTokens += Tokens::$bracketTokens;
+        $staticTokens += [
+            T_DOUBLE_ARROW => T_DOUBLE_ARROW,
+            T_COMMA        => T_COMMA,
+            T_TRUE         => T_TRUE,
+            T_FALSE        => T_FALSE,
+        ];
+
+        for ($i = ($start + 1); $i < $end; $i++) {
+            if (isset($tokens[$i]['scope_closer']) === true) {
+                $i = $tokens[$i]['scope_closer'];
+                continue;
+            }
+
+            if (isset($staticTokens[$tokens[$i]['code']]) === false) {
+                return false;
+            }
+        }
+
+        return true;
+
+    }//end isArrayStatic()
+
+
 }//end class

src/Standards/Generic/Tests/ControlStructures/DisallowYodaConditionsUnitTest.inc

@@ -132,3 +132,35 @@ if ((int) 5 > $var) {}
 if ((int) $var > (int) 5) {}
 if (true == function() { return false;}){}
 if (function() { return false;} == true){}
+
+if (is_array($val)
+    && array($foo) === array($bar)
+    && [$foo] === [$bar]
+    && array('foo', 'bar') === array($foo, $bar)
+    && ['foo', 'bar'] === [$foo, $bar]
+    && array('foo' => true, 'bar' => false) === array(getContents())
+    && ['foo' => true, 'bar' => false] === array(getContents())
+    && array(getContents()) === ['foo' => true, 'bar' => false]
+) {
+}
+
+if ($this->cfg['some_closure']() == 2) {
+}
+
+if (is_array($val)
+    && array(get_class($val[0]), $val[1]) == array('someNamespace\\className', 'method')
+) {
+}
+
+if (is_array($val)
+    && array('someNamespace\\className', 'method') == array(get_class($val[0]), $val[1])
+) {
+}
+
+if ([function() { echo 'hi'; }] === [$foo]
+    && [$foo] === [function() { echo 'hi'; }]
+    && [function() { echo 'hi'; }, $bar] === [$foo]
+    && [$foo] === [function() { echo 'hi'; }, $bar]
+) {
+}
+

src/Standards/Generic/Tests/ControlStructures/DisallowYodaConditionsUnitTest.php

@@ -48,15 +48,17 @@ public function getErrorList()
             97  => 3,
             98  => 3,
             105 => 1,
-            119 => 1,
-            120 => 1,
-            122 => 1,
-            123 => 1,
             128 => 1,
             129 => 2,
             130 => 1,
             131 => 1,
             133 => 1,
+            139 => 1,
+            140 => 1,
+            141 => 1,
+            142 => 1,
+            156 => 1,
+            160 => 1,
         ];
 
     }//end getErrorList()