Allow for loop increment expression to use variable defined inside lo…

…op (#262) * Add tests for for loops * Reprocess for loop increment vars after loop ends * Prevent treating for loops (and other blocks) as function calls * Add test for for more specialized for loops * Add tests for unusual loops to test third expressions * Handle for loops without scope * Support 'endfor' as scope closer * Handle inline for loops * Fix indentation
sirbrillig · Aug 10, 2022 · 1087f11 · 1087f11
1 parent 2747116
commit 1087f11
Show file tree

Hide file tree

Showing 5 changed files with 549 additions and 10 deletions.
diff --git a/Tests/VariableAnalysisSniff/ForLoopTest.php b/Tests/VariableAnalysisSniff/ForLoopTest.php
@@ -0,0 +1,47 @@
+<?php
+
+namespace VariableAnalysis\Tests\VariableAnalysisSniff;
+
+use VariableAnalysis\Tests\BaseTestCase;
+
+class ForLoopTest extends BaseTestCase
+{
+	public function testFunctionWithForLoopWarningsReportsCorrectLines()
+	{
+		$fixtureFile = $this->getFixture('FunctionWithForLoopFixture.php');
+		$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+		$phpcsFile->process();
+		$lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+		$expectedWarnings = [
+			22,
+			65,
+			80,
+			94,
+			110,
+			112,
+			113,
+			118,
+			123,
+			129,
+		];
+		$this->assertSame($expectedWarnings, $lines);
+	}
+
+	public function testFunctionWithForLoopWarningsHasCorrectSniffCodes()
+	{
+		$fixtureFile = $this->getFixture('FunctionWithForLoopFixture.php');
+		$phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+		$phpcsFile->process();
+		$warnings = $phpcsFile->getWarnings();
+		$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[22][17][0]['source']);
+		$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[65][16][0]['source']);
+		$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[80][16][0]['source']);
+		$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[94][5][0]['source']);
+		$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[110][7][0]['source']);
+		$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[112][7][0]['source']);
+		$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[113][16][0]['source']);
+		$this->assertSame(self::UNUSED_ERROR_CODE, $warnings[118][5][0]['source']);
+		$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[123][5][0]['source']);
+		$this->assertSame(self::UNDEFINED_ERROR_CODE, $warnings[129][14][0]['source']);
+	}
+}
diff --git a/Tests/VariableAnalysisSniff/fixtures/FunctionWithForLoopFixture.php b/Tests/VariableAnalysisSniff/fixtures/FunctionWithForLoopFixture.php
@@ -0,0 +1,164 @@
+<?php
+
+function defineIncrementBeforeLoop($fp, $string) {
+  $count = 10;
+  for (
+    $written = 0;
+    $written < strlen($string);
+    $written += $count
+  ) {
+    $fwrite = fwrite($fp, substr($string, $written));
+    if ($fwrite === false) {
+      return $written;
+    }
+  }
+  return $written;
+}
+
+function defineIncrementAfterLoop($fp, $string) {
+  for (
+    $written = 0;
+    $written < strlen($string);
+    $written += $count // Undefined variable $count
+  ) {
+    $fwrite = fwrite($fp, substr($string, $written));
+    if ($fwrite === false) {
+      return $written;
+    }
+  }
+  $count = 10; // This is an unused variable, but it's hard to tell because it has been read above in the same scope.
+  return $written;
+}
+
+function defineIncrementInsideLoop($fp, $string) {
+  for (
+    $written = 0;
+    $written < strlen($string);
+    $written += $fwrite
+  ) {
+    $fwrite = fwrite($fp, substr($string, $written));
+    if ($fwrite === false) {
+      return $written;
+    }
+  }
+  return $written;
+}
+
+function defineConditionBeforeLoop($fp, $string) {
+  $count = 10;
+  for (
+    $written = 0;
+    $written < $count;
+    $written += 1
+  ) {
+    $fwrite = fwrite($fp, substr($string, $written));
+    if ($fwrite === false) {
+      return $written;
+    }
+  }
+  return $written;
+}
+
+function defineConditionAfterLoop($fp, $string) {
+  for (
+    $written = 0;
+    $written < $count; // Undefined variable $count
+    $written += 1
+  ) {
+    $fwrite = fwrite($fp, substr($string, $written));
+    if ($fwrite === false) {
+      return $written;
+    }
+  }
+  $count = 10;
+  return $written;
+}
+
+function defineConditionInsideLoop($fp, $string) {
+  for (
+    $written = 0;
+    $written < $fwrite; // Undefined variable $fwrite
+    $written += 1
+  ) {
+    $fwrite = fwrite($fp, substr($string, $written));
+    if ($fwrite === false) {
+      return $written;
+    }
+  }
+  return $written;
+}
+
+function unusedInitializer($fp, $string) {
+  $written = 0;
+  for (
+    $foo = 0; // Unused variable $foo
+    ;
+    $written += 1
+  ) {
+    fwrite($fp, substr($string, $written));
+    if ($written > 10) {
+      break;
+    }
+  }
+}
+
+function closureInsideLoopInit() {
+  for (
+    $closure = function(
+      $a,
+      $b,
+      $c // Unused variable $c
+    ) {
+      $m = 1; // Unused variable $m
+      var_dump($i); // Undefined variable $i
+      return $a * $b;
+    },
+    $a = 0,
+    $b = 0,
+    $c = 0, // Unused variable $c
+    $i = 10;
+  $closure($a, $b, 4) < $i;
+  $i++,
+    $a++,
+    $t++, // Undefined variable $t
+    $b++
+  )
+  {
+    var_dump($a);
+    var_dump($b);
+    var_dump($m); // Undefined variable $m
+  }
+}
+
+function veryBoringForLoop() {
+  for (
+    ;
+    ;
+  ) {
+  }
+}
+
+function reallySmallForLoop() {
+  for ($i = 1, $j = 0; $i <= 10; $j += $i, print $i, $i++);
+}
+
+function colonSyntaxForLoop() {
+  for ($i = 0; $i < 10; $i++):
+    print $i;
+  endfor;
+}
+
+function colonSyntaxForLoopAndLateDefinition() {
+  for ($i = 0; $i < 10; var_dump($foo), $i++):
+    $foo = 'hello';
+    print $i;
+  endfor;
+}
+
+function forLoopWithOneStatement() {
+  for ($i = 0; $i < 10; $i++) echo $i;
+}
+
+function forLoopWithOneStatementAndLateDefinition() {
+  for ($i = 0; $i < 10; var_dump($foo), $i++) $foo = 'hello';
+}
diff --git a/VariableAnalysis/Lib/ForLoopInfo.php b/VariableAnalysis/Lib/ForLoopInfo.php
@@ -0,0 +1,114 @@
+<?php
+
+namespace VariableAnalysis\Lib;
+
+/**
+ * Holds details of a for loop.
+ */
+class ForLoopInfo
+{
+	/**
+	 * The position of the `for` token.
+	 *
+	 * @var int
+	 */
+	public $forIndex;
+
+	/**
+	 * The position of the initialization expression opener for the loop.
+	 *
+	 * @var int
+	 */
+	public $initStart;
+
+	/**
+	 * The position of the initialization expression closer for the loop.
+	 *
+	 * @var int
+	 */
+	public $initEnd;
+
+	/**
+	 * The position of the condition expression opener for the loop.
+	 *
+	 * @var int
+	 */
+	public $conditionStart;
+
+	/**
+	 * The position of the condition expression closer for the loop.
+	 *
+	 * @var int
+	 */
+	public $conditionEnd;
+
+	/**
+	 * The position of the increment expression opener for the loop.
+	 *
+	 * @var int
+	 */
+	public $incrementStart;
+
+	/**
+	 * The position of the increment expression closer for the loop.
+	 *
+	 * @var int
+	 */
+	public $incrementEnd;
+
+	/**
+	 * The position of the block opener for the loop.
+	 *
+	 * @var int
+	 */
+	public $blockStart;
+
+	/**
+	 * The position of the block closer for the loop.
+	 *
+	 * @var int
+	 */
+	public $blockEnd;
+
+	/**
+	 * Any variables defined inside the third expression of the loop.
+	 *
+	 * The key is the variable index.
+	 *
+	 * @var array<int, \VariableAnalysis\Lib\VariableInfo>
+	 */
+	public $incrementVariables = [];
+
+	/**
+	 * @param int $forIndex
+	 * @param int $blockStart
+	 * @param int $blockEnd
+	 * @param int $initStart
+	 * @param int $initEnd
+	 * @param int $conditionStart
+	 * @param int $conditionEnd
+	 * @param int $incrementStart
+	 * @param int $incrementEnd
+	 */
+	public function __construct(
+		$forIndex,
+		$blockStart,
+		$blockEnd,
+		$initStart,
+		$initEnd,
+		$conditionStart,
+		$conditionEnd,
+		$incrementStart,
+		$incrementEnd
+	) {
+		$this->forIndex = $forIndex;
+		$this->blockStart = $blockStart;
+		$this->blockEnd = $blockEnd;
+		$this->initStart = $initStart;
+		$this->initEnd = $initEnd;
+		$this->conditionStart = $conditionStart;
+		$this->conditionEnd = $conditionEnd;
+		$this->incrementStart = $incrementStart;
+		$this->incrementEnd = $incrementEnd;
+	}
+}