diff --git a/phpstan.neon b/phpstan.neon index 26b131aa034..947e43b5a06 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -338,3 +338,7 @@ parameters: - message: '#Offset float\|int\|string might not exist on string#' path: rules/Php70/EregToPcreTransformer.php + + - + identifier: symplify.noReference + message: '#Use explicit return value over magic &reference#' diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/keep_comments.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/include_empty_assign.php.inc similarity index 50% rename from rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/keep_comments.php.inc rename to rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/include_empty_assign.php.inc index c2c085d4d49..084e1f7e4c7 100644 --- a/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/keep_comments.php.inc +++ b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/include_empty_assign.php.inc @@ -2,14 +2,13 @@ namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture; -final class KeepComments +final class IncludeEmptyAssign { - public function getPerson() + public function run($person2) { $person = []; - // name - $person['name'] = 'Timmy'; - // surname + + $person[] = 'Timmy'; $person['surname'] = 'Back'; return $person; @@ -22,16 +21,11 @@ final class KeepComments namespace Rector\Tests\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector\Fixture; -final class KeepComments +final class IncludeEmptyAssign { - public function getPerson() + public function run($person2) { - return [ - // name - 'name' => 'Timmy', - // surname - 'surname' => 'Back', - ]; + return ['Timmy', 'surname' => 'Back']; } } diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/include_nested_variable.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/include_nested_variable.php.inc new file mode 100644 index 00000000000..9f2d0393d81 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/include_nested_variable.php.inc @@ -0,0 +1,30 @@ + +----- + 'John', 'surname' => 'Doe', 'age' => 50]]; + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/nested_array_with_multiple_keys.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/nested_array_with_multiple_keys.php.inc new file mode 100644 index 00000000000..09586b81094 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/nested_array_with_multiple_keys.php.inc @@ -0,0 +1,32 @@ + 1, 'b' => 2]; + + return $arr; + } +} + + +?> +----- + ['bar' => ['baz' => ['a' => 1, 'b' => 2]]]]; + } +} + + +?> diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_different_variable.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_different_variable.php.inc new file mode 100644 index 00000000000..63b3e02cddb --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_different_variable.php.inc @@ -0,0 +1,17 @@ + 1, 'b' => 2]; - - return $arr; - } -} - diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_reuse_variable_in_assign_expr.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_reuse_variable_in_assign_expr.php.inc deleted file mode 100644 index 73df1404d6d..00000000000 --- a/rules-tests/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector/Fixture/skip_reuse_variable_in_assign_expr.php.inc +++ /dev/null @@ -1,18 +0,0 @@ - +----- + 'John', 'surname' => 'Doe', 'age' => 50]; + } +} + +?> diff --git a/rules/CodeQuality/NodeAnalyzer/VariableDimFetchAssignResolver.php b/rules/CodeQuality/NodeAnalyzer/VariableDimFetchAssignResolver.php index 24054feae2b..de83d55ca87 100644 --- a/rules/CodeQuality/NodeAnalyzer/VariableDimFetchAssignResolver.php +++ b/rules/CodeQuality/NodeAnalyzer/VariableDimFetchAssignResolver.php @@ -4,34 +4,41 @@ namespace Rector\CodeQuality\NodeAnalyzer; -use PhpParser\Node; use PhpParser\Node\Expr; use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\Assign; -use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Expression; +use PhpParser\Node\Stmt\Return_; use Rector\CodeQuality\ValueObject\KeyAndExpr; -use Rector\PhpParser\Comparing\NodeComparator; -use Rector\PhpParser\Node\BetterNodeFinder; +use Rector\PhpParser\Node\Value\ValueResolver; final readonly class VariableDimFetchAssignResolver { public function __construct( - private NodeComparator $nodeComparator, - private BetterNodeFinder $betterNodeFinder + private ValueResolver $valueResolver ) { } /** * @param Stmt[] $stmts - * @return KeyAndExpr[] + * @return array */ - public function resolveFromStmtsAndVariable(array $stmts, Variable $variable): array + public function resolveFromStmtsAndVariable(array $stmts, ?Assign $emptyArrayAssign): array { - $keysAndExprs = []; + $exprs = []; + + $key = 0; foreach ($stmts as $stmt) { + if ($stmt instanceof Expression && $stmt->expr === $emptyArrayAssign) { + continue; + } + + if ($stmt instanceof Return_) { + continue; + } + if (! $stmt instanceof Expression) { return []; } @@ -43,66 +50,40 @@ public function resolveFromStmtsAndVariable(array $stmts, Variable $variable): a $assign = $stmtExpr; - $keyExpr = $this->matchKeyOnArrayDimFetchOfVariable($assign, $variable); - if ($assign->var instanceof ArrayDimFetch && $assign->var->var instanceof ArrayDimFetch) { - return []; - } - - $keysAndExprs[] = new KeyAndExpr($keyExpr, $assign->expr, $stmt->getComments()); - } - - // we can only work with same variable - // and exclusively various keys or empty keys - if (! $this->hasExclusivelyNullKeyOrFilledKey($keysAndExprs)) { - return []; - } - - return $keysAndExprs; - } + $dimValues = []; - private function matchKeyOnArrayDimFetchOfVariable(Assign $assign, Variable $variable): ?Expr - { - if (! $assign->var instanceof ArrayDimFetch) { - return null; - } + $arrayDimFetch = $assign->var; + while ($arrayDimFetch instanceof ArrayDimFetch) { + $dimValues[] = $arrayDimFetch->dim instanceof Expr ? $this->valueResolver->getValue( + $arrayDimFetch->dim + ) : $key; - $arrayDimFetch = $assign->var; - if (! $this->nodeComparator->areNodesEqual($arrayDimFetch->var, $variable)) { - return null; - } + $arrayDimFetch = $arrayDimFetch->var; + } - $isFoundInExpr = (bool) $this->betterNodeFinder->findFirst( - $assign->expr, - fn (Node $subNode): bool => $this->nodeComparator->areNodesEqual($subNode, $variable) - ); + ++$key; - if ($isFoundInExpr) { - return null; + $this->setNestedKeysExpr($exprs, $dimValues, $assign->expr); } - return $arrayDimFetch->dim; + return $exprs; } /** - * @param KeyAndExpr[] $keysAndExprs + * @param mixed[] $exprsByKeys + * @param array $keys */ - private function hasExclusivelyNullKeyOrFilledKey(array $keysAndExprs): bool + private function setNestedKeysExpr(array &$exprsByKeys, array $keys, Expr $expr): void { - $alwaysNullKey = true; - $alwaysStringKey = true; - - foreach ($keysAndExprs as $keyAndExpr) { - if ($keyAndExpr->getKeyExpr() instanceof Expr) { - $alwaysNullKey = false; - } else { - $alwaysStringKey = false; - } - } + $reference = &$exprsByKeys; + + $keys = array_reverse($keys); - if ($alwaysNullKey) { - return true; + foreach ($keys as $key) { + // create intermediate arrays automatically + $reference = &$reference[$key]; } - return $alwaysStringKey; + $reference = $expr; } } diff --git a/rules/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector.php b/rules/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector.php index ce1105db0ac..a0554c6cb3f 100644 --- a/rules/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/InlineArrayReturnAssignRector.php @@ -5,7 +5,6 @@ namespace Rector\CodeQuality\Rector\ClassMethod; use PhpParser\Node; -use PhpParser\Node\ArrayItem; use PhpParser\Node\Expr\Array_; use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\Assign; @@ -15,10 +14,7 @@ use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\Return_; use Rector\CodeQuality\NodeAnalyzer\VariableDimFetchAssignResolver; -use Rector\CodeQuality\ValueObject\KeyAndExpr; use Rector\Contract\PhpParser\Node\StmtsAwareInterface; -use Rector\NodeTypeResolver\Node\AttributeKey; -use Rector\PhpParser\Node\Value\ValueResolver; use Rector\Rector\AbstractRector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -30,7 +26,6 @@ final class InlineArrayReturnAssignRector extends AbstractRector { public function __construct( private readonly VariableDimFetchAssignResolver $variableDimFetchAssignResolver, - private readonly ValueResolver $valueResolver ) { } @@ -77,100 +72,57 @@ public function getNodeTypes(): array public function refactor(Node $node): ?Node { $stmts = (array) $node->stmts; + + // skip primitive cases, as may be on purpose if (count($stmts) < 3) { return null; } - $firstStmt = array_shift($stmts); - $variable = $this->matchVariableAssignOfEmptyArray($firstStmt); - if (! $variable instanceof Variable) { + $lastStmt = $node->stmts[count($stmts) - 1] ?? null; + if (! $lastStmt instanceof Return_) { return null; } - if (! $this->areAssignExclusiveToDimFetch($stmts)) { + if (! $lastStmt->expr instanceof Variable) { return null; } - $lastStmt = array_pop($stmts); - if (! $lastStmt instanceof Stmt) { + $returnedVariableName = $this->getName($lastStmt->expr); + if (! is_string($returnedVariableName)) { return null; } - if (! $this->isReturnOfVariable($lastStmt, $variable)) { + $emptyArrayAssign = $this->resolveDefaultEmptyArrayAssign($stmts, $returnedVariableName); + if (! $this->areAssignExclusiveToDimFetchVariable($stmts, $emptyArrayAssign, $returnedVariableName)) { return null; } - $keysAndExprs = $this->variableDimFetchAssignResolver->resolveFromStmtsAndVariable($stmts, $variable); - if ($keysAndExprs === []) { + $keysAndExprsByKey = $this->variableDimFetchAssignResolver->resolveFromStmtsAndVariable( + $stmts, + $emptyArrayAssign, + ); + + if ($keysAndExprsByKey === []) { return null; } - $array = $this->createArray($keysAndExprs); + $array = $this->nodeFactory->createArray($keysAndExprsByKey); $node->stmts = [new Return_($array)]; return $node; } - private function matchVariableAssignOfEmptyArray(Stmt $stmt): ?Variable - { - if (! $stmt instanceof Expression) { - return null; - } - - if (! $stmt->expr instanceof Assign) { - return null; - } - - $assign = $stmt->expr; - - if (! $this->valueResolver->isValue($assign->expr, [])) { - return null; - } - - if (! $assign->var instanceof Variable) { - return null; - } - - return $assign->var; - } - - private function isReturnOfVariable(Stmt $stmt, Variable $variable): bool - { - if (! $stmt instanceof Return_) { - return false; - } - - if (! $stmt->expr instanceof Variable) { - return false; - } - - return $this->nodeComparator->areNodesEqual($stmt->expr, $variable); - } - - /** - * @param KeyAndExpr[] $keysAndExprs - */ - private function createArray(array $keysAndExprs): Array_ - { - $arrayItems = []; - foreach ($keysAndExprs as $keyAndExpr) { - $arrayItem = new ArrayItem($keyAndExpr->getExpr(), $keyAndExpr->getKeyExpr()); - $arrayItem->setAttribute(AttributeKey::COMMENTS, $keyAndExpr->getComments()); - - $arrayItems[] = $arrayItem; - } - - return new Array_($arrayItems); - } - /** * Only: * $items['...'] = $result; * * @param Stmt[] $stmts */ - private function areAssignExclusiveToDimFetch(array $stmts): bool - { + private function areAssignExclusiveToDimFetchVariable( + array $stmts, + ?Assign $emptyArrayAssign, + string $variableName + ): bool { $lastKey = array_key_last($stmts); foreach ($stmts as $key => $stmt) { @@ -189,6 +141,11 @@ private function areAssignExclusiveToDimFetch(array $stmts): bool $assign = $stmt->expr; + // skip initial assign + if ($assign === $emptyArrayAssign) { + continue; + } + // skip new X instance with args to keep complex assign readable if ($assign->expr instanceof New_ && ! $assign->expr->isFirstClassCallable() && $assign->expr->getArgs() !== []) { return false; @@ -197,8 +154,56 @@ private function areAssignExclusiveToDimFetch(array $stmts): bool if (! $assign->var instanceof ArrayDimFetch) { return false; } + + $arrayDimFetch = $assign->var; + + // traverse all nested variables up + while ($arrayDimFetch instanceof ArrayDimFetch) { + $arrayDimFetch = $arrayDimFetch->var; + } + + if (! $arrayDimFetch instanceof Variable) { + return false; + } + + if (! $this->isName($arrayDimFetch, $variableName)) { + return false; + } } return true; } + + /** + * @param Expression[] $stmts + */ + private function resolveDefaultEmptyArrayAssign(array $stmts, string $returnedVariableName): ?Assign + { + foreach ($stmts as $stmt) { + if (! $stmt instanceof Expression) { + continue; + } + + if (! $stmt->expr instanceof Assign) { + continue; + } + + $assign = $stmt->expr; + if (! $assign->var instanceof Variable) { + continue; + } + + if (! $this->isName($assign->var, $returnedVariableName)) { + continue; + } + + if (! $assign->expr instanceof Array_) { + continue; + } + + return $assign; + } + + return null; + } } diff --git a/rules/Privatization/TypeManipulator/TypeNormalizer.php b/rules/Privatization/TypeManipulator/TypeNormalizer.php index 540dcd44a8c..04b4e1b08c6 100644 --- a/rules/Privatization/TypeManipulator/TypeNormalizer.php +++ b/rules/Privatization/TypeManipulator/TypeNormalizer.php @@ -25,7 +25,7 @@ final class TypeNormalizer * @deprecated This method is deprecated and will be removed in the next major release. * Use @see generalizeConstantTypes() instead. */ - public function generalizeConstantBoolTypes(\PHPStan\Type\Type $type): Type + public function generalizeConstantBoolTypes(Type $type): Type { return $this->generalizeConstantTypes($type); } diff --git a/rules/TypeDeclarationDocblocks/Rector/Class_/AddReturnDocblockDataProviderRector.php b/rules/TypeDeclarationDocblocks/Rector/Class_/AddReturnDocblockDataProviderRector.php index f1ccc16733e..19c3ac93acf 100644 --- a/rules/TypeDeclarationDocblocks/Rector/Class_/AddReturnDocblockDataProviderRector.php +++ b/rules/TypeDeclarationDocblocks/Rector/Class_/AddReturnDocblockDataProviderRector.php @@ -154,11 +154,9 @@ public function refactor(Node $node): ?Node if ($yields !== []) { $yieldType = $this->yieldTypeResolver->resolveFromYieldNodes($yields, $dataProviderClassMethod); - if ($yieldType instanceof FullyQualifiedGenericObjectType) { - if ($yieldType->getClassName() === 'Generator') { - // most likely, a static iterator is used in data test fixtures - $yieldType = new FullyQualifiedGenericObjectType('Iterator', $yieldType->getTypes()); - } + if ($yieldType instanceof FullyQualifiedGenericObjectType && $yieldType->getClassName() === 'Generator') { + // most likely, a static iterator is used in data test fixtures + $yieldType = new FullyQualifiedGenericObjectType('Iterator', $yieldType->getTypes()); } $this->addGeneratedTypeReturnDocblockType($yieldType, $classMethodPhpDocInfo, $dataProviderClassMethod); diff --git a/src/PhpParser/Node/NodeFactory.php b/src/PhpParser/Node/NodeFactory.php index 8cfb1e4ca7d..f1b486070ce 100644 --- a/src/PhpParser/Node/NodeFactory.php +++ b/src/PhpParser/Node/NodeFactory.php @@ -4,6 +4,7 @@ namespace Rector\PhpParser\Node; +use PhpParser\Node\Expr\New_; use PhpParser\Builder\Method; use PhpParser\Builder\Param as ParamBuilder; use PhpParser\Builder\Property as PropertyBuilder; @@ -396,6 +397,12 @@ private function createArrayItem(mixed $item, string | int | null $key = null): return $arrayItem; } + if ($item instanceof New_) { + $arrayItem = new ArrayItem($item); + $this->decorateArrayItemWithKey($key, $arrayItem); + return $arrayItem; + } + $nodeClass = is_object($item) ? $item::class : $item; throw new NotImplementedYetException(sprintf( 'Not implemented yet. Go to "%s()" and add check for "%s" node.',