Classes referenced in PHP doc are reported unused

[ruudk]

Given you have the following code:

final readonly class MyValue
{
    public function __construct(
        public string $value,
    ) {}
}

final readonly class MyResponse
{
    /**
     * @param list<MyValue> $values
     */
    public function __construct(
        public array $values,
    ) {}
}

It results in MyValue being reported as unused.

This is because class-leak does not check for PHPDocs.

I would like to enhance class-leak so that it is able to do this. But manually parsing the PHPDocs feels a bit contra productive to me.

@TomasVotruba Given your experience with this problem in Rector, you probably have an idea how to achieve it. Could you point me in the right direction so that I can give it a go?

[ruudk]

@samsonasik You might also know something about this 😁

[samsonasik]

@ruudk it may need a NameScope usage like in rector, and decorate it, something like https://github.com/rectorphp/rector-src/pull/5657

[TomasVotruba]

Indeed, this would require something like phpdoc-parser or PHPStan internals. Not sure if worth it :)

Where is the MyValue created?

[ruudk]

Where is the MyValue created?

But us. When create these objects when decoding JSON objects to have type safety. We use Symfony Serializer for this. Works great.

But now they are detected as dead.

[TomasVotruba]

I see, we use serializer as well, but we use at least single place with real type that skips these.

I don't have time and resources to handle this, but if you're bale to add such a support, please send a PR :+1:

It could be also simple way to check for a serialized type, then try to find all use statements in it and mark them as used.

[ruudk]

I'm going to have a look at phpdoc-parser and see if we can have some basic functionality that is able to find used classes in arrays and lists.

[staabm]

we have a similar problem for classes only referenced via magic @property above classes:

/**
 * @property ClassB[]                   $ClassB
 */
class ClassA
{
}

[TomasVotruba]

I forgot to close this one 🤦‍♂️

Phpdoc won't be supported, see https://github.com/TomasVotruba/class-leak/pull/38#issuecomment-2091238224

[staabm]

I think phpdoc based typing is something really required nowadays (the language itself is not good enough yet) and leaving the developer in juding about all the false-positives is a waste of resources.

@ruudk we could maintain a fork if you agree

[ruudk]

I was thinking the same. Without support for PHPDoc I can't really use class-leak. It would be so good if this thing could run in CI and warn whenever something becomes obsolete (dead). But having false positives here and there will make my team not happy 😊

Before we fork, It would be good to know if @TomasVotruba can reconsider.

[TomasVotruba]

I think those cases where class is located in a docblock only an nowhere in the code should be refactored into more explicit ones. Once docblock are seen as a valid PHP code, the codebase looses it's reliablity and trustworthy. It's like allowing Rector to add types based on docblocks - it broke more code than it helped :)

I've been using this package on many legacy projects and haven't found single false positive due to docblocks that would be valid.

Despite that, feel free to maintain a fork :+1:

[staabm]

I've been using this package on many legacy projects and haven't found single false positive due to docblocks that would be valid.

then your code base does not make use of useful features like generics, conditional return types etc. ;-).

we have several hundred false positives across our apps

[ruudk]

@staabm would be great if you could fork, I will contribute there.

[staabm]

@ruudk here we go: https://github.com/staabm/class-leak

but please small focused PRs. I don't want to review big overhauls and it would be great we could stay as compatible as possible with the upstream so we can easily contribute changes and also get back improvements into the fork without hassle

[ruudk]

I already did the work to get it working. Not going to split this up in smaller PR's because I think a change like this can't be done smaller, without knowing how the thing will look like. It was disappointing to see the PR got rejected so I won't invest more time in it other than doing a rebase if needed.