WIP: Fix phpstan issues

[agentrickard]

Description

Explain the technical implementation of the work done.

To Test

• Add steps to test this feature

Drupal.org issue

Provide a link to the issue from https://www.drupal.org/project/rector/issues. If no issue exists, please create one and link to this PR.

[agentrickard]

Remaining changes:

• https://github.com/search?q=repo%3Arectorphp%2Frector-src+PARENT_NODE&type=commits
• https://github.com/search?q=repo%3Arectorphp%2Frector-src+NEXT_NODE&type=commits
• https://github.com/search?q=repo%3Arectorphp%2Frector-src+findParentType()&type=commits

[bbrala]

Some of these will be... interesting.

Anyways; this might be relevant: https://phpstan.org/blog/preprocessing-ast-for-custom-rules

[agentrickard]

Some notes

• I added the neon rule suggested by the phpstan link
• The memory leak concerns seem trivial for our test needs -- we are also on PHPSTAN 1.0 -- so we can handle that in a followup.
• The PhpParser library is still using parent and next keys, so I just hardcoded those.

[agentrickard]

I simply don't follow how to replace these calls:

https://github.com/search?q=repo%3Arectorphp%2Frector-src+findParentType()&type=commits

[bbrala]

For LinkGeneratorTraitLRector:

    public function refactor(Node $node): ?Node
    {
        /** @var Node\Expr\MethodCall $node */
          if ($this->getName($node->name) === 'l') {
              $class = $this->betterNodeFinder->findParentType($node, Node\Stmt\Class_::class);

            // Check if class has LinkGeneratorTrait.
            if ($this->checkClassTypeHasTrait($class, 'Drupal\Core\Routing\LinkGeneratorTrait')) {
              $this->addDrupalRectorComment($node, 'Please manually remove the `use LinkGeneratorTrait;` statement from this class.');

The findParentType probably needs to use reflection on the method, then ask reflection the class and from there continue to find the traits.

Unfortunately there are no tests, and no example. I think this might be because it needed a stub.

[bbrala]

Same logic for DrupalSetMessageRector i think

[bbrala]

drupal_set_message it actually tested in the examples, so that might be easier.

Seems logic is actually a little different, it might need to change de Node\Expr to make sure you can get the class the expression lives in. Not completely sure without actually debugging the output :P

[agentrickard]

I took this as far as I could today. Here's the error on phpunit:

5) DrupalRector\Tests\Rector\Deprecation\DrupalSetMessageRector\DrupalSetMessageRectorTest::test with data set #4
assert($classReflection !== null)

/Users/rickard/Sites/drupal-rector/src/Utility/GetDeclaringSourceTrait.php:29
/Users/rickard/Sites/drupal-rector/src/Rector/Deprecation/DrupalSetMessageRector.php:81
/Users/rickard/Sites/drupal-rector/vendor/rector/rector-src/src/Rector/AbstractRector.php:201

This likely means that we are passing the wrong var to the method:

protected function getDeclaringSource(Node\Expr $expr): ?string

[bbrala]

Ok, comments thing broke because of this: https://github.com/rectorphp/rector/releases/tag/0.17.3

0.17.2 is not broken. (perhaps its time for some matrix tests on last x releases or something xD)

And i think because of this: rectorphp/rector-src#4463

Now it cannot resolve the parent we change in the comment. So basically, we wither need a personal base which loads that connector if possible, or we have to change the one using comments to always require the node that is worked on to be the Stmt. Then we can add comments.

So i guess, choose your poison ;)

Thats all the help i can give right now.

[mglaman]

One thing I discovered: the current way we attach comments, php-parser ends up treating them as $origComments and now the pretty printer breaks since there is no original start position or line.

                $comments = $arrItem->getComments();
                $origComments = $origArrItem->getComments();

The comments end up on $origArrItem when they should be attached our replacement.

[mglaman]

Adding context

• Inline comment belong to the next line nikic/PHP-Parser#791
• How to add inline comment? rectorphp/rector#7489

[mglaman]

Found more in php-parser PrettyPrinterAbstract:

                if ($comments !== $origComments) {
                    if ($comments) {
                        $result .= $this->pComments($comments) . $this->nl;
                    }

The current way comments are added, as previously stated have them treated as original comments. That means $comments is an empty array, so the printer acts as if comments were removed from the node.

[mglaman]

My thought was to:

• Have addDrupalRectorComment set an attribute on the node to add a comment, the attribute value is the comment.
• Have a PostRector which post-processed files to check each Node\Stmt\Expression if a child node has the attribute set and apply comments.

We cannot register PostRectors. They are hardcoded in PostFileProcessor.

I also tried changing a rule to listen for Node\Stmt\Expression::class and verify the expression type (like MethodCall.) Then we'd need to refactor how addDrupalRectorComment works or its callers.

[mglaman]

Here's an example with checking against the root rexpression: https://github.com/palantirnet/drupal-rector/compare/stan-fails...mglaman:drupal-rector:parent-traverse-fix?expand=1

[bbrala]

Wooo! Getting there <3

[bbrala]

I think we should close this in favor of #238 which is a cleaned up version with also less changes. :)