Use visitor to assign comments

This fixes the long-standing issue where a comment would get assigned
to all nodes with the same starting position, instead of only the
outer-most one.

Fixes #253.
This commit is contained in:
Nikita Popov 2022-06-06 16:32:48 +02:00
parent de84f76766
commit 4e27a17cd8
23 changed files with 97 additions and 371 deletions

View File

@ -0,0 +1,82 @@
<?php declare(strict_types=1);
namespace PhpParser\NodeVisitor;
use PhpParser\Comment;
use PhpParser\Node;
use PhpParser\NodeVisitorAbstract;
use PhpParser\Token;
class CommentAnnotatingVisitor extends NodeVisitorAbstract {
/** @var int Last seen token start position */
private int $pos = 0;
/** @var Token[] Token array */
private array $tokens;
/** @var list<int> Token positions of comments */
private array $commentPositions = [];
/**
* Create a comment annotation visitor.
*
* @param Token[] $tokens Token array
*/
public function __construct(array $tokens) {
$this->tokens = $tokens;
// Collect positions of comments. We use this to avoid traversing parts of the AST where
// there are no comments.
foreach ($tokens as $i => $token) {
if ($token->id === \T_COMMENT || $token->id === \T_DOC_COMMENT) {
$this->commentPositions[] = $i;
}
}
}
public function enterNode(Node $node) {
$nextCommentPos = current($this->commentPositions);
if ($nextCommentPos === false) {
// No more comments.
return self::STOP_TRAVERSAL;
}
$oldPos = $this->pos;
$this->pos = $pos = $node->getStartTokenPos();
if ($nextCommentPos > $oldPos && $nextCommentPos < $pos) {
$comments = [];
while (--$pos >= $oldPos) {
$token = $this->tokens[$pos];
if ($token->id === \T_DOC_COMMENT) {
$comments[] = new Comment\Doc(
$token->text, $token->line, $token->pos, $pos,
$token->getEndLine(), $token->getEndPos() - 1, $pos);
continue;
}
if ($token->id === \T_COMMENT) {
$comments[] = new Comment(
$token->text, $token->line, $token->pos, $pos,
$token->getEndLine(), $token->getEndPos() - 1, $pos);
continue;
}
if ($token->id !== \T_WHITESPACE) {
break;
}
}
if (!empty($comments)) {
$node->setAttribute('comments', array_reverse($comments));
}
do {
$nextCommentPos = next($this->commentPositions);
} while ($nextCommentPos !== false && $nextCommentPos < $this->pos);
}
$endPos = $node->getEndTokenPos();
if ($nextCommentPos > $endPos) {
// Skip children if there are no comments located inside this node.
$this->pos = $endPos;
return self::DONT_TRAVERSE_CHILDREN;
}
return null;
}
}

View File

