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

Commit bfd9a80

Browse files
beberleibeberlei
committed
[GH-28] Fix bug with relative changes
1 parent 7c43ece commit bfd9a80

File tree

3 files changed

+102
-37
lines changed

3 files changed

+102
-37
lines changed

‎features/fix_class_names.feature

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,3 +287,43 @@ Feature: Fix Class Names
287287
-$foo = new \Foo\Foo\Foo();
288288
+$foo = new \Foo\Bar\Baz\Boing();
289289
"""
290+
291+
Scenario: "Removing a slice of a namespace"
292+
Given a PHP File named "src/Foo/Boing.php" with:
293+
"""
294+
<?php
295+
namespace Foo\Foo;
296+
297+
class Foo
298+
{
299+
}
300+
"""
301+
Given a PHP File named "src/index.php" with:
302+
"""
303+
<?php
304+
$foo = new \Foo\Foo\Foo();
305+
"""
306+
When I use refactoring "fix-class-names" with:
307+
| arg | value |
308+
| dir | src/ |
309+
Then the PHP File "src/Foo.php" should be refactored:
310+
"""
311+
--- a/src/Foo/Boing.php
312+
+++ b/src/Foo/Boing.php
313+
@@ -1,6 +1,6 @@
314+
<?php
315+
-namespace Foo\Foo;
316+
+namespace Foo;
317+
318+
-class Foo
319+
+class Boing
320+
{
321+
}
322+
--- a/index.php
323+
+++ b/index.php
324+
@@ -1,2 +1,2 @@
325+
<?php
326+
-$foo = new \Foo\Foo\Foo();
327+
+$foo = new \Foo\Boing();
328+
"""
329+

‎src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class PhpName implements Hashable
2525

2626
static public function createDeclarationName($fullyQualifiedName)
2727
{
28-
$parts = explode("\\", $fullyQualifiedName);
28+
$parts = self::stringToParts($fullyQualifiedName);
2929

3030
return new PhpName(
3131
$fullyQualifiedName,
@@ -55,12 +55,12 @@ public function isAffectedByChangesTo(PhpName $other)
5555

5656
private function overlaps(PhpName $other)
5757
{
58-
$otherParts = explode("\\", $other->fullyQualifiedName);
59-
$thisParts = explode("\\", $this->fullyQualifiedName);
58+
$otherParts = self::stringToParts($other->fullyQualifiedName);
59+
$thisParts = self::stringToParts($this->fullyQualifiedName);
6060

6161
$otherLength = count($otherParts) - 1;
62-
$otherRelativeLength = count(explode("\\", $other->relativeName));
63-
$thisRelativeStart = count($thisParts) - count(explode("\\", $this->relativeName)) - 1;
62+
$otherRelativeLength = count(self::stringToParts($other->relativeName));
63+
$thisRelativeStart = count($thisParts) - count(self::stringToParts($this->relativeName)) - 1;
6464

6565
$matches = array();
6666

@@ -87,26 +87,29 @@ public function change(PhpName $from, PhpName $to)
8787
return $to;
8888
}
8989

90-
$toParts = explode("\\", $to->fullyQualifiedName);
91-
$thisParts = explode("\\", $this->fullyQualifiedName);
92-
$count = max(count($thisParts), count($toParts));
93-
94-
$newParts = array();
95-
for ($i = 0; $i < $count; $i++) {
96-
if ( ! isset($toParts[$i])) {
97-
$newParts[] = $thisParts[$i];
98-
} else {
99-
$newParts[] = $toParts[$i];
100-
}
101-
}
90+
$newParts = self::stringToParts($to->fullyQualifiedName);
91+
$newParts = $this->appendMissingParts($from, $newParts);
10292

10393
if ($this->fullyQualifiedName === $this->relativeName) {
10494
$relativeNewParts = $newParts;
10595
} else {
10696
$relativeNewParts = array_slice($newParts, -1 * count(explode('\\', $this->relativeName)));
10797
}
10898

109-
return new PhpName(implode('\\', $newParts), implode('\\', $relativeNewParts));
99+
return new PhpName(self::partsToString($newParts), self::partsToString($relativeNewParts));
100+
}
101+
102+
private function appendMissingParts($from, $newParts)
103+
{
104+
$fromParts = self::stringToParts($from->fullyQualifiedName);
105+
$thisParts = self::stringToParts($this->fullyQualifiedName);
106+
$sizeChange = count($fromParts) - count($newParts);
107+
108+
if (count($thisParts) > (count($newParts)+$sizeChange)) {
109+
$newParts = array_merge($newParts, array_slice($thisParts, count($newParts)+$sizeChange));
110+
}
111+
112+
return $newParts;
110113
}
111114

112115
public function fullyQualifiedName()
@@ -116,17 +119,17 @@ public function fullyQualifiedName()
116119

117120
public function namespaceName()
118121
{
119-
$parts = explode("\\", $this->fullyQualifiedName);
122+
$parts = self::stringToParts($this->fullyQualifiedName);
120123
array_pop($parts);
121124

122-
$name = implode("\\", $parts);
125+
$name = self::partsToString($parts);
123126

124127
return new PhpName($name, $name);
125128
}
126129

127130
public function shortName()
128131
{
129-
$parts = explode("\\", $this->fullyQualifiedName);
132+
$parts = self::stringToParts($this->fullyQualifiedName);
130133

131134
return array_pop($parts);
132135
}
@@ -156,4 +159,14 @@ public function fullyQualified()
156159
{
157160
return new PhpName($this->fullyQualifiedName, $this->fullyQualifiedName);
158161
}
162+
163+
static private function partsToString($parts)
164+
{
165+
return implode('\\', $parts);
166+
}
167+
168+
static private function stringToParts($string)
169+
{
170+
return explode('\\', $string);
171+
}
159172
}

‎src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,32 +17,32 @@ class PhpNameTest extends \PHPUnit_Framework_TestCase
1717
{
1818
public function testIsAffectedByChangesToItself()
1919
{
20-
$name = new PhpName("Foo\Bar\Baz", "Baz", null, null);
20+
$name = new PhpName("Foo\Bar\Baz", "Baz");
2121

2222
$this->assertTrue($name->isAffectedByChangesTo($name));
2323
}
2424

2525
public function testIsNotAffectedByChangesToNonRelativePart()
2626
{
27-
$name = new PhpName("Foo\Bar\Baz", "Baz", null, null);
28-
$changing = new PhpName("Foo\Bar", "Foo\Bar", null, null);
27+
$name = new PhpName("Foo\Bar\Baz", "Baz");
28+
$changing = new PhpName("Foo\Bar", "Foo\Bar");
2929

3030
$this->assertFalse($name->isAffectedByChangesTo($changing));
3131
}
3232

3333
public function testIsAffectedByRelativeChanges()
3434
{
35-
$name = new PhpName("Foo\Bar\Baz", "Bar\Baz", null, null);
36-
$changing = new PhpName("Foo\Bar", "Foo\Bar", null, null);
35+
$name = new PhpName("Foo\Bar\Baz", "Bar\Baz");
36+
$changing = new PhpName("Foo\Bar", "Foo\Bar");
3737

3838
$this->assertTrue($name->isAffectedByChangesTo($changing));
3939
}
4040

4141
public function testRelativeChanges()
4242
{
43-
$name = new PhpName("Foo\Bar\Baz", "Bar\Baz", null, null);
44-
$from = new PhpName("Foo\Bar", "Foo\Bar", null, null);
45-
$to = new PhpName("Foo\Baz", "Foo\Baz", null, null);
43+
$name = new PhpName("Foo\Bar\Baz", "Bar\Baz");
44+
$from = new PhpName("Foo\Bar", "Foo\Bar");
45+
$to = new PhpName("Foo\Baz", "Foo\Baz");
4646

4747
$newName = $name->change($from, $to);
4848

@@ -52,17 +52,17 @@ public function testRelativeChanges()
5252

5353
public function testRegression()
5454
{
55-
$name = new PhpName("Bar\Bar", "Bar\Bar", null, null);
56-
$changing = new PhpName("Bar", "Bar", null, null);
55+
$name = new PhpName("Bar\Bar", "Bar\Bar");
56+
$changing = new PhpName("Bar", "Bar");
5757

5858
$this->assertTrue($name->isAffectedByChangesTo($changing));
5959
}
6060

6161
public function testRegression2()
6262
{
63-
$name = new PhpName("Foo\\Foo", "Foo\\Foo", null, null);
64-
$from = new PhpName("Foo\\Foo", "Foo", null, null);
65-
$to = new PhpName("Foo\\Bar", "Bar", null, null);
63+
$name = new PhpName("Foo\\Foo", "Foo\\Foo");
64+
$from = new PhpName("Foo\\Foo", "Foo");
65+
$to = new PhpName("Foo\\Bar", "Bar");
6666

6767
$changed = $name->change($from, $to);
6868

@@ -72,9 +72,9 @@ public function testRegression2()
7272

7373
public function testRegression3()
7474
{
75-
$name = new PhpName("Foo\\Foo", "Foo\\Foo", null, null);
76-
$from = new PhpName("Foo\\Foo", "Foo\\Foo", null, null);
77-
$to = new PhpName("Foo\\Bar\\Foo", "Foo\\Bar\\Foo", null, null);
75+
$name = new PhpName("Foo\\Foo", "Foo\\Foo");
76+
$from = new PhpName("Foo\\Foo", "Foo\\Foo");
77+
$to = new PhpName("Foo\\Bar\\Foo", "Foo\\Bar\\Foo");
7878

7979
$changed = $name->change($from, $to);
8080

@@ -122,4 +122,16 @@ public function testRegression6()
122122
$this->assertEquals('Foo\Bar\Baz\Boing', $changed->fullyQualifiedName());
123123
$this->assertEquals('Foo\Bar\Baz\Boing', $changed->relativeName());
124124
}
125+
126+
public function testRegression7()
127+
{
128+
$from = new PhpName('Foo\Foo\Foo', 'Foo');
129+
$to = new PhpName('Foo\Boing', 'Boing');
130+
131+
$name = new PhpName('Foo\Foo\Foo', 'Foo\Foo\Foo');
132+
$changed = $name->change($from, $to);
133+
134+
$this->assertEquals('Foo\Boing', $changed->fullyQualifiedName());
135+
$this->assertEquals('Foo\Boing', $changed->relativeName());
136+
}
125137
}

0 commit comments

Comments
 (0)