diff --git a/features/convert_local_to_instance_variable.feature b/features/convert_local_to_instance_variable.feature index eb5aaa2..effb2ee 100644 --- a/features/convert_local_to_instance_variable.feature +++ b/features/convert_local_to_instance_variable.feature @@ -46,3 +46,44 @@ Feature: Convert Local to Instance Variable } """ + Scenario: Convert Argument Variable + Given a PHP File named "src/FooWithArgument.php" with: + """ + 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 @@ + operation(); + + $this->service = $service; + + - return $service; + + $this->service->operation(); + + + + return $this->service; + } + } + """ diff --git a/src/main/QafooLabs/Refactoring/Adapters/PatchBuilder/PatchBuilder.php b/src/main/QafooLabs/Refactoring/Adapters/PatchBuilder/PatchBuilder.php index 4da855e..ef8cb7e 100644 --- a/src/main/QafooLabs/Refactoring/Adapters/PatchBuilder/PatchBuilder.php +++ b/src/main/QafooLabs/Refactoring/Adapters/PatchBuilder/PatchBuilder.php @@ -94,10 +94,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); } /** diff --git a/src/main/QafooLabs/Refactoring/Adapters/TokenReflection/StaticCodeAnalysis.php b/src/main/QafooLabs/Refactoring/Adapters/TokenReflection/StaticCodeAnalysis.php index c766693..d5484b0 100644 --- a/src/main/QafooLabs/Refactoring/Adapters/TokenReflection/StaticCodeAnalysis.php +++ b/src/main/QafooLabs/Refactoring/Adapters/TokenReflection/StaticCodeAnalysis.php @@ -14,11 +14,12 @@ namespace QafooLabs\Refactoring\Adapters\TokenReflection; -use QafooLabs\Refactoring\Domain\Services\CodeAnalysis; -use QafooLabs\Refactoring\Domain\Model\LineRange; use QafooLabs\Refactoring\Domain\Model\File; +use QafooLabs\Refactoring\Domain\Model\LineRange; use QafooLabs\Refactoring\Domain\Model\PhpClass; use QafooLabs\Refactoring\Domain\Model\PhpName; +use QafooLabs\Refactoring\Domain\Model\RefactoringException; +use QafooLabs\Refactoring\Domain\Services\CodeAnalysis; use TokenReflection\Broker; use TokenReflection\Broker\Backend\Memory; @@ -91,6 +92,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[] diff --git a/src/main/QafooLabs/Refactoring/Application/ConvertLocalToInstanceVariable.php b/src/main/QafooLabs/Refactoring/Application/ConvertLocalToInstanceVariable.php index 5c7e925..75848ab 100644 --- a/src/main/QafooLabs/Refactoring/Application/ConvertLocalToInstanceVariable.php +++ b/src/main/QafooLabs/Refactoring/Application/ConvertLocalToInstanceVariable.php @@ -13,9 +13,12 @@ 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 { /** @@ -23,6 +26,11 @@ class ConvertLocalToInstanceVariable extends SingleFileRefactoring */ 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)); } } diff --git a/src/main/QafooLabs/Refactoring/Domain/Model/EditingAction/AddAssignment.php b/src/main/QafooLabs/Refactoring/Domain/Model/EditingAction/AddAssignment.php new file mode 100644 index 0000000..cc59e3b --- /dev/null +++ b/src/main/QafooLabs/Refactoring/Domain/Model/EditingAction/AddAssignment.php @@ -0,0 +1,44 @@ +line = $line; + $this->lhs = $lhs; + $this->rhs = $rhs; + } + + public function performEdit(EditorBuffer $buffer) + { + $buffer->append($this->line, array( + ' ' . $this->lhs->getToken() . ' = ' . $this->rhs . ';', + '' + )); + } +} diff --git a/src/test/QafooLabs/Refactoring/Domain/Model/EditingAction/AddAssignementTest.php b/src/test/QafooLabs/Refactoring/Domain/Model/EditingAction/AddAssignementTest.php new file mode 100644 index 0000000..b22bff2 --- /dev/null +++ b/src/test/QafooLabs/Refactoring/Domain/Model/EditingAction/AddAssignementTest.php @@ -0,0 +1,52 @@ +buffer = $this->getMock('QafooLabs\Refactoring\Domain\Model\EditorBuffer'); + } + + public function testItIsAnEditingAction() + { + $this->assertInstanceOf( + 'QafooLabs\Refactoring\Domain\Model\EditingAction', + new AddAssignment(4, new Variable('lhs'), 'rhs') + ); + } + + public function testAssignementIsAppenedAtGivenLine() + { + $line = 27; + + $action = new AddAssignment($line, new Variable(''), ''); + + $this->buffer + ->expects($this->once()) + ->method('append') + ->with($line, $this->anything()); + + $action->performEdit($this->buffer); + } + + public function testPropertyCodeIsCorrect() + { + $action = new AddAssignment(5, new Variable('$this->value'), '$value'); + + $this->buffer + ->expects($this->once()) + ->method('append') + ->with($this->anything(), $this->equalTo(array( + ' $this->value = $value;', + '' + ))); + + $action->performEdit($this->buffer); + } +}