From d3d1ee470a67998b14b5c04b07134e1480d14dd9 Mon Sep 17 00:00:00 2001
From: Nikita Popov <nikita.ppv@gmail.com>
Date: Tue, 8 Dec 2020 23:04:55 +0100
Subject: [PATCH] Fix #738 incorrect start line for traits

Empty productions are supposed to be assigned the start attributes
of the lookahead token. Currently, this happens by assigning above
the current stack position when the token it read.

This fails in a situation where we first reduce an empty production
higher up in the stack, and then again reduce an empty production
lower in the stack, without consuming the lookahead token in the
meantime.

Fix this by moving the assignment into the reduction phase. We
also need to do this for error productions, which are effectively
empty.
---
 lib/PhpParser/ParserAbstract.php              |  13 ++-
 test/code/parser/errorHandling/recovery.test  |   3 -
 .../parser/stmt/class/class_position.test     | 100 +++++++++++-------
 3 files changed, 72 insertions(+), 44 deletions(-)

diff --git a/lib/PhpParser/ParserAbstract.php b/lib/PhpParser/ParserAbstract.php
index 11c8568..3ff4742 100644
--- a/lib/PhpParser/ParserAbstract.php
+++ b/lib/PhpParser/ParserAbstract.php
@@ -219,10 +219,7 @@ abstract class ParserAbstract implements Parser
                         ));
                     }
 
-                    // This is necessary to assign some meaningful attributes to /* empty */ productions. They'll get
-                    // the attributes of the next token, even though they don't contain it themselves.
-                    $this->startAttributeStack[$stackPos+1] = $startAttributes;
-                    $this->endAttributeStack[$stackPos+1] = $endAttributes;
+                    // Allow productions to access the start attributes of the lookahead token.
                     $this->lookaheadStartAttributes = $startAttributes;
 
                     //$this->traceRead($symbol);
@@ -294,7 +291,8 @@ abstract class ParserAbstract implements Parser
 
                     /* Goto - shift nonterminal */
                     $lastEndAttributes = $this->endAttributeStack[$stackPos];
-                    $stackPos -= $this->ruleToLength[$rule];
+                    $ruleLength = $this->ruleToLength[$rule];
+                    $stackPos -= $ruleLength;
                     $nonTerminal = $this->ruleToNonTerminal[$rule];
                     $idx = $this->gotoBase[$nonTerminal] + $stateStack[$stackPos];
                     if ($idx >= 0 && $idx < $this->gotoTableSize && $this->gotoCheck[$idx] === $nonTerminal) {
@@ -307,6 +305,10 @@ abstract class ParserAbstract implements Parser
                     $stateStack[$stackPos]     = $state;
                     $this->semStack[$stackPos] = $this->semValue;
                     $this->endAttributeStack[$stackPos] = $lastEndAttributes;
+                    if ($ruleLength === 0) {
+                        // Empty productions use the start attributes of the lookahead token.
+                        $this->startAttributeStack[$stackPos] = $this->lookaheadStartAttributes;
+                    }
                 } else {
                     /* error */
                     switch ($this->errorState) {
@@ -340,6 +342,7 @@ abstract class ParserAbstract implements Parser
 
                             // We treat the error symbol as being empty, so we reset the end attributes
                             // to the end attributes of the last non-error symbol
+                            $this->startAttributeStack[$stackPos] = $this->lookaheadStartAttributes;
                             $this->endAttributeStack[$stackPos] = $this->endAttributeStack[$stackPos - 1];
                             $this->endAttributes = $this->endAttributeStack[$stackPos - 1];
                             break;
diff --git a/test/code/parser/errorHandling/recovery.test b/test/code/parser/errorHandling/recovery.test
index d7d4256..27f7cb4 100644
--- a/test/code/parser/errorHandling/recovery.test
+++ b/test/code/parser/errorHandling/recovery.test
@@ -1490,9 +1490,6 @@ array(
                         )
                     )
                 )
-                comments: array(
-                    0: /** @var ?string */
-                )
             )
         )
     )
diff --git a/test/code/parser/stmt/class/class_position.test b/test/code/parser/stmt/class/class_position.test
index b657e85..f90ad9d 100644
--- a/test/code/parser/stmt/class/class_position.test
+++ b/test/code/parser/stmt/class/class_position.test
@@ -2,26 +2,28 @@ Class position
 -----
 <?php
 
-class A {
-}
+if (1);
 
-try {
-} catch (Exception $e) {
-}
-
-class B {
-}
-
-?>
+class C {}
 -----
 !!positions
 array(
-    0: Stmt_Class[3:1 - 4:1](
+    0: Stmt_If[3:1 - 3:7](
+        cond: Scalar_LNumber[3:5 - 3:5](
+            value: 1
+        )
+        stmts: array(
+        )
+        elseifs: array(
+        )
+        else: null
+    )
+    1: Stmt_Class[5:1 - 5:10](
         attrGroups: array(
         )
         flags: 0
-        name: Identifier[3:7 - 3:7](
-            name: A
+        name: Identifier[5:7 - 5:7](
+            name: C
         )
         extends: null
         implements: array(
@@ -29,36 +31,62 @@ array(
         stmts: array(
         )
     )
-    1: Stmt_TryCatch[6:1 - 8:1](
+)
+-----
+<?php
+
+if (1);
+
+trait X {}
+-----
+!!positions
+array(
+    0: Stmt_If[3:1 - 3:7](
+        cond: Scalar_LNumber[3:5 - 3:5](
+            value: 1
+        )
         stmts: array(
         )
-        catches: array(
-            0: Stmt_Catch[7:3 - 8:1](
-                types: array(
-                    0: Name[7:10 - 7:18](
-                        parts: array(
-                            0: Exception
-                        )
-                    )
-                )
-                var: Expr_Variable[7:20 - 7:21](
-                    name: e
-                )
-                stmts: array(
-                )
-            )
+        elseifs: array(
         )
-        finally: null
+        else: null
     )
-    2: Stmt_Class[10:1 - 11:1](
+    1: Stmt_Trait[5:1 - 5:10](
         attrGroups: array(
         )
-        flags: 0
-        name: Identifier[10:7 - 10:7](
-            name: B
+        name: Identifier[5:7 - 5:7](
+            name: X
         )
-        extends: null
-        implements: array(
+        stmts: array(
+        )
+    )
+)
+-----
+<?php
+
+if (1);
+
+interface X {}
+-----
+!!positions
+array(
+    0: Stmt_If[3:1 - 3:7](
+        cond: Scalar_LNumber[3:5 - 3:5](
+            value: 1
+        )
+        stmts: array(
+        )
+        elseifs: array(
+        )
+        else: null
+    )
+    1: Stmt_Interface[5:1 - 5:14](
+        attrGroups: array(
+        )
+        name: Identifier[5:11 - 5:11](
+            name: X
+        )
+        extends: array(
         )
         stmts: array(
         )