Incorrect start line for trait that has a conditional class_alias in the same file

[mglaman]

I have been tracking a weird bug in phpstan-drupal where PHPStan cannot discover a trait provided by Drupal. There is a PHPUnit compatibility trait used in Drupal's PHPUnit tests. It uses a dynamic alias to support both PHPUnit 6 and 7.

This is the trait in question: https://git.drupalcode.org/project/drupal/-/blob/8.8.x/core/tests/Drupal/Tests/PhpunitCompatibilityTrait.php.

<?php

namespace Drupal\Tests;

use Drupal\TestTools\PhpUnitCompatibility\RunnerVersion;

// In order to manage different method signatures between PHPUnit versions, we
// dynamically load a compatibility trait dependent on the PHPUnit runner
// version.
if (!trait_exists(PhpunitVersionDependentTestCompatibilityTrait::class, FALSE)) {
  class_alias("Drupal\TestTools\PhpUnitCompatibility\PhpUnit" . RunnerVersion::getMajor() . "\TestCompatibilityTrait", PhpunitVersionDependentTestCompatibilityTrait::class);
}

/**
 * Makes Drupal's test API forward compatible with multiple versions of PHPUnit.
 */
trait PhpunitCompatibilityTrait {

  use PhpunitVersionDependentTestCompatibilityTrait;

PHP's internal reflection says the start line is on line 17. However, nikic/php-parser says it is on line 10 – the same line as the if with a class_exists check.

Here is a screenshot from an Xdebug session after the library parsed the file

This worked without issue in v4.9.1 but broke with v4.10.0. I haven't started to dig in entirely.

[mglaman]

I went into it a bit more. The $stackPos for the trait is 8. And it gets set correctly in the startAttributeStack but for some reason, after it is parsed it has the attributes from the if statement.

[nikic]

Probably related to empty optional attributes before the trait.

[mglaman]

@nikic would that roughly refer to this part of the lexer:

            // Emulate PHP 8 T_NAME_* tokens, by combining sequences of T_NS_SEPARATOR and T_STRING
            // into a single token.
            if (\is_array($token)
                    && ($token[0] === \T_NS_SEPARATOR || isset($this->identifierTokens[$token[0]]))) {
                $lastWasSeparator = $token[0] === \T_NS_SEPARATOR;

Because that seemed to be where things went awry, but I wasn't sure.

[mglaman]

I've added a test in #739.

[mglaman]

I wasn't able to dig in much further, but realized I should breakpoint in the Trait_ constructor which let me see it was in this callback

            201 => function ($stackPos) {
                 $this->semValue = new Stmt\Trait_($this->semStack[$stackPos-(6-3)], ['stmts' => $this->semStack[$stackPos-(6-5)], 'attrGroups' => $this->semStack[$stackPos-(6-1)]], $this->startAttributeStack[$stackPos-(6-1)] + $this->endAttributes);
            },

[mglaman]

With: $stackPos = 7

$this->startAttributeStack[$stackPos-(6-1)]

This returns 2 when it should be 3.

Manually changing the file to the following gets a pass:

$this->startAttributeStack[$stackPos-(6-2)]

[mglaman]

I've tried to discern this:

    | optional_attributes T_TRAIT identifier '{' class_statement_list '}'
          { $$ = Stmt\Trait_[$3, ['stmts' => $5, 'attrGroups' => $1]]; }

but I have no idea how to influence $this->startAttributeStack[$stackPos-(6-1)]

[nikic]

This was pretty tricky... I'm not completely confident in my fix. Let's see if there will be any regressions in other position information :)

[ondrejmirtes]

@mglaman FYI you can play with phpstan/phpstan dev-master as I just upgraded to PHP-Parser with the fix :)

[mglaman]

😱 @nikic thanks!

@ondrejmirtes I will!

Seriously, thank you. I'm going to try this today.

[mglaman]

🥳 The build passes with phpstan:dev-master mglaman/phpstan-drupal#149

Thank you both!

[TomasVotruba]

@mglaman Thank you for reporting and investigation. We had this bug in @Rector for weeks rectorphp/rector#4556 I assumed it's somewhere in our side, so it's feels good to see we're not the only ones :) thank you!

@nikic Thank you for the fix and fast release 👍