Merge pull request #564 from rectorphp/controller-trait

[FrameworkBundle] Let GetToConstructorInjectionRector work with trait as well

packages/NodeTypeResolver/src/Reflection/ClassReflectionTypesResolver.php

@@ -2,10 +2,21 @@
 
 namespace Rector\NodeTypeResolver\Reflection;
 
+use PHPStan\Broker\Broker;
 use PHPStan\Reflection\ClassReflection;
 
 final class ClassReflectionTypesResolver
 {
+    /**
+     * @var Broker
+     */
+    private $broker;
+
+    public function __construct(Broker $broker)
+    {
+        $this->broker = $broker;
+    }
+
     /**
      * @return string[]
      */
@@ -21,13 +32,22 @@ final class ClassReflectionTypesResolver
         $types = array_merge($types, $classReflection->getParentClassesNames());
 
         // interfaces
-        foreach ($classReflection->getInterfaces() as $classReflection) {
-            $types[] = $classReflection->getName();
+        foreach ($classReflection->getInterfaces() as $interfaceReflection) {
+            $types[] = $interfaceReflection->getName();
         }
 
         // traits
-        foreach ($classReflection->getTraits() as $classReflection) {
-            $types[] = $classReflection->getName();
+        foreach ($classReflection->getTraits() as $traitReflection) {
+            $types[] = $traitReflection->getName();
+        }
+
+        // to cover traits of parent classes
+        foreach ($classReflection->getParentClassesNames() as $parentClassName) {
+            $parentClassReflection = $this->broker->getClass($parentClassName);
+
+            foreach ($parentClassReflection->getTraits() as $parentClassTrait) {
+                $types[] = $parentClassTrait->getName();
+            }
         }
 
         return $types;

packages/NodeTypeResolver/tests/PerNodeTypeResolver/ClassAndInterfaceTypeResolver/ClassAndInterfaceTypeResolverTest.php

@@ -0,0 +1,57 @@
+<?php declare(strict_types=1);
+
+namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver;
+
+use Iterator;
+use PhpParser\Node\Stmt\Class_;
+use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\AbstractNodeTypeResolverTest;
+use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source\AnotherTrait;
+use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source\ClassWithParentClass;
+use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source\ClassWithParentInterface;
+use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source\ClassWithParentTrait;
+use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source\ClassWithTrait;
+use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source\ParentClass;
+use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source\SomeInterface;
+
+/**
+ * @covers \Rector\NodeTypeResolver\PerNodeTypeResolver\ClassAndInterfaceTypeResolver
+ */
+final class ClassAndInterfaceTypeResolverTest extends AbstractNodeTypeResolverTest
+{
+    /**
+     * @dataProvider dataProvider()
+     * @param string[] $expectedTypes
+     */
+    public function test(string $file, int $nodePosition, array $expectedTypes): void
+    {
+        $variableNodes = $this->getNodesForFileOfType($file, Class_::class);
+
+        $this->assertSame($expectedTypes, $this->nodeTypeResolver->resolve($variableNodes[$nodePosition]));
+    }
+
+    public function dataProvider(): Iterator
+    {
+        yield [__DIR__ . '/Source/ClassWithParentInterface.php', 0, [
+            ClassWithParentInterface::class,
+            SomeInterface::class,
+        ]];
+
+        yield [__DIR__ . '/Source/ClassWithParentClass.php', 0, [
+            ClassWithParentClass::class,
+            ParentClass::class,
+        ]];
+
+        yield [__DIR__ . '/Source/ClassWithTrait.php', 0, [ClassWithTrait::class, AnotherTrait::class]];
+
+        yield [
+            __DIR__ . '/Source/ClassWithParentTrait.php',
+            0,
+            [ClassWithParentTrait::class, ClassWithTrait::class, AnotherTrait::class],
+        ];
+        yield [
+            __DIR__ . '/Source/AnonymousClass.php',
+            0,
+            [ParentClass::class, SomeInterface::class, AnotherTrait::class],
+        ];
+    }
+}

packages/NodeTypeResolver/tests/PerNodeTypeResolver/ClassAndInterfaceTypeResolver/Source/AnonymousClass.php

@@ -1,6 +1,6 @@
 <?php declare(strict_types=1);
 
-namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source;
+namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source;
 
 new class extends ParentClass implements SomeInterface
 {

packages/NodeTypeResolver/tests/PerNodeTypeResolver/ClassAndInterfaceTypeResolver/Source/AnotherTrait.php

@@ -1,6 +1,6 @@
 <?php declare(strict_types=1);
 
-namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source;
+namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source;
 
 trait AnotherTrait
 {

packages/NodeTypeResolver/tests/PerNodeTypeResolver/ClassAndInterfaceTypeResolver/Source/ClassWithParentClass.php

@@ -1,6 +1,6 @@
 <?php declare(strict_types=1);
 
-namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source;
+namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source;
 
 final class ClassWithParentClass extends ParentClass
 {

packages/NodeTypeResolver/tests/PerNodeTypeResolver/ClassAndInterfaceTypeResolver/Source/ClassWithParentInterface.php

@@ -1,6 +1,6 @@
 <?php declare(strict_types=1);
 
-namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source;
+namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source;
 
 final class ClassWithParentInterface implements SomeInterface
 {

packages/NodeTypeResolver/tests/PerNodeTypeResolver/ClassAndInterfaceTypeResolver/Source/ClassWithParentTrait.php

@@ -0,0 +1,7 @@
+<?php declare(strict_types=1);
+
+namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source;
+
+class ClassWithParentTrait extends ClassWithTrait
+{
+}

packages/NodeTypeResolver/tests/PerNodeTypeResolver/ClassAndInterfaceTypeResolver/Source/ClassWithTrait.php

@@ -1,8 +1,8 @@
 <?php declare(strict_types=1);
 
-namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source;
+namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source;
 
-final class ClassWithTrait
+class ClassWithTrait
 {
     use AnotherTrait;
 }

packages/NodeTypeResolver/tests/PerNodeTypeResolver/ClassAndInterfaceTypeResolver/Source/ParentClass.php

@@ -1,6 +1,6 @@
 <?php declare(strict_types=1);
 
-namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source;
+namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source;
 
 class ParentClass
 {

packages/NodeTypeResolver/tests/PerNodeTypeResolver/ClassAndInterfaceTypeResolver/Source/SomeInterface.php

@@ -1,6 +1,6 @@
 <?php declare(strict_types=1);
 
-namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source;
+namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassAndInterfaceTypeResolver\Source;
 
 interface SomeInterface
 {

packages/NodeTypeResolver/tests/PerNodeTypeResolver/ClassTypeResolver/ClassTypeResolverTest.php

@@ -1,48 +0,0 @@
-<?php declare(strict_types=1);
-
-namespace Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver;
-
-use Iterator;
-use PhpParser\Node\Stmt\Class_;
-use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\AbstractNodeTypeResolverTest;
-use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source\AnotherTrait;
-use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source\ClassWithParentClass;
-use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source\ClassWithParentInterface;
-use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source\ClassWithTrait;
-use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source\ParentClass;
-use Rector\NodeTypeResolver\Tests\PerNodeTypeResolver\ClassTypeResolver\Source\SomeInterface;
-
-/**
- * @covers \Rector\NodeTypeResolver\PerNodeTypeResolver\ClassTypeResolver
- */
-final class ClassTypeResolverTest extends AbstractNodeTypeResolverTest
-{
-    /**
-     * @dataProvider dataProvider()
-     * @param string[] $expectedTypes
-     */
-    public function test(string $file, int $nodePosition, array $expectedTypes): void
-    {
-        $variableNodes = $this->getNodesForFileOfType($file, Class_::class);
-
-        $this->assertSame($expectedTypes, $this->nodeTypeResolver->resolve($variableNodes[$nodePosition]));
-    }
-
-    public function dataProvider(): Iterator
-    {
-        yield [__DIR__ . '/Source/ClassWithParentInterface.php', 0, [
-            ClassWithParentInterface::class,
-            SomeInterface::class,
-        ]];
-
-        yield [__DIR__ . '/Source/ClassWithParentClass.php', 0, [
-            ClassWithParentClass::class,
-            ParentClass::class,
-        ]];
-
-        yield [__DIR__ . '/Source/ClassWithTrait.php', 0, [ClassWithTrait::class, AnotherTrait::class]];
-
-        // traits in anonymous classes are ignored in PHPStan
-        yield [__DIR__ . '/Source/AnonymousClass.php', 0, [ParentClass::class, SomeInterface::class]];
-    }
-}

packages/Symfony/src/Bridge/DependencyInjection/ContainerFactory.php

@@ -25,7 +25,7 @@ final class ContainerFactory
     }
 
     /**
-     * Mimics https://github.com/symfony/symfony/blob/226e2f3949c5843b67826aca4839c2c6b95743cf/src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php#L200-L203
+     * Mimics https://github.com/symfony/symfony/blob/f834c9262b411aa5793fcea23694e3ad3b5acbb4/src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php#L200-L203
      */
     private function createContainerFromKernelClass(string $kernelClass): Container
     {
@@ -35,6 +35,7 @@ final class ContainerFactory
 
         /** @var ContainerBuilder $containerBuilder */
         $containerBuilder = (new PrivatesCaller())->callPrivateMethod($kernel, 'buildContainer');
+        $containerBuilder->getCompilerPassConfig()->setRemovingPasses([]);
 
         // anonymous class on intention, since this depends on Symfony\DependencyInjection in rector-prefixed
         $containerBuilder->getCompilerPassConfig()->addPass(new class() implements CompilerPassInterface {
@@ -49,6 +50,8 @@ final class ContainerFactory
             }
         });
 
+        $containerBuilder->compile();
+
         return $containerBuilder;
     }
 

packages/Symfony/src/Rector/FrameworkBundle/GetToConstructorInjectionRector.php

@@ -14,9 +14,17 @@ final class GetToConstructorInjectionRector extends AbstractToConstructorInjecti
      */
     private $controllerClass;
 
-    public function __construct(string $controllerClass = 'Symfony\Bundle\FrameworkBundle\Controller\Controller')
-    {
+    /**
+     * @var string
+     */
+    private $traitClass;
+
+    public function __construct(
+        string $controllerClass = 'Symfony\Bundle\FrameworkBundle\Controller\Controller',
+        string $traitClass = 'Symfony\Bundle\FrameworkBundle\Controller\ControllerTrait'
+    ) {
         $this->controllerClass = $controllerClass;
+        $this->traitClass = $traitClass;
     }
 
     public function getDefinition(): RectorDefinition
@@ -61,6 +69,6 @@ CODE_SAMPLE
             return false;
         }
 
-        return $this->methodCallAnalyzer->isTypeAndMethod($node, $this->controllerClass, 'get');
+        return $this->methodCallAnalyzer->isTypesAndMethod($node, [$this->controllerClass, $this->traitClass], 'get');
     }
 }

packages/Symfony/tests/Rector/FrameworkBundle/GetToConstructorInjectionRector/Correct/correct3.php.inc

@@ -0,0 +1,21 @@
+<?php declare (strict_types=1);
+
+use Rector\Symfony\Tests\Rector\FrameworkBundle\GetToConstructorInjectionRector\Source\GetTrait;
+
+class ClassWithNamedServiceAndTrait
+{
+    use GetTrait;
+    /**
+     * @var \Rector\Symfony\Tests\Rector\FrameworkBundle\AbstractToConstructorInjectionRectorSource\SomeTranslator
+     */
+    private $someTranslator;
+    public function __construct(\Rector\Symfony\Tests\Rector\FrameworkBundle\AbstractToConstructorInjectionRectorSource\SomeTranslator $someTranslator)
+    {
+        $this->someTranslator = $someTranslator;
+    }
+
+    public function render()
+    {
+        $this->someTranslator;
+    }
+}

packages/Symfony/tests/Rector/FrameworkBundle/GetToConstructorInjectionRector/Correct/correct4.php.inc

@@ -0,0 +1,19 @@
+<?php declare (strict_types=1);
+
+use Rector\Symfony\Tests\Rector\FrameworkBundle\GetToConstructorInjectionRector\Source\ParentClassWithGetTrait;
+
+class ClassWithNamedServiceAndParentTrait extends ParentClassWithGetTrait
+{
+    /**
+     * @var \Rector\Symfony\Tests\Rector\FrameworkBundle\AbstractToConstructorInjectionRectorSource\SomeTranslator
+     */
+    private $someTranslator;
+    public function __construct(\Rector\Symfony\Tests\Rector\FrameworkBundle\AbstractToConstructorInjectionRectorSource\SomeTranslator $someTranslator)
+    {
+        $this->someTranslator = $someTranslator;
+    }
+    public function render()
+    {
+        $this->someTranslator;
+    }
+}

packages/Symfony/tests/Rector/FrameworkBundle/GetToConstructorInjectionRector/GetToConstructorInjectionRectorTest.php

@@ -29,6 +29,8 @@ final class GetToConstructorInjectionRectorTest extends AbstractRectorTestCase
     {
         yield [__DIR__ . '/Wrong/wrong.php.inc', __DIR__ . '/Correct/correct.php.inc'];
         yield [__DIR__ . '/Wrong/wrong2.php.inc', __DIR__ . '/Correct/correct2.php.inc'];
+        yield [__DIR__ . '/Wrong/wrong3.php.inc', __DIR__ . '/Correct/correct3.php.inc'];
+        yield [__DIR__ . '/Wrong/wrong4.php.inc', __DIR__ . '/Correct/correct4.php.inc'];
     }
 
     protected function provideConfig(): string

packages/Symfony/tests/Rector/FrameworkBundle/GetToConstructorInjectionRector/Source/GetTrait.php

@@ -0,0 +1,13 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\Rector\FrameworkBundle\GetToConstructorInjectionRector\Source;
+
+trait GetTrait
+{
+    /**
+     * @return object
+     */
+    public function get(string $serviceName)
+    {
+    }
+}

packages/Symfony/tests/Rector/FrameworkBundle/GetToConstructorInjectionRector/Source/ParentClassWithGetTrait.php

@@ -0,0 +1,9 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\Rector\FrameworkBundle\GetToConstructorInjectionRector\Source;
+
+class ParentClassWithGetTrait
+{
+    use GetTrait;
+
+}

packages/Symfony/tests/Rector/FrameworkBundle/GetToConstructorInjectionRector/Wrong/wrong3.php.inc

@@ -0,0 +1,13 @@
+<?php declare (strict_types=1);
+
+use Rector\Symfony\Tests\Rector\FrameworkBundle\GetToConstructorInjectionRector\Source\GetTrait;
+
+class ClassWithNamedServiceAndTrait
+{
+    use GetTrait;
+
+    public function render()
+    {
+        $this->get('translator');
+    }
+}

packages/Symfony/tests/Rector/FrameworkBundle/GetToConstructorInjectionRector/Wrong/wrong4.php.inc

@@ -0,0 +1,11 @@
+<?php declare (strict_types=1);
+
+use Rector\Symfony\Tests\Rector\FrameworkBundle\GetToConstructorInjectionRector\Source\ParentClassWithGetTrait;
+
+class ClassWithNamedServiceAndParentTrait extends ParentClassWithGetTrait
+{
+    public function render()
+    {
+        $this->get('translator');
+    }
+}

packages/Symfony/tests/Rector/FrameworkBundle/GetToConstructorInjectionRector/config.yml

@@ -4,3 +4,4 @@ parameters:
 services:
     Rector\Symfony\Rector\FrameworkBundle\GetToConstructorInjectionRector:
         $controllerClass: 'Rector\Symfony\Tests\Rector\Source\SymfonyController'
+        $traitClass: 'Rector\Symfony\Tests\Rector\FrameworkBundle\GetToConstructorInjectionRector\Source\GetTrait'

src/NodeAnalyzer/MethodCallAnalyzer.php

@@ -30,7 +30,7 @@ final class MethodCallAnalyzer
      */
     public function isTypeAndMethods(Node $node, string $type, array $methods): bool
     {
-        if (! $this->isType($node, $type)) {
+        if (! $this->isTypes($node, [$type])) {
             return false;
         }
 
@@ -45,7 +45,24 @@ final class MethodCallAnalyzer
      */
     public function isTypeAndMethod(Node $node, string $type, string $method): bool
     {
-        if (! $this->isType($node, $type)) {
+        if (! $this->isTypes($node, [$type])) {
+            return false;
+        }
+
+        /** @var Identifier $methodName */
+        $methodName = $node->name;
+
+        return $methodName->toString() === $method;
+    }
+
+    /**
+     * Checks "$this->classOfSpecificType->specificMethodName()"
+     *
+     * @param string[] $types
+     */
+    public function isTypesAndMethod(Node $node, array $types, string $method): bool
+    {
+        if (! $this->isTypes($node, $types)) {
             return false;
         }
 
@@ -87,20 +104,6 @@ final class MethodCallAnalyzer
         return in_array($node->name->name, $methods, true);
     }
 
-    /**
-     * Checks "$this->methodCall()"
-     */
-    public function isType(Node $node, string $type): bool
-    {
-        if (! $node instanceof MethodCall) {
-            return false;
-        }
-
-        $calledNodeTypes = $this->nodeTypeResolver->resolve($node->var);
-
-        return in_array($type, $calledNodeTypes, true);
-    }
-
     /**
      * @param string[] $types
      * @return string[]
@@ -115,4 +118,18 @@ final class MethodCallAnalyzer
 
         return array_intersect($nodeTypes, $types) ? $nodeTypes : null;
     }
+
+    /**
+     * @param string[] $types
+     */
+    private function isTypes(Node $node, array $types): bool
+    {
+        if (! $node instanceof MethodCall) {
+            return false;
+        }
+
+        $calledNodeTypes = $this->nodeTypeResolver->resolve($node->var);
+
+        return (bool) array_intersect($types, $calledNodeTypes);
+    }
 }