Fix parentheses for moved nodes in format-preserving printer

lib/PhpParser/PrettyPrinterAbstract.php

@@ -458,6 +458,63 @@ protected function pPostfixOp(string $class, Node $node, string $operatorString,
         return $prefix . $this->p($node, $opPrecedence, $lhsPrecedence) . $operatorString . $suffix;
     }
 
+    /**
+     * Determine whether an operator node printed with preserved formatting needs parentheses.
+     *
+     * Parentheses for an operator are printed by the operator node itself (see pInfixOp(),
+     * pPrefixOp() and pPostfixOp()). If the formatting of the node is preserved, its original code
+     * is reused instead, so the same check has to be performed separately. This matters if the
+     * node has been moved into a context that binds more tightly than the original one.
+     *
+     * @param Node $node Node whose formatting is preserved
+     * @param int $precedence Precedence of parent operator
+     * @param int $lhsPrecedence Precedence for unary operator on LHS of binary operator
+     *
+     * @return bool Whether parentheses are required
+     */
+    protected function preservedOpRequiresParens(Node $node, int $precedence, int $lhsPrecedence): bool {
+        $class = \get_class($node);
+        if (!isset($this->precedenceMap[$class])) {
+            return false;
+        }
+
+        $opPrecedence = $this->precedenceMap[$class][0];
+        if ($node instanceof BinaryOp || $node instanceof Expr\Ternary || $node instanceof Expr\Instanceof_) {
+            // Infix and postfix operators
+            return $opPrecedence >= $precedence;
+        }
+
+        // Prefix operators
+        return $opPrecedence >= $lhsPrecedence;
+    }
+
+    /**
+     * Get the name of the operand at the end of an operator node.
+     *
+     * A prefix operator at the end of an expression (e.g. the assignment in `$a === $b = $c`)
+     * extends as far to the right as possible. The operand at the end of an operator therefore
+     * inherits the $lhsPrecedence of the operator (see pInfixOp() and pPrefixOp()).
+     *
+     * @param Node $node Operator node
+     *
+     * @return string|null Subnode name, or null if the node does not end with an operand
+     */
+    protected function getTrailingOperandName(Node $node): ?string {
+        if ($node instanceof BinaryOp) {
+            return 'right';
+        }
+        if ($node instanceof Expr\Ternary) {
+            return 'else';
+        }
+        if ($node instanceof Expr\Yield_) {
+            return 'value';
+        }
+        if ($node instanceof Expr\Instanceof_ || !isset($this->precedenceMap[\get_class($node)])) {
+            return null;
+        }
+        return 'expr';
+    }
+
     /**
      * Pretty prints an array of nodes and implodes the printed values.
      *
@@ -644,6 +701,12 @@ protected function p(
         $type = $node->getType();
         $fixupInfo = $this->fixupMap[$class] ?? null;
 
+        // The original code may have been printed in a different context, so the parentheses
+        // required by the current one have to be added here.
+        $parens = $this->preservedOpRequiresParens($node, $precedence, $lhsPrecedence);
+        $trailingOperand = !$parens && $lhsPrecedence !== self::MAX_PRECEDENCE
+            ? $this->getTrailingOperandName($node) : null;
+
         $result = '';
         $pos = $startPos;
         foreach ($node->getSubNodeNames() as $subNodeName) {
@@ -752,6 +815,10 @@ protected function p(
                 ) {
                     $fixup = $fixupInfo[$subNodeName];
                     $res = $this->pFixup($fixup, $subNode, $class, $subStartPos, $subEndPos);
+                } elseif ($subNodeName === $trailingOperand
+                    && !$this->origTokens->haveParens($subStartPos, $subEndPos)
+                ) {
+                    $res = $this->p($subNode, self::MAX_PRECEDENCE, $lhsPrecedence, true);
                 } else {
                     $res = $this->p($subNode, self::MAX_PRECEDENCE, self::MAX_PRECEDENCE, true);
                 }
@@ -766,6 +833,9 @@ protected function p(
         }
 
         $result .= $this->origTokens->getTokenCode($pos, $endPos + 1, $indentAdjustment);
+        if ($parens) {
+            $result = '(' . $result . ')';
+        }
         return $result;
     }
 

test/code/formatPreservation/fixupMovedNodes.test

@@ -0,0 +1,62 @@
+Precedence of nodes with preserved formatting moved into a new context
+-----
+<?php
+$x = a() && b();
+$x = $a !== null ? $a : ($b ? 'yes' : 'no');
+$x = $b - $c;
+$x = $b - $c;
+$a = $b;
+$a = $b;
+-----
+// New parent around a preserved operand
+$stmts[0]->expr->expr = new Expr\BooleanNot($stmts[0]->expr->expr);
+$stmts[1]->expr->expr = new Expr\BinaryOp\Coalesce(
+    $stmts[1]->expr->expr->if, $stmts[1]->expr->expr->else);
+$stmts[2]->expr->expr = new Expr\BinaryOp\Minus(new Expr\Variable('a'), $stmts[2]->expr->expr);
+// No parens necessary, minus is left assoc
+$stmts[3]->expr->expr = new Expr\BinaryOp\Minus($stmts[3]->expr->expr, new Expr\Variable('d'));
+// Assignment on the LHS needs parens, on the RHS it does not
+$stmts[4]->expr = new Expr\BinaryOp\BooleanAnd($stmts[4]->expr, new Expr\Variable('c'));
+$stmts[5]->expr = new Expr\BinaryOp\BooleanAnd(new Expr\Variable('c'), $stmts[5]->expr);
+-----
+<?php
+$x = !(a() && b());
+$x = $a ?? ($b ? 'yes' : 'no');
+$x = $a - ($b - $c);
+$x = $b - $c - $d;
+($a = $b) && $c;
+$c && $a = $b;
+-----
+<?php
+$a  *  $b;
+$c + $d;
+$a  *  ($b);
+$c + $d;
+-----
+// Preserved operand moved into a preserved parent
+$stmts[0]->expr->right = $stmts[1]->expr;
+// Already has parens
+$stmts[2]->expr->right = $stmts[3]->expr;
+-----
+<?php
+$a  *  ($c + $d);
+$c + $d;
+$a  *  ($c + $d);
+$c + $d;
+-----
+<?php
+$a  ||  $b;
+$c === $d = $e;
+$f >> print $g;
+$x = @$h = $i;
+-----
+// Prefix operator at the end of a preserved operand, which is now followed by an operator
+$stmts[0]->expr->left = $stmts[1]->expr;
+$stmts[2]->expr = new Expr\BinaryOp\Identical($stmts[2]->expr, new Expr\Variable('j'));
+$stmts[3]->expr->expr = new Expr\Instanceof_($stmts[3]->expr->expr, new Node\Name('Foo'));
+-----
+<?php
+$c === ($d = $e)  ||  $b;
+$c === $d = $e;
+$f >> (print $g) === $j;
+$x = @($h = $i) instanceof Foo;