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

Some tidying to the fix class names code #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
109 changes: 55 additions & 54 deletions features/fix_class_names.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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 @@
Expand All @@ -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:
Expand All @@ -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 @@
Expand All @@ -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"
Expand All @@ -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 @@
Expand All @@ -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:
Expand All @@ -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 @@
Expand All @@ -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:
Expand All @@ -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
{
}

Expand All @@ -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
{
}

Expand All @@ -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;
Expand All @@ -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"
Expand All @@ -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 @@
Expand All @@ -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();
"""

147 changes: 106 additions & 41 deletions src/main/QafooLabs/Refactoring/Application/FixClassNames.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}