Some tidying to the fix class names code

Author: Tom Oram

features/fix_class_names.feature

@@ -75,6 +75,13 @@ Feature: Fix Class Names
             | dir   | src/  |
         Then the PHP File "src/Foo.php" should be refactored:
             """
+            --- a/vfs://project/src/Foo.php
+            +++ b/vfs://project/src/Foo.php
+            @@ -1,2 +1,2 @@
+             <?php
+            -use Foo\Foo;
+            +use Foo\Bar;
+
             --- a/vfs://project/src/src/Foo/Bar.php
             +++ b/vfs://project/src/src/Foo/Bar.php
             @@ -1,6 +1,6 @@
@@ -85,13 +92,6 @@ Feature: Fix Class Names
             +class Bar
              {
              }
-
-            --- a/vfs://project/src/Foo.php
-            +++ b/vfs://project/src/Foo.php
-            @@ -1,2 +1,2 @@
-             <?php
-            -use Foo\Foo;
-            +use Foo\Bar;
             """
     Scenario: "Namespace moved changes use statements"
         Given a PHP File named "src/Foo/Bar.php" with:
@@ -113,6 +113,13 @@ Feature: Fix Class Names
             | dir   | src/  |
         Then the PHP File "src/Foo.php" should be refactored:
             """
+            --- a/vfs://project/src/Foo.php
+            +++ b/vfs://project/src/Foo.php
+            @@ -1,2 +1,2 @@
+             <?php
+            -use Bar\Bar;
+            +use Foo\Bar;
+
             --- a/vfs://project/src/src/Foo/Bar.php
             +++ b/vfs://project/src/src/Foo/Bar.php
             @@ -1,5 +1,5 @@
@@ -122,13 +129,6 @@ Feature: Fix Class Names
 
              class Bar
              {
-
-            --- a/vfs://project/src/Foo.php
-            +++ b/vfs://project/src/Foo.php
-            @@ -1,2 +1,2 @@
-             <?php
-            -use Bar\Bar;
-            +use Foo\Bar;
             """
 
     Scenario: "Rename class changes static occurances"
@@ -151,6 +151,13 @@ Feature: Fix Class Names
             | dir   | src/  |
         Then the PHP File "src/Foo.php" should be refactored:
             """
+            --- a/vfs://project/src/Foo.php
+            +++ b/vfs://project/src/Foo.php
+            @@ -1,2 +1,2 @@
+             <?php
+            -Foo\Foo::bar();
+            +Foo\Bar::bar();
+
             --- a/vfs://project/src/src/Foo/Bar.php
             +++ b/vfs://project/src/src/Foo/Bar.php
             @@ -1,6 +1,6 @@
@@ -161,13 +168,6 @@ Feature: Fix Class Names
             +class Bar
              {
              }
-
-            --- a/vfs://project/src/Foo.php
-            +++ b/vfs://project/src/Foo.php
-            @@ -1,2 +1,2 @@
-             <?php
-            -Foo\Foo::bar();
-            +Foo\Bar::bar();
             """
     Scenario: "Rename class changes new instantiations"
         Given a PHP File named "src/Foo/Bar.php" with:
@@ -189,6 +189,13 @@ Feature: Fix Class Names
             | dir   | src/  |
         Then the PHP File "src/Foo.php" should be refactored:
             """
+            --- a/vfs://project/src/Foo.php
+            +++ b/vfs://project/src/Foo.php
+            @@ -1,2 +1,2 @@
+             <?php
+            -new Foo\Foo();
+            +new Foo\Bar();
+
             --- a/vfs://project/src/src/Foo/Bar.php
             +++ b/vfs://project/src/src/Foo/Bar.php
             @@ -1,6 +1,6 @@
@@ -200,12 +207,6 @@ Feature: Fix Class Names
              {
              }
 
-            --- a/vfs://project/src/Foo.php
-            +++ b/vfs://project/src/Foo.php
-            @@ -1,2 +1,2 @@
-             <?php
-            -new Foo\Foo();
-            +new Foo\Bar();
             """
     Scenario: "Rename class changes that is extended"
         Given a PHP File named "src/Foo/Bar.php" with:
