Wrong doc node type detection

[simonschaufi]

Bug Report

Subject	Details
Rector version	2.1.7

I have a strange scenario in TYPO3 Rector where I want to convert the @TYPO3\CMS\Extbase\Annotation\Validate into a php attribute: #[\TYPO3\CMS\Extbase\Annotation\Validate(['validator' => 'NotEmpty'])]. The migration works well if the validate annotation is the only one but once you add @var string to it, it doesn't work anymore. The issue is this one: sabbelasichon/typo3-rector/issues/4625

I've tried my best to find the exact location where the error is but I couldn't find it as there is so much going on in the PhpDocInfoFactory and especially the BetterPhpDocParser that I got lost on the way.

Minimal PHP Code Causing Issue

The following code leads to an invalid result:

<?php

class HelloWorld
{
    /**
     * @var string
     * @TYPO3\CMS\Extbase\Annotation\Validate("NotEmpty")
     */
    protected $hash = '';
}

results into (note the missing \ in the beginning):

<?php

class HelloWorld
{
    /**
     * @var string
     */
    #[TYPO3\CMS\Extbase\Annotation\Validate(['validator' => 'NotEmpty'])]
    protected $name = '';
}

This code works:

<?php

class HelloWorld
{
    /**
     * @TYPO3\CMS\Extbase\Annotation\Validate("NotEmpty")
     */
    protected $hash = '';
}

and results in:

<?php

class HelloWorld
{
    #[\TYPO3\CMS\Extbase\Annotation\Validate(['validator' => 'NotEmpty'])]
    protected $name = '';
}

Expected Behaviour

The applied rule https://github.com/sabbelasichon/typo3-rector/blob/main/rules/TYPO312/v0/ExtbaseAnnotationToAttributeRector.php does it's job correctly

The most important line in the rector rule is:

if (str_starts_with($doctrineTagValueNode->identifierTypeNode->name, '@TYPO3\CMS')) {

If that is true, the rest works. In the case of the additional @var string annotation, this is false.

[samsonasik]

If you read namespaced annotation under namespace, without \\ prefix, you will get current namespace + annotation name, if there is partial namespace in use statement, then it will be partial namespace + substr(annotation, partial).

If you want non prefixed detected as dedicated fqcn, I think you can replace Identifier FullyQualifiedIdentifierTypeNode, however, the risk will on yours, as it will possibly cause invalid assumption :)

https://github.com/rectorphp/rector-src/blob/d7c7d359cbaa87158507a59d4edf582f39f29c21/src/BetterPhpDocParser/ValueObject/Type/FullyQualifiedIdentifierTypeNode.php#L10

[simonschaufi]

I've done a bit more debugging because I couldn't reproduce this in my tests but only in my projects and then I realized that Rector\Renaming\Rector\Name\RenameClassRector is causing this issue! When I skip the rule, it works! So there must be something else going on here than just renaming classes! This is definitely a Rector bug!

[samsonasik]

We need failing test case then, you can create fixture for multi rules under tests/Issues

https://github.com/rectorphp/rector-src/tree/main/tests/Issues

[simonschaufi]

@samsonasik please reopen as there is a bug in Rector

[samsonasik]

You created custom rule, which may can cause the issue that may unrelated to internal rector code.

To prove that it is a rector bug, failing test case is needed.

Thank you.

[simonschaufi]

See rectorphp/rector-src/pull/7326

[samsonasik]

as I commented before, this is expected :), namespaced annotation under namespace, without \\ prefix, become namespace + annotation, add:

use TYPO;

and it will be correctly detected :)