Allow convert local variable to instance variable to work with function arguments

Author: Tom Oram

composer.lock

@@ -46,3 +46,44 @@ Feature: Convert Local to Instance Variable
              }
             """
 
+    Scenario: Convert Argument Variable
+        Given a PHP File named "src/FooWithArgument.php" with:
+            """
+            <?php
+            class FooWithArgument
+            {
+                public function operation($service)
+                {
+                    $service->operation();
+
+                    return $service;
+                }
+            }
+            """
+        When I use refactoring "convert-local-to-instance-variable" with:
+            | arg       | value       |
+            | file      | src/FooWithArgument.php |
+            | line      | 6           |
+            | variable  | service     |
+        Then the PHP File "src/FooWithArgument.php" should be refactored:
+            """
+            --- a/vfs://project/src/FooWithArgument.php
+            +++ b/vfs://project/src/FooWithArgument.php
+            @@ -1,10 +1,14 @@
+             <?php
+             class FooWithArgument
+             {
+            +    private $service;
+            +
+                 public function operation($service)
+                 {
+            -        $service->operation();
+            +        $this->service = $service;
+             
+            -        return $service;
+            +        $this->service->operation();
+            +
+            +        return $this->service;
+                 }
+             }
+            """

features/convert_local_to_instance_variable.feature

@@ -86,10 +86,7 @@ public function changeToken($originalLine, $oldToken, $newToken)
      */
     public function appendToLine($originalLine, array $lines)
     {
-        // Why is this one method different to the rest?
-        $originalLine++;
-
-        $this->buffer->insert($this->createLineNumber($originalLine), $lines);
+        $this->buffer->append($this->createLineNumber($originalLine), $lines);
     }
 
     /**

src/main/QafooLabs/Refactoring/Adapters/PatchBuilder/PatchBuilder.php

@@ -22,6 +22,7 @@
 
 use TokenReflection\Broker;
 use TokenReflection\Broker\Backend\Memory;
+use QafooLabs\Refactoring\Domain\Model\RefactoringException;
 
 class StaticCodeAnalysis extends CodeAnalysis
 {
@@ -90,6 +91,17 @@ public function isInsideMethod(File $file, LineRange $range)
         return $this->findMatchingMethod($file, $range) !== null;
     }
 
+    public function getMethod(File $file, LineRange $range)
+    {
+        $method = $this->findMatchingMethod($file, $range);
+
+        if ($method === null) {
+            throw RefactoringException::rangeIsNotInsideMethod($range);
+        }
+
+        return $method;
+    }
+
     /**
      * @param File $file
      * @return PhpClass[]

src/main/QafooLabs/Refactoring/Adapters/TokenReflection/StaticCodeAnalysis.php

@@ -13,16 +13,24 @@
 use QafooLabs\Refactoring\Domain\Services\VariableScanner;
 use QafooLabs\Refactoring\Domain\Services\CodeAnalysis;
 use QafooLabs\Refactoring\Domain\Services\Editor;
+use QafooLabs\Refactoring\Domain\Model\EditingAction\AddAssignment;
 use QafooLabs\Refactoring\Domain\Model\EditingAction\AddProperty;
 use QafooLabs\Refactoring\Domain\Model\EditingAction\LocalVariableToInstance;
 
+use TokenReflection\ReflectionMethod;
+
 class ConvertLocalToInstanceVariable extends SingleFileRefactoring
 {
     /**
      * @var Variable
      */
     private $convertVariable;
 
+    /**
+     * @var ReflectionMethod
+     */
+    private $method;
+
     /**
      * @param int $line
      */
@@ -32,11 +40,19 @@ public function refactor(File $file, $line, Variable $convertVariable)
         $this->line = $line;
         $this->convertVariable = $convertVariable;
 
+        $this->method = $this->codeAnalysis->getMethod($this->file, LineRange::fromSingleLine($line));
+
         $this->assertIsInsideMethod();
 
         $this->startEditingSession();
+
         $this->addProperty();
         $this->convertVariablesToInstanceVariables();
+
+        if ($this->variableIsMethodParameter()) {
+            $this->assignArgumentVariableToInstanceVariable();
+        }
+
         $this->completeEditingSession();
     }
 
@@ -51,12 +67,55 @@ private function addProperty()
 
     private function convertVariablesToInstanceVariables()
     {
+        $range = $this->getMethodBodyRange();
+
         $definedVariables = $this->getDefinedVariables();
 
+        $this->assertVariableIsDefiniedInScope($range, $definedVariables);
+
+        $this->session->addEdit(new LocalVariableToInstance($definedVariables, $this->convertVariable));
+    }
+
+    private function variableIsMethodParameter()
+    {
+        return in_array($this->convertVariable->getName(), array_map(function ($parameter) {
+            return $parameter->getName();
+        }, $this->method->getParameters()));
+    }
+
+    private function assignArgumentVariableToInstanceVariable()
+    {
+        $instanceVariable = $this->convertVariable->convertToInstance();
+
+        $line = $this->method->getStartLine() + 1;
+
+        // The +1 assumes that the function definition is followed by a newline with the
+        // opening brace. Ideally this needs to be detected.
+        $this->session->addEdit(new AddAssignment($line, $instanceVariable, $this->convertVariable->getToken()));
+    }
+
+    protected function getDefinedVariables()
+    {
+        $selectedMethodLineRange = $this->getMethodBodyRange();
+
+        $definedVariables = $this->variableScanner->scanForVariables(
+            $this->file, $selectedMethodLineRange
+        );
+
+        return $definedVariables;
+    }
+
+    private function getMethodBodyRange()
+    {
+        $methodLineRange = $this->codeAnalysis->findMethodRange($this->file, LineRange::fromSingleLine($this->line));
+
+        return LineRange::fromLines($methodLineRange->getStart() + 1, $methodLineRange->getEnd());
+    }
+
+    private function assertVariableIsDefiniedInScope(LineRange $selectedMethodLineRange, DefinedVariables $definedVariables)
+    {
         if ( ! $definedVariables->contains($this->convertVariable)) {
             throw RefactoringException::variableNotInRange($this->convertVariable, $selectedMethodLineRange);
         }
-
-        $this->session->addEdit(new LocalVariableToInstance($definedVariables, $this->convertVariable));
     }
 }

src/main/QafooLabs/Refactoring/Application/ConvertLocalToInstanceVariable.php

@@ -0,0 +1,44 @@
+<?php
+
+namespace QafooLabs\Refactoring\Domain\Model\EditingAction;
+
+use QafooLabs\Refactoring\Domain\Model\EditingAction;
+use QafooLabs\Refactoring\Domain\Model\EditorBuffer;
+use QafooLabs\Refactoring\Domain\Model\Variable;
+
+class AddAssignment implements EditingAction
+{
+    /**
+     * @var int
+     */
+    private $line;
+
+    /**
+     * @var Variable
+     */
+    private $lhs;
+
+    /**
+     * @var string
+     */
+    private $rhs;
+
+    /**
+     * @param int    $line
+     * @param string $rhs
+     */
+    public function __construct($line, Variable $lhs, $rhs)
+    {
+        $this->line = $line;
+        $this->lhs  = $lhs;
+        $this->rhs  = $rhs;
+    }
+
+    public function performEdit(EditorBuffer $buffer)
+    {
+        $buffer->append($this->line, array(
+            '        ' . $this->lhs->getToken() . ' = ' . $this->rhs . ';',
+            ''
+        ));
+    }
+}