Fix formatting preservation for alternative elseif/else syntax

Test taken from PR #797.
nikic · Sep 11, 2022 · 9b46dff · 9b46dff
1 parent 205bd75
commit 9b46dff
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 8 deletions.
diff --git a/grammar/php.y b/grammar/php.y
@@ -628,7 +628,8 @@ new_elseif_list:
 ;
 
 new_elseif:
-     T_ELSEIF '(' expr ')' ':' inner_statement_list         { $$ = Stmt\ElseIf_[$3, $6]; }
+     T_ELSEIF '(' expr ')' ':' inner_statement_list
+         { $$ = Stmt\ElseIf_[$3, $6]; $this->fixupAlternativeElse($$); }
 ;
 
 else_single:
@@ -638,7 +639,8 @@ else_single:
 
 new_else_single:
       /* empty */                                           { $$ = null; }
-    | T_ELSE ':' inner_statement_list                       { $$ = Stmt\Else_[$3]; }
+    | T_ELSE ':' inner_statement_list
+          { $$ = Stmt\Else_[$3]; $this->fixupAlternativeElse($$); }
 ;
 
 foreach_variable:

diff --git a/lib/PhpParser/Parser/Php7.php b/lib/PhpParser/Parser/Php7.php
@@ -1966,7 +1966,7 @@ protected function initReduceCallbacks() {
                  $this->semStack[$stackPos-(2-1)][] = $this->semStack[$stackPos-(2-2)]; $this->semValue = $this->semStack[$stackPos-(2-1)];
             },
             263 => function ($stackPos) {
-                 $this->semValue = new Stmt\ElseIf_($this->semStack[$stackPos-(6-3)], $this->semStack[$stackPos-(6-6)], $this->startAttributeStack[$stackPos-(6-1)] + $this->endAttributes);
+                 $this->semValue = new Stmt\ElseIf_($this->semStack[$stackPos-(6-3)], $this->semStack[$stackPos-(6-6)], $this->startAttributeStack[$stackPos-(6-1)] + $this->endAttributes); $this->fixupAlternativeElse($this->semValue);
             },
             264 => function ($stackPos) {
                  $this->semValue = null;
@@ -1978,7 +1978,7 @@ protected function initReduceCallbacks() {
                  $this->semValue = null;
             },
             267 => function ($stackPos) {
-                 $this->semValue = new Stmt\Else_($this->semStack[$stackPos-(3-3)], $this->startAttributeStack[$stackPos-(3-1)] + $this->endAttributes);
+                 $this->semValue = new Stmt\Else_($this->semStack[$stackPos-(3-3)], $this->startAttributeStack[$stackPos-(3-1)] + $this->endAttributes); $this->fixupAlternativeElse($this->semValue);
             },
             268 => function ($stackPos) {
                  $this->semValue = array($this->semStack[$stackPos-(1-1)], false);

diff --git a/lib/PhpParser/Parser/Php8.php b/lib/PhpParser/Parser/Php8.php
@@ -1984,7 +1984,7 @@ protected function initReduceCallbacks() {
                  $this->semStack[$stackPos-(2-1)][] = $this->semStack[$stackPos-(2-2)]; $this->semValue = $this->semStack[$stackPos-(2-1)];
             },
             263 => function ($stackPos) {
-                 $this->semValue = new Stmt\ElseIf_($this->semStack[$stackPos-(6-3)], $this->semStack[$stackPos-(6-6)], $this->startAttributeStack[$stackPos-(6-1)] + $this->endAttributes);
+                 $this->semValue = new Stmt\ElseIf_($this->semStack[$stackPos-(6-3)], $this->semStack[$stackPos-(6-6)], $this->startAttributeStack[$stackPos-(6-1)] + $this->endAttributes); $this->fixupAlternativeElse($this->semValue);
             },
             264 => function ($stackPos) {
                  $this->semValue = null;
@@ -1996,7 +1996,7 @@ protected function initReduceCallbacks() {
                  $this->semValue = null;
             },
             267 => function ($stackPos) {
-                 $this->semValue = new Stmt\Else_($this->semStack[$stackPos-(3-3)], $this->startAttributeStack[$stackPos-(3-1)] + $this->endAttributes);
+                 $this->semValue = new Stmt\Else_($this->semStack[$stackPos-(3-3)], $this->startAttributeStack[$stackPos-(3-1)] + $this->endAttributes); $this->fixupAlternativeElse($this->semValue);
             },
             268 => function ($stackPos) {
                  $this->semValue = array($this->semStack[$stackPos-(1-1)], false);

diff --git a/lib/PhpParser/ParserAbstract.php b/lib/PhpParser/ParserAbstract.php
@@ -20,9 +20,12 @@
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassConst;
 use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Else_;
+use PhpParser\Node\Stmt\ElseIf_;
 use PhpParser\Node\Stmt\Enum_;
 use PhpParser\Node\Stmt\Interface_;
 use PhpParser\Node\Stmt\Namespace_;
+use PhpParser\Node\Stmt\Nop;
 use PhpParser\Node\Stmt\Property;
 use PhpParser\Node\Stmt\TryCatch;
 use PhpParser\Node\UseItem;
@@ -874,6 +877,24 @@ protected function postprocessList(Expr\List_ $node): void {
         }
     }
 
+    /** @param ElseIf_|Else_ $node */
+    protected function fixupAlternativeElse($node): void {
+        // Make sure a trailing nop statement carrying comments is part of the node.
+        $numStmts = \count($node->stmts);
+        if ($numStmts !== 0 && $node->stmts[$numStmts - 1] instanceof Nop) {
+            $nopAttrs = $node->stmts[$numStmts - 1]->getAttributes();
+            if (isset($nopAttrs['endLine'])) {
+                $node->setAttribute('endLine', $nopAttrs['endLine']);
+            }
+            if (isset($nopAttrs['endFilePos'])) {
+                $node->setAttribute('endFilePos', $nopAttrs['endFilePos']);
+            }
+            if (isset($nopAttrs['endTokenPos'])) {
+                $node->setAttribute('endTokenPos', $nopAttrs['endTokenPos']);
+            }
+        }
+    }
+
     protected function checkClassModifier(int $a, int $b, int $modifierPos) {
         try {
             Modifiers::verifyClassModifier($a, $b);

diff --git a/test/code/formatPreservation/comments.test b/test/code/formatPreservation/comments.test
@@ -49,4 +49,4 @@ class Test {
     public function test() {
         // some code
     }
-}
+}
diff --git a/test/code/prettyPrinter/comments.test b/test/code/prettyPrinter/comments.test
@@ -64,4 +64,28 @@ function test()
 function test()
 {
     // empty
-}
+}
+-----
+<?php
+
+function noDuplicateComment()
+{
+    if (true):
+        // TEST 1
+    elseif (true):
+        // TEST 2
+    else:
+        // TEST 3
+    endif;
+}
+-----
+function noDuplicateComment()
+{
+    if (true) {
+        // TEST 1
+    } elseif (true) {
+        // TEST 2
+    } else {
+        // TEST 3
+    }
+}
-Original file line number
+Diff line change
@@ Expand Up / @@ -49,4 +49,4 @@ class Test { @@
         public function test() {
             // some code
         }
-    }
+    }