Bleeding edge - AssignToByRefExprFromForeachRule (level 1)

Author: ondrejmirtes

conf/bleedingEdge.neon

@@ -13,3 +13,4 @@ parameters:
 		reportTooWideBool: true
 		rawMessageInBaseline: true
 		reportNestedTooWideType: false
+		assignToByRefForeachExpr: true

conf/config.level1.neon

@@ -10,3 +10,11 @@ parameters:
 autowiredAttributeServices:
 	# registers rules with #[RegisteredRule] attribute
 	level: 1
+
+conditionalTags:
+	PHPStan\Rules\Variables\AssignToByRefExprFromForeachRule:
+		phpstan.rules.rule: %featureToggles.assignToByRefForeachExpr%
+
+services:
+	-
+		class: PHPStan\Rules\Variables\AssignToByRefExprFromForeachRule

conf/config.neon

@@ -37,6 +37,7 @@ parameters:
 		reportTooWideBool: false
 		rawMessageInBaseline: false
 		reportNestedTooWideType: false
+		assignToByRefForeachExpr: false
 	fileExtensions:
 		- php
 	checkAdvancedIsset: false

conf/parametersSchema.neon

@@ -40,6 +40,7 @@ parametersSchema:
 		reportTooWideBool: bool()
 		rawMessageInBaseline: bool()
 		reportNestedTooWideType: bool()
+		assignToByRefForeachExpr: bool()
 	])
 	fileExtensions: listOf(string())
 	checkAdvancedIsset: bool()

src/Analyser/NodeScopeResolver.php

@@ -86,6 +86,7 @@
 use PHPStan\Node\ExecutionEndNode;
 use PHPStan\Node\Expr\AlwaysRememberedExpr;
 use PHPStan\Node\Expr\ExistingArrayDimFetch;
+use PHPStan\Node\Expr\ForeachValueByRefExpr;
 use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
 use PHPStan\Node\Expr\GetIterableValueTypeExpr;
 use PHPStan\Node\Expr\GetOffsetValueTypeExpr;
