vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.52k stars 656 forks source link

Add UnusedConstructor error #9531

Open jacekkarczmarczyk opened 1 year ago

jacekkarczmarczyk commented 1 year ago

Hi there,

I'd like to request support for the UnusedConstructor issue in Psalm. Currently, Psalm does not check for unused constructors in PHP classes.

The issue arises when constructors are created indirectly through dependency injection containers. In such cases, Psalm is not able to detect the usage of the constructor and marks it as unused, even though it is being used.

Here's a sample code snippet that illustrates the issue:

class Example {
    private $dependency;

    public function __construct(Dependency $dependency) {
        $this->dependency = $dependency;
    }

    public function doSomething() {
        $this->dependency->doSomethingElse();
    }
}

class Dependency {
    public function doSomethingElse() {}
}

$container = new Container();
$example = $container->get(Dependency::class);
$example->doSomething();

In this example, Psalm will report that the Example constructor is unused, even though it is being called indirectly through the container.

I think it would be useful to have support for the UnusedConstructor issue in Psalm to avoid false positives in such scenarios.

psalm-github-bot[bot] commented 1 year ago

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

weirdan commented 1 year ago

https://psalm.dev/r/14e393be0c

psalm-github-bot[bot] commented 1 year ago

I found these snippets:

https://psalm.dev/r/14e393be0c ```php dependency->doSomethingElse(); } } class Dependency { public function doSomethingElse(): void {} } interface Container { /** * @template T of object * @param class-string $class * @return T */ public function get(string $class): object; } function f(Container $container): void { $example = $container->get(Example::class); $example->doSomething(); } ``` ``` Psalm output (using commit 531eec6): INFO: PossiblyUnusedMethod - 3:21 - Cannot find any calls to method Example::__construct ```
weirdan commented 1 year ago

You can use regular expressions in referencedMethod element. Either of the following should work:

<issueHandlers>
  <PossiblyUnusedMethod>
    <errorLevel type="suppress">
      <referencedMethod name="*::__construct" />
      <referencedMethod name="/.*::__construct/" />
    <errorLevel>
  </PossiblyUnusedMethod>
</issueHandlers>
ygottschalk commented 1 year ago

Does @psalm-api work only for classes? Maybe we could use it for methods/functions, too...

weirdan commented 1 year ago

Does @psalm-api work only for classes?

Since 5.8.0 it's processed on methods as well.

jacekkarczmarczyk commented 11 months ago

Since there can be only one constructor in PHP - can't we just assume that if there is a (not unused) code that is using Dependency class then the constructor must have been called somehow?

weirdan commented 11 months ago

Actually UnusedConstructor already exists: https://psalm.dev/docs/running_psalm/issues/UnusedConstructor/

We may consider emitting it instead of *UnusedMethod for non-private constructors as well