fix get request for GetRequestRector [closes #594]

README.md

@@ -1,6 +1,6 @@
 # Rector - Upgrade your Legacy App to Modern Codebase
 
-Rector is a **rec**onstruc**tor** tool - it does **instant upgrades** and **instant refactoring** of your code. 
+Rector is a **rec**onstruc**tor** tool - it does **instant upgrades** and **instant refactoring** of your code.
 I mean, why do it manually if 80 % can Rector handle for you?
 
 [![Build Status](https://img.shields.io/travis/rectorphp/rector/master.svg?style=flat-square)](https://travis-ci.org/rectorphp/rector)

composer.json

@@ -64,6 +64,7 @@
         },
         "classmap": [
             "packages",
+            "tests/Issues",
             "tests/Rector"
         ]
     },

packages/Symfony/src/Bridge/NodeAnalyzer/ControllerMethodAnalyzer.php

@@ -20,13 +20,17 @@ final class ControllerMethodAnalyzer
         }
 
         $parentClassName = $node->getAttribute(Attribute::PARENT_CLASS_NAME);
-        if (! Strings::endsWith($parentClassName, 'Controller')) {
-            return false;
+
+        if (Strings::endsWith($parentClassName, 'Controller')) {
+            return true;
         }
 
         /** @var Identifier $identifierNode */
         $identifierNode = $node->name;
+        if (Strings::endsWith($identifierNode->toString(), 'Action')) {
+            return true;
+        }
 
-        return Strings::endsWith($identifierNode->toString(), 'Action');
+        return $node->isPublic();
     }
 }

packages/Symfony/src/Rector/HttpKernel/GetRequestRector.php

@@ -4,6 +4,7 @@ namespace Rector\Symfony\Rector\HttpKernel;
 
 use PhpParser\Node;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Scalar\String_;
 use PhpParser\Node\Stmt\ClassMethod;
 use Rector\Node\NodeFactory;
 use Rector\NodeAnalyzer\MethodCallAnalyzer;
@@ -114,19 +115,64 @@ CODE_SAMPLE
             return false;
         }
 
-        return (bool) $this->betterNodeFinder->find($node, function (Node $node) {
+        // "$this->getRequest()"
+        $isGetRequestMethod = (bool) $this->betterNodeFinder->find($node, function (Node $node) {
             return $this->methodCallAnalyzer->isMethod($node, 'getRequest');
         });
+
+        if ($isGetRequestMethod) {
+            return true;
+        }
+
+        // "$this->get('request')"
+        /** @var MethodCall[] $getMethodCalls */
+        $getMethodCalls = $this->betterNodeFinder->find($node, function (Node $node) {
+            return $this->methodCallAnalyzer->isMethod($node, 'get');
+        });
+
+        foreach ($getMethodCalls as $getMethodCall) {
+            if ($this->isGetMethodCallWithRequestParameters($getMethodCall)) {
+                return true;
+            }
+        }
+
+        return false;
     }
 
     private function isGetRequestInAction(Node $node): bool
     {
-        if (! $this->methodCallAnalyzer->isMethod($node, 'getRequest')) {
+        if (! $node instanceof MethodCall) {
+            return false;
+        }
+
+        if (! $this->methodCallAnalyzer->isMethods(
+            $node,
+            ['getRequest']
+        ) && ! $this->isGetMethodCallWithRequestParameters($node)) {
             return false;
         }
 
         $methodNode = $node->getAttribute(Attribute::METHOD_NODE);
-
         return $this->controllerMethodAnalyzer->isAction($methodNode);
     }
+
+    private function isGetMethodCallWithRequestParameters(MethodCall $methodCall): bool
+    {
+        if ((string) $methodCall->name !== 'get') {
+            return false;
+        }
+
+        if (count($methodCall->args) !== 1) {
+            return false;
+        }
+
+        if (! $methodCall->args[0]->value instanceof String_) {
+            return false;
+        }
+
+        /** @var String_ $stringValue */
+        $stringValue = $methodCall->args[0]->value;
+
+        return $stringValue->value === 'request';
+    }
 }

tests/Issues/Issue594/Correct/correct594.php

@@ -0,0 +1,13 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Tests\Issues\Issue594\Wrong;
+
+use Rector\Symfony\Tests\Rector\Source\AbstractSymfonyController;
+
+class SomeController extends AbstractSymfonyController
+{
+    public function action(\Symfony\Component\HttpFoundation\Request $request)
+    {
+        $request = $request;
+    }
+}

tests/Issues/Issue594/Issue594Test.php

@@ -0,0 +1,27 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Tests\Issues\Issue594;
+
+use Iterator;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class Issue594Test extends AbstractRectorTestCase
+{
+    /**
+     * @dataProvider provideWrongToFixedFiles()
+     */
+    public function test(string $wrong, string $fixed): void
+    {
+        $this->doTestFileMatchesExpectedContent($wrong, $fixed);
+    }
+
+    public function provideWrongToFixedFiles(): Iterator
+    {
+        yield [__DIR__ . '/Wrong/wrong594.php', __DIR__ . '/Correct/correct594.php'];
+    }
+
+    protected function provideConfig(): string
+    {
+        return __DIR__ . '/config594.yml';
+    }
+}

tests/Issues/Issue594/Wrong/wrong594.php

@@ -0,0 +1,13 @@
+<?php declare(strict_types=1);
+
+namespace Rector\Tests\Issues\Issue594\Wrong;
+
+use Rector\Symfony\Tests\Rector\Source\AbstractSymfonyController;
+
+class SomeController extends AbstractSymfonyController
+{
+    public function action()
+    {
+        $request = $this->get('request');
+    }
+}

tests/Issues/Issue594/config594.yml

@@ -0,0 +1,2 @@
+services:
+    Rector\Symfony\Rector\HttpKernel\GetRequestRector: ~