@@ -231,14 +232,14 @@ Feature: Fix Class Names
             | dir   | src/  |
         Then the PHP File "src/Foo.php" should be refactored:
             """
-            --- a/vfs://project/src/src/Foo/Bar.php
-            +++ b/vfs://project/src/src/Foo/Bar.php
+            --- a/vfs://project/src/Foo/Baz.php
+            +++ b/vfs://project/src/Foo/Baz.php
             @@ -1,6 +1,6 @@
-            <?php
-            namespace Foo;
+             <?php
+             namespace Foo;
 
-            -class Foo
-            +class Bar
+            -class Baz extends Foo
+            +class Baz extends Bar
              {
              }
 
@@ -253,14 +254,14 @@ Feature: Fix Class Names
              {
              }
 
-            --- a/vfs://project/src/Foo/Baz.php
-            +++ b/vfs://project/src/Foo/Baz.php
+            --- a/vfs://project/src/src/Foo/Bar.php
+            +++ b/vfs://project/src/src/Foo/Bar.php
             @@ -1,6 +1,6 @@
-             <?php
-             namespace Foo;
+            <?php
+            namespace Foo;
 
-            -class Baz extends Foo
-            +class Baz extends Bar
+            -class Foo
+            +class Bar
              {
              }
 
@@ -285,9 +286,16 @@ Feature: Fix Class Names
             | dir   | src/  |
         Then the PHP File "src/Foo.php" should be refactored:
             """
+            --- a/vfs://project/src/index.php
+            +++ b/vfs://project/src/index.php
+            @@ -1,2 +1,2 @@
+             <?php
+            -$foo = new \Foo\Foo\Foo();
+            +$foo = new \Foo\Bar\Baz\Boing();
+
             --- a/vfs://project/src/src/Foo/Bar/Baz/Boing.php
             +++ b/vfs://project/src/src/Foo/Bar/Baz/Boing.php
-           @@ -1,6 +1,6 @@
+            @@ -1,6 +1,6 @@
              <?php
             -namespace Foo\Foo;
             +namespace Foo\Bar\Baz;
@@ -296,13 +304,6 @@ Feature: Fix Class Names
             +class Boing
              {
              }
-
-            --- a/vfs://project/src/index.php
-            +++ b/vfs://project/src/index.php
-            @@ -1,2 +1,2 @@
-             <?php
-            -$foo = new \Foo\Foo\Foo();
-            +$foo = new \Foo\Bar\Baz\Boing();
             """
 
     Scenario: "Removing a slice of a namespace"
@@ -325,6 +326,13 @@ Feature: Fix Class Names
             | dir   | src/  |
         Then the PHP File "src/Foo.php" should be refactored:
             """
+            --- a/vfs://project/src/index.php
+            +++ b/vfs://project/src/index.php
+            @@ -1,2 +1,2 @@
+             <?php
+            -$foo = new \Foo\Foo\Foo();
+            +$foo = new \Foo\Boing();
+
             --- a/vfs://project/src/src/Foo/Boing.php
             +++ b/vfs://project/src/src/Foo/Boing.php
             @@ -1,6 +1,6 @@
@@ -336,12 +344,5 @@ Feature: Fix Class Names
             +class Boing
              {
              }
