Allow other namespaces for @Inject in JmsInjectAnnotationRector

[holtkamp]

I was checking whether I could use rector to change my PHP-DI @Inject annotation-based dependency injections to constructor based injections.

To my big surprise I found out about an already existing rector:

https://github.com/rectorphp/rector/blob/master/docs/AllRectorsOverview.md#jmsinjectannotationrector

The only difference seems to be that the PHP-DI @Inject annotation does not have/use a namespace. That made me wonder, instead of making a new rector, would it be a (good) idea to change https://github.com/rectorphp/rector/blob/fac1503915aea843a3c123a5c622ecf0708bda62/packages/Jms/src/Rector/Property/JmsInjectAnnotationRector.php in such a way that the namespace of the annotation can be configured / left empty, probably defaulting to

rector/packages/Jms/src/Rector/Property/JmsInjectAnnotationRector.php

Line 28 in fac1503

private const INJECT_ANNOTATION = 'JMS\DiExtraBundle\Annotation\Inject';

[TomasVotruba]

Hi,

so it's basically the very same change just with different annotation?

What will happen when you change it in JmsInjectAnnotationRector manually?

If that's what helps, we can make the rule configurable.

[holtkamp]

What will happen when you change it in JmsInjectAnnotationRector manually?

Will give it a shot and report back here, this week is quite busy, so might take some time.

[holtkamp]

I finally found some time to try this out.

Using rector 0.5.5 and the following rector.yaml file:

# rector.yaml
services:
  Rector\Rector\Property\InjectAnnotationClassRector:
    $annotationClass: 'Inject'

This results in an error:

Make sure "kernel_class" parameters is set in rector.yaml in "parameters:" section

Which seems to indicate this functionality is only available for Symfony applications?

When the property type indicates that it is a FQCN (using class_exists()?), this type could probably be used directly instead of having the AnalyzedApplicationContainer determine the type. This way it should also work for "non-Symfony" applications.

Not sure this is possible, just thinking out loud 😄

class Example {

  /**
   * @Inject
   * @var \Fully\Qualified\Class\Name\To\Dependency
   */
  private $someDependency;

}

can then become:

class Example {

  /**
   * @var \Fully\Qualified\Class\Name\Of\Dependency
   */
  private $someDependency;

  public function __construct(\Fully\Qualified\Class\Name\Of\Dependency $someDependency)
  {
    $this->someDependency = $someDependency
  }
}

[TomasVotruba]

Thanks for testing!

What exact bin command do you run?

[holtkamp]

I used:

./vendor/bin/rector process path/to/some/files/with/simple/inject/annotations  

[holtkamp]

@TomasVotruba nice, works great!

A "nice to have" I encountered:

• re-use an imported class alias if applicable

Currently the FQCN of the dependency is used, but it might be an alias:

use Fully\Qualified\Class\Name\To\Dependency as MyDependency;

class Example {

  /**
   * @Inject
   * @var MyDependency
   */
  private $someDependency;

}

Maybe I can come up with a PR for this 😄

UPDATE
Tried to use an import by changing resolveType() to :

        $varTypeInfo = $this->docBlockManipulator->getVarTypeInfo($node);
        if ($varTypeInfo !== null) {
            if($varTypeInfo->getType() !== null){
                return ltrim($varTypeInfo->getType(), '\\');
            }
            if($varTypeInfo->getFqnType() !== null){
                return ltrim($varTypeInfo->getFqnType(), '\\');
            }
        }

but apparently the type gets always prefixed with a backslash. So let's leave it for now / have another rector apply this optimization.

[TomasVotruba]

That could be done :)

Tried to use an import by changing resolveType() to :

Good start!

When you send PR with test case for this, I'll navigate you to the finish.

[holtkamp]

When you send PR with test case for this, I'll navigate you to the finish.

To be honest: by using "Code" => "Code Cleanup" in PHPStorm the aliases of the imported classes are used properly, so this kind of "reduces" this "nice to have" request 😉 .

Protected properties change to private?
Something more important: it seems that the visibility of protected properties is changed to private due to this line:

rector/src/Rector/Property/InjectAnnotationClassRector.php

Line 199 in 125dac4

$property->flags = Class_::MODIFIER_PRIVATE;

Not sure what the rationale / idea of this is. But in case specialized classes extend the class which is "rectored", than these properties become unavailable to them..