Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions src/Rules/Arrays/InvalidKeyInArrayDimFetchRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -41,19 +40,29 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

$dimensionType = $scope->getType($node->dim);
if ($dimensionType instanceof MixedType) {
return [];
}

$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(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still wondering about what's the effect of the $varType going through RuleLevelHelper. Does something change if we get rid of it only continue if $scope->getType($node->var)->isArray()->yes()?

It's weird to me that this rule checks maybe-arrays. It's okay for array|null for example, but I'm not sure if it ever should report something on mixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added more test case here with the commit 6f99f04

You can see that

  • array|foo is checked
  • explicit and implicit mixed are not checked (because transformed to ErrorType or to a StrictMixedType in which isArray() return no.

But it was right to report I need to check the result for maybe array.
The error message was Invalid array key type DateTimeImmutable., I think it should be Possibly invalid array key type DateTimeImmutable. then. And I fix that in the commit 7e0f73b

)->getType();

if ($varType instanceof ErrorType || $varType->isArray()->no()) {
if ($varType instanceof ErrorType) {
return [];
}

$isArray = $varType->isArray();
if ($isArray->no() || ($isArray->maybe() && !$this->reportMaybes)) {
return [];
}

$dimensionType = $this->ruleLevelHelper->findTypeToCheck(
$scope,
$node->dim,
'',
static fn (Type $dimType): bool => AllowedArrayKeysTypes::getType()->isSuperTypeOf($dimType)->yes(),
)->getType();
if ($dimensionType instanceof ErrorType) {
return [];
}

Expand All @@ -64,7 +73,11 @@ public function processNode(Node $node, Scope $scope): array

return [
RuleErrorBuilder::message(
sprintf('%s array key type %s.', $isSuperType->no() ? 'Invalid' : 'Possibly invalid', $dimensionType->describe(VerbosityLevel::typeOnly())),
sprintf(
'%s array key type %s.',
$isArray->yes() && $isSuperType->no() ? 'Invalid' : 'Possibly invalid',
$dimensionType->describe(VerbosityLevel::typeOnly()),
),
)->identifier('offsetAccess.invalidOffset')->build(),
];
}
Expand Down
6 changes: 5 additions & 1 deletion src/Rules/RuleLevelHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,11 @@ private function findTypeToCheckImplementation(
bool $isTopLevel = false,
): FoundTypeResult
{
if (!$this->checkNullables && !$type->isNull()->yes()) {
if (
!$this->checkNullables
&& !$type->isNull()->yes()
&& !$unionTypeCriteriaCallback(new NullType())
) {
$type = TypeCombinator::removeNull($type);
}

Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Levels/LevelsIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public static function dataTopics(): array
['arrayDestructuring'],
['listType'],
['missingTypes'],
['arrayOffsetAccess'],
];
if (PHP_VERSION_ID >= 80300) {
$topics[] = ['constantAccesses83'];
Expand Down
52 changes: 52 additions & 0 deletions tests/PHPStan/Levels/data/arrayOffsetAccess-10.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
[
{
"message": "Possibly invalid array key type mixed.",
"line": 22,
"ignorable": true
},
{
"message": "Possibly invalid array key type mixed.",
"line": 31,
"ignorable": true
},
{
"message": "Cannot access offset 42 on mixed.",
"line": 42,
"ignorable": true
},
{
"message": "Cannot access offset null on mixed.",
"line": 43,
"ignorable": true
},
{
"message": "Cannot access offset DateTimeImmutable on mixed.",
"line": 44,
"ignorable": true
},
{
"message": "Cannot access offset int|null on mixed.",
"line": 45,
"ignorable": true
},
{
"message": "Cannot access offset int|object on mixed.",
"line": 46,
"ignorable": true
},
{
"message": "Cannot access offset object|null on mixed.",
"line": 47,
"ignorable": true
},
{
"message": "Cannot access offset mixed on mixed.",
"line": 48,
"ignorable": true
},
{
"message": "Cannot access offset mixed on mixed.",
"line": 49,
"ignorable": true
}
]
7 changes: 7 additions & 0 deletions tests/PHPStan/Levels/data/arrayOffsetAccess-3.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"message": "Invalid array key type DateTimeImmutable.",
"line": 17,
"ignorable": true
}
]
142 changes: 142 additions & 0 deletions tests/PHPStan/Levels/data/arrayOffsetAccess-4.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
[
{
"message": "Expression \"$a[42]\" on a separate line does not do anything.",
"line": 15,
"ignorable": true
},
{
"message": "Expression \"$a[null]\" on a separate line does not do anything.",
"line": 16,
"ignorable": true
},
{
"message": "Expression \"$a[$intOrNull]\" on a separate line does not do anything.",
"line": 18,
"ignorable": true
},
{
"message": "Expression \"$a[$objectOrInt]\" on a separate line does not do anything.",
"line": 19,
"ignorable": true
},
{
"message": "Expression \"$a[$objectOrNull]\" on a separate line does not do anything.",
"line": 20,
"ignorable": true
},
{
"message": "Expression \"$a[$explicitlyMixed]\" on a separate line does not do anything.",
"line": 21,
"ignorable": true
},
{
"message": "Expression \"$a[$implicitlyMixed]\" on a separate line does not do anything.",
"line": 22,
"ignorable": true
},
{
"message": "Expression \"$arrayOrObject[42]\" on a separate line does not do anything.",
"line": 24,
"ignorable": true
},
{
"message": "Expression \"$arrayOrObject[null]\" on a separate line does not do anything.",
"line": 25,
"ignorable": true
},
{
"message": "Expression \"$arrayOrObject[$intOrNull]\" on a separate line does not do anything.",
"line": 27,
"ignorable": true
},
{
"message": "Expression \"$arrayOrObject[$objectOrInt]\" on a separate line does not do anything.",
"line": 28,
"ignorable": true
},
{
"message": "Expression \"$arrayOrObject[$objectOrNull]\" on a separate line does not do anything.",
"line": 29,
"ignorable": true
},
{
"message": "Expression \"$arrayOrObject[$explicitlyMixed]\" on a separate line does not do anything.",
"line": 30,
"ignorable": true
},
{
"message": "Expression \"$arrayOrObject[$implicitlyMixed]\" on a separate line does not do anything.",
"line": 31,
"ignorable": true
},
{
"message": "Expression \"$explicitlyMixed[42]\" on a separate line does not do anything.",
"line": 33,
"ignorable": true
},
{
"message": "Expression \"$explicitlyMixed[null]\" on a separate line does not do anything.",
"line": 34,
"ignorable": true
},
{
"message": "Expression \"$explicitlyMixed[$intOrNull]\" on a separate line does not do anything.",
"line": 36,
"ignorable": true
},
{
"message": "Expression \"$explicitlyMixed[$objectOrInt]\" on a separate line does not do anything.",
"line": 37,
"ignorable": true
},
{
"message": "Expression \"$explicitlyMixed[$objectOrNull]\" on a separate line does not do anything.",
"line": 38,
"ignorable": true
},
{
"message": "Expression \"$explicitlyMixed[$explicitlyMixed]\" on a separate line does not do anything.",
"line": 39,
"ignorable": true
},
{
"message": "Expression \"$explicitlyMixed[$implicitlyMixed]\" on a separate line does not do anything.",
"line": 40,
"ignorable": true
},
{
"message": "Expression \"$implicitlyMixed[42]\" on a separate line does not do anything.",
"line": 42,
"ignorable": true
},
{
"message": "Expression \"$implicitlyMixed[null]\" on a separate line does not do anything.",
"line": 43,
"ignorable": true
},
{
"message": "Expression \"$implicitlyMixed[$intOrNull]\" on a separate line does not do anything.",
"line": 45,
"ignorable": true
},
{
"message": "Expression \"$implicitlyMixed[$objectOrInt]\" on a separate line does not do anything.",
"line": 46,
"ignorable": true
},
{
"message": "Expression \"$implicitlyMixed[$objectOrNull]\" on a separate line does not do anything.",
"line": 47,
"ignorable": true
},
{
"message": "Expression \"$implicitlyMixed[$explicitlyMixed]\" on a separate line does not do anything.",
"line": 48,
"ignorable": true
},
{
"message": "Expression \"$implicitlyMixed[$implicitlyMixed]\" on a separate line does not do anything.",
"line": 49,
"ignorable": true
}
]
22 changes: 22 additions & 0 deletions tests/PHPStan/Levels/data/arrayOffsetAccess-6.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[
{
"message": "Method Levels\\ArrayOffsetAccess\\Foo::test() has parameter $a with no value type specified in iterable type array.",
"line": 13,
"ignorable": true
},
{
"message": "Method Levels\\ArrayOffsetAccess\\Foo::test() has parameter $arrayOrObject with generic interface ArrayAccess but does not specify its types: TKey, TValue",
"line": 13,
"ignorable": true
},
{
"message": "Method Levels\\ArrayOffsetAccess\\Foo::test() has parameter $arrayOrObject with no value type specified in iterable type array.",
"line": 13,
"ignorable": true
},
{
"message": "Method Levels\\ArrayOffsetAccess\\Foo::test() has parameter $implicitlyMixed with no type specified.",
"line": 13,
"ignorable": true
}
]
27 changes: 27 additions & 0 deletions tests/PHPStan/Levels/data/arrayOffsetAccess-7.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
[
{
"message": "Possibly invalid array key type int|object.",
"line": 19,
"ignorable": true
},
{
"message": "Possibly invalid array key type object|null.",
"line": 20,
"ignorable": true
},
{
"message": "Possibly invalid array key type DateTimeImmutable.",
"line": 26,
"ignorable": true
},
{
"message": "Possibly invalid array key type int|object.",
"line": 28,
"ignorable": true
},
{
"message": "Possibly invalid array key type object|null.",
"line": 29,
"ignorable": true
}
]
52 changes: 52 additions & 0 deletions tests/PHPStan/Levels/data/arrayOffsetAccess-9.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
[
{
"message": "Possibly invalid array key type mixed.",
"line": 21,
"ignorable": true
},
{
"message": "Possibly invalid array key type mixed.",
"line": 30,
"ignorable": true
},
{
"message": "Cannot access offset 42 on mixed.",
"line": 33,
"ignorable": true
},
{
"message": "Cannot access offset null on mixed.",
"line": 34,
"ignorable": true
},
{
"message": "Cannot access offset DateTimeImmutable on mixed.",
"line": 35,
"ignorable": true
},
{
"message": "Cannot access offset int|null on mixed.",
"line": 36,
"ignorable": true
},
{
"message": "Cannot access offset int|object on mixed.",
"line": 37,
"ignorable": true
},
{
"message": "Cannot access offset object|null on mixed.",
"line": 38,
"ignorable": true
},
{
"message": "Cannot access offset mixed on mixed.",
"line": 39,
"ignorable": true
},
{
"message": "Cannot access offset mixed on mixed.",
"line": 40,
"ignorable": true
}
]
Loading
Loading