Add special cases for variables used in else blocks

Tests/VariableAnalysisSniff/IfConditionTest.php

@@ -0,0 +1,34 @@
+<?php
+namespace VariableAnalysis\Tests\VariableAnalysisSniff;
+
+use VariableAnalysis\Tests\BaseTestCase;
+
+class IfConditionTest extends BaseTestCase {
+  public function testIfConditionWarnings() {
+    $fixtureFile = $this->getFixture('FunctionWithIfConditionFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+    $phpcsFile->ruleset->setSniffProperty(
+      'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
+      'allowUnusedParametersBeforeUsed',
+      'true'
+    );
+    $phpcsFile->process();
+    $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+    $expectedWarnings = [
+      15,
+      27,
+      36,
+      38,
+      47,
+      58,
+      62,
+      70,
+      74,
+      82,
+      87,
+      98,
+      101,
+    ];
+    $this->assertEquals($expectedWarnings, $lines);
+  }
+}

Tests/VariableAnalysisSniff/fixtures/FunctionWithIfConditionFixture.php

@@ -0,0 +1,147 @@
+<?php
+
+function normalIfCondition($first, $second) {
+  $name = 'human';
+  if ($first) {
+    $words = "hello {$name}";
+  } elseif ($second) {
+    $words = "bye {$name}";
+  }
+  echo $words;
+}
+
+function undefinedInsideIfCondition($second) {
+  $name = 'human';
+  if ($first) { // undefined variable $first
+    $words = "hello {$name}";
+  } elseif ($second) {
+    $words = "bye {$name}";
+  }
+  echo $words;
+}
+
+function undefinedInsideElseCondition($first) {
+  $name = 'human';
+  if ($first) {
+    $words = "hello {$name}";
+  } elseif ($second) { // undefined variable $second
+    $words = "bye {$name}";
+  }
+  echo $words;
+}
+
+function definedInsideFirstBlockUndefinedInsideElseCondition($first) {
+  $name = 'human';
+  if ($first) {
+    $second = true; // unused variable $second
+    $words = "hello {$name}";
+  } elseif ($second) { // undefined variable $second
+    $words = "bye {$name}";
+  }
+  echo $words;
+}
+
+function unusedInsideFirstBlock($first, $second) {
+  $name = 'human';
+  if ($first) {
+    $unused = true; // unused variable $unused
+    $words = "hello {$name}";
+  } elseif ($second) {
+    $words = "bye {$name}";
+  }
+  echo $words;
+}
+
+function definedInsideFirstBlockUndefinedInsideElseIfBlock($first) {
+  $name = 'human';
+  if ($first) {
+    $second = true; // unused variable $second
+    $words = "hello {$name}";
+  } elseif ($name) {
+    $words = "bye {$name}";
+    echo $second; // undefined variable $second
+  }
+  echo $words;
+}
+
+function definedInsideFirstBlockUndefinedInsideElseBlock($first) {
+  $name = 'human';
+  if ($first) {
+    $second = true; // unused variable $second
+    $words = "hello {$name}";
+  } else {
+    $words = "bye {$name}";
+    echo $second; // undefined variable $second
+  }
+  echo $words;
+}
+
+function definedInsideFirstBlockUndefinedInsideElseBlockInsideAnotherIf($first) {
+  $name = 'human';
+  if ($first) {
+    $second = true; // unused variable $second
+    $words = "hello {$name}";
+  } else {
+    if ($name) {
+      $words = "bye {$name}";
+      echo $second; // undefined variable $second
+    }
+  }
+  echo $words;
+}
+
+function definedInsideElseIfBlockUndefinedInsideElseBlock($first) {
+  $name = 'human';
+  if ($first) {
+    $words = "hello {$name}";
+  } elseif ($name) {
+    $second = true; // unused variable $second
+  } else {
+    $words = "bye {$name}";
+    echo $second; // undefined variable $second
+  }
+  echo $words;
+}
+
+function definedInsideFirstBlockUndefinedInsideUnconnectedElseCondition($first) {
+  $name = 'human';
+  if ($first) {
+    $second = true;
+    $words = "hello {$name}";
+  } elseif ($name) {
+    $words = "bye {$name}";
+  }
+  if ($first) {
+    $second = true;
+    $words = "hello {$name}";
+  } elseif ($second) {
+    $words = "bye {$name}";
+  }
+  echo $words;
+}
+
+function definedInsideFirstBlockUndefinedInsideSecondCondition($first) {
+  $name = 'human';
+  if ($first) {
+    $second = true;
+    $words = "hello {$name}";
+  }
+  if ($second) {
+    $words = "bye {$name}";
+  }
+  echo $words;
+}
+
+function ifConditionWithPossibleDefinition($first) {
+  if ($first) {
+    $name = 'person';
+  }
+  echo $name;
+}
+
+function ifConditionWithPossibleUse($first) {
+  $name = 'person';
+  if ($first) {
+    echo $name;
+  }
+}

VariableAnalysis/Lib/Helpers.php

