From 154bb87dd7c805963e0ab222a33e94444ffb3b11 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 24 May 2025 13:44:05 +0200 Subject: [PATCH] Introduce reportCastedArrayKey parameter --- conf/config.neon | 1 + conf/parametersSchema.neon | 1 + src/Rules/Arrays/AllowedArrayKeysTypes.php | 9 +- .../Arrays/InvalidKeyInArrayDimFetchRule.php | 7 +- .../Arrays/InvalidKeyInArrayItemRule.php | 4 +- .../InvalidKeyInArrayDimFetchRuleTest.php | 83 ++++++++++++++++++- .../Arrays/InvalidKeyInArrayItemRuleTest.php | 27 +++++- .../Rules/Arrays/data/unset-false-key.php | 16 ++++ 8 files changed, 142 insertions(+), 6 deletions(-) create mode 100644 tests/PHPStan/Rules/Arrays/data/unset-false-key.php diff --git a/conf/config.neon b/conf/config.neon index 73a8062c27..ebd58909c6 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -65,6 +65,7 @@ parameters: reportStaticMethodSignatures: false reportWrongPhpDocTypeInVarTag: false reportAnyTypeWideningInVarTag: false + reportCastedArrayKey: false reportPossiblyNonexistentGeneralArrayOffset: false reportPossiblyNonexistentConstantArrayOffset: false checkMissingOverrideMethodAttribute: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 6af9e2737e..6bd136ca23 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -75,6 +75,7 @@ parametersSchema: reportStaticMethodSignatures: bool() reportWrongPhpDocTypeInVarTag: bool() reportAnyTypeWideningInVarTag: bool() + reportCastedArrayKey: bool() reportPossiblyNonexistentGeneralArrayOffset: bool() reportPossiblyNonexistentConstantArrayOffset: bool() checkMissingOverrideMethodAttribute: bool() diff --git a/src/Rules/Arrays/AllowedArrayKeysTypes.php b/src/Rules/Arrays/AllowedArrayKeysTypes.php index 2178b579fb..d685b777f3 100644 --- a/src/Rules/Arrays/AllowedArrayKeysTypes.php +++ b/src/Rules/Arrays/AllowedArrayKeysTypes.php @@ -21,8 +21,15 @@ final class AllowedArrayKeysTypes { - public static function getType(): Type + public static function getType(bool $strict = false): Type { + if ($strict) { + return new UnionType([ + new IntegerType(), + new StringType(), + ]); + } + return new UnionType([ new IntegerType(), new StringType(), diff --git a/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php b/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php index 30f7febe49..bfbdcc0f1e 100644 --- a/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php +++ b/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php @@ -26,6 +26,8 @@ public function __construct( private RuleLevelHelper $ruleLevelHelper, #[AutowiredParameter] private bool $reportMaybes, + #[AutowiredParameter] + private bool $reportCastedArrayKey, ) { } @@ -46,18 +48,19 @@ public function processNode(Node $node, Scope $scope): array return []; } + $reportCastedArrayKey = $this->reportCastedArrayKey; $varType = $this->ruleLevelHelper->findTypeToCheck( $scope, $node->var, '', - static fn (Type $varType): bool => $varType->isArray()->no() || AllowedArrayKeysTypes::getType()->isSuperTypeOf($dimensionType)->yes(), + static fn (Type $varType): bool => $varType->isArray()->no() || AllowedArrayKeysTypes::getType($reportCastedArrayKey)->isSuperTypeOf($dimensionType)->yes(), )->getType(); if ($varType instanceof ErrorType || $varType->isArray()->no()) { return []; } - $isSuperType = AllowedArrayKeysTypes::getType()->isSuperTypeOf($dimensionType); + $isSuperType = AllowedArrayKeysTypes::getType($this->reportCastedArrayKey)->isSuperTypeOf($dimensionType); if ($isSuperType->yes() || ($isSuperType->maybe() && !$this->reportMaybes)) { return []; } diff --git a/src/Rules/Arrays/InvalidKeyInArrayItemRule.php b/src/Rules/Arrays/InvalidKeyInArrayItemRule.php index 993a6dde61..b0e3ae393d 100644 --- a/src/Rules/Arrays/InvalidKeyInArrayItemRule.php +++ b/src/Rules/Arrays/InvalidKeyInArrayItemRule.php @@ -22,6 +22,8 @@ final class InvalidKeyInArrayItemRule implements Rule public function __construct( #[AutowiredParameter] private bool $reportMaybes, + #[AutowiredParameter] + private bool $reportCastedArrayKey, ) { } @@ -38,7 +40,7 @@ public function processNode(Node $node, Scope $scope): array } $dimensionType = $scope->getType($node->key); - $isSuperType = AllowedArrayKeysTypes::getType()->isSuperTypeOf($dimensionType); + $isSuperType = AllowedArrayKeysTypes::getType($this->reportCastedArrayKey)->isSuperTypeOf($dimensionType); if ($isSuperType->no()) { return [ RuleErrorBuilder::message( diff --git a/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php index 0d42bca9e9..7551e10cd8 100644 --- a/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php @@ -13,10 +13,12 @@ class InvalidKeyInArrayDimFetchRuleTest extends RuleTestCase { + private bool $reportCastedArrayKey = false; + protected function getRule(): Rule { $ruleLevelHelper = new RuleLevelHelper(self::createReflectionProvider(), true, false, true, false, false, false, true); - return new InvalidKeyInArrayDimFetchRule($ruleLevelHelper, true); + return new InvalidKeyInArrayDimFetchRule($ruleLevelHelper, true, $this->reportCastedArrayKey); } public function testInvalidKey(): void @@ -61,6 +63,69 @@ public function testInvalidKey(): void ]); } + public function testInvalidKeyReportingCastedArrayKey(): void + { + $this->reportCastedArrayKey = true; + $this->analyse([__DIR__ . '/data/invalid-key-array-dim-fetch.php'], [ + [ + 'Invalid array key type null.', + 6, + ], + [ + 'Invalid array key type DateTimeImmutable.', + 7, + ], + [ + 'Invalid array key type array.', + 8, + ], + [ + 'Invalid array key type float.', + 10, + ], + [ + 'Invalid array key type true.', + 12, + ], + [ + 'Invalid array key type false.', + 13, + ], + [ + 'Possibly invalid array key type string|null.', + 17, + ], + [ + 'Possibly invalid array key type stdClass|string.', + 24, + ], + [ + 'Invalid array key type DateTimeImmutable.', + 31, + ], + [ + 'Invalid array key type DateTimeImmutable.', + 45, + ], + [ + 'Invalid array key type DateTimeImmutable.', + 46, + ], + [ + 'Invalid array key type DateTimeImmutable.', + 47, + ], + [ + 'Invalid array key type stdClass.', + 47, + ], + [ + 'Invalid array key type DateTimeImmutable.', + 48, + ], + ]); + } + #[RequiresPhp('>= 8.1')] public function testBug6315(): void { @@ -92,4 +157,20 @@ public function testBug6315(): void ]); } + public function testUnsetFalseKey(): void + { + $this->reportCastedArrayKey = true; + + $this->analyse([__DIR__ . '/data/unset-false-key.php'], [ + [ + 'Invalid array key type false.', + 6, + ], + [ + 'Invalid array key type false.', + 13, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayItemRuleTest.php b/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayItemRuleTest.php index ab486eeacf..824cfe9058 100644 --- a/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayItemRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayItemRuleTest.php @@ -12,9 +12,11 @@ class InvalidKeyInArrayItemRuleTest extends RuleTestCase { + private bool $reportCastedArrayKey = false; + protected function getRule(): Rule { - return new InvalidKeyInArrayItemRule(true); + return new InvalidKeyInArrayItemRule(true, $this->reportCastedArrayKey); } public function testInvalidKey(): void @@ -35,6 +37,29 @@ public function testInvalidKey(): void ]); } + public function testInvalidKeyReportingCastedArrayKey(): void + { + $this->reportCastedArrayKey = true; + $this->analyse([__DIR__ . '/data/invalid-key-array-item.php'], [ + [ + 'Invalid array key type null.', + 12, + ], + [ + 'Invalid array key type DateTimeImmutable.', + 13, + ], + [ + 'Invalid array key type array.', + 14, + ], + [ + 'Possibly invalid array key type stdClass|string.', + 15, + ], + ]); + } + public function testInvalidKeyInList(): void { $this->analyse([__DIR__ . '/data/invalid-key-list.php'], [ diff --git a/tests/PHPStan/Rules/Arrays/data/unset-false-key.php b/tests/PHPStan/Rules/Arrays/data/unset-false-key.php new file mode 100644 index 0000000000..d47f059600 --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/unset-false-key.php @@ -0,0 +1,16 @@ + $data */ +unset($data[false]); + +function test_remove_element(): void { + $modified = [1, 4, 6, 8]; + + // this would happen in the SUT + unset($modified[array_search(4, $modified, true)]); + unset($modified[array_search(5, $modified, true)]); // bug is here - will unset key `0` by accident + + assert([1, 6, 8] === $modified); // actually is [6, 8] +}