From d789c75c0084cacac09baa33d8ec5462d1f9c089 Mon Sep 17 00:00:00 2001
From: "Dylan K. Taylor" <dktapps@pmmp.io>
Date: Thu, 8 May 2025 02:21:39 +0100
Subject: [PATCH] Improve PHPStan error reporting for unsafe foreaches these
 are actually two separate concerns: one for dodgy PHPStan type suppression on
 implicit keys, and the other for arrays being casted to strings by PHP.

---
 phpstan.neon.dist                             |  2 +-
 ...OfStringRule.php => UnsafeForeachRule.php} | 43 +++++++++++--------
 2 files changed, 26 insertions(+), 19 deletions(-)
 rename tests/phpstan/rules/{UnsafeForeachArrayOfStringRule.php => UnsafeForeachRule.php} (69%)

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
index 13f35c121..12c739f2f 100644
--- a/phpstan.neon.dist
+++ b/phpstan.neon.dist
@@ -15,7 +15,7 @@ rules:
 	- pocketmine\phpstan\rules\DisallowEnumComparisonRule
 	- pocketmine\phpstan\rules\DisallowForeachByReferenceRule
 	- pocketmine\phpstan\rules\ExplodeLimitRule
-	- pocketmine\phpstan\rules\UnsafeForeachArrayOfStringRule
+	- pocketmine\phpstan\rules\UnsafeForeachRule
 #	- pocketmine\phpstan\rules\ThreadedSupportedTypesRule
 
 parameters:
diff --git a/tests/phpstan/rules/UnsafeForeachArrayOfStringRule.php b/tests/phpstan/rules/UnsafeForeachRule.php
similarity index 69%
rename from tests/phpstan/rules/UnsafeForeachArrayOfStringRule.php
rename to tests/phpstan/rules/UnsafeForeachRule.php
index 34056131b..cb463c61d 100644
--- a/tests/phpstan/rules/UnsafeForeachArrayOfStringRule.php
+++ b/tests/phpstan/rules/UnsafeForeachRule.php
@@ -41,7 +41,7 @@ use function sprintf;
 /**
  * @implements Rule<Foreach_>
  */
-final class UnsafeForeachArrayOfStringRule implements Rule{
+final class UnsafeForeachRule implements Rule{
 
 	public function getNodeType() : string{
 		return Foreach_::class;
@@ -73,7 +73,7 @@ final class UnsafeForeachArrayOfStringRule implements Rule{
 				$benevolentUnionDepth--;
 				return $result;
 			}
-			if($type instanceof IntegerType && $benevolentUnionDepth === 0){
+			if($type instanceof IntegerType){
 				$expectsIntKeyTypes = true;
 				return $type;
 			}
@@ -87,24 +87,31 @@ final class UnsafeForeachArrayOfStringRule implements Rule{
 			$hasCastableKeyTypes = true;
 			return $type;
 		});
-		if($hasCastableKeyTypes && !$expectsIntKeyTypes){
-			$tip = $implicitType ?
-				sprintf(
-					"Declare a key type using @phpstan-var or @phpstan-param, or use %s() to promote the key type to get proper error reporting",
+		$errors = [];
+		if($implicitType){
+			$errors[] = RuleErrorBuilder::message("Possible unreported errors in foreach on array with unspecified key type.")
+				->tip(sprintf(
+					<<<TIP
+PHPStan might not be reporting some type errors if the key type is not specified.
+Declare a key type using @phpstan-var or @phpstan-param, or use %s() to force PHPStan to report proper errors.
+TIP,
 					Utils::getNiceClosureName(Utils::promoteKeys(...))
-				) :
-				sprintf(
-					"Use %s() to get a \Generator that will force the keys to string",
-					Utils::getNiceClosureName(Utils::stringifyKeys(...)),
-				);
-			return [
-				RuleErrorBuilder::message(sprintf(
-					"Unsafe foreach on array with key type %s (they might be casted to int).",
-					$iterableType->getIterableKeyType()->describe(VerbosityLevel::value())
-				))->tip($tip)->identifier('pocketmine.foreach.stringKeys')->build()
-			];
+				))->identifier('pocketmine.foreach.implicitKeys')->build();
 		}
-		return [];
+		if($hasCastableKeyTypes && !$expectsIntKeyTypes){
+			$errors[] = RuleErrorBuilder::message(sprintf(
+				"Unsafe foreach on array with key type %s.",
+				$iterableType->getIterableKeyType()->describe(VerbosityLevel::value())
+			))
+				->tip(sprintf(
+					<<<TIP
+PHP coerces numeric strings to integers when used as array keys, which can lead to unexpected type errors when passing the key to a function.
+Use %s() to wrap the array in a \Generator that will force the keys back to string during iteration.
+TIP,
+					Utils::getNiceClosureName(Utils::stringifyKeys(...))
+				))->identifier('pocketmine.foreach.stringKeys')->build();
+		}
+		return $errors;
 	}
 
 }