Controller action dependencies rector

[ghost]

Hi, I was sure there was a rule to move $this->get('service_id') to the action method parameters. However I can't find this rule in rector-symfony. Did I dream it or what?

class MyController
{
-   public function myAction()
+   public function myAction(MyService $service)
  {
-    $myService = $this->get('my_service');
  }
}

[TomasVotruba]

Hi, all the Symfony rules are here: https://github.com/rectorphp/rector-symfony/blob/main/docs/rector_rules_overview.md
And this one is a dream for now :)

[ghost]

Hmm, is it a dream because it's hard to implement? I'm asking because we already have get() to constructor injection Rector that does pretty similar thing.

[TomasVotruba]

I mean this rule does not exist.

It's quite easy to implement. You can re-use this rule: https://github.com/rectorphp/rector-symfony/blob/main/docs/rector_rules_overview.md#containergettoconstructorinjectionrector

And instead of adding a class dependency, you add it to class method.

[ghost]

I think I'm willing to implement this rule. However I have a request for you. Could you write out what classes I have to create and in which directories (including tests)? Do we have an analogous class for this case as DependencyInjectionMethodCallAnalyzer is for constructor injection? The same question for PropertyToAddCollector <- do we have sth like this for parameters?

[TomasVotruba]

Sure :) This would be the docs to check: https://github.com/rectorphp/rector/blob/main/docs/create_own_rule.md#file-structure
IMO the best source to learn is to check other contributors' PRs.

Do we have an analogous class for this case as DependencyInjectionMethodCallAnalyzer is for constructor injection?

No, we do not have any method injection logic at all.

[ghost]

I can see one big problem with the existing rule and the potential new one. Currently Rector adds a TH based on the type of the class under the service id inside container configured in rector.php. However this type can be different based on the environment. E.g. translator service is DataCollectorTranslator when dev and LoggingTranslator when prod. That's why there is an alias TranslatorInterface. Rector should be smart enough to add valid TH. We have access to the container and aliases are there so it might be doable.