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
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,4 @@ final class DifferentUsage {
}
}

?>
?>
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,4 @@ final class WithArrayFilterAndArrayWalk
}
}

?>
?>
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,4 @@ final class TryCatchFinallySameType
}
}

?>
?>
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ class FuncCallFound
}
}

?>
?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\StrictArrayParamDimFetchRector\Fixture;

final class CoverIssetWithDimFetch
{
public function run($param, $item): bool
{
if (! isset($param[$item])) {
return false;
}

if ($param[$item] === 100) {
return true;
}

return false;
}
}

?>
-----
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\StrictArrayParamDimFetchRector\Fixture;

final class CoverIssetWithDimFetch
{
public function run(array $param, $item): bool
Copy link
Contributor

@staabm staabm Oct 1, 2025

Choose a reason for hiding this comment

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

in this case $param could be anything which implements ArrayAccess not just the rare case of string-offset access.

in case you "wrongly guess" the array type a call with a ArrayAccess object will produce a fatal error at runtime - see https://3v4l.org/csROV

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, it's a choice of practice over risk of few edge cases. We apply same logic to other rules too, to ease manual daunting work on project upgrades.

If your codebase uses any type in this case, exclude the rule.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, it's a choice of practice over risk of few edge cases. We apply same logic to other rules too, to ease manual daunting work on project upgrades.

If your codebase uses any type in this case, exclude the rule.

{
if (! isset($param[$item])) {
return false;
}

if ($param[$item] === 100) {
return true;
}

return false;
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\StrictArrayParamDimFetchRector\Fixture;

final class IncludeIssetIndex
{
public function run($param): void
{
echo isset($param['index']) ? $param['index'] : 'test';
}
}

?>
-----
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\StrictArrayParamDimFetchRector\Fixture;

final class IncludeIssetIndex
{
public function run(array $param): void
{
echo isset($param['index']) ? $param['index'] : 'test';
}
}

?>

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Expr\Empty_;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\Isset_;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\FunctionLike;
Expand All @@ -28,6 +27,7 @@
use PhpParser\Node\Stmt\Function_;
use PhpParser\NodeVisitor;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\UnionType;
use Rector\NodeTypeResolver\PHPStan\Type\TypeFactory;
use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
Expand Down Expand Up @@ -182,10 +182,7 @@ private function isParamAccessedArrayDimFetch(Param $param, ClassMethod|Function
return NodeVisitor::STOP_TRAVERSAL;
}

if ($variableType instanceof ObjectType && $this->typeComparator->isSubtype(
$variableType,
new ObjectType('ArrayAccess')
)) {
if ($this->isArrayAccess($variableType)) {
$isParamAccessedArrayDimFetch = false;
return NodeVisitor::STOP_TRAVERSAL;
}
Expand Down Expand Up @@ -216,18 +213,8 @@ private function shouldStop(Node $node, Param $param, string $paramName): bool
{
$nodeToCheck = null;

if (! $param->default instanceof Expr) {
if ($node instanceof Isset_) {
foreach ($node->vars as $var) {
if ($var instanceof ArrayDimFetch && $var->var instanceof Variable && $var->var->name === $paramName) {
return true;
}
}
}

if ($node instanceof Empty_ && $node->expr instanceof ArrayDimFetch && $node->expr->var instanceof Variable && $node->expr->var->name === $paramName) {
return true;
}
if (! $param->default instanceof Expr && ($node instanceof Empty_ && $node->expr instanceof ArrayDimFetch && $node->expr->var instanceof Variable && $node->expr->var->name === $paramName)) {
return true;
}

if ($node instanceof FuncCall
Expand Down Expand Up @@ -320,4 +307,13 @@ private function isMethodCallOrArrayDimFetch(string $paramName, ?Node $node): bo

return false;
}

private function isArrayAccess(Type $type): bool
{
if (! $type instanceof ObjectType) {
return false;
}

return $this->typeComparator->isSubtype($type, new ObjectType('ArrayAccess'));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function __construct(

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Add function/closure param array type, if dim fetch is inside', [
return new RuleDefinition('Add closure and arrow function param array type, if dim fetch is used inside', [
new CodeSample(
<<<'CODE_SAMPLE'
$array = [['name' => 'John']];
Expand Down
4 changes: 3 additions & 1 deletion src/Config/Level/TypeDeclarationLevel.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,12 @@ final class TypeDeclarationLevel
AddReturnTypeDeclarationBasedOnParentClassMethodRector::class,
ReturnTypeFromStrictFluentReturnRector::class,
ReturnNeverTypeRector::class,
StrictArrayParamDimFetchRector::class,
StrictStringParamConcatRector::class,
TypedPropertyFromJMSSerializerAttributeTypeRector::class,

// array parameter from dim fetch assign inside
StrictArrayParamDimFetchRector::class,

// possibly based on docblocks, but also helpful, intentionally last
AddArrayFunctionClosureParamTypeRector::class,
TypedPropertyFromDocblockSetUpDefinedRector::class,
Expand Down
Loading