Psalm: Feature request: classes used through their interface should not be marked as unused

[githoober]

``` interface I {};

class C implements I {};

class A {
public function useI(I $i): void {
$name = get_class($i);
echo "HERE is $name\n";
}
}

$a = new A();

The dead code detection produces the following errors:

ERROR: UnusedClass - src/index.php:4:7 - Class C is never used (see https://psalm.dev/075)
class C implements I {};

ERROR: UnusedClass - src/index.php:6:7 - Class A is never used (see https://psalm.dev/075)
class A {
```

The class C in this example is used through its interface I. This approach is widely utilized in applications based on abstractions/contracts, i.g. onion architectures, domain-driven design, etc.

It would be very helpful to detect classes usages through their interfaces and thus not to report them as not used.

[psalm-github-bot[bot]]

Hey @githoober, can you reproduce the issue on https://psalm.dev ?

[muglug]

The class C in this example is used through its interface I

It still has to be initialised _somewhere_ though – where is that happening?

[githoober]

Yes, it is, and IMO is irrelevant.

[mr-feek]

I am 👎🏼on this feature request. Just because a class implements an interface does not mean it is used.

[githoober]

I think I did a poor job explaining myself.

• The class A has a reference to the interface I.
• The class A is instantiated for example by a DI container.
• The implementations of I are also instantiated by the same DI container (This is IMO irrelevant as it cannot fix our issue). There are no any references to these classes in the code!
• With the request feature enabled via an extra option --dont-report-contract-implementations, psalm would not mark any implementations of the interface I as not used because they are used through the class A.
• As an alternative, psalm could mark the implementations of the interface I as possibly not used. Right now, it's a hard 'NotUsed' and it creates a lot of noise in our apps, as there are no any references to the implementation classes of the interface I in the code. This solution is not ideal, but at least, psalm would be admitting that way that it cannot figure out the usage.

[weirdan]

The implementations of I are also instantiated by the same DI container (This is IMO irrelevant as it cannot fix our issue). There are no any references to these classes in the code!

There's a decision made by DI container to use specific implementation of I, and Psalm needs to know that. There could be a factory instantiating A, or a configuration mentioning A or some convention telling DIC to prefer A implements I over B implements I. How to supply this data to Psalm heavily depends on how that decision is made. So this is actually most relevant.

[muglug]

Yes – there has to be information somewhere that specifies what classes are actually to be instantiated – maybe it's in XML configuration, maybe it's in code – but that data needs to be sent to Psalm somehow (presumably via a plugin), otherwise it won't be able to connect the dots.

[githoober]

@muglug our use case is creating libraries that become part of applications. These libraries do not have a DIC or anything else that creates class instances. I suppose we could create factories that create these classes, but then, what would be instantiating these factories?

IMO, psalm can support a mode where it calculates usage based on relations between classes.

[muglug]

In your example code you had class C implements I {}. For that code to be useful, C has to be instatiated _somewhere_.

Where is it instantiated, and what decides how that instantiation happens?

[zmitic]

I just noticed this as well. In my case, I am using Symfony DIC to inject iterable of ViewFactoryInterface into ViewFactory service.

All of them can be seen in this screenshot:

Main class (ViewFactory) is used everywhere and it is not a problem. What is does is strategy pattern i.e. delegate the job of doing something to some of other classes.

But all of them are reported as unused, even though their interface is used in

    /**
     * @param iterable<ViewFactoryInterface> $factories
     */
    public function __construct(iterable $factories)
    {
        $this->factories = $factories;
    }

If of any information, unused constructors are not a problem:

        <PossiblyUnusedMethod>
            <errorLevel type="suppress">
                <referencedMethod name="*::__construct"/>
                <directory name="src/Service"/>

[dbalabka]

@githoober IMO any class must be used somewhere if we are building an application. In the case of libraries, you should suppress such issues case-by-case (see @psalm-suppress)

Or you should disable this check project configuration of your library.