Add UndefinedUnsetVariable sniff code (#208)

* Add test for UndefinedUnsetVariable sniff code

* Add UndefinedUnsetVariable sniff code

* Add UndefinedUnsetVariable to README

Author: sirbrillig

README.md

@@ -8,6 +8,7 @@ Please note that this README is for VariableAnalysis v3. For documentation about
 
 - Warns if variables are used without being defined. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable`)
 - Warns if variables are used for an array push shortcut without being defined. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedArrayVariable`)
+- Warns if variables are used inside `unset()` without being defined. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedUnsetVariable`)
 - Warns if variables are set or declared but never used. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable`)
 - Warns if function parameters are declared but never used. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedParameter`)
 - Warns if function parameters are declared but never used before other parameters that are used. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedParameterBeforeUsed`)

Tests/VariableAnalysisSniff/UnsetTest.php

@@ -0,0 +1,29 @@
+<?php
+namespace VariableAnalysis\Tests\VariableAnalysisSniff;
+
+use VariableAnalysis\Tests\BaseTestCase;
+
+class UnsetTest extends BaseTestCase {
+  public function testUnsetReportsUndefinedVariables() {
+    $fixtureFile = $this->getFixture('UnsetFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+    $phpcsFile->process();
+    $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+    // Technically, these are not illegal, but they may be typos. See https://github.com/sirbrillig/phpcs-variable-analysis/issues/174
+    $expectedWarnings = [
+      6,
+      11,
+    ];
+    $this->assertEquals($expectedWarnings, $lines);
+  }
+
+  public function testUnsetHasCorrectSniffCodes() {
+    $fixtureFile = $this->getFixture('UnsetFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+    $phpcsFile->process();
+
+    $warnings = $phpcsFile->getWarnings();
+    $this->assertEquals('VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedUnsetVariable', $warnings[6][7][0]['source']);
+    $this->assertEquals('VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedUnsetVariable', $warnings[11][9][0]['source']);
+  }
+}

Tests/VariableAnalysisSniff/fixtures/UnsetFixture.php

@@ -0,0 +1,12 @@
+<?php
+
+$foo = 'hello';
+
+unset($foo);
+unset($bar); // undefined variable $bar
+
+function unset_loop($array) {
+  foreach ($array as $value) {
+  }
+  unset($key, $value); // undefined variable $key
+}

VariableAnalysis/Lib/Helpers.php

@@ -830,4 +830,25 @@ public static function isVariableArrayPushShortcut(File $phpcsFile, $stackPtr) {
 
     return true;
   }
+
+  /**
+   * @param File $phpcsFile
+   * @param int $stackPtr
+   *
+   * @return bool
+   */
+  public static function isVariableInsideUnset(File $phpcsFile, $stackPtr) {
+    $functionIndex = self::getFunctionIndexForFunctionCallArgument($phpcsFile, $stackPtr);
+    if (! is_int($functionIndex)) {
+      return false;
+    }
+    $tokens = $phpcsFile->getTokens();
+    if (! isset($tokens[$functionIndex])) {
+      return false;
+    }
+    if ($tokens[$functionIndex]['content'] === 'unset') {
+      return true;
+    }
+    return false;
+  }
 }

VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

@@ -560,12 +560,19 @@ protected function markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $sta
     $this->markVariableRead($varName, $stackPtr, $currScope);
     if ($this->isVariableUndefined($varName, $stackPtr, $currScope) === true) {
       Helpers::debug("variable $varName looks undefined");
+
       if (Helpers::isVariableArrayPushShortcut($phpcsFile, $stackPtr)) {
         $this->warnAboutUndefinedArrayPushShortcut($phpcsFile, $varName, $stackPtr);
         // Mark the variable as defined if it's of the form `$x[] = 1;`
         $this->markVariableAssignment($varName, $stackPtr, $currScope);
         return;
       }
+
+      if (Helpers::isVariableInsideUnset($phpcsFile, $stackPtr)) {
+        $this->warnAboutUndefinedUnset($phpcsFile, $varName, $stackPtr);
+        return;
+      }
+
       $this->warnAboutUndefinedVariable($phpcsFile, $varName, $stackPtr);
     }
   }
@@ -1773,6 +1780,7 @@ protected function warnAboutUndefinedVariable(File $phpcsFile, $varName, $stackP
         ["\${$varName}"]
       );
   }
+
   /**
    * @param File $phpcsFile
    * @param string $varName
@@ -1788,4 +1796,20 @@ protected function warnAboutUndefinedArrayPushShortcut(File $phpcsFile, $varName
         ["\${$varName}"]
       );
   }
+
+  /**
+   * @param File $phpcsFile
+   * @param string $varName
+   * @param int $stackPtr
+   *
+   * @return void
+   */
+  protected function warnAboutUndefinedUnset(File $phpcsFile, $varName, $stackPtr) {
+      $phpcsFile->addWarning(
+        "Variable %s inside unset call is undefined.",
+        $stackPtr,
+        'UndefinedUnsetVariable',
+        ["\${$varName}"]
+      );
+  }
 }