Skip to content

[CodeQuality] Preserve parentheses around right-side assign in LogicalToBooleanRector#8152

Merged
TomasVotruba merged 1 commit into
mainfrom
fix-logical-to-boolean-assign-parens
Jul 8, 2026
Merged

[CodeQuality] Preserve parentheses around right-side assign in LogicalToBooleanRector#8152
TomasVotruba merged 1 commit into
mainfrom
fix-logical-to-boolean-assign-parens

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

Fixes rectorphp/rector#9799

LogicalToBooleanRector dropped the parentheses around an assignment sitting on the right/nested side of a boolean operator, silently changing semantics.

Before

true and ($x = 42) and false;

was rewritten to:

true && $x = 42 && false;

which PHP parses as true && ($x = (42 && false)) — so $x becomes false instead of 42.

After

true && ($x = 42) && false;

Cause

BetterStandardPrinter::wrapBinaryOpWithBrackets() reprinted the right operand only when !haveParens. But the source parens in ($x = 42) live in the parent node's token range, not the Assign node's — so haveParens reports true, the reprint is skipped, and the newly pretty-printed && node drops them anyway.

Reprint the right Assign unconditionally. The pretty-printer is context-aware and only adds parentheses when semantically needed:

-is_int(1) and $x = 5;
+is_int(1) && $x = 5;        // no following op -> stays paren-free

-true and $x = 42 and false;
+true && ($x = 42) && false; // following && -> parens added

Left-operand handling keeps its !haveParens guard: removing it double-wraps cases like ($c = $x) ?? $d whose parent node preserves its own token gaps.

…lToBooleanRector

Fixes rectorphp/rector#9799

When an assignment sits on the right/nested side of a converted boolean
operator, the printer reused the Assign node verbatim and dropped the
parentheses the pretty-printer would add in context:

    true and ($x = 42) and false;
    // became: true && $x = 42 && false;  ->  $x = (42 && false) = false

The right-side reprint was guarded by !haveParens, but source parens live
in the parent node token range (not the Assign node), so the guard skipped
the reprint and the parens were lost anyway. Reprint unconditionally; the
pretty-printer only adds parens when semantically needed.
@TomasVotruba
TomasVotruba force-pushed the fix-logical-to-boolean-assign-parens branch from b24637a to 6475904 Compare July 8, 2026 10:37
@TomasVotruba

Copy link
Copy Markdown
Member Author

Let's ship it 👍

@TomasVotruba
TomasVotruba merged commit cfea3ba into main Jul 8, 2026
65 checks passed
@TomasVotruba
TomasVotruba deleted the fix-logical-to-boolean-assign-parens branch July 8, 2026 10:42
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.

Incorrect behavior of LogicalToBooleanRector: removing important parentheses in some cases

1 participant