Check - overwriting scope variables with foreach

Author: ondrejmirtes

README.md

@@ -19,6 +19,7 @@
 * Statically declared methods are called statically.
 * Disallow `empty()` - it's a very loose comparison (see [manual](https://php.net/empty)), it's recommended to use more strict one.
 * Disallow variable variables (`$$foo`, `$this->$method()` etc.)
+* Disallow overwriting variables with foreach key and value variables
 * Always true `instanceof`, type-checking `is_*` functions and strict comparisons `===`/`!==`. These checks can be turned off by setting `checkAlwaysTrueInstanceof`/`checkAlwaysTrueCheckTypeFunctionCall`/`checkAlwaysTrueStrictComparison` to false.
 * Require parameter and return typehints for functions and methods (either native or phpDocs)
 * Correct case for referenced and called function names.

rules.neon

@@ -17,6 +17,7 @@ rules:
 	- PHPStan\Rules\Cast\UselessCastRule
 	- PHPStan\Rules\DisallowedConstructs\DisallowedEmptyRule
 	- PHPStan\Rules\DisallowedConstructs\DisallowedImplicitArrayCreationRule
+	- PHPStan\Rules\ForeachLoop\OverwriteVariablesWithForeachRule
 	- PHPStan\Rules\Functions\MissingFunctionParameterTypehintRule
 	- PHPStan\Rules\Functions\MissingFunctionReturnTypehintRule
 	- PHPStan\Rules\Methods\MissingMethodParameterTypehintRule

src/Rules/ForeachLoop/OverwriteVariablesWithForeachRule.php

@@ -0,0 +1,75 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\ForeachLoop;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+
+class OverwriteVariablesWithForeachRule implements Rule
+{
+
+	public function getNodeType(): string
+	{
+		return Node\Stmt\Foreach_::class;
+	}
+
+	/**
+	 * @param \PhpParser\Node\Stmt\Foreach_ $node
+	 * @param \PHPStan\Analyser\Scope $scope
+	 * @return string[]
+	 */
+	public function processNode(Node $node, Scope $scope): array
+	{
+		$errors = [];
+		if (
+			$node->keyVar instanceof Node\Expr\Variable
+			&& is_string($node->keyVar->name)
+			&& $scope->hasVariableType($node->keyVar->name)->yes()
+		) {
+			$errors[] = sprintf('Foreach overwrites $%s with its key variable.', $node->keyVar->name);
+		}
+
+		foreach ($this->checkValueVar($scope, $node->valueVar) as $error) {
+			$errors[] = $error;
+		}
+
+		return $errors;
+	}
+
+	/**
+	 * @param Scope $scope
+	 * @param Expr $expr
+	 * @return string[]
+	 */
+	private function checkValueVar(Scope $scope, Expr $expr): array
+	{
+		$errors = [];
+		if (
+			$expr instanceof Node\Expr\Variable
+			&& is_string($expr->name)
+			&& $scope->hasVariableType($expr->name)->yes()
+		) {
+			$errors[] = sprintf('Foreach overwrites $%s with its value variable.', $expr->name);
+		}
+
+		if (
+			$expr instanceof Node\Expr\List_
+			|| $expr instanceof Node\Expr\Array_
+		) {
+			foreach ($expr->items as $item) {
+				if ($item === null) {
+					continue;
+				}
+
+				foreach ($this->checkValueVar($scope, $item->value) as $error) {
+					$errors[] = $error;
+				}
+			}
+		}
+
+		return $errors;
+	}
+
+}

tests/Rules/ForeachLoop/OverwriteVariablesWithForeachRuleTest.php

@@ -0,0 +1,42 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\ForeachLoop;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+class OverwriteVariablesWithForeachRuleTest extends RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new OverwriteVariablesWithForeachRule();
+	}
+
+	public function testRule(): void
+	{
+		$this->analyse([__DIR__ . '/data/foreach.php'], [
+			[
+				'Foreach overwrites $str with its value variable.',
+				14,
+			],
+			[
+				'Foreach overwrites $b with its value variable.',
+				26,
+			],
+			[
+				'Foreach overwrites $d with its value variable.',
+				26,
+			],
+			[
+				'Foreach overwrites $b with its value variable.',
+				32,
+			],
+			[
+				'Foreach overwrites $d with its value variable.',
+				32,
+			],
+		]);
+	}
+
+}

tests/Rules/ForeachLoop/data/foreach.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace OverwriteVariablesWithForeach;
+
+class Foo
+{
+
+	public function doFoo(array $a, array $b, string $str)
+	{
+		foreach ($a as $val) {
+
+		}
+
+		foreach ($a as $str) {
+
+		}
+
+		foreach ($a as $val) {
+			foreach ($b as $var) {
+
+			}
+		}
+	}
+
+	public function doBar(array $a, string $b, string $d) {
+		foreach ($a as [$b, $c, [$d, $e]]) {
+
+		}
+	}
+
+	public function doBaz(array $a, string $b, string $d) {
+		foreach ($a as list($b, $c, list($d, $e))) {
+
+		}
+	}
+
+}