[Php80] ClassPropertyAssignToConstructorPromotionRector can break Symfony auto-wiring

[kick-the-bucket]

Bug Report

Subject	Details
Rector version	v0.13.6

When promoting constructor properties and there is a name miss-match between the property name and constructor argument name - the property name is used. This leads to problems, when there are more than one instance of the class available and named aliases are used (the class to auto-wire in selected by constructor argument name). So if the argument was auto-wired by name and this name is changed - it either can no longer be auto-wired or it can be auto-wired to a different (usually default instead of specific one referenced by the argument name) instance and break intended behavior.

Minimal PHP Code Causing Issue

Example borrowed from How to Log Messages to different Files

With monolog config

monolog:
    channels:
        - foo

this is auto-wired the LoggerInterface instance for the foo channel.

class Service
{
    private LoggerInterface $logger;

    public function construct(LoggerInterface $fooLogger)
    {
        $this->logger = $fooLogger;
    }
}

Currently it will be refactored by ClassPropertyAssignToConstructorPromotionRector to

class Service
{
    public function construct(private LoggerInterface $logger)
    {
    }
}

which will be auto-wired to a different LoggerInterface instance for the app channel.

Expected Behaviour

There are quire some options how to dealt with this:

• Provide an option for this rector to allow or disallow constructor argument renaming
• Provide an option to choose if the property name or the constructor argument name is used for the promoted property
• Provide an option to set a list of argument types which should not be renamed
• If phpstan/phpstan-symfony extension is enabled and containerXmlPath is set - maybe it would be possible to make sure if we can safely rename the constructor argument or not?
• If we don't know that symfony dependency injection auto-wiring is used
  • we could skip such properties/constructor arguments
  • we could rename the property instead of the constructor argument

[TomasVotruba]

It seems that keeping $fooLogger name would be the best way to go. Then rename the property fetches accordingly

[TomasVotruba]

Closing as not an issue and suggestion is accepted, to keep focus on relevant and opened issues.

On the other hand, renaming properties can be dangerous and lead to another unexpected consequences. In this case, I'd recommend to handle Rector the 1:1 type rename the constructor params. Then update the configs to match the new names and types. That way you can also fix missnames of property names.

The best solution IMO is not to use named arguments at all and go for type-based autowiring or positions. That way you remove this issue completelly from your project :+1:

[Geolim4]

Hello,

Sorry to bump this @TomasVotruba, but I'm unable to find out how to disable constructor argument renaming using rule configuration ?