Skip to content

[EarlyReturn] Split nested && operand into its own early return in ReturnBinaryOrToEarlyReturnRector#8156

Open
TomasVotruba wants to merge 2 commits into
mainfrom
fix-return-binary-or-nested-and-9802
Open

[EarlyReturn] Split nested && operand into its own early return in ReturnBinaryOrToEarlyReturnRector#8156
TomasVotruba wants to merge 2 commits into
mainfrom
fix-return-binary-or-nested-and-9802

Conversation

@TomasVotruba

@TomasVotruba TomasVotruba commented Jul 8, 2026

Copy link
Copy Markdown
Member

Fixes rectorphp/rector#9802

ReturnBinaryOrToEarlyReturnRector silently dropped operands when a returned || chain contained a parenthesized && in a nested-left position.

Problem

The rule used an empty array [] as an abort signal when it met a && operand. But when the && sat deep in the left branch of a longer || chain, that [] was spread back into the collected ifs during recursion — indistinguishable from "nothing collected here" — so collection resumed and the earlier operands were discarded:

 public function hasStale(): bool
 {
-    return $this->donation === null
-        || ($this->commission > 0 && $this->transaction === null)
-        || $this->somethingElse()
-        || $this->anotherThing();
+    if ($this->somethingElse()) {
+        return true;
+    }
+    return $this->anotherThing();
 }

The first two operands ($this->donation === null and the && group) vanished — the method no longer meant the same thing.

Fix

A && operand is a perfectly valid early-return condition, so instead of skipping it is now kept whole inside its own if. The convoluted recursive createMultipleIfs/collectLeftBooleanOrToIfs pair (and its ambiguous [] abort) is replaced by a simple flatten of the left-associative || chain into ordered operands — each becomes one early return, the last becomes the final return.

 public function hasStale(): bool
 {
-    return $this->donation === null
-        || ($this->commission > 0 && $this->transaction === null)
-        || $this->somethingElse()
-        || $this->anotherThing();
+    if ($this->donation === null) {
+        return true;
+    }
+    if ($this->commission > 0 && $this->transaction === null) {
+        return true;
+    }
+    if ($this->somethingElse()) {
+        return true;
+    }
+    return (bool) $this->anotherThing();
 }

Tests

  • Added and_nested_deep.php.inc (the reported case).
  • The former skip_and_in_next.php.inc is now and_in_next.php.inc — a && in the middle of a chain now transforms instead of being skipped.
  • All existing fixtures still pass unchanged.

@TomasVotruba TomasVotruba force-pushed the fix-return-binary-or-nested-and-9802 branch from 9591092 to 786d227 Compare July 8, 2026 12:24
@TomasVotruba TomasVotruba changed the title [EarlyReturn] Skip nested && in || chain in ReturnBinaryOrToEarlyReturnRector [EarlyReturn] Split nested && operand into its own early return in ReturnBinaryOrToEarlyReturnRector Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

ReturnBinaryOrToEarlyReturnRector removes operands from a returned || chain containing a parenthesized &&

2 participants