-
-            --- a/vfs://project/src/index.php
-            +++ b/vfs://project/src/index.php
-            @@ -1,2 +1,2 @@
-             <?php
-            -$foo = new \Foo\Foo\Foo();
-            +$foo = new \Foo\Boing();
             """
 

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

@@ -19,78 +19,143 @@
 use QafooLabs\Refactoring\Domain\Model\PhpName;
 use QafooLabs\Refactoring\Domain\Model\PhpNameChange;
 use QafooLabs\Refactoring\Domain\Model\PhpNames\NoImportedUsagesFilter;
+use QafooLabs\Refactoring\Domain\Services\CodeAnalysis;
+use QafooLabs\Refactoring\Domain\Services\Editor;
+use QafooLabs\Refactoring\Adapters\PHPParser\ParserPhpNameScanner;
+use QafooLabs\Refactoring\Domain\Model\PhpNameOccurance;
 
 class FixClassNames
 {
+    /**
+     * @var CodeAnalysis
+     */
     private $codeAnalysis;
+
+    /**
+     * @var Editor
+     */
     private $editor;
+
+    /**
+     * @var ParserPhpNameScanner
+     */
     private $nameScanner;
 
-    public function __construct($codeAnalysis, $editor, $nameScanner)
+    /**
+     * @var Set
+     */
+    private $renames;
+
+
+    public function __construct(CodeAnalysis $codeAnalysis, Editor $editor, ParserPhpNameScanner $nameScanner)
     {
         $this->codeAnalysis = $codeAnalysis;
         $this->editor = $editor;
         $this->nameScanner = $nameScanner;
     }
 
+
     public function refactor(Directory $directory)
     {
         $phpFiles = $directory->findAllPhpFilesRecursivly();
 
-        $renames = new Set();
-        $occurances = array();
-        $noImportedUsages = new NoImportedUsagesFilter();
+        $this->renames = new Set();
 
         foreach ($phpFiles as $phpFile) {
-            $classes = $this->codeAnalysis->findClasses($phpFile);
+            $this->checkIfRenameIsRequired($phpFile);
+        }
 
-            $occurances = array_merge(
-                $noImportedUsages->filter($this->nameScanner->findNames($phpFile)),
-                $occurances
-            );
+        $occurances = $this->findOccurances($phpFiles);
 
-            if (count($classes) !== 1) {
-                continue;
-            }
+        foreach ($occurances as $occurance) {
+            $this->performRename($occurance);
+        }
 
-            $class = $classes[0];
-            $currentClassName = $class->declarationName();
-            $expectedClassName = $phpFile->extractPsr0ClassName();
+        $this->editor->save();
+    }
 
-            $buffer = $this->editor->openBuffer($phpFile); // This is weird to be required here
 
-            if ($expectedClassName->shortName() !== $currentClassName->shortName()) {
-                $renames->add(new PhpNameChange($currentClassName, $expectedClassName));
-            }
+    private function checkIfRenameIsRequired(File $phpFile)
+    {
+        $classes = $this->codeAnalysis->findClasses($phpFile);
+
+        // Why skip for multiple classes in a file?
+        if (count($classes) !== 1) {
+            return;
+        }
 
-            if (!$expectedClassName->namespaceName()->equals($currentClassName->namespaceName())) {
-                $renames->add(new PhpNameChange($currentClassName->fullyQualified(), $expectedClassName->fullyQualified()));
+        $class = $classes[0];
 
-                $buffer->replaceString(
-                    $class->namespaceDeclarationLine(),
-                    $currentClassName->namespaceName()->fullyQualifiedName(),
-                    $expectedClassName->namespaceName()->fullyQualifiedName()
-                );
-            }
+        $currentClassName = $class->declarationName();
+        $expectedClassName = $phpFile->extractPsr0ClassName();
+
+        if ($this->shortNameHasChanged($expectedClassName, $currentClassName)) {
+            // Queue a rename to happen in the next loop
+            $this->renames->add(new PhpNameChange($currentClassName, $expectedClassName));
         }
 
-        $occurances = array_filter($occurances, function ($occurance) {
-            return $occurance->name()->type() !== PhpName::TYPE_NAMESPACE;
-        });
+        if ($this->namespaceHasChanged($expectedClassName, $currentClassName)) {
+            $this->renames->add(new PhpNameChange($currentClassName->fullyQualified(), $expectedClassName->fullyQualified()));
+        }
+    }
 
-        foreach ($occurances as $occurance) {
-            $name = $occurance->name();
-
-            foreach ($renames as $rename) {
-                if ($rename->affects($name)) {
-                    $buffer = $this->editor->openBuffer($occurance->file());
-                    $buffer->replaceString($occurance->declarationLine(), $name->relativeName(), $rename->change($name)->relativeName());
-                    continue 2;
-                }
+    /**
+     * @return boolean
+     */
+    private function shortNameHasChanged(PhpName $expectedClassName, PhpName $currentClassName)
+    {
+        return $expectedClassName->shortName() !== $currentClassName->shortName();
+    }
+
+    /**
+     * @return boolean
+     */
+    private function namespaceHasChanged(PhpName $expectedClassName, PhpName $currentClassName)
+    {
+        return !$expectedClassName->namespaceName()->equals($currentClassName->namespaceName());
+    }
+
+    private function performRename(PhpNameOccurance $occurance)
+    {
+        $name = $occurance->name();
+
+        foreach ($this->renames as $rename) {
+            if (!$rename->affects($name)) {
+                continue;
             }
+
+            $buffer = $this->editor->openBuffer($occurance->file());
+
+            $buffer->replaceString(
+                $occurance->declarationLine(),
+                $name->relativeName(),
+                $rename->change($name)->relativeName()
+            );
+
+            // Why is a contnue required? Surely 2 renames can't apply
+            // to the same occurance?
+            break;
         }
+    }
 
-        $this->editor->save();
+    /**
+     * @param File[] $phpFiles
+     *
+     * @return PhpNameOccurance[]
+     */
+    private function findOccurances($phpFiles)
+    {
+        $occurances = array();
+
+        $noImportedUsages = new NoImportedUsagesFilter();
+
+        foreach ($phpFiles as $phpFile) {
+            $occurances = array_merge(
+                $noImportedUsages->filter($this->nameScanner->findNames($phpFile)),
+                $occurances
+            );
+        }
+
+        return $occurances;
     }
 }
-