Fix nikic#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.

Author: nikic

lib/PhpParser/ParserAbstract.php

@@ -219,10 +219,7 @@ protected function doParse() {
                         ));
                     }
 
-                    // 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 @@ protected function doParse() {
 
                     /* 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 @@ protected function doParse() {
                     $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 @@ protected function doParse() {
 
                             // 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;

test/code/parser/errorHandling/recovery.test

@@ -1490,9 +1490,6 @@ array(
                         )
                     )
                 )
-                comments: array(
-                    0: /** @var ?string */
-                )
             )
         )
     )

test/code/parser/stmt/class/class_position.test

@@ -2,63 +2,91 @@ 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(
         )
         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(
-                )
-            )
-        )
-        finally: null
+        elseifs: array(
+        )
+        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(
         )