How to deal with command / event busses?

[ruudk]

Let's say you have the following:

class MyHandler {
    public function handle(MyCommand $command) {}
}

class MyCommand {
    public function __construct(public string $name) {}
}

class MyController {
    public function __invoke() {
        $this->commandBus->dispatch(new MyCommand('Ruud'));
    }
}

The above should return in no dead classes.

But when MyCommand is not dispatched anywhere, it should mark MyHandler + MyCommand as dead.

Same with an Event and multiple event subscribers to that event. If the event is never dispatched, the event + all the subscribers should be marked as dead.

https://symfony.com/doc/current/components/messenger.html

[janedbal]

I believe that in order to support this (and #83), we need some HiddenCallProvider which will be based on AST, not based on reflection like EntrypointProvider is.

[janedbal]

Since 0.7.0, we can deduce usages from AST, but I'm still unsure if it can solve this problem.

Because when we are analysing $this->commandBus->dispatch, we would need to know which handler have the MyCommand in its __invoke method to be able to mark it as used. This is possible in PHPStan only via collectors, which might be really tricky to inject into dead-code-detector's extension point without breaking its performance (mainly because you MUST NOT use reflection in all CollectedData rules).

Or do you see any option without collector-like logic?

[ruudk]

I was thinking about this today. With this example:

class MyHandler {
    #[AsCommandHandler]
    public function handle(MyCommand $command) {}
}

class MyCommand {
    public function __construct(public string $name) {}
}

We get the following error:

MyHandler is not used

It does not complain about MyCommand, because that's used by MyHandler.

If we could somehow flip this, with some extension, that triggers on:

• Methods with AsCommandHandler attribute
• Take the object type of the first parameter
  The object type of the first parameter will then be marked as "opposite" ownership / usage.

It would then report the following error:

MyCommand is not used
and thus MyHandler is unused (transitive)

Now the next part becomes simple, as soon as we add:

class MyController {
    public function __invoke() {
        $this->commandBus->dispatch(new MyCommand('Ruud'));
    }
}

It sees that MyCommand is used, and the errors go away.

[ruudk]

Also something else I wonder, with this example:

class MyHandler {
    #[AsCommandHandler]
    public function handle(MyCommand $command) {}
}

class MyCommand {
    public function __construct(public string $name) {}
}

It currently says

MyHandler is not used

But why doesn't it say

MyHandler is not used
And thus MyCommand is unused (transitive)

[janedbal]

First of all, please keep in mind we still work only with class methods, not classes itself.

So if MyCommand::__construct in you example is not reported as unused, it means that it is used somewhere:

• new MyCommand
• it is created by reflection and understood by our extension
• it is present in your DIC xml file
• or the violation is ignored

To detect where, I started working on this idea, so it might be easier to debug this in near future.

But there is no call transitivity from MyHandler::handle to MyCommand::__construct in your example as there is no new MyCommand() inside MyHandler::handle method.

[janedbal]

But otherwise yes, we could probably emit usage of MyHandler::handle with origin in MyCommand::__construct based on the attribute.

[ruudk]

First of all, please keep in mind we still work only with class methods, not classes itself.

I think this is key. I forgot about that.

Trying to hook this into usage of the __construct feels like a hack.

We should try to work on detecting unused classes.

[janedbal]

We should try to work on detecting unused classes.

Yeah, I already tried POCing that, it will take some time.

[ruudk]

I think we can close this. I created custom rules in our project to check if a command handler uses a command that is never dispatched. The same for event subscribers, when the event is never dispatched, it will report an error.