@@ -1207,6 +1208,14 @@ private function processStmtNode(
 			$originalScope = $scope;
 			$bodyScope = $scope;
 
+			if ($stmt->keyVar instanceof Variable) {
+				$nodeCallback(new VariableAssignNode($stmt->keyVar, new GetIterableKeyTypeExpr($stmt->expr)), $originalScope);
+			}
+
+			if ($stmt->valueVar instanceof Variable) {
+				$nodeCallback(new VariableAssignNode($stmt->valueVar, new GetIterableValueTypeExpr($stmt->expr)), $originalScope);
+			}
+
 			if ($context->isTopLevel()) {
 				$originalScope = $this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope;
 				$bodyScope = $this->enterForeach($originalScope, $originalScope, $stmt, $nodeCallback);
@@ -1269,6 +1278,9 @@ private function processStmtNode(
 			if (!(new ObjectType(Traversable::class))->isSuperTypeOf($scope->getType($stmt->expr))->no()) {
 				$throwPoints[] = ThrowPoint::createImplicit($scope, $stmt->expr);
 			}
+			if ($stmt->byRef) {
+				$finalScope = $finalScope->assignExpression(new ForeachValueByRefExpr($stmt->valueVar), new MixedType(), new MixedType());
+			}
 
 			return new StatementResult(
 				$finalScope,
@@ -1926,6 +1938,7 @@ public function leaveNode(Node $node): ?ExistingArrayDimFetch
 					$scope = $scope->invalidateExpression($var);
 				}
 
+				$scope = $scope->invalidateExpression(new ForeachValueByRefExpr($var));
 			}
 		} elseif ($stmt instanceof Node\Stmt\Use_) {
 			$hasYield = false;

src/Node/Expr/ForeachValueByRefExpr.php

@@ -0,0 +1,37 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Node\Expr;
+
+use Override;
+use PhpParser\Node\Expr;
+use PHPStan\Node\VirtualNode;
+
+final class ForeachValueByRefExpr extends Expr implements VirtualNode
+{
+
+	public function __construct(private Expr $expr)
+	{
+		parent::__construct([]);
+	}
+
+	public function getExpr(): Expr
+	{
+		return $this->expr;
+	}
+
+	#[Override]
+	public function getType(): string
+	{
+		return 'PHPStan_Node_ForeachValueByRefExpr';
+	}
+
+	/**
+	 * @return string[]
+	 */
+	#[Override]
+	public function getSubNodeNames(): array
+	{
+		return [];
+	}
+
+}

src/Node/Printer/Printer.php

@@ -6,6 +6,7 @@
 use PHPStan\DependencyInjection\AutowiredService;
 use PHPStan\Node\Expr\AlwaysRememberedExpr;
 use PHPStan\Node\Expr\ExistingArrayDimFetch;
+use PHPStan\Node\Expr\ForeachValueByRefExpr;
 use PHPStan\Node\Expr\GetIterableKeyTypeExpr;
 use PHPStan\Node\Expr\GetIterableValueTypeExpr;
 use PHPStan\Node\Expr\GetOffsetValueTypeExpr;
@@ -88,6 +89,11 @@ protected function pPHPStan_Node_PropertyInitializationExpr(PropertyInitializati
 		return sprintf('__phpstanPropertyInitialization(%s)', $expr->getPropertyName());
 	}
 
+	protected function pPHPStan_Node_ForeachValueByRefExpr(ForeachValueByRefExpr $expr): string // phpcs:ignore
+	{
+		return sprintf('__phpstanForeachValueByRef(%s)', $this->p($expr->getExpr()));
+	}
+
 	protected function pPHPStan_Node_ParameterVariableOriginalValueExpr(ParameterVariableOriginalValueExpr $expr): string // phpcs:ignore
 	{
 		return sprintf('__phpstanParameterVariableOriginalValue(%s)', $expr->getVariableName());

src/Rules/Variables/AssignToByRefExprFromForeachRule.php

@@ -0,0 +1,54 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Variables;
+
+use PhpParser\Node;
+use PhpParser\NodeAbstract;
+use PHPStan\Analyser\Scope;
+use PHPStan\Node\Expr\ForeachValueByRefExpr;
+use PHPStan\Node\Printer\ExprPrinter;
+use PHPStan\Node\PropertyAssignNode;
+use PHPStan\Node\VariableAssignNode;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use function sprintf;
+
+/**
+ * @implements Rule<NodeAbstract>
+ */
+final class AssignToByRefExprFromForeachRule implements Rule
+{
+
+	public function __construct(private ExprPrinter $printer)
+	{
+	}
+
+	public function getNodeType(): string
+	{
+		return NodeAbstract::class;
+	}
+
+	public function processNode(Node $node, Scope $scope): array
+	{
+		if ($node instanceof VariableAssignNode) {
+			$expr = $node->getVariable();
+		} elseif ($node instanceof PropertyAssignNode) {
+			$expr = $node->getPropertyFetch();
+		} else {
+			return [];
+		}
+
+		if ($scope->hasExpressionType(new ForeachValueByRefExpr($expr))->no()) {
+			return [];
+		}
+
+		return [
+			RuleErrorBuilder::message(sprintf(
+				'Assign to %s overwrites the last element from array.',
+				$this->printer->printExpr($expr),
+			))->identifier('assign.byRefForeachExpr')
+				->tip('Unset it right after foreach to avoid this problem.')->build(),
+		];
+	}
+
+}

tests/PHPStan/Rules/Variables/AssignToByRefExprFromForeachRuleTest.php

@@ -0,0 +1,48 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Variables;
+
+use PHPStan\Node\Printer\ExprPrinter;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+/**
+ * @extends RuleTestCase<AssignToByRefExprFromForeachRule>
+ */
+class AssignToByRefExprFromForeachRuleTest extends RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new AssignToByRefExprFromForeachRule(
+			self::getContainer()->getByType(ExprPrinter::class),
+		);
+	}
+
+	public function testRule(): void
+	{
+		$this->analyse([__DIR__ . '/data/assign-to-by-ref-expr-from-foreach.php'], [
+			[
+				'Assign to $item overwrites the last element from array.',
+				26,
+				'Unset it right after foreach to avoid this problem.',
+			],
+			[
+				'Assign to $item overwrites the last element from array.',
+				50,
+				'Unset it right after foreach to avoid this problem.',
+			],
+			[
+				'Assign to $item overwrites the last element from array.',
+				51,
+				'Unset it right after foreach to avoid this problem.',
+			],
+			[
+				'Assign to $item overwrites the last element from array.',
+				62,
+				'Unset it right after foreach to avoid this problem.',
+			],
+		]);
+	}
+
+}

tests/PHPStan/Rules/Variables/data/assign-to-by-ref-expr-from-foreach.php

@@ -0,0 +1,67 @@
+<?php
+
+namespace AssignToByRefExprFromForeach;
+
+class Foo
+{
+
+	/** @param list<array<string, mixed>> $input */
+	public function sayHello(array $input): void
+	{
+		foreach ($input as &$item) {
+			$item = 2;
+		}
+		unset($item);
+
+		$item = 'foo';
+	}
+
+	/** @param list<array<string, mixed>> $input */
+	public function sayHello2(array $input): void
+	{
+		foreach ($input as &$item) {
+			$item = 2;
+		}
+
+		$item = 'foo';
+	}
+
+	public function doFoo(): void
+	{
+		$array = [0, 1, 2, 3];
+		foreach ($array as &$item) {
+			$item = 2;
+		}
+
+		unset($item);
+
+		foreach ($array as $item) {
+
+		}
+	}
+
+	public function doFoo2(): void
+	{
+		$array = [0, 1, 2, 3];
+		foreach ($array as &$item) {
+			$item = 2;
+		}
+
+		foreach ($array as $item) {
+			$item = 2;
+		}
+	}
+
+	public function doFoo3(): void
+	{
+		$array = [0, 1, 2, 3];
+		foreach ($array as &$item) {
+
+		}
+
+		foreach ($array as $item) {
+
+		}
+	}
+
+}