[BUG] New __construct() does not call parent::__construct()

[cweiske]

Minimal PHP Code Causing Issue

<?php
use TYPO3Fluid\Fluid\Core\ViewHelper\AbstractTagBasedViewHelper;

class SourceViewHelper extends AbstractTagBasedViewHelper
{
    public function injectImageService(ImageService $imageService)
    {
        $this->imageService = $imageService;
    }
}

When applying the InjectMethodToConstructorInjectionRector rule, a new __construct() method is created.
Unfortunately, the parent::__construct() method is not called, preventing initialization of internal properties.

This leads to an error:

(1/1) Error
Call to a member function reset() on null
in /var/www/typo3/vendor/typo3fluid/fluid/src/Core/ViewHelper/AbstractTagBasedViewHelper.php line 99

     */
    public function initialize()
    {
        parent::initialize();
        $this->tag->reset();
        $this->tag->setTagName($this->tagName);

Applied rules

InjectMethodToConstructorInjectionRector

Expected Behaviour

If the parent class has a __construct() method, it should be called in the newly generated constructor.

Package Version

2.12.0

PHP Version

8.2.24

TYPO3 Version

10.4.37

Notes

No response

[cweiske]

I also had a similar problem when extending TYPO3\CMS\Form\Domain\Model\FormElements\GenericFormElement, and typo3-rector converted the injection method into a __construct method:

Return value of TYPO3\CMS\Form\Domain\Model\Renderable\AbstractRenderable::getIdentifier() must be of the type string, null returned
in /var/www/typo3/public/typo3/sysext/form/Classes/Domain/Model/Renderable/AbstractRenderable.php line 119

The reason is that while GenericFormElement itself has no __construct method, the parent class AbstractFormElement has:

public function __construct(string $identifier, string $type)

The solution was to add the required constructor properties into the newly created __construct method and pass them on to parent::__construct($identifier, $type).

The correct constructor signature is:

public function __construct(string $identifier, string $type, ApiService $apiClient)

(first two are the required parameters from the grandparent class, the last one is the injected service)

[cweiske]

The constructor injection with

public function __construct(string $identifier, string $type, ApiService $apiClient)

did work in TYPO3v10, but does not anymore with TYPO3v11.

The dependency injection documentation says:

In some APIs dependency injection cannot be used yet. This applies to classes that need specific data in their constructors [...]

Maybe this is the reason. (v13 still has the same note).

So typo3-rector rule should generate different code depending on the constructor parameters - when the class has required parameters, it should keep the injectX methods.

[simonschaufi]

Sometimes you have to exclude certain directories from Rector and this is such a thing, as covering all different kind of edge cases would be way too complicated.

    ->withSkip([
        InjectMethodToConstructorInjectionRector::class => [
            __DIR__ . '/../../Classes/ViewHelpers/',
        ],
    ])

[cweiske]

But then I have to know about the problem, which I don't.
In my eyes the rules should be safe to use, and not introduce new bugs.

[simonschaufi]

This rule does exactly what is says, transform inject methods into a constructor without checking for a parent constructor. If you can't apply this rule, you have to exclude it yourself. That is why you should always dry-run rector before applying the code changes.

Writing rector rules is already quite challenging and thinking about every possible edge case is almost impossible. We as a team have also already thrown away some rules that became so complicated to implement that it wasn't worth it to maintain them.

[cweiske]

I found out that dependency injection in TYPO3 does not work for classes that have non-service parameters in their __construct() method.

So the "only" thing the InjectMethodToConstructorInjectionRector needs is to skip the class if a constructor exists with flat non-object parameters (either the class itself or any parent classes).