contao/contao-rector

System::getContainer() services shouldn't go into the class constructor

qzminski opened this issue · 6 comments

qzminski commented

Input:

public function method()
{
    $service = System::getContainer()->get(\App\Service::class);
}

Output:

public function __construct(private \App\Service $service)
{
}

public function method()
{
    $service = $this->service;
}

Problem:

Getting a service from a container in the old way is used mainly in places where it's impossible to use dependency injection. For example, in the form fields, models, or page-type classes. Those cannot be changed because they are not services.

In my opinion, this rule should be disabled completely, or it should somehow detect whether the class that will be changed is a service. Otherwise, every run would break the stuff.

aschempp commented

what rule exactly are you talking about?

qzminski commented

Yeah it apparently is the ContainerGetToConstructorInjectionRector rule. Should this be then excluded on the project level?

aschempp commented

Who is adding it in the first place? What does your config look like?

aschempp commented

yes, so it's not related to this rector, is it? 😉