From b0a6e9402644c165cafb88fe78bd0da805d517de Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 24 May 2025 13:44:05 +0200 Subject: [PATCH 1/3] 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 65cda918e7..9345db597b 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -66,6 +66,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 3f60d6383d..f168f35ce8 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -76,6 +76,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] +} From ef976756d7054dcae24b8a0f1725fc566e4e89fc Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 17 Jul 2025 21:53:35 +0200 Subject: [PATCH 2/3] Rename and use RuleLevelHelper --- conf/config.neon | 2 +- conf/parametersSchema.neon | 2 +- .../Arrays/InvalidKeyInArrayDimFetchRule.php | 24 ++-- .../Arrays/InvalidKeyInArrayItemRule.php | 16 ++- .../InvalidKeyInArrayDimFetchRuleTest.php | 103 +++++++++++++++++- .../Arrays/InvalidKeyInArrayItemRuleTest.php | 49 ++++++++- .../Arrays/data/invalid-key-array-item.php | 7 ++ 7 files changed, 186 insertions(+), 17 deletions(-) diff --git a/conf/config.neon b/conf/config.neon index 9345db597b..6a05a17961 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -66,7 +66,7 @@ parameters: reportStaticMethodSignatures: false reportWrongPhpDocTypeInVarTag: false reportAnyTypeWideningInVarTag: false - reportCastedArrayKey: false + reportArrayKeyCast: false reportPossiblyNonexistentGeneralArrayOffset: false reportPossiblyNonexistentConstantArrayOffset: false checkMissingOverrideMethodAttribute: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index f168f35ce8..90f734620d 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -76,7 +76,7 @@ parametersSchema: reportStaticMethodSignatures: bool() reportWrongPhpDocTypeInVarTag: bool() reportAnyTypeWideningInVarTag: bool() - reportCastedArrayKey: bool() + reportArrayKeyCast: bool() reportPossiblyNonexistentGeneralArrayOffset: bool() reportPossiblyNonexistentConstantArrayOffset: bool() checkMissingOverrideMethodAttribute: bool() diff --git a/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php b/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php index bfbdcc0f1e..f197038e12 100644 --- a/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php +++ b/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php @@ -27,7 +27,7 @@ public function __construct( #[AutowiredParameter] private bool $reportMaybes, #[AutowiredParameter] - private bool $reportCastedArrayKey, + private bool $reportArrayKeyCast, ) { } @@ -43,24 +43,28 @@ public function processNode(Node $node, Scope $scope): array return []; } - $dimensionType = $scope->getType($node->dim); - if ($dimensionType instanceof MixedType) { - return []; - } - - $reportCastedArrayKey = $this->reportCastedArrayKey; $varType = $this->ruleLevelHelper->findTypeToCheck( $scope, $node->var, '', - static fn (Type $varType): bool => $varType->isArray()->no() || AllowedArrayKeysTypes::getType($reportCastedArrayKey)->isSuperTypeOf($dimensionType)->yes(), + static fn (Type $varType): bool => $varType->isArray()->no(), )->getType(); - if ($varType instanceof ErrorType || $varType->isArray()->no()) { return []; } - $isSuperType = AllowedArrayKeysTypes::getType($this->reportCastedArrayKey)->isSuperTypeOf($dimensionType); + $reportArrayKeyCast = $this->reportArrayKeyCast; + $dimensionType = $this->ruleLevelHelper->findTypeToCheck( + $scope, + $node->dim, + '', + static fn (Type $dimType): bool => AllowedArrayKeysTypes::getType($reportArrayKeyCast)->isSuperTypeOf($dimType)->yes(), + )->getType(); + if ($dimensionType instanceof MixedType) { + return []; + } + + $isSuperType = AllowedArrayKeysTypes::getType($this->reportArrayKeyCast)->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 b0e3ae393d..9ec10640ad 100644 --- a/src/Rules/Arrays/InvalidKeyInArrayItemRule.php +++ b/src/Rules/Arrays/InvalidKeyInArrayItemRule.php @@ -8,7 +8,9 @@ use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\Rules\RuleLevelHelper; use PHPStan\Type\MixedType; +use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; use function sprintf; @@ -20,10 +22,11 @@ final class InvalidKeyInArrayItemRule implements Rule { public function __construct( + private RuleLevelHelper $ruleLevelHelper, #[AutowiredParameter] private bool $reportMaybes, #[AutowiredParameter] - private bool $reportCastedArrayKey, + private bool $reportArrayKeyCast, ) { } @@ -39,8 +42,15 @@ public function processNode(Node $node, Scope $scope): array return []; } - $dimensionType = $scope->getType($node->key); - $isSuperType = AllowedArrayKeysTypes::getType($this->reportCastedArrayKey)->isSuperTypeOf($dimensionType); + $reportArrayKeyCast = $this->reportArrayKeyCast; + $dimensionType = $this->ruleLevelHelper->findTypeToCheck( + $scope, + $node->key, + '', + static fn (Type $dimType): bool => AllowedArrayKeysTypes::getType($reportArrayKeyCast)->isSuperTypeOf($dimType)->yes(), + )->getType(); + + $isSuperType = AllowedArrayKeysTypes::getType($reportArrayKeyCast)->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 7551e10cd8..c066521456 100644 --- a/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayDimFetchRuleTest.php @@ -15,9 +15,13 @@ class InvalidKeyInArrayDimFetchRuleTest extends RuleTestCase private bool $reportCastedArrayKey = false; + private bool $checkUnionType = true; + + private bool $checkNullable = true; + protected function getRule(): Rule { - $ruleLevelHelper = new RuleLevelHelper(self::createReflectionProvider(), true, false, true, false, false, false, true); + $ruleLevelHelper = new RuleLevelHelper(self::createReflectionProvider(), $this->checkNullable, false, $this->checkUnionType, false, false, false, true); return new InvalidKeyInArrayDimFetchRule($ruleLevelHelper, true, $this->reportCastedArrayKey); } @@ -63,6 +67,46 @@ public function testInvalidKey(): void ]); } + public function testInvalidKeyOnLevel6(): void + { + $this->checkNullable = false; + $this->checkUnionType = false; + $this->analyse([__DIR__ . '/data/invalid-key-array-dim-fetch.php'], [ + [ + 'Invalid array key type DateTimeImmutable.', + 7, + ], + [ + 'Invalid array key type array.', + 8, + ], + [ + '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, + ], + ]); + } + public function testInvalidKeyReportingCastedArrayKey(): void { $this->reportCastedArrayKey = true; @@ -126,6 +170,63 @@ public function testInvalidKeyReportingCastedArrayKey(): void ]); } + public function testInvalidKeyReportingCastedArrayKeyOnLevel6(): void + { + $this->checkNullable = false; + $this->checkUnionType = false; + $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, + ], + [ + '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 { diff --git a/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayItemRuleTest.php b/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayItemRuleTest.php index 824cfe9058..52765d73af 100644 --- a/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayItemRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/InvalidKeyInArrayItemRuleTest.php @@ -3,6 +3,7 @@ namespace PHPStan\Rules\Arrays; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleLevelHelper; use PHPStan\Testing\RuleTestCase; use PHPUnit\Framework\Attributes\RequiresPhp; @@ -14,9 +15,14 @@ class InvalidKeyInArrayItemRuleTest extends RuleTestCase private bool $reportCastedArrayKey = false; + private bool $checkUnionType = true; + + private bool $checkNullable = true; + protected function getRule(): Rule { - return new InvalidKeyInArrayItemRule(true, $this->reportCastedArrayKey); + $ruleLevelHelper = new RuleLevelHelper(self::createReflectionProvider(), $this->checkNullable, false, $this->checkUnionType, false, false, false, true); + return new InvalidKeyInArrayItemRule($ruleLevelHelper, true, $this->reportCastedArrayKey); } public function testInvalidKey(): void @@ -37,6 +43,22 @@ public function testInvalidKey(): void ]); } + public function testInvalidKeyOnLevel6(): void + { + $this->checkNullable = false; + $this->checkUnionType = false; + $this->analyse([__DIR__ . '/data/invalid-key-array-item.php'], [ + [ + 'Invalid array key type DateTimeImmutable.', + 13, + ], + [ + 'Invalid array key type array.', + 14, + ], + ]); + } + public function testInvalidKeyReportingCastedArrayKey(): void { $this->reportCastedArrayKey = true; @@ -57,6 +79,31 @@ public function testInvalidKeyReportingCastedArrayKey(): void 'Possibly invalid array key type stdClass|string.', 15, ], + [ + 'Possibly invalid array key type string|null.', + 22, + ], + ]); + } + + public function testInvalidKeyReportingCastedArrayKeyOnLevel6(): void + { + $this->checkNullable = false; + $this->checkUnionType = false; + $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, + ], ]); } diff --git a/tests/PHPStan/Rules/Arrays/data/invalid-key-array-item.php b/tests/PHPStan/Rules/Arrays/data/invalid-key-array-item.php index cde86133df..2ff10b5127 100644 --- a/tests/PHPStan/Rules/Arrays/data/invalid-key-array-item.php +++ b/tests/PHPStan/Rules/Arrays/data/invalid-key-array-item.php @@ -14,3 +14,10 @@ [] => 'bbb', $stringOrObject => 'aaa', ]; + +/** @var string|null $stringOrNull */ +$stringOrNull = doFoo(); + +$b = [ + $stringOrNull => 'aaa', +]; From 627be15379ccf7d3f39f218fe31d2feb5dd63cf3 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 26 Jul 2025 18:26:04 +0200 Subject: [PATCH 3/3] Fix --- src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php b/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php index f197038e12..eb98468167 100644 --- a/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php +++ b/src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php @@ -10,7 +10,6 @@ use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Rules\RuleLevelHelper; use PHPStan\Type\ErrorType; -use PHPStan\Type\MixedType; use PHPStan\Type\Type; use PHPStan\Type\VerbosityLevel; use function sprintf; @@ -60,7 +59,7 @@ public function processNode(Node $node, Scope $scope): array '', static fn (Type $dimType): bool => AllowedArrayKeysTypes::getType($reportArrayKeyCast)->isSuperTypeOf($dimType)->yes(), )->getType(); - if ($dimensionType instanceof MixedType) { + if ($dimensionType instanceof ErrorType) { return []; }