Consider get_defined_vars() to be a read (#92)

* Tests: add tests for get_defined_vars

* Tests: add a variable named get_defined_vars

* Remove unused import

* Add some phpdoc

* Mark all vars read at get_defined_vars

* Add more guards to checkForPassByReferenceFunctionCall

Author: sirbrillig

VariableAnalysis/Lib/Helpers.php

@@ -3,7 +3,6 @@
 namespace VariableAnalysis\Lib;
 
 use PHP_CodeSniffer\Files\File;
-use VariableAnalysis\Lib\VariableInfo;
 
 class Helpers {
   public static function findContainingOpeningSquareBracket(File $phpcsFile, $stackPtr) {
@@ -84,6 +83,12 @@ public static function findPreviousFunctionPtr(File $phpcsFile, $openPtr) {
     return $phpcsFile->findPrevious($functionPtrTypes, $openPtr - 1, null, true, null, true);
   }
 
+  /**
+   * @param File $phpcsFile
+   * @param int $stackPtr
+   *
+   * @return int|false
+   */
   public static function findFunctionCall(File $phpcsFile, $stackPtr) {
     $tokens = $phpcsFile->getTokens();
 
@@ -198,6 +203,12 @@ public static function findFunctionPrototype(File $phpcsFile, $stackPtr) {
     return false;
   }
 
+  /**
+   * @param File $phpcsFile
+   * @param int $stackPtr
+   *
+   * @return int|false
+   */
   public static function findVariableScope(File $phpcsFile, $stackPtr) {
     $tokens = $phpcsFile->getTokens();
     $token  = $tokens[$stackPtr];

VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

@@ -123,23 +123,51 @@ public function process(File $phpcsFile, $stackPtr) {
     if (($token['code'] === T_STRING) && ($token['content'] === 'compact')) {
       return $this->processCompact($phpcsFile, $stackPtr);
     }
+    if ($this->isGetDefinedVars($phpcsFile, $stackPtr)) {
+      Helpers::debug('get_defined_vars is being called');
+      return $this->markAllVariablesRead($phpcsFile, $stackPtr);
+    }
     if (($token['code'] === T_CLOSE_CURLY_BRACKET) && isset($token['scope_condition'])) {
       return $this->processScopeClose($phpcsFile, $token['scope_condition']);
     }
   }
 
+  protected function isGetDefinedVars(File $phpcsFile, $stackPtr) {
+    $tokens = $phpcsFile->getTokens();
+    $token = $tokens[$stackPtr];
+    if (! $token || $token['content'] !== 'get_defined_vars') {
+      return false;
+    }
+    // Make sure this is a function call
+    $parenPointer = $phpcsFile->findNext(T_OPEN_PARENTHESIS, $stackPtr, $stackPtr + 2);
+    if (! $parenPointer) {
+      return false;
+    }
+    return true;
+  }
+
   protected function getScopeKey($currScope) {
     if ($currScope === false) {
       $currScope = 'file';
     }
     return ($this->currentFile ? $this->currentFile->getFilename() : 'unknown file') . ':' . $currScope;
   }
 
+  /**
+   * @param int $currScope
+   *
+   * @return ?ScopeInfo
+   */
   protected function getScopeInfo($currScope) {
     $scopeKey = $this->getScopeKey($currScope);
     return isset($this->scopes[$scopeKey]) ? $this->scopes[$scopeKey] : null;
   }
 
+  /**
+   * @param int $currScope
+   *
+   * @return ScopeInfo
+   */
   protected function getOrCreateScopeInfo($currScope) {
     $scopeKey = $this->getScopeKey($currScope);
     if (!isset($this->scopes[$scopeKey])) {
@@ -241,6 +269,13 @@ protected function markVariableDeclaration(
     $varInfo->firstDeclared = $stackPtr;
   }
 
+  /**
+   * @param string $varName
+   * @param int $stackPtr
+   * @param int $currScope
+   *
+   * @return void
+   */
   protected function markVariableRead($varName, $stackPtr, $currScope) {
     $varInfo = $this->getOrCreateVariableInfo($varName, $currScope);
     if (isset($varInfo->firstRead) && ($varInfo->firstRead <= $stackPtr)) {
@@ -284,6 +319,25 @@ protected function markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $sta
     }
   }
 
+  /**
+   * @param File $phpcsFile
+   * @param int $stackPtr
+   *
+   * @return void
+   */
+  protected function markAllVariablesRead(File $phpcsFile, $stackPtr) {
+    $currScope = Helpers::findVariableScope($phpcsFile, $stackPtr);
+    if (! $currScope) {
+      return false;
+    }
+    $scopeInfo = $this->getOrCreateScopeInfo($currScope);
+    $count = count($scopeInfo->variables);
+    Helpers::debug("marking all $count variables in scope as read");
+    foreach ($scopeInfo->variables as $varInfo) {
+      $this->markVariableRead($varInfo->name, $stackPtr, $scopeInfo->owner);
+    }
+  }
+
   protected function checkForFunctionPrototype(File $phpcsFile, $stackPtr, $varName, $currScope) {
     $tokens = $phpcsFile->getTokens();
     $token  = $tokens[$stackPtr];
@@ -723,7 +777,7 @@ protected function checkForPassByReferenceFunctionCall(File $phpcsFile, $stackPt
 
     // Are we pass-by-reference to known pass-by-reference function?
     $functionPtr = Helpers::findFunctionCall($phpcsFile, $stackPtr);
-    if ($functionPtr === false) {
+    if ($functionPtr === false || ! isset($tokens[$functionPtr])) {
       return false;
     }
 

VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php

@@ -895,4 +895,18 @@ public function testUnusedForeachVariablesAreIgnoredIfSet() {
     ];
     $this->assertEquals($expectedWarnings, $lines);
   }
+
+  public function testGetDefinedVarsCountsAsRead() {
+    $fixtureFile = $this->getFixture('GetDefinedVarsFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
+    $phpcsFile->process();
+    $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+    $expectedWarnings = [
+			6,
+			18,
+			22,
+			29,
+    ];
+    $this->assertEquals($expectedWarnings, $lines);
+  }
 }

VariableAnalysis/Tests/CodeAnalysis/fixtures/GetDefinedVarsFixture.php

@@ -0,0 +1,31 @@
+<?php
+
+function send_vars_to_method( $object, $data ) {
+    $some_number = 4;
+    $some_string = 'hello';
+    echo $undefined_data; // should be a warning
+    $new_data = $object->transform_data( $data );
+    $object->continue_things( get_defined_vars() );
+}
+
+function send_vars_to_method_with_global( $object, $data ) {
+    global $global_data;
+    $new_data = $object->transform_data( $data );
+    $object->continue_things( get_defined_vars() );
+}
+
+function send_vars_to_method_with_scope_import( $object, $data ) {
+    $unused_data = 42; // should be a warning
+    $imported_data = 76;
+    return array_map( function( $datum ) use ( $object, $imported_data ) {
+        $new_data = $object->transform_data( $datum );
+        echo $undefined_data; // should be a warning
+        $object->continue_things( get_defined_vars() );
+    }, $data );
+}
+
+function send_var_with_var_named_get_defined_vars( $object, $data ) {
+    $get_defined_vars = 'hi';
+    $new_data = $object->transform_data( $data ); // should be a warning
+    $object->continue_things( $get_defined_vars );
+}