Skip to content
Merged
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
24 changes: 12 additions & 12 deletions rules/Transform/Rector/ClassMethod/WrapReturnRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,9 @@ public function refactor(Node $node): ?Node
continue;
}

$this->wrap($classMethod, $typeMethodWrap->isArrayWrap());
$hasChanged = true;
if ($typeMethodWrap->isArrayWrap() && $this->wrap($classMethod)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of passing the isArrayWrap() value to the function, if it is not true we just don't call the function

$hasChanged = true;
}
}
}

Expand All @@ -107,22 +108,21 @@ public function configure(array $configuration): void
$this->typeMethodWraps = $configuration;
}

private function wrap(ClassMethod $classMethod, bool $isArrayWrap): ?ClassMethod
private function wrap(ClassMethod $classMethod): bool
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of returning the ClassMethod (which was not used), we return a boolean indicating if the value has changed

{
if (! is_iterable($classMethod->stmts)) {
return null;
return false;
}

foreach ($classMethod->stmts as $key => $stmt) {
if ($stmt instanceof Return_ && $stmt->expr instanceof Expr) {
if ($isArrayWrap && ! $stmt->expr instanceof Array_) {
$stmt->expr = new Array_([new ArrayItem($stmt->expr)]);
}

$classMethod->stmts[$key] = $stmt;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not needed, this code just replaces a statement with itself

$hasChanged = false;
foreach ($classMethod->stmts as $stmt) {
if ($stmt instanceof Return_ && $stmt->expr instanceof Expr
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplify the if statement into a single statement

&& ! $stmt->expr instanceof Array_) {
$stmt->expr = new Array_([new ArrayItem($stmt->expr)]);
$hasChanged = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only when we actually change the return expression we mark the node as changed

}
}

return $classMethod;
return $hasChanged;
}
}