Bug: ContainerGetToConstructorInjectionRector replaces constant with object

[vlados]

Code:

        $alertNotifications = $this->getAlertNotificationService()->updateAlertsNotificationsVisibility(
            $request->get((string) Constants::ALERT_NOTIFICATIONS)
        );

Result:

    ---------- begin diff ----------
@@ @@
     use ControllerExceptionHandling;

     public function __construct(
-        private readonly ManagerRegistry $managerRegistry
+        private readonly ManagerRegistry $managerRegistry, private readonly Constants $constants
     )
     {
     }
@@ @@
         return $em->wrapInTransaction(
             function () use ($request) {
                 $alertNotifications = $this->getAlertNotificationService()->updateAlertsNotificationsVisibility(
-                    $request->get(Constants::ALERT_NOTIFICATIONS),
+                    $this->constants,
                     false
                 );
    ----------- end diff -----------

Applied rules:
 * ReadOnlyPropertyRector (https://wiki.php.net/rfc/readonly_properties_v2)
 * ContainerGetToConstructorInjectionRector

Expected result

    ---------- begin diff ----------
@@ @@
     use ControllerExceptionHandling;

     public function __construct(
-        private readonly ManagerRegistry $managerRegistry
+        private readonly ManagerRegistry $managerRegistry, private readonly Constants $constants
     )
     {
     }
@@ @@
         return $em->wrapInTransaction(
             function () use ($request) {
                 $alertNotifications = $this->getAlertNotificationService()->updateAlertsNotificationsVisibility(
-                    $request->get(Constants::ALERT_NOTIFICATIONS),
+                    $this->constants::ALERT_NOTIFICATIONS,
                     false
                 );
    ----------- end diff -----------

Applied rules:
 * ReadOnlyPropertyRector (https://wiki.php.net/rfc/readonly_properties_v2)
 * ContainerGetToConstructorInjectionRector

Or how I can make this rule to be skipped for Constants?

[TomasVotruba]

Thank you for your report!

We'll need an isolated failing demo link from: http://getrector.org/demo,
that way we can reproduce the bug.

[vlados]

I've tried but it returns no code change - https://getrector.com/demo/fcd3131b-2333-45d4-934b-e3d38f9b9acc
I am locally using latest version - 0.17.1

[stefantalen]

The demo cannot guess your service definition so it doesn't know about AlertNotificationService and its method updateAlertsNotificationsVisibility 😉

You can add the class in the example

[vlados]

https://getrector.com/demo/d086fa78-f0b8-4c32-a7ac-6bc50c0d00d0
still nothing ... I think this demo have some problem

[stefantalen]

Can you share the full file content of your first message? The diff shows an anonymous function structure that is not in any of your demo examples 🙂

My first thought is the ContainerGetToConstructorInjectionRector somehow gets triggered on the get() method for $request

[TomasVotruba]

Demo is not suitable for such a complex rule reproducer, as it need the class to exist and be part of dumped xml.
We'll probably need a reproducible Github repository, that will speed up the process :)

What are the parent types of your $request variable?

[samsonasik]

I am closing it as no reproducer repo, feel free to reopen new issue with reproducer repo.

[bbrala]

Reproduced the bug, yay, see related issue.