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.
This commit is contained in:
Dylan K. Taylor 2025-05-08 02:21:39 +01:00
parent f2e7473629
commit d789c75c00
No known key found for this signature in database
GPG Key ID: 8927471A91CAFD3D
2 changed files with 26 additions and 19 deletions

View File

@ -15,7 +15,7 @@ rules:
- pocketmine\phpstan\rules\DisallowEnumComparisonRule - pocketmine\phpstan\rules\DisallowEnumComparisonRule
- pocketmine\phpstan\rules\DisallowForeachByReferenceRule - pocketmine\phpstan\rules\DisallowForeachByReferenceRule
- pocketmine\phpstan\rules\ExplodeLimitRule - pocketmine\phpstan\rules\ExplodeLimitRule
- pocketmine\phpstan\rules\UnsafeForeachArrayOfStringRule - pocketmine\phpstan\rules\UnsafeForeachRule
# - pocketmine\phpstan\rules\ThreadedSupportedTypesRule # - pocketmine\phpstan\rules\ThreadedSupportedTypesRule
parameters: parameters:

View File

@ -41,7 +41,7 @@ use function sprintf;
/** /**
* @implements Rule<Foreach_> * @implements Rule<Foreach_>
*/ */
final class UnsafeForeachArrayOfStringRule implements Rule{ final class UnsafeForeachRule implements Rule{
public function getNodeType() : string{ public function getNodeType() : string{
return Foreach_::class; return Foreach_::class;
@ -73,7 +73,7 @@ final class UnsafeForeachArrayOfStringRule implements Rule{
$benevolentUnionDepth--; $benevolentUnionDepth--;
return $result; return $result;
} }
if($type instanceof IntegerType && $benevolentUnionDepth === 0){ if($type instanceof IntegerType){
$expectsIntKeyTypes = true; $expectsIntKeyTypes = true;
return $type; return $type;
} }
@ -87,24 +87,31 @@ final class UnsafeForeachArrayOfStringRule implements Rule{
$hasCastableKeyTypes = true; $hasCastableKeyTypes = true;
return $type; return $type;
}); });
if($hasCastableKeyTypes && !$expectsIntKeyTypes){ $errors = [];
$tip = $implicitType ? if($implicitType){
sprintf( $errors[] = RuleErrorBuilder::message("Possible unreported errors in foreach on array with unspecified key type.")
"Declare a key type using @phpstan-var or @phpstan-param, or use %s() to promote the key type to get proper error reporting", ->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(...)) Utils::getNiceClosureName(Utils::promoteKeys(...))
) : ))->identifier('pocketmine.foreach.implicitKeys')->build();
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()
];
} }
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;
} }
} }