Edge Cases with TemplateAnnotationRector

[stedekay]

Question

Thanks for fixing my issues with the TemplateAnnotationRector before.

Now, I've found two more edge cases where I don't know if it is a case for Rector to solve it, so I just thought opening a Support Ticket:

In one case I've got a Controller which doesn't extend any class (no AbstractController or Controller). While the Template annotation works, the result ($this->render(...)) won't work, because the render method is a helper method from AbstractController (ControllerTrait to be more specific).

So I think the rector shouldn't do anything in this case or what do you think?

The other case is that some of our actions call another private method like in the following example:

    /**
     * @Route("/create", name="survey_create")
     * @Template("PAPPSurveyBundle:Survey:create.html.twig")
     *
     * @return array
     */
    public function createAction(Request $request, Party $currentParty)
    {
        $survey = $this->surveyHelper->createSurvey($currentParty);

        return $this->processForm($request, $currentParty, $survey, $this->generateUrl('survey_save'));
    }

    private function processForm(Request $request, Party $party, Survey $survey, $url)
    {
        ...

        return [
            'form' => $form->createView(),
        ];
    }

In this case the Template annotation will be removed, but nothing more which breaks the application.

In both cases, I am not sure what the rector should do. If you want me to open issues for one or both of these cases, just let me know, I could add some failing tests.

[TomasVotruba]

In one case I've got a Controller which doesn't extend any class (no AbstractController or Controller). While the Template annotation works, the result ($this->render(...)) won't work, because the render method is a helper method from AbstractController (ControllerTrait to be more specific).

How do you approach it manually? Do you intent to keep it change it?

[stedekay]

I think the best solution is to keep it and change it manually.

[TomasVotruba]

I think the best solution is to keep it and change it manually.

How do you change it manually?

[TomasVotruba]

In this case the Template annotation will be removed, but nothing more which breaks the application.

Could you create a new issue for this? So we don't have 2-threads here :)

[stedekay]

Could you create a new issue for this? So we don't have 2-threads here :)

Sure.

[stedekay]

How do you change it manually?

There are 2 possibilities: Add the AbstractController as base class or adding the Twig Environment service as dependency to use its render method.

[TomasVotruba]

I see. Which do you prefer and why?

[stedekay]

To be more framework agnostic I would go for option 2

[TomasVotruba]

I see. We could make it default configurable.

Btw, how is this controller different to others, so it's missing the parent controller?

[stedekay]

Exactly, it extends no other class

[TomasVotruba]

I know, but why?

[stedekay]

I think its better to make its dependencies in the controller explicit instead of using the given helper methods from the base class and if you don't use any of these methods there is no need to use the AbstractController.

But I think it would be perfectly fine when Rector will add the AbstractController class as base class, as its from the Symfony set and use the render method of the AbstractController.

[TomasVotruba]

But I think it would be perfectly fine when Rector will add the AbstractController class as base class, as its from the Symfony set and use the render method of the AbstractController.

Exactly my thoughts! Let's go for it then
Do you want to try to add such feature?

[TomasVotruba]

Btw, there could be another rule, which would remove parent AbstractController and replace helper methods with specific service calls