phpDocumentor / Reflection

Reflection library to do Static Analysis for PHP Projects
MIT License
117 stars 51 forks source link

Return tag with constant wildcard list parsed into InvalidTag instead of Return_ #261

Open andrew-demb opened 2 years ago

andrew-demb commented 2 years ago

Documentation for this syntax: https://psalm.dev/docs/annotating_code/typing_in_psalm/#specifying-stringint-options-aka-enums

Example code:

class Some
{
    public const STATUS_FOO = 'foo';
    public const STATUS_BAR = 'bar';

    /**
     * @return self::STATUS_*
     */
    public function dummy(): string
    {
        return self::STATUS_FOO;
    }
}

Test:

<?php

declare(strict_types=1);

use phpDocumentor\Reflection\DocBlock\Tags\Return_;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Cache\Adapter\ArrayAdapter;
use Symfony\Component\Cache\Psr16Cache;
use TheCodingMachine\GraphQLite\Reflection\CachedDocBlockFactory;

final class GithubTest extends TestCase
{
    public function testGithub(): void
    {
        $cachedDocBlockFactory = new CachedDocBlockFactory(new Psr16Cache(new ArrayAdapter()));

        $refMethod = new \ReflectionMethod($this, 'dummy');
        $docBlockObj = $cachedDocBlockFactory->getDocBlock($refMethod);

        $returnTag = $docBlockObj->getTagsByName('return')[0] ?? null;

        $this->assertInstanceOf(Return_::class, $returnTag);
    }

    public const STATUS_FOO = 'foo';
    public const STATUS_BAR = 'bar';

    /**
     * @return self::STATUS_*
     */
    public function dummy(): string
    {
        return self::STATUS_FOO;
    }
}

Current output:

Failed asserting that phpDocumentor\Reflection\DocBlock\Tags\InvalidTag Object (...) is an instance of class "phpDocumentor\Reflection\DocBlock\Tags\Return_".
 /.../tests/GithubTest.php:24
jaapio commented 2 years ago

hm, our type resolver doesn't has support for this type of notations. And it would be hard to build in. In your example the self::STATUS_* resolves into the correct class, so that would be possible. But if it would refer to something else, like My\Namespaced\Class_ we would need some kind of autoloading or discovery after we parsed the file. This will have an impact on the way our parser works as we are parsing per file.

The best I could think of is that we would introduce some new type. that needs to be resolved in some post processing. The reason why our libraries are fast, is because there are no relations between files and classes. So if you ask for the docblock of a class / method / property we will just process that particular file.

An UnresolvedType would allow us to detect these types while processing the Typed tags. But it will be up to the consumer to find the correct referenced type. And that might not be easy. Psalm and other analyzers are loading the entire project, then do post-processing and other magic to detect errors. As those are applications it is possible for them to build this. As this is a library, it will be way harder to solve this issue.

My intuition says I won't be able to build such a feature in the short term. If it would ever be built in this or any of our libs.

jaapio commented 6 months ago

I added support for contant references to the docblock reflection component of phpDocumentor. This feature will be part of a future release. It gives a ConstExpression object containing the expression STATUS_* and the referencing class / enum. as a type.

From there people using this project should be able to find the correct constants. however this will not be very straight forward. Extra documentation will be needed to make this use-case more clear.