Skip to content
This repository was archived by the owner on Jul 12, 2020. It is now read-only.

Extract method works properly with code indented more than one level #41

Merged
merged 1 commit into from
Apr 14, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions features/extract_method.feature
Original file line number Diff line number Diff line change
Expand Up @@ -265,3 +265,43 @@ Feature: Extract Method
}
}
"""

Scenario: Extract method from inside a block
Given a PHP File named "src/ExtractMethodFromBlock.php" with:
"""
<?php
class ExtractMethodFromBlock
{
public function operation()
{
for ($i=0; $i<5; $i++) {
echo "Hello World";
}
}
}
"""
When I use refactoring "extract-method" with:
| arg | value |
| file | src/ExtractMethodFromBlock.php |
| range | 7-7 |
| newmethod | hello |
Then the PHP File "src/ExtractMethodFromBlock.php" should be refactored:
"""
--- a/vfs://project/src/ExtractMethodFromBlock.php
+++ b/vfs://project/src/ExtractMethodFromBlock.php
@@ -4,7 +4,12 @@
public function operation()
{
for ($i=0; $i<5; $i++) {
- echo "Hello World";
+ $this->hello();
}
}
+
+ private function hello()
+ {
+ echo "Hello World";
+ }
}

"""
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
* to kontakt@beberlei.de so I can send you a copy immediately.
*/


namespace QafooLabs\Refactoring\Adapters\PatchBuilder;

use QafooLabs\Refactoring\Domain\Model\EditorBuffer;
Expand All @@ -30,6 +29,11 @@ public function __construct(PatchBuilder $builder)
$this->builder = $builder;
}

public function getLines(LineRange $range)
{
return $this->builder->getOriginalLines($range->getStart(), $range->getEnd());
}

public function replace(LineRange $range, array $newLines)
{
$this->builder->replaceLines($range->getStart(), $range->getEnd(), $newLines);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ public function __construct($contents, $path = null)
$this->path = $path;
}

public function getOriginalLines($start, $end)
{
return array_slice(
$this->buffer->getOriginalContents(),
$start - 1,
$end - $start + 1
);
}

/**
* Change Token in given line from old to new.
*
Expand All @@ -75,7 +84,6 @@ public function changeToken($originalLine, $oldToken, $newToken)
);
}


/**
* Append new lines to an original line of the file.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
use QafooLabs\Refactoring\Domain\Model\EditorBuffer;
use QafooLabs\Refactoring\Domain\Model\MethodSignature;
use QafooLabs\Refactoring\Domain\Model\LineRange;
use QafooLabs\Refactoring\Domain\Model\LineCollection;
use QafooLabs\Refactoring\Domain\Model\IndentationDetector;

class ReplaceWithMethodCall implements EditingAction
{
Expand All @@ -27,12 +29,23 @@ public function __construct(LineRange $range, MethodSignature $newMethod)

public function performEdit(EditorBuffer $buffer)
{
$buffer->replace($this->range, array($this->getIndent() . $this->getMethodCall()));
$extractedCode = $buffer->getLines($this->range);

$buffer->replace($this->range, array($this->getIndent($extractedCode) . $this->getMethodCall()));
}

private function getIndent()
/**
* @param string[] $lines
*
* @return string
*/
private function getIndent(array $lines)
{
return ' ';
$detector = new IndentationDetector(
LineCollection::createFromArray($lines)
);

return str_repeat(' ', $detector->getFirstLineIndentation());
}

private function getMethodCall()
Expand Down
9 changes: 9 additions & 0 deletions src/main/QafooLabs/Refactoring/Domain/Model/EditorBuffer.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@
*/
interface EditorBuffer
{
/**
* Return the given range of lines from the buffer.
*
* @param LineRange $range
*
* @return string[]
*/
public function getLines(LineRange $range);

/**
* Replace LineRange with new lines.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,4 +200,12 @@ public function testReplaceLines()
DIFF;
$this->assertEquals($expected, $this->builder->generateUnifiedDiff());
}

public function testGetOriginalLines()
{
$this->assertEquals(
array('line4', 'line5', 'line6'),
$this->builder->getOriginalLines(4, 6)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ public function testBufferReplacesAtGivenRange()
->method('replace')
->with($this->equalTo($range), $this->anything());

$this->setCodeBeingReplaced();

$action->performEdit($this->buffer);
}

Expand All @@ -49,6 +51,8 @@ public function testMethodCallIsCorrectForSimpleMethod()
new MethodSignature('testMethod')
);

$this->setCodeBeingReplaced();

$this->assertGeneratedMethodCallMatches('$this->testMethod();', $action);
}

Expand All @@ -59,6 +63,8 @@ public function testMethodCallUsesGivenMethodName()
new MethodSignature('realMethod')
);

$this->setCodeBeingReplaced();

$this->assertGeneratedMethodCallMatches('$this->realMethod();', $action);
}

Expand All @@ -69,6 +75,8 @@ public function testStaticMethodCall()
new MethodSignature('testMethod', MethodSignature::IS_STATIC)
);

$this->setCodeBeingReplaced();

$this->assertGeneratedMethodCallMatches('self::testMethod();', $action);
}

Expand All @@ -79,6 +87,8 @@ public function testMethodCallWithSingleReturnVariable()
new MethodSignature('testMethod', 0, array(), array('result'))
);

$this->setCodeBeingReplaced();

$this->assertGeneratedMethodCallMatches('$result = $this->testMethod();', $action);
}

Expand All @@ -89,6 +99,8 @@ public function testMethodCallWithMultipleReturnVariables()
new MethodSignature('testMethod', 0, array(), array('result1', 'result2'))
);

$this->setCodeBeingReplaced();

$this->assertGeneratedMethodCallMatches(
'list($result1, $result2) = $this->testMethod();',
$action
Expand All @@ -102,15 +114,67 @@ public function testMethodCallWithArguments()
new MethodSignature('testMethod', 0, array('arg1', 'arg2'))
);

$this->setCodeBeingReplaced();

$this->assertGeneratedMethodCallMatches(
'$this->testMethod($arg1, $arg2);',
$action
);
}

private function assertGeneratedMethodCallMatches($expected, $action)
public function testExtractedRangeIsReadFromTheBuffer()
{
$range = LineRange::fromLines(1, 2);

$this->buffer
->expects($this->once())
->method('getLines')
->with($this->equalTo($range))
->will($this->returnValue(array()));

$action = new ReplaceWithMethodCall(
$range,
new MethodSignature('testMethod')
);

$action->performEdit($this->buffer);
}

public function testExtractRangeIndentsMethodCallForFirstLineWithExtraIndent()
{
$lines = array(
' echo "Something";',
);

$this->buffer
->expects($this->once())
->method('getLines')
->will($this->returnValue($lines));

$action = new ReplaceWithMethodCall(
LineRange::fromLines(1, 2),
new MethodSignature('testMethod')
);

$this->assertGeneratedMethodCallMatches(
'$this->testMethod();',
$action,
12
);
}

private function setCodeBeingReplaced(
array $lines = array(' echo "Replace me";')
) {
$this->buffer
->expects($this->any())
->method('getLines')
->will($this->returnValue($lines));
}

private function assertGeneratedMethodCallMatches($expected, $action, $indentSize = 8)
{
$expected = ' ' . $expected;
$expected = str_repeat(' ', $indentSize) . $expected;

$this->buffer
->expects($this->once())
Expand Down
11 changes: 11 additions & 0 deletions src/test/QafooLabs/Refactoring/Domain/Model/EditingSessionTest.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
<?php
/**
* Qafoo PHP Refactoring Browser
*
* LICENSE
*
* This source file is subject to the MIT license that is bundled
* with this package in the file LICENSE.txt.
* If you did not receive a copy of the license and are unable to
* obtain it through the world-wide-web, please send an email
* to kontakt@beberlei.de so I can send you a copy immediately.
*/

namespace QafooLabs\Refactoring\Domain\Model;

Expand Down