@@ -560,4 +560,93 @@ public static function splitStringToArray($pattern, $value) {
   public static function isVariableANumericVariable($varName) {
     return is_numeric(substr($varName, 0, 1));
   }
+
+  /**
+   * @param File $phpcsFile
+   * @param int $stackPtr
+   *
+   * @return bool
+   */
+  public static function isVariableInsideElseCondition(File $phpcsFile, $stackPtr) {
+    $tokens = $phpcsFile->getTokens();
+    $nonFunctionTokenTypes = array_values(Tokens::$emptyTokens);
+    $nonFunctionTokenTypes[] = T_OPEN_PARENTHESIS;
+    $nonFunctionTokenTypes[] = T_VARIABLE;
+    $nonFunctionTokenTypes[] = T_ELLIPSIS;
+    $nonFunctionTokenTypes[] = T_COMMA;
+    $nonFunctionTokenTypes[] = T_STRING;
+    $nonFunctionTokenTypes[] = T_BITWISE_AND;
+    $elsePtr = self::getIntOrNull($phpcsFile->findPrevious($nonFunctionTokenTypes, $stackPtr - 1, null, true, null, true));
+    $elseTokenTypes = [
+      T_ELSE,
+      T_ELSEIF,
+    ];
+    if (is_int($elsePtr) && in_array($tokens[$elsePtr]['code'], $elseTokenTypes, true)) {
+      return true;
+    }
+    return false;
+  }
+
+  /**
+   * @param File $phpcsFile
+   * @param int $stackPtr
+   *
+   * @return bool
+   */
+  public static function isVariableInsideElseBody(File $phpcsFile, $stackPtr) {
+    $tokens = $phpcsFile->getTokens();
+    $token = $tokens[$stackPtr];
+    $conditions = isset($token['conditions']) ? $token['conditions'] : [];
+    $elseTokenTypes = [
+      T_ELSE,
+      T_ELSEIF,
+    ];
+    foreach (array_reverse($conditions, true) as $scopeCode) {
+      if (in_array($scopeCode, $elseTokenTypes, true)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /**
+   * @param File $phpcsFile
+   * @param int $stackPtr
+   *
+   * @return int[]
+   */
+  public static function getAttachedBlockIndicesForElse(File $phpcsFile, $stackPtr) {
+    $currentElsePtr = $phpcsFile->findPrevious([T_ELSE, T_ELSEIF], $stackPtr - 1);
+    if (! is_int($currentElsePtr)) {
+      throw new \Exception("Cannot find expected else at {$stackPtr}");
+    }
+
+    $ifPtr = $phpcsFile->findPrevious([T_IF], $currentElsePtr - 1);
+    if (! is_int($ifPtr)) {
+      throw new \Exception("Cannot find if for else at {$stackPtr}");
+    }
+    $blockIndices = [$ifPtr];
+
+    $previousElseIfPtr = $currentElsePtr;
+    do {
+      $elseIfPtr = $phpcsFile->findPrevious([T_ELSEIF], $previousElseIfPtr - 1, $ifPtr);
+      if (is_int($elseIfPtr)) {
+        $blockIndices[] = $elseIfPtr;
+        $previousElseIfPtr = $elseIfPtr;
+      }
+    } while (is_int($elseIfPtr));
+
+    return $blockIndices;
+  }
+
+  /**
+   * @param int $needle
+   * @param int $scopeStart
+   * @param int $scopeEnd
+   *
+   * @return bool
+   */
+  public static function isIndexInsideScope($needle, $scopeStart, $scopeEnd) {
+    return ($needle > $scopeStart && $needle < $scopeEnd);
+  }
 }

VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

@@ -1390,11 +1390,59 @@ protected function processVariable(File $phpcsFile, $stackPtr) {
       return;
     }
 
+    if (Helpers::isVariableInsideElseCondition($phpcsFile, $stackPtr) || Helpers::isVariableInsideElseBody($phpcsFile, $stackPtr)) {
+      Helpers::debug('found variable inside else condition or body');
+      $this->processVaribleInsideElse($phpcsFile, $stackPtr, $varName, $currScope);
+      return;
+    }
+
     // OK, we don't appear to be a write to the var, assume we're a read.
     Helpers::debug('looks like a variable read');
     $this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $stackPtr, $currScope);
   }
 
+  /**
+   * @param File $phpcsFile
+   * @param int $stackPtr
+   * @param string $varName
+   * @param int $currScope
+   *
+   * @return void
+   */
+  protected function processVaribleInsideElse(File $phpcsFile, $stackPtr, $varName, $currScope) {
+    // Find all assignments to this variable inside the current scope.
+    $varInfo = $this->getOrCreateVariableInfo($varName, $currScope);
+    $allAssignmentIndices = array_unique($varInfo->allAssignments);
+    // Find the attached 'if' and 'elseif' block start and end indices.
+    $blockIndices = Helpers::getAttachedBlockIndicesForElse($phpcsFile, $stackPtr);
+
+    // If all of the assignments are within the previous attached blocks, then warn about undefined.
+    $tokens = $phpcsFile->getTokens();
+    $assignmentsInsideAttachedBlocks = [];
+    foreach ($allAssignmentIndices as $index) {
+      foreach ($blockIndices as $blockIndex) {
+        Helpers::debug('looking at scope', $index, 'between', $tokens[$blockIndex]['scope_opener'], 'and', $tokens[$blockIndex]['scope_closer']);
+        if (Helpers::isIndexInsideScope($index, $tokens[$blockIndex]['scope_opener'], $tokens[$blockIndex]['scope_closer'])) {
+          $assignmentsInsideAttachedBlocks[] = $index;
+        }
+      }
+    }
+
+    if (count($assignmentsInsideAttachedBlocks) === count($allAssignmentIndices)) {
+      Helpers::debug("variable $varName inside else looks undefined");
+      $phpcsFile->addWarning(
+        "Variable %s is undefined.",
+        $stackPtr,
+        'UndefinedVariable',
+        ["\${$varName}"]
+      );
+      return;
+    }
+
+    Helpers::debug('looks like a variable read inside else');
+    $this->markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $stackPtr, $currScope);
+  }
+
   /**
    * Called to process variables found in double quoted strings.
    *