Merge pull request #41 from tomphp/fix/extract-method-from-block

Extract method works properly with code indented more than one level

Author: beberlei

features/extract_method.feature

@@ -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";
+            +    }
+             }
+
+            """

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

@@ -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;
@@ -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);

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

@@ -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.
      *
@@ -75,7 +84,6 @@ public function changeToken($originalLine, $oldToken, $newToken)
         );
     }
 
-
     /**
      * Append new lines to an original line of the file.
      *

src/main/QafooLabs/Refactoring/Domain/Model/EditingAction/ReplaceWithMethodCall.php

@@ -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
 {
@@ -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()

src/main/QafooLabs/Refactoring/Domain/Model/EditorBuffer.php

@@ -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.
      *

src/test/QafooLabs/Refactoring/Adapters/PatchBuilder/PatchBuilderTest.php

@@ -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)
+        );
+    }
 }

src/test/QafooLabs/Refactoring/Domain/Model/EditingAction/ReplaceWithMethodCallTest.php

@@ -39,6 +39,8 @@ public function testBufferReplacesAtGivenRange()
              ->method('replace')
              ->with($this->equalTo($range), $this->anything());
 
+        $this->setCodeBeingReplaced();
+
         $action->performEdit($this->buffer);
     }
 
@@ -49,6 +51,8 @@ public function testMethodCallIsCorrectForSimpleMethod()
             new MethodSignature('testMethod')
         );
 
+        $this->setCodeBeingReplaced();
+
         $this->assertGeneratedMethodCallMatches('$this->testMethod();', $action);
     }
 
@@ -59,6 +63,8 @@ public function testMethodCallUsesGivenMethodName()
             new MethodSignature('realMethod')
         );
 
+        $this->setCodeBeingReplaced();
+
         $this->assertGeneratedMethodCallMatches('$this->realMethod();', $action);
     }
 
@@ -69,6 +75,8 @@ public function testStaticMethodCall()
             new MethodSignature('testMethod', MethodSignature::IS_STATIC)
         );
 
+        $this->setCodeBeingReplaced();
+
         $this->assertGeneratedMethodCallMatches('self::testMethod();', $action);
     }
 
@@ -79,6 +87,8 @@ public function testMethodCallWithSingleReturnVariable()
             new MethodSignature('testMethod', 0, array(), array('result'))
         );
 
+        $this->setCodeBeingReplaced();
+
         $this->assertGeneratedMethodCallMatches('$result = $this->testMethod();', $action);
     }
 
@@ -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
@@ -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())

src/test/QafooLabs/Refactoring/Domain/Model/EditingSessionTest.php

@@ -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;