Ban foreach(arrayWithStringKeys as k => v)

this is not as good as phpstan/phpstan-src#769 (e.g. array_key_first()/array_key_last() aren't covered by this, nor is array_rand()) but it does eliminate the most infuriating cases where this usually crops up.

build/generate-known-translation-apis.php

@@ -24,6 +24,7 @@ declare(strict_types=1);
 namespace pocketmine\build\generate_known_translation_apis;
 
 use pocketmine\lang\Translatable;
+use pocketmine\utils\Utils;
 use Webmozart\PathUtil\Path;
 use function array_map;
 use function count;
@@ -100,7 +101,7 @@ final class KnownTranslationKeys{
 HEADER;
 
 	ksort($languageDefinitions, SORT_STRING);
-	foreach($languageDefinitions as $k => $_){
+	foreach(Utils::stringifyKeys($languageDefinitions) as $k => $_){
 		echo "\tpublic const ";
 		echo constantify($k);
 		echo " = \"" . $k . "\";\n";
@@ -135,7 +136,7 @@ HEADER;
 	$parameterRegex = '/{%(.+?)}/';
 
 	$translationContainerClass = (new \ReflectionClass(Translatable::class))->getShortName();
-	foreach($languageDefinitions as $key => $value){
+	foreach(Utils::stringifyKeys($languageDefinitions) as $key => $value){
 		$parameters = [];
 		if(preg_match_all($parameterRegex, $value, $matches) > 0){
 			foreach($matches[1] as $parameterName){

build/make-release.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\build\make_release;
 
+use pocketmine\utils\Utils;
 use pocketmine\utils\VersionString;
 use pocketmine\VersionInfo;
 use function array_keys;
@@ -76,7 +77,7 @@ const ACCEPTED_OPTS = [
 
 function main() : void{
 	$filteredOpts = [];
-	foreach(getopt("", ["current:", "next:", "channel:", "help"]) as $optName => $optValue){
+	foreach(Utils::stringifyKeys(getopt("", ["current:", "next:", "channel:", "help"])) as $optName => $optValue){
 		if($optName === "help"){
 			fwrite(STDOUT, "Options:\n");
 

phpstan.neon.dist

@@ -13,6 +13,7 @@ includes:
 
 rules:
 	- pocketmine\phpstan\rules\DisallowEnumComparisonRule
+	- pocketmine\phpstan\rules\UnsafeForeachArrayOfStringRule
 #	- pocketmine\phpstan\rules\ThreadedSupportedTypesRule
 
 parameters:

src/MemoryManager.php

@@ -368,7 +368,7 @@ class MemoryManager{
 				'_SESSION' => true
 			];
 
-			foreach($GLOBALS as $varName => $value){
+			foreach(Utils::stringifyKeys($GLOBALS) as $varName => $value){
 				if(isset($ignoredGlobals[$varName])){
 					continue;
 				}

src/Server.php

@@ -1201,7 +1201,7 @@ class Server{
 	 * Unsubscribes from all broadcast channels.
 	 */
 	public function unsubscribeFromAllBroadcastChannels(CommandSender $subscriber) : void{
-		foreach($this->broadcastSubscribers as $channelId => $recipients){
+		foreach(Utils::stringifyKeys($this->broadcastSubscribers) as $channelId => $recipients){
 			$this->unsubscribeFromBroadcastChannel($channelId, $subscriber);
 		}
 	}

src/crash/CrashDump.php

@@ -350,7 +350,7 @@ class CrashDump{
 		$this->addLine("Zend version: " . zend_version());
 		$this->addLine("OS: " . PHP_OS . ", " . Utils::getOS());
 		$this->addLine("Composer libraries: ");
-		foreach($composerLibraries as $library => $libraryVersion){
+		foreach(Utils::stringifyKeys($composerLibraries) as $library => $libraryVersion){
 			$this->addLine("- $library $libraryVersion");
 		}
 	}

src/network/mcpe/convert/ItemTranslator.php

@@ -27,6 +27,7 @@ use pocketmine\data\bedrock\LegacyItemIdToStringIdMap;
 use pocketmine\network\mcpe\protocol\serializer\ItemTypeDictionary;
 use pocketmine\utils\AssumptionFailedError;
 use pocketmine\utils\SingletonTrait;
+use pocketmine\utils\Utils;
 use Webmozart\PathUtil\Path;
 use function array_key_exists;
 use function file_get_contents;
@@ -92,7 +93,7 @@ final class ItemTranslator{
 			}
 			$simpleMappings[$newId] = $intId;
 		}
-		foreach($legacyStringToIntMap->getStringToLegacyMap() as $stringId => $intId){
+		foreach(Utils::stringifyKeys($legacyStringToIntMap->getStringToLegacyMap()) as $stringId => $intId){
 			if(isset($simpleMappings[$stringId])){
 				throw new \UnexpectedValueException("Old ID $stringId collides with new ID");
 			}

src/network/query/QueryInfo.php

@@ -233,7 +233,7 @@ final class QueryInfo{
 			$query .= $key . "\x00" . $value . "\x00";
 		}
 
-		foreach($this->extraData as $key => $value){
+		foreach(Utils::stringifyKeys($this->extraData) as $key => $value){
 			$query .= $key . "\x00" . $value . "\x00";
 		}
 

src/permission/PermissibleInternal.php

@@ -27,6 +27,7 @@ use pocketmine\plugin\Plugin;
 use pocketmine\plugin\PluginException;
 use pocketmine\timings\Timings;
 use pocketmine\utils\ObjectSet;
+use pocketmine\utils\Utils;
 use function count;
 use function spl_object_id;
 
@@ -143,7 +144,7 @@ class PermissibleInternal implements Permissible{
 		$oldPermissions = $this->permissions;
 		$this->permissions = [];
 
-		foreach($this->rootPermissions as $name => $isGranted){
+		foreach(Utils::stringifyKeys($this->rootPermissions) as $name => $isGranted){
 			$perm = $permManager->getPermission($name);
 			if($perm === null){
 				throw new \LogicException("Unregistered root permission $name");
@@ -187,11 +188,12 @@ class PermissibleInternal implements Permissible{
 	}
 
 	/**
-	 * @param bool[]                    $children
+	 * @param bool[] $children
+	 * @phpstan-param array<string, bool> $children
 	 */
 	private function calculateChildPermissions(array $children, bool $invert, ?PermissionAttachment $attachment, ?PermissionAttachmentInfo $parent) : void{
 		$permManager = PermissionManager::getInstance();
-		foreach($children as $name => $v){
+		foreach(Utils::stringifyKeys($children) as $name => $v){
 			$perm = $permManager->getPermission($name);
 			$value = ($v xor $invert);
 			$this->permissions[$name] = new PermissionAttachmentInfo($name, $attachment, $value, $parent);

src/permission/PermissionParser.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\permission;
 
+use pocketmine\utils\Utils;
 use function is_bool;
 use function strtolower;
 
@@ -83,7 +84,7 @@ class PermissionParser{
 	 */
 	public static function loadPermissions(array $data, string $default = self::DEFAULT_FALSE) : array{
 		$result = [];
-		foreach($data as $name => $entry){
+		foreach(Utils::stringifyKeys($data) as $name => $entry){
 			$desc = null;
 			if(isset($entry["default"])){
 				$default = PermissionParser::defaultFromString($entry["default"]);

src/plugin/PluginBase.php

@@ -32,6 +32,7 @@ use pocketmine\scheduler\TaskScheduler;
 use pocketmine\Server;
 use pocketmine\utils\AssumptionFailedError;
 use pocketmine\utils\Config;
+use pocketmine\utils\Utils;
 use Webmozart\PathUtil\Path;
 use function count;
 use function dirname;
@@ -162,7 +163,7 @@ abstract class PluginBase implements Plugin, CommandExecutor{
 	private function registerYamlCommands() : void{
 		$pluginCmds = [];
 
-		foreach($this->getDescription()->getCommands() as $key => $data){
+		foreach(Utils::stringifyKeys($this->getDescription()->getCommands()) as $key => $data){
 			if(strpos($key, ":") !== false){
 				$this->logger->error($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_commandError($key, $this->getDescription()->getFullName(), ":")));
 				continue;

src/plugin/PluginLoadabilityChecker.php

@@ -76,7 +76,7 @@ final class PluginLoadabilityChecker{
 			}
 		}
 
-		foreach($description->getRequiredExtensions() as $extensionName => $versionConstrs){
+		foreach(Utils::stringifyKeys($description->getRequiredExtensions()) as $extensionName => $versionConstrs){
 			$gotVersion = phpversion($extensionName);
 			if($gotVersion === false){
 				return KnownTranslationFactory::pocketmine_plugin_extensionNotLoaded($extensionName);

src/plugin/PluginManager.php

@@ -181,7 +181,7 @@ class PluginManager{
 		}
 		$opRoot = $permManager->getPermission(DefaultPermissions::ROOT_OPERATOR);
 		$everyoneRoot = $permManager->getPermission(DefaultPermissions::ROOT_USER);
-		foreach($description->getPermissions() as $default => $perms){
+		foreach(Utils::stringifyKeys($description->getPermissions()) as $default => $perms){
 			foreach($perms as $perm){
 				$permManager->addPermission($perm);
 				switch($default){
@@ -345,7 +345,7 @@ class PluginManager{
 
 		while(count($triage->plugins) > 0){
 			$loadedThisLoop = 0;
-			foreach($triage->plugins as $name => $entry){
+			foreach(Utils::stringifyKeys($triage->plugins) as $name => $entry){
 				$this->checkDepsForTriage($name, "hard", $triage->dependencies, $loadedPlugins, $triage);
 				$this->checkDepsForTriage($name, "soft", $triage->softDependencies, $loadedPlugins, $triage);
 
@@ -377,7 +377,7 @@ class PluginManager{
 				//No plugins loaded :(
 
 				//check for skippable soft dependencies first, in case the dependents could resolve hard dependencies
-				foreach($triage->plugins as $name => $file){
+				foreach(Utils::stringifyKeys($triage->plugins) as $name => $file){
 					if(isset($triage->softDependencies[$name]) && !isset($triage->dependencies[$name])){
 						foreach($triage->softDependencies[$name] as $k => $dependency){
 							if($this->getPlugin($dependency) === null && !array_key_exists($dependency, $triage->plugins)){
@@ -392,7 +392,7 @@ class PluginManager{
 					}
 				}
 
-				foreach($triage->plugins as $name => $file){
+				foreach(Utils::stringifyKeys($triage->plugins) as $name => $file){
 					if(isset($triage->dependencies[$name])){
 						$unknownDependencies = [];
 
@@ -415,7 +415,7 @@ class PluginManager{
 					}
 				}
 
-				foreach($triage->plugins as $name => $file){
+				foreach(Utils::stringifyKeys($triage->plugins) as $name => $file){
 					$this->server->getLogger()->critical($this->server->getLanguage()->translate(KnownTranslationFactory::pocketmine_plugin_loadError($name, KnownTranslationFactory::pocketmine_plugin_circularDependency())));
 				}
 				break;

src/stats/SendUsageTask.php

@@ -123,9 +123,7 @@ class SendUsageTask extends AsyncTask{
 				];
 
 				//This anonymizes the user ids so they cannot be reversed to the original
-				foreach($playerList as $k => $v){
-					$playerList[$k] = md5($v);
-				}
+				$playerList = array_map('md5', $playerList);
 
 				$players = array_map(function(Player $p) : string{ return md5($p->getUniqueId()->getBytes()); }, $server->getOnlinePlayers());
 

src/utils/Config.php

@@ -425,7 +425,7 @@ class Config{
 	public function set($k, $v = true) : void{
 		$this->config[$k] = $v;
 		$this->changed = true;
-		foreach($this->nestedCache as $nestedKey => $nvalue){
+		foreach(Utils::stringifyKeys($this->nestedCache) as $nestedKey => $nvalue){
 			if(substr($nestedKey, 0, strlen($k) + 1) === ($k . ".")){
 				unset($this->nestedCache[$nestedKey]);
 			}
@@ -487,7 +487,7 @@ class Config{
 	 */
 	private function fillDefaults(array $default, &$data) : int{
 		$changed = 0;
-		foreach($default as $k => $v){
+		foreach(Utils::stringifyKeys($default) as $k => $v){
 			if(is_array($v)){
 				if(!isset($data[$k]) or !is_array($data[$k])){
 					$data[$k] = [];
@@ -536,7 +536,7 @@ class Config{
 	 */
 	public static function writeProperties(array $config) : string{
 		$content = "#Properties Config file\r\n#" . date("D M j H:i:s T Y") . "\r\n";
-		foreach($config as $k => $v){
+		foreach(Utils::stringifyKeys($config) as $k => $v){
 			if(is_bool($v)){
 				$v = $v ? "on" : "off";
 			}

src/utils/Filesystem.php

@@ -163,7 +163,8 @@ final class Filesystem{
 		$result = str_replace([DIRECTORY_SEPARATOR, ".php", "phar://"], ["/", "", ""], $path);
 
 		//remove relative paths
-		foreach(self::$cleanedPaths as $cleanPath => $replacement){
+		//this should probably never have integer keys, but it's safer than making PHPStan ignore it
+		foreach(Utils::stringifyKeys(self::$cleanedPaths) as $cleanPath => $replacement){
 			$cleanPath = rtrim(str_replace([DIRECTORY_SEPARATOR, "phar://"], ["/", ""], $cleanPath), "/");
 			if(strpos($result, $cleanPath) === 0){
 				$result = ltrim(str_replace($cleanPath, $replacement, $result), "/");

src/utils/StringToTParser.php

@@ -75,7 +75,7 @@ abstract class StringToTParser{
 		return strtolower(str_replace([" ", "minecraft:"], ["_", ""], trim($input)));
 	}
 
-	/** @return string[] */
+	/** @return string[]|int[] */
 	public function getKnownAliases() : array{
 		return array_keys($this->callbackMap);
 	}

src/utils/Utils.php

@@ -571,6 +571,22 @@ final class Utils{
 		}
 	}
 
+	/**
+	 * Generator which forces array keys to string during iteration.
+	 * This is necessary because PHP has an anti-feature where it casts numeric string keys to integers, leading to
+	 * various crashes.
+	 *
+	 * @phpstan-template TKeyType of string
+	 * @phpstan-template TValueType
+	 * @phpstan-param array<TKeyType, TValueType> $array
+	 * @phpstan-return \Generator<TKeyType, TValueType, void, void>
+	 */
+	public static function stringifyKeys(array $array) : \Generator{
+		foreach($array as $key => $value){ // @phpstan-ignore-line - this is where we fix the stupid bullshit with array keys :)
+			yield (string) $key => $value;
+		}
+	}
+
 	public static function checkUTF8(string $string) : void{
 		if(!mb_check_encoding($string, 'UTF-8')){
 			throw new \InvalidArgumentException("Text must be valid UTF-8");

src/wizard/SetupWizard.php

@@ -35,6 +35,7 @@ use pocketmine\player\GameMode;
 use pocketmine\utils\Config;
 use pocketmine\utils\Internet;
 use pocketmine\utils\InternetException;
+use pocketmine\utils\Utils;
 use pocketmine\VersionInfo;
 use Webmozart\PathUtil\Path;
 use function fgets;
@@ -69,7 +70,7 @@ class SetupWizard{
 		}
 
 		$this->message("Please select a language");
-		foreach($langs as $short => $native){
+		foreach(Utils::stringifyKeys($langs) as $short => $native){
 			$this->writeLine(" $native => $short");
 		}
 

src/world/format/io/WorldProviderManager.php

@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace pocketmine\world\format\io;
 
+use pocketmine\utils\Utils;
 use pocketmine\world\format\io\leveldb\LevelDB;
 use pocketmine\world\format\io\region\Anvil;
 use pocketmine\world\format\io\region\McRegion;
@@ -77,7 +78,7 @@ final class WorldProviderManager{
 	 */
 	public function getMatchingProviders(string $path) : array{
 		$result = [];
-		foreach($this->providers as $alias => $providerEntry){
+		foreach(Utils::stringifyKeys($this->providers) as $alias => $providerEntry){
 			if($providerEntry->isValid($path)){
 				$result[$alias] = $providerEntry;
 			}

src/world/generator/GeneratorManager.php

@@ -105,7 +105,7 @@ final class GeneratorManager{
 	 */
 	public function getGeneratorName(string $class) : string{
 		Utils::testValidInstance($class, Generator::class);
-		foreach($this->list as $name => $c){
+		foreach(Utils::stringifyKeys($this->list) as $name => $c){
 			if($c->getGeneratorClass() === $class){
 				return $name;
 			}

tests/phpstan/rules/UnsafeForeachArrayOfStringRule.php

@@ -0,0 +1,90 @@
+<?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;
+use PHPStan\Type\ClassStringType;
+use PHPStan\Type\IntegerType;
+use PHPStan\Type\StringType;
+use PHPStan\Type\Type;
+use PHPStan\Type\TypeTraverser;
+use PHPStan\Type\VerbosityLevel;
+use function sprintf;
+
+/**
+ * @implements Rule<Foreach_>
+ */
+final class UnsafeForeachArrayOfStringRule implements Rule{
+
+	public function getNodeType() : string{
+		return Foreach_::class;
+	}
+
+	public function processNode(Node $node, Scope $scope) : array{
+		/** @var Foreach_ $node */
+		if($node->keyVar === null){
+			return [];
+		}
+		$iterableType = $scope->getType($node->expr);
+
+		if($iterableType->isArray()->no()){
+			return [];
+		}
+		if($iterableType->isIterableAtLeastOnce()->no()){
+			return [];
+		}
+
+		$hasCastableKeyTypes = false;
+		$expectsIntKeyTypes = false;
+		TypeTraverser::map($iterableType->getIterableKeyType(), function(Type $type, callable $traverse) use (&$hasCastableKeyTypes, &$expectsIntKeyTypes) : Type{
+			if($type instanceof IntegerType){
+				$expectsIntKeyTypes = true;
+				return $type;
+			}
+			if(!$type instanceof StringType){
+				return $traverse($type);
+			}
+			if($type->isNumericString()->no() || $type instanceof ClassStringType){
+				//class-string cannot be numeric, even if PHPStan thinks they can be
+				return $type;
+			}
+			$hasCastableKeyTypes = true;
+			return $type;
+		});
+		if($hasCastableKeyTypes && !$expectsIntKeyTypes){
+			return [
+				RuleErrorBuilder::message(sprintf(
+					"Unsafe foreach on array with key type %s (they might be casted to int).",
+					$iterableType->getIterableKeyType()->describe(VerbosityLevel::value())
+				))->tip("Use Utils::foreachWithStringKeys() for a safe Generator-based iterator.")->build()
+			];
+		}
+		return [];
+	}
+
+}

tools/generate-permission-doc.php

@@ -26,6 +26,7 @@ namespace pocketmine\generate_permission_doc;
 use pocketmine\permission\DefaultPermissions;
 use pocketmine\permission\PermissionManager;
 use pocketmine\utils\AssumptionFailedError;
+use pocketmine\utils\Utils;
 use pocketmine\VersionInfo;
 use Webmozart\PathUtil\Path;
 use function count;
@@ -90,7 +91,7 @@ foreach($permissions as $permission){
 	fwrite($doc, "|:-----|:----:|\n");
 	$children = $permission->getChildren();
 	ksort($children, SORT_STRING);
-	foreach($children as $childName => $isGranted){
+	foreach(Utils::stringifyKeys($children) as $childName => $isGranted){
 		fwrite($doc, "| `$childName` | " . ($isGranted ? "Granted" : "Denied") . " |\n");
 	}
 	fwrite($doc, "\n");

tools/simulate-chunk-selector.php

@@ -6,6 +6,7 @@ namespace pocketmine\tools\simulate_chunk_selector;
 
 use pocketmine\player\ChunkSelector;
 use pocketmine\utils\AssumptionFailedError;
+use pocketmine\utils\Utils;
 use pocketmine\world\format\Chunk;
 use pocketmine\world\World;
 use Webmozart\PathUtil\Path;
@@ -115,7 +116,7 @@ if(count(getopt("", ["help"])) !== 0){
 	exit(0);
 }
 
-foreach(getopt("", ["radius:", "baseX:", "baseZ:", "scale:", "chunksPerStep:"]) as $name => $value){
+foreach(Utils::stringifyKeys(getopt("", ["radius:", "baseX:", "baseZ:", "scale:", "chunksPerStep:"])) as $name => $value){
 	if(!is_string($value) || (string) ((int) $value) !== $value){
 		fwrite(STDERR, "Value for --$name must be an integer\n");
 		exit(1);
@@ -136,7 +137,7 @@ if($radius === null){
 }
 
 $outputDirectory = null;
-foreach(getopt("", ["output:"]) as $name => $value){
+foreach(Utils::stringifyKeys(getopt("", ["output:"])) as $name => $value){
 	assert($name === "output");
 	if(!is_string($value)){
 		fwrite(STDERR, "Value for --$name must be a string\n");