Skip to content

Commit 2fd1aa5

Browse files
committed
Fix double-wrap regression: gate nested compound-assignment reprint to source operands
1 parent 391bc0b commit 2fd1aa5

2 files changed

Lines changed: 31 additions & 5 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
$s = 0;
4+
2 * ($s += 42) and true;
5+
6+
?>
7+
-----
8+
<?php
9+
10+
$s = 0;
11+
2 * ($s += 42) && true;
12+
13+
?>

src/PhpParser/Printer/BetterStandardPrinter.php

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ private function wrapBinaryOpWithBrackets(Node $node): void
479479
// an assignment nested directly under a unary operator (e.g. "!$x = 1", "-$x += 1",
480480
// "(int) $x = 1") loses its required parentheses once "and"/"or" become "&&"/"||"; force
481481
// the assignment to reprint so the pretty-printer re-adds them based on precedence
482-
if ($this->isUnaryExprWithAssign($node) && $this->origTokens instanceof TokenStream) {
482+
if ($node->getStartTokenPos() >= 0 && $this->isUnaryExprWithAssign($node) && $this->origTokens instanceof TokenStream) {
483483
$node->expr->setAttribute(AttributeKey::ORIGINAL_NODE, null);
484484
}
485485

@@ -491,16 +491,29 @@ private function wrapBinaryOpWithBrackets(Node $node): void
491491
return;
492492
}
493493

494-
if ($this->isAssignExpr($node->left)
494+
if ($node->left instanceof Assign
495495
&& $this->origTokens instanceof TokenStream
496496
&& ! $this->origTokens->haveParens($node->left->getStartTokenPos(), $node->left->getEndTokenPos())) {
497497
$node->left->setAttribute(AttributeKey::ORIGINAL_NODE, null);
498498
}
499499

500-
if ($this->isAssignExpr($node->right) && $this->origTokens instanceof TokenStream) {
500+
if ($node->right instanceof Assign && $this->origTokens instanceof TokenStream) {
501501
$node->right->setAttribute(AttributeKey::ORIGINAL_NODE, null);
502502
}
503503

504+
// a compound assignment ("+=", "-=", …) nested in an originally printed operand keeps its
505+
// verbatim, parenless tokens; force it to reprint so the required parentheses are re-added.
506+
// only for source operands: freshly created binary nodes already get them from precedence
507+
if ($node->getStartTokenPos() >= 0) {
508+
if ($node->left instanceof AssignOp || $node->left instanceof AssignRef) {
509+
$node->left->setAttribute(AttributeKey::ORIGINAL_NODE, null);
510+
}
511+
512+
if ($node->right instanceof AssignOp || $node->right instanceof AssignRef) {
513+
$node->right->setAttribute(AttributeKey::ORIGINAL_NODE, null);
514+
}
515+
}
516+
504517
// boolean operators (&&, ||) bind tighter than the logical ones (and, or); when
505518
// LogicalToBooleanRector swaps them, operands with lower precedence must be reprinted
506519
// so the pretty-printer re-adds the parentheses that keep the original semantics
@@ -537,9 +550,9 @@ private function isUnaryExprWithAssign(Node $node): bool
537550
return $this->isAssignExpr($node->expr);
538551
}
539552

540-
private function isAssignExpr(Node $node): bool
553+
private function isAssignExpr(Expr $expr): bool
541554
{
542-
return $node instanceof Assign || $node instanceof AssignOp || $node instanceof AssignRef;
555+
return $expr instanceof Assign || $expr instanceof AssignOp || $expr instanceof AssignRef;
543556
}
544557

545558
/**

0 commit comments

Comments
 (0)