@ -30,6 +30,7 @@ use PhpParser\Node\Stmt\Nop;
use PhpParser\Node\Stmt\Property;
use PhpParser\Node\Stmt\TryCatch;
use PhpParser\Node\UseItem;
use PhpParser\NodeVisitor\CommentAnnotatingVisitor;
abstract class ParserAbstract implements Parser {
private const SYMBOL_NONE = -1;
@ -201,6 +202,11 @@ abstract class ParserAbstract implements Parser {
$this->semValue = null;
$this->createdArrays = null;
if ($result !== null) {
$traverser = new NodeTraverser(new CommentAnnotatingVisitor($this->tokens));
$traverser->traverse($result);
}
return $result;
}
@ -473,7 +479,7 @@ abstract class ParserAbstract implements Parser {
protected function getAttributes(int $tokenStartPos, int $tokenEndPos): array {
$startToken = $this->tokens[$tokenStartPos];
$afterEndToken = $this->tokens[$tokenEndPos + 1];
$attributes = [
return [
'startLine' => $startToken->line,
'startTokenPos' => $tokenStartPos,
'startFilePos' => $startToken->pos,
@ -481,11 +487,6 @@ abstract class ParserAbstract implements Parser {
'endTokenPos' => $tokenEndPos,
'endFilePos' => $afterEndToken->pos - 1,
];
$comments = $this->getCommentsBeforeToken($tokenStartPos);
if (!empty($comments)) {
$attributes['comments'] = $comments;
}
return $attributes;
}
/**
@ -500,7 +501,7 @@ abstract class ParserAbstract implements Parser {
// Get attributes for the sentinel token.
$token = $this->tokens[$tokenPos];
$attributes = [
return [
'startLine' => $token->line,
'startTokenPos' => $tokenPos,
'startFilePos' => $token->pos,
@ -508,11 +509,6 @@ abstract class ParserAbstract implements Parser {
'endTokenPos' => $tokenPos,
'endFilePos' => $token->pos,
];
$comments = $this->getCommentsBeforeToken($tokenPos);
if (!empty($comments)) {
$attributes['comments'] = $comments;
}
return $attributes;
}
/*
@ -902,12 +898,9 @@ abstract class ParserAbstract implements Parser {
}
/**
* Get comments before the given token position.
*
* @return Comment[] Comments
* Get last comment before the given token position, if any
*/
protected function getCommentsBeforeToken(int $tokenPos): array {
$comments = [];
protected function getCommentBeforeToken(int $tokenPos): ?Comment {
while (--$tokenPos >= 0) {
$token = $this->tokens[$tokenPos];
if (!isset($this->dropTokens[$token->id])) {
@ -915,22 +908,21 @@ abstract class ParserAbstract implements Parser {
}
if ($token->id === \T_COMMENT || $token->id === \T_DOC_COMMENT) {
$comments[] = $this->createCommentFromToken($token, $tokenPos);
return $this->createCommentFromToken($token, $tokenPos);
}
}
return \array_reverse($comments);
return null;
}
/**
* Create a zero-length nop to capture preceding comments, if any.
*/
protected function maybeCreateZeroLengthNop(int $tokenPos): ?Nop {
$comments = $this->getCommentsBeforeToken($tokenPos);
if (empty($comments)) {
$comment = $this->getCommentBeforeToken($tokenPos);
if ($comment === null) {
return null;
}
$comment = $comments[\count($comments) - 1];
$commentEndLine = $comment->getEndLine();
$commentEndFilePos = $comment->getEndFilePos();
$commentEndTokenPos = $comment->getEndTokenPos();
@ -941,14 +933,12 @@ abstract class ParserAbstract implements Parser {
'endFilePos' => $commentEndFilePos,
'startTokenPos' => $commentEndTokenPos + 1,
'endTokenPos' => $commentEndTokenPos,
'comments' => $comments,
];
return new Nop($attributes);
}
protected function maybeCreateNop(int $tokenStartPos, int $tokenEndPos): ?Nop {
$comments = $this->getCommentsBeforeToken($tokenStartPos);
if (empty($comments)) {
if ($this->getCommentBeforeToken($tokenStartPos) === null) {
return null;
}
return new Nop($this->getAttributes($tokenStartPos, $tokenEndPos));

View File

@ -22,9 +22,6 @@ array(
0: Stmt_Expression(
expr: Expr_Variable(
name: a
comments: array(
0: // baz
)
)
comments: array(
0: // baz

View File

@ -24,12 +24,6 @@ array(
0: Stmt_Expression(
expr: Expr_Variable(
name: var
comments: array(
0: /** doc 1 */
1: /* foobar 1 */
2: // foo 1
3: // bar 1
)
)
comments: array(
0: /** doc 1 */

View File

@ -116,9 +116,6 @@ array(
expr: Expr_Array(
items: array(
)
comments: array(
0: // short array syntax
)
)
comments: array(
0: // short array syntax

View File

@ -41,16 +41,10 @@ array(
expr: Expr_Assign(
var: Expr_Variable(
name: a
comments: array(
0: // simple assign
)
)
expr: Expr_Variable(
name: b
)
comments: array(
0: // simple assign
)
)
comments: array(
0: // simple assign
@ -60,16 +54,10 @@ array(
expr: Expr_AssignOp_BitwiseAnd(
var: Expr_Variable(
name: a
comments: array(
0: // combined assign
)
)
expr: Expr_Variable(
name: b
)
comments: array(
0: // combined assign
)
)
comments: array(
0: // combined assign
@ -199,9 +187,6 @@ array(
expr: Expr_Assign(
var: Expr_Variable(
name: a
comments: array(
0: // chained assign
)
)
expr: Expr_AssignOp_Mul(
var: Expr_Variable(
@ -216,9 +201,6 @@ array(
)
)
)
comments: array(
0: // chained assign
)
)
comments: array(
0: // chained assign
@ -228,16 +210,10 @@ array(
expr: Expr_AssignRef(
var: Expr_Variable(
name: a
comments: array(
0: // by ref assign
)
)
expr: Expr_Variable(
name: b
)
comments: array(
0: // by ref assign
)
)
comments: array(
0: // by ref assign
@ -256,16 +232,10 @@ array(
unpack: false
)
)
comments: array(
0: // list() assign
)
)
expr: Expr_Variable(
name: b
)
comments: array(
0: // list() assign
)
)
comments: array(
0: // list() assign
@ -349,9 +319,6 @@ array(
var: Expr_Variable(
name: a
)
comments: array(
0: // inc/dec
)
)
comments: array(
0: // inc/dec

View File

@ -17,9 +17,6 @@ array(
name: b
)
)
comments: array(
0: // This is legal.
)
)
comments: array(
0: // This is legal.
@ -37,9 +34,6 @@ array(
)
)
)
comments: array(
0: // This is illegal, but not a syntax error.
)
)
comments: array(
0: // This is illegal, but not a syntax error.

View File

@ -29,16 +29,10 @@ array(
unpack: false
)
)
comments: array(
0: // This is legal.
)
)
expr: Expr_Variable(
name: x
)
comments: array(
0: // This is legal.
)
)
comments: array(
0: // This is legal.
@ -62,16 +56,10 @@ array(
unpack: false
)
)
comments: array(
0: // This is illegal, but not a syntax error.
)
)
expr: Expr_Variable(
name: x
)
comments: array(
0: // This is illegal, but not a syntax error.
)
)
comments: array(
0: // This is illegal, but not a syntax error.

View File

@ -20,15 +20,9 @@ array(
expr: Expr_FuncCall(
name: Name(
name: a
comments: array(
0: // function name variations
)
)
args: array(
)
comments: array(
0: // function name variations
)
)
comments: array(
0: // function name variations
@ -130,22 +124,13 @@ array(
var: Expr_FuncCall(
name: Name(
name: a
comments: array(
0: // array dereferencing
)
)
args: array(
)
comments: array(
0: // array dereferencing
)
)
dim: Scalar_String(
value: b
)
comments: array(
0: // array dereferencing
)
)
comments: array(
0: // array dereferencing

View File

@ -22,16 +22,10 @@ array(
expr: Expr_PropertyFetch(
var: Expr_Variable(
name: a
comments: array(
0: // property fetch variations
)
)
name: Identifier(
name: b
)
comments: array(
0: // property fetch variations
)
)
comments: array(
0: // property fetch variations
@ -71,18 +65,12 @@ array(
expr: Expr_MethodCall(
var: Expr_Variable(
name: a
comments: array(
0: // method call variations
)
)
name: Identifier(
name: b
)
args: array(
)
comments: array(
0: // method call variations
)
)
comments: array(
0: // method call variations
@ -136,25 +124,16 @@ array(
var: Expr_MethodCall(
var: Expr_Variable(
name: a
comments: array(
0: // array dereferencing
)
)
name: Identifier(
name: b
)
args: array(
)
comments: array(
0: // array dereferencing
)
)
dim: Scalar_String(
value: c
)
comments: array(
0: // array dereferencing
)
)
comments: array(
0: // array dereferencing

View File

@ -23,18 +23,12 @@ array(
expr: Expr_StaticCall(
class: Name(
name: A
comments: array(
0: // method name variations
)
)
name: Identifier(
name: b
)
args: array(
)
comments: array(
0: // method name variations
)
)
comments: array(
0: // method name variations
@ -112,25 +106,16 @@ array(
var: Expr_StaticCall(
class: Name(
name: A
comments: array(
0: // array dereferencing
)
)
name: Identifier(
name: b
)
args: array(
)
comments: array(
0: // array dereferencing
)
)
dim: Scalar_String(
value: c
)
comments: array(
0: // array dereferencing
)
)
comments: array(
0: // array dereferencing
@ -140,18 +125,12 @@ array(
expr: Expr_StaticCall(
class: Name(
name: static
comments: array(
0: // class name variations
)
)
name: Identifier(
name: b
)
args: array(
)
comments: array(
0: // class name variations
)
)
comments: array(
0: // class name variations

View File

@ -18,16 +18,10 @@ array(
expr: Expr_StaticPropertyFetch(
class: Name(
name: A
comments: array(
0: // property name variations
)
)
name: VarLikeIdentifier(
name: b
)
comments: array(
0: // property name variations
)
)
comments: array(
0: // property name variations
@ -58,23 +52,14 @@ array(
var: Expr_StaticPropertyFetch(
class: Name(
name: A
comments: array(
0: // array access
)
)
name: VarLikeIdentifier(
name: b
)
comments: array(
0: // array access
)
)
dim: Scalar_String(
value: c
)
comments: array(
0: // array access
)
)
comments: array(
0: // array access

View File

@ -61,9 +61,6 @@ array(
0: VariadicPlaceholder(
)
)
comments: array(
0: // These are invalid, but accepted on the parser level.
)
)
comments: array(
0: // These are invalid, but accepted on the parser level.

View File

@ -25,16 +25,10 @@ array(
expr: Expr_BinaryOp_BooleanAnd(
left: Expr_Variable(
name: a
comments: array(
0: // boolean ops
)
)
right: Expr_Variable(
name: b
)
comments: array(
0: // boolean ops
)
)
comments: array(
0: // boolean ops
@ -70,16 +64,10 @@ array(
expr: Expr_BinaryOp_LogicalAnd(
left: Expr_Variable(
name: a
comments: array(
0: // logical ops
)
)
right: Expr_Variable(
name: b
)
comments: array(
0: // logical ops
)
)
comments: array(
0: // logical ops
@ -110,16 +98,10 @@ array(
left: Expr_BinaryOp_BooleanAnd(
left: Expr_Variable(
name: a
comments: array(
0: // precedence
)
)
right: Expr_Variable(
name: b
)
comments: array(
0: // precedence
)
)
right: Expr_BinaryOp_BooleanAnd(
left: Expr_Variable(
@ -129,9 +111,6 @@ array(
name: d
)
)
comments: array(
0: // precedence
)
)
comments: array(
0: // precedence

View File

@ -63,9 +63,6 @@ array(
conds: array(
0: Scalar_Int(
value: 0
comments: array(
0: // list of conditions
)
)
1: Scalar_Int(
value: 1

View File

@ -39,9 +39,6 @@ array(
expr: Expr_Variable(
name: a
)
comments: array(
0: // unary ops
)
)
comments: array(
0: // unary ops
@ -65,16 +62,10 @@ array(
expr: Expr_BinaryOp_BitwiseAnd(
left: Expr_Variable(
name: a
comments: array(
0: // binary ops
)
)
right: Expr_Variable(
name: b
)
comments: array(
0: // binary ops
)
)
comments: array(
0: // binary ops
@ -195,23 +186,14 @@ array(
left: Expr_BinaryOp_Mul(
left: Expr_Variable(
name: a
comments: array(
0: // associativity
)
)
right: Expr_Variable(
name: b
)
comments: array(
0: // associativity
)
)
right: Expr_Variable(
name: c
)
comments: array(
0: // associativity
)
)
comments: array(
0: // associativity
@ -236,9 +218,6 @@ array(
expr: Expr_BinaryOp_Plus(
left: Expr_Variable(
name: a
comments: array(
0: // precedence
)
)
right: Expr_BinaryOp_Mul(
left: Expr_Variable(
@ -248,9 +227,6 @@ array(
name: c
)
)
comments: array(
0: // precedence
)
)
comments: array(
0: // precedence
@ -275,9 +251,6 @@ array(
expr: Expr_BinaryOp_Pow(
left: Expr_Variable(
name: a
comments: array(
0: // pow is special
)
)
right: Expr_BinaryOp_Pow(
left: Expr_Variable(
@ -287,9 +260,6 @@ array(
name: c
)
)
comments: array(
0: // pow is special
)
)
comments: array(
0: // pow is special

View File

@ -52,9 +52,6 @@ array(
)
args: array(
)
comments: array(
0: // class name variations
)
)
comments: array(
0: // class name variations
@ -100,9 +97,6 @@ array(
)
args: array(
)
comments: array(
0: // DNCR object access
)
)
comments: array(
0: // DNCR object access

View File

@ -21,9 +21,6 @@ array(
expr: Expr_Ternary(
cond: Expr_Variable(
name: a
comments: array(
0: // ternary
)
)
if: Expr_Variable(
name: b
@ -31,9 +28,6 @@ array(
else: Expr_Variable(
name: c
)
comments: array(
0: // ternary
)
)
comments: array(
0: // ternary
@ -55,9 +49,6 @@ array(
cond: Expr_Ternary(
cond: Expr_Variable(
name: a
comments: array(
0: // precedence
)
)
if: Expr_Variable(
name: b
@ -65,9 +56,6 @@ array(
else: Expr_Variable(
name: c
)
comments: array(
0: // precedence
)
)
if: Expr_Variable(
name: d
@ -75,9 +63,6 @@ array(
else: Expr_Variable(
name: e
)
comments: array(
0: // precedence
)
)
comments: array(
0: // precedence
@ -108,16 +93,10 @@ array(
expr: Expr_BinaryOp_Coalesce(
left: Expr_Variable(
name: a
comments: array(
0: // null coalesce
)
)
right: Expr_Variable(
name: b
)
comments: array(
0: // null coalesce
)
)
comments: array(
0: // null coalesce

View File

@ -37,9 +37,6 @@ array(
0: Stmt_Expression(
expr: Scalar_String(
value:
comments: array(
0: // empty strings
)
)
comments: array(
0: // empty strings
@ -53,9 +50,6 @@ array(
2: Stmt_Expression(
expr: Scalar_String(
value: Test '" $a \n
comments: array(
0: // constant encapsed strings
)
)
comments: array(
0: // constant encapsed strings
@ -77,9 +71,6 @@ array(
name: a
)
)
comments: array(
0: // encapsed strings
)
)
comments: array(
0: // encapsed strings

View File

@ -76,10 +76,6 @@ array(
10: Stmt_Expression(
expr: Scalar_Float(
value: 1.844674407371E+19
comments: array(
0: // various integer -> float overflows
1: // (all are actually the same number, just in different representations)
)
)
comments: array(
0: // various integer -> float overflows

View File

@ -59,12 +59,6 @@ array(
expr: Expr_ConstFetch(
name: Name(
name: _100
comments: array(
0: // already a valid constant name
)
)
comments: array(
0: // already a valid constant name
)
)
comments: array(
@ -74,9 +68,6 @@ array(
6: Stmt_Expression(
expr: Scalar_Int(
value: 100
comments: array(
0: // syntax errors
)
)
comments: array(
0: // syntax errors

View File

@ -83,9 +83,6 @@ array(
flags: 0
type: Name(
name: iterable
comments: array(
0: // PHP 7.0
)
)
byRef: false
variadic: false
@ -103,9 +100,6 @@ array(
flags: 0
type: Name(
name: object
comments: array(
0: // PHP 7.1
)
)
byRef: false
variadic: false
@ -123,9 +117,6 @@ array(
flags: 0
type: Name(
name: mixed
comments: array(
0: // PHP 7.2
)
)
byRef: false
variadic: false
@ -143,9 +134,6 @@ array(
flags: 0
type: Name(
name: null
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false
@ -163,9 +151,6 @@ array(
flags: 0
type: Name(
name: false
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false
@ -292,9 +277,6 @@ array(
flags: 0
type: Name(
name: iterable
comments: array(
0: // PHP 7.0
)
)
byRef: false
variadic: false
@ -312,9 +294,6 @@ array(
flags: 0
type: Name(
name: object
comments: array(
0: // PHP 7.1
)
)
byRef: false
variadic: false
@ -332,9 +311,6 @@ array(
flags: 0
type: Name(
name: mixed
comments: array(
0: // PHP 7.2
)
)
byRef: false
variadic: false
@ -352,9 +328,6 @@ array(
flags: 0
type: Name(
name: null
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false
@ -372,9 +345,6 @@ array(
flags: 0
type: Name(
name: false
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false
@ -501,9 +471,6 @@ array(
flags: 0
type: Identifier(
name: iterable
comments: array(
0: // PHP 7.0
)
)
byRef: false
variadic: false
@ -521,9 +488,6 @@ array(
flags: 0
type: Name(
name: object
comments: array(
0: // PHP 7.1
)
)
byRef: false
variadic: false
@ -541,9 +505,6 @@ array(
flags: 0
type: Name(
name: mixed
comments: array(
0: // PHP 7.2
)
)
byRef: false
variadic: false
@ -561,9 +522,6 @@ array(
flags: 0
type: Name(
name: null
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false
@ -581,9 +539,6 @@ array(
flags: 0
type: Name(
name: false
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false
@ -710,9 +665,6 @@ array(
flags: 0
type: Identifier(
name: iterable
comments: array(
0: // PHP 7.0
)
)
byRef: false
variadic: false
@ -730,9 +682,6 @@ array(
flags: 0
type: Identifier(
name: object
comments: array(
0: // PHP 7.1
)
)
byRef: false
variadic: false
@ -750,9 +699,6 @@ array(
flags: 0
type: Name(
name: mixed
comments: array(
0: // PHP 7.2
)
)
byRef: false
variadic: false
@ -770,9 +716,6 @@ array(
flags: 0
type: Name(
name: null
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false
@ -790,9 +733,6 @@ array(
flags: 0
type: Name(
name: false
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false
@ -919,9 +859,6 @@ array(
flags: 0
type: Identifier(
name: iterable
comments: array(
0: // PHP 7.0
)
)
byRef: false
variadic: false
@ -939,9 +876,6 @@ array(
flags: 0
type: Identifier(
name: object
comments: array(
0: // PHP 7.1
)
)
byRef: false
variadic: false
@ -959,9 +893,6 @@ array(
flags: 0
type: Identifier(
name: mixed
comments: array(
0: // PHP 7.2
)
)
byRef: false
variadic: false
@ -979,9 +910,6 @@ array(
flags: 0
type: Identifier(
name: null
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false
@ -999,9 +927,6 @@ array(
flags: 0
type: Identifier(
name: false
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false
@ -1128,9 +1053,6 @@ array(
flags: 0
type: Identifier(
name: iterable
comments: array(
0: // PHP 7.0
)
)
byRef: false
variadic: false
@ -1148,9 +1070,6 @@ array(
flags: 0
type: Identifier(
name: object
comments: array(
0: // PHP 7.1
)
)
byRef: false
variadic: false
@ -1168,9 +1087,6 @@ array(
flags: 0
type: Identifier(
name: mixed
comments: array(
0: // PHP 7.2
)
)
byRef: false
variadic: false
@ -1188,9 +1104,6 @@ array(
flags: 0
type: Identifier(
name: null
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false
@ -1208,9 +1121,6 @@ array(
flags: 0
type: Identifier(
name: false
comments: array(
0: // PHP 8.0
)
)
byRef: false
variadic: false

View File

@ -47,9 +47,6 @@ array(
expr: Expr_Yield(
key: null
value: null
comments: array(
0: // statements
)
)
comments: array(
0: // statements
@ -77,17 +74,11 @@ array(
expr: Expr_Assign(
var: Expr_Variable(
name: data
comments: array(
0: // expressions
)
)
expr: Expr_Yield(
key: null
value: null
)
comments: array(
0: // expressions
)
)
comments: array(
0: // expressions
@ -214,9 +205,6 @@ array(
expr: Expr_FuncCall(
name: Name(
name: func
comments: array(
0: // yield in function calls
)
)
args: array(
0: Arg(
@ -231,9 +219,6 @@ array(
unpack: false
)
)
comments: array(
0: // yield in function calls
)
)
comments: array(
0: // yield in function calls