Start line of class is reported to be start line of attribute when class has attribute

[sebastianbergmann]

I ran into the following situation while implementing support for PHPUnit 10's PHPUnit\Framework\Attributes\CodeCoverageIgnore attribute to phpunit/php-code-coverage (which uses PHP-Parser for static analysis):

Reproducing example

<?php declare(strict_types=1);
use PhpParser\Lexer;
use PhpParser\Node;
use PhpParser\Node\Stmt\Class_;
use PhpParser\NodeTraverser;
use PhpParser\NodeVisitor\NameResolver;
use PhpParser\NodeVisitor\ParentConnectingVisitor;
use PhpParser\NodeVisitorAbstract;
use PhpParser\ParserFactory;

require __DIR__ . '/vendor/autoload.php';

$classWithoutAttribute = <<<EOT
<?php declare(strict_types=1);
final class Foo
{
}
EOT;

$classWithAttribute = <<<EOT
<?php declare(strict_types=1);
#[Bar]
final class Foo
{
}
EOT;

$parser = (new ParserFactory)->create(
    ParserFactory::PREFER_PHP7,
    new Lexer
);

$traverser = new NodeTraverser;

$traverser->addVisitor(new NameResolver);
$traverser->addVisitor(new ParentConnectingVisitor);

$traverser->addVisitor(
    new class extends NodeVisitorAbstract {
        public function enterNode(Node $node): void
        {
            if ($node instanceof Class_) {
                var_dump($node->getStartLine());
                var_dump($node->getEndLine());
            }
        }
    }
);

$traverser->traverse($parser->parse($classWithoutAttribute));
$traverser->traverse($parser->parse($classWithAttribute));

Actual output

int(2)
int(4)
int(2)
int(5)

Expected output

int(2)
int(4)
int(3)
int(5)

The fact that PHP-Parser considers the line with #[Bar] to be the starting line of the class declaration of Foo is, at least to me, unexpected.

I am not sure whether this is the correct behaviour or if I stumbled upon a bug.

[ondrejmirtes]

Probably connected to #762

the attributes are considered part of the class

[sebastianbergmann]

Probably connected to #762

Sounds reasonable (that it's connected to #762). I now worked around this like so: https://github.com/sebastianbergmann/php-code-coverage/blob/9.2/src/StaticAnalysis/IgnoredLinesFindingVisitor.php#L75

[WinterSilence]

@sebastianbergmann

PHP-Parser/lib/PhpParser/ParserFactory.php

Line 20 in b4b60c8

* @deprecated Use createForVersion(), createForNewestSupportedVersion() or createForHostVersion() instead.

[rulatir]

Just to be sure - if unmitigated, this completely destroys the format-preserving pretty printer for affected code, right?

[nikic]

@rulatir It should work fine. In fact, the current behavior of considering attributes part of the class is a requirement for making formatting preservation work. Are you seeing any issues with this?

[rulatir]

No, I am diving into using this library for the first time in ~4y, so I took a look at the open issues list and it just happens that the 2nd from the top seems to affect my use case. Glad the format preserving prettyprint can be hoped to work.

[nikic]

This is working as intended as far as I'm concerned. Taking the line of the name seems like a reasonably low effort workaround if you don't want to count the attributes.