Allow undefined variables inside isset and empty (#204)

* Add tests for isset and empty

* Ignore undefined variables inside isset or empty

* Make sure we detect the start of arguments

* Add test to make sure isset still counts as read

* Explicitly do not count variable definitions for getFunctionIndexForFunctionCallArgument

* Mark isset/empty usage as a read

Author: sirbrillig

Tests/VariableAnalysisSniff/IssetTest.php

@@ -0,0 +1,18 @@
+<?php
+namespace VariableAnalysis\Tests\VariableAnalysisSniff;
+
+use VariableAnalysis\Tests\BaseTestCase;
+
+class IssetTest extends BaseTestCase {
+  public function testIssetVariableUse() {
+    $fixtureFile = $this->getFixture('IssetFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+    $phpcsFile->process();
+    $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+    $expectedWarnings = [
+      4,
+      23, // ideally this should not be a warning, but will be because it is difficult to know: https://github.com/sirbrillig/phpcs-variable-analysis/issues/202#issuecomment-688507314
+    ];
+    $this->assertEquals($expectedWarnings, $lines);
+  }
+}

Tests/VariableAnalysisSniff/fixtures/IssetFixture.php

@@ -0,0 +1,37 @@
+<?php
+
+function normalFunctionShouldNotIgnoreUndefinedVariable() {
+  if (isActive($undefinedVar)) { // should report undefined variable $undefinedVar
+    doSomething();
+  }
+}
+
+function issetShouldIgnoreUndefinedVariable() {
+  if (isset($undefinedVar)) {
+    doSomething();
+  }
+}
+
+function emptyShouldIgnoreUndefinedVariable() {
+  if (! empty($undefinedVar)) {
+    doSomething();
+  }
+}
+
+function shouldIgnoreUndefinedVariableUseAfterIsset() {
+  if (isset($undefinedVar)) {
+    doSomething($undefinedVar); // ideally this should not be a warning, but will be because it is difficult to know: https://github.com/sirbrillig/phpcs-variable-analysis/issues/202#issuecomment-688507314
+  }
+}
+
+function shouldCountVariableUseInsideIssetAsRead($definedVar) {
+  if (isset($definedVar)) {
+    doSomething();
+  }
+}
+
+function shouldCountVariableUseInsideEmptyAsRead($definedVar) {
+  if (empty($definedVar)) {
+    doSomething();
+  }
+}

VariableAnalysis/Lib/Helpers.php

@@ -107,6 +107,9 @@ public static function isTokenInsideFunctionDefinitionArgumentList(File $phpcsFi
    *
    * Will also work for the parenthesis that make up the function definition's arguments list.
    *
+   * For arguments inside a function call, rather than a definition, use
+   * `getFunctionIndexForFunctionCallArgument`.
+   *
    * @param File $phpcsFile
    * @param int $stackPtr
    *
@@ -130,6 +133,10 @@ public static function getFunctionIndexForFunctionArgument(File $phpcsFile, $sta
       $startOfArguments = end($startingParenthesis);
     }
 
+    if (! is_int($startOfArguments)) {
+      return null;
+    }
+
     $nonFunctionTokenTypes = array_values(Tokens::$emptyTokens);
     $nonFunctionTokenTypes[] = T_STRING;
     $nonFunctionTokenTypes[] = T_BITWISE_AND;
@@ -730,4 +737,60 @@ public static function isRequireInScopeAfter(File $phpcsFile, VariableInfo $varI
     }
     return false;
   }
+
+  /**
+   * Find the index of the function keyword for a token in a function call's arguments
+   *
+   * @param File $phpcsFile
+   * @param int $stackPtr
+   *
+   * @return ?int
+   */
+  public static function getFunctionIndexForFunctionCallArgument(File $phpcsFile, $stackPtr) {
+    $tokens = $phpcsFile->getTokens();
+    $token = $tokens[$stackPtr];
+    if (empty($token['nested_parenthesis'])) {
+      return null;
+    }
+    $startingParenthesis = array_keys($token['nested_parenthesis']);
+    $startOfArguments = end($startingParenthesis);
+    if (! is_int($startOfArguments)) {
+      return null;
+    }
+
+    $nonFunctionTokenTypes = array_values(Tokens::$emptyTokens);
+    $functionPtr = self::getIntOrNull($phpcsFile->findPrevious($nonFunctionTokenTypes, $startOfArguments - 1, null, true, null, true));
+    if (! is_int($functionPtr) || ! isset($tokens[$functionPtr]['code'])) {
+      return null;
+    }
+    if ($tokens[$functionPtr]['code'] === 'function' || ($tokens[$functionPtr]['content'] === 'fn' && FunctionDeclarations::isArrowFunction($phpcsFile, $functionPtr))) {
+      return null;
+    }
+    return $functionPtr;
+  }
+
+  /**
+   * @param File $phpcsFile
+   * @param int $stackPtr
+   *
+   * @return bool
+   */
+  public static function isVariableInsideIssetOrEmpty(File $phpcsFile, $stackPtr) {
+    $functionIndex = self::getFunctionIndexForFunctionCallArgument($phpcsFile, $stackPtr);
+    if (! is_int($functionIndex)) {
+      return false;
+    }
+    $tokens = $phpcsFile->getTokens();
+    if (! isset($tokens[$functionIndex])) {
+      return false;
+    }
+    $allowedFunctionNames = [
+      'isset',
+      'empty',
+    ];
+    if (in_array($tokens[$functionIndex]['content'], $allowedFunctionNames, true)) {
+      return true;
+    }
+    return false;
+  }
 }

VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

@@ -1440,6 +1440,13 @@ protected function processVariable(File $phpcsFile, $stackPtr) {
       return;
     }
 
+    // Are we an isset or empty call?
+    if (Helpers::isVariableInsideIssetOrEmpty($phpcsFile, $stackPtr)) {
+      Helpers::debug('found isset or empty');
+      $this->markVariableRead($varName, $stackPtr, $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);