From c1638ffaabfb8ffad41e530532f7a131db30a8e8 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Tue, 8 Aug 2023 17:08:13 +0100
Subject: [PATCH] Ban foreach by-reference at the PHPStan level

---
 phpstan.neon.dist                             |  1 +
 src/permission/PermissionManager.php          |  6 +--
 .../rules/DisallowForeachByReferenceRule.php  | 53 +++++++++++++++++++
 .../block/regenerate_consistency_check.php    |  4 +-
 4 files changed, 60 insertions(+), 4 deletions(-)
 create mode 100644 tests/phpstan/rules/DisallowForeachByReferenceRule.php

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
index b3aeaf4f6..1d72511f7 100644
--- a/phpstan.neon.dist
+++ b/phpstan.neon.dist
@@ -11,6 +11,7 @@ includes:
 
 rules:
 	- pocketmine\phpstan\rules\DisallowEnumComparisonRule
+	- pocketmine\phpstan\rules\DisallowForeachByReferenceRule
 	- pocketmine\phpstan\rules\UnsafeForeachArrayOfStringRule
 #	- pocketmine\phpstan\rules\ThreadedSupportedTypesRule
 
diff --git a/src/permission/PermissionManager.php b/src/permission/PermissionManager.php
index b7e622934..2d8324887 100644
--- a/src/permission/PermissionManager.php
+++ b/src/permission/PermissionManager.php
@@ -81,9 +81,9 @@ class PermissionManager{
 	}
 
 	public function unsubscribeFromAllPermissions(PermissibleInternal $permissible) : void{
-		foreach($this->permSubs as $permission => &$subs){
-			unset($subs[spl_object_id($permissible)]);
-			if(count($subs) === 0){
+		foreach($this->permSubs as $permission => $subs){
+			unset($this->permSubs[$permission][spl_object_id($permissible)]);
+			if(count($this->permSubs[$permission]) === 0){
 				unset($this->permSubs[$permission]);
 			}
 		}
diff --git a/tests/phpstan/rules/DisallowForeachByReferenceRule.php b/tests/phpstan/rules/DisallowForeachByReferenceRule.php
new file mode 100644
index 000000000..79124d328
--- /dev/null
+++ b/tests/phpstan/rules/DisallowForeachByReferenceRule.php
@@ -0,0 +1,53 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+ */
+
+declare(strict_types=1);
+
+namespace pocketmine\phpstan\rules;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\Foreach_;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+
+/**
+ * @phpstan-implements Rule<Foreach_>
+ */
+final class DisallowForeachByReferenceRule implements Rule{
+
+	public function getNodeType() : string{
+		return Foreach_::class;
+	}
+
+	public function processNode(Node $node, Scope $scope) : array{
+		/** @var Foreach_ $node */
+		if($node->byRef){
+			return [
+				RuleErrorBuilder::message("Foreach by-reference is not allowed, because it has surprising behaviour.")
+					->tip("If the value variable is used outside of the foreach construct (e.g. in a second foreach), the iterable's contents will be unexpectedly altered.")
+					->build()
+			];
+		}
+
+		return [];
+	}
+}
diff --git a/tests/phpunit/block/regenerate_consistency_check.php b/tests/phpunit/block/regenerate_consistency_check.php
index 9930c54f0..ac8a1ad9d 100644
--- a/tests/phpunit/block/regenerate_consistency_check.php
+++ b/tests/phpunit/block/regenerate_consistency_check.php
@@ -24,6 +24,7 @@ declare(strict_types=1);
 use pocketmine\block\Block;
 use pocketmine\block\RuntimeBlockStateRegistry;
 use pocketmine\utils\AssumptionFailedError;
+use pocketmine\utils\Utils;
 
 require dirname(__DIR__, 3) . '/vendor/autoload.php';
 
@@ -91,8 +92,9 @@ foreach($new as $stateId => $name){
 	$newTable[$name][] = $stateId;
 }
 ksort($newTable, SORT_STRING);
-foreach($newTable as &$stateIds){
+foreach(Utils::stringifyKeys($newTable) as $name => $stateIds){
 	sort($stateIds, SORT_NUMERIC);
+	$newTable[$name] = $stateIds;
 }
 
 file_put_contents(__DIR__ . '/block_factory_consistency_check.json', json_encode(