From 21c8cf5d3b2399ce1e2cdc191bd6be3a85390b57 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Mon, 17 Jun 2013 07:57:15 +0200 Subject: [PATCH 01/17] [OptimizeUse] add behat feature --- features/optimize_use.feature | 58 +++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 features/optimize_use.feature diff --git a/features/optimize_use.feature b/features/optimize_use.feature new file mode 100644 index 0000000..e31889a --- /dev/null +++ b/features/optimize_use.feature @@ -0,0 +1,58 @@ +Feature: Optimize use + To optimize the use statements in my code + As a developer + I need to convert every FQN in the code to a use statement in the file + + Scenario: Convert FQN and leave relative names + Given a PHP File named "src/Foo.php" with: + """ + operation(new \ACME\Qux\Adapter($flag)); + + return $service; + } + } + """ + When I use refactoring "optimize-use" with: + | arg | value | + | file | src/Foo.php | + Then the PHP File "src/Foo.php" should be refactored: + """ + --- a/vfs://project/src/Foo.php + +++ b/vfs://project/src/Foo.php + @@ -1,11 +1,13 @@ + operation(new \ACME\Qux\Adapter($flag)); + + $service->operation(new Adapter($flag)); + + return $service; + } + } + """ + From a5f70924bf7119871fb19d2b1b9ffe088c0b136c Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Mon, 17 Jun 2013 10:56:52 +0200 Subject: [PATCH 02/17] File: fix relativePath calculation for working dirs with trailing slash, add a test --- .../Refactoring/Domain/Model/File.php | 3 +- .../Refactoring/Domain/Model/FileTest.php | 38 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 src/test/QafooLabs/Refactoring/Domain/Model/FileTest.php diff --git a/src/main/QafooLabs/Refactoring/Domain/Model/File.php b/src/main/QafooLabs/Refactoring/Domain/Model/File.php index d63e258..88b9afa 100644 --- a/src/main/QafooLabs/Refactoring/Domain/Model/File.php +++ b/src/main/QafooLabs/Refactoring/Domain/Model/File.php @@ -35,7 +35,8 @@ public static function createFromPath($path, $workingDirectory) } $code = file_get_contents($path); - $relativePath = ltrim(str_replace($workingDirectory, "", $path), "/"); + $workingDirectory = rtrim($workingDirectory, '/\\'); + $relativePath = ltrim(str_replace($workingDirectory, "", $path), "/\\"); return new self($relativePath, $code); } diff --git a/src/test/QafooLabs/Refactoring/Domain/Model/FileTest.php b/src/test/QafooLabs/Refactoring/Domain/Model/FileTest.php new file mode 100644 index 0000000..34c2f45 --- /dev/null +++ b/src/test/QafooLabs/Refactoring/Domain/Model/FileTest.php @@ -0,0 +1,38 @@ +root = vfsStream::setup('project', 0644, + array( + 'src'=> + array( + 'Foo'=> + array( + 'Bar.php'=>'' + ) + ) + ) + ); + } + + public function testGetRelativePathRespectsMixedWindowsPathsAndWorkingDirectoryTrailingSlashs() + { + $workingDir = $this->root->getChild('src')->url().'/'; + + $file = File::createFromPath( + $this->root->getChild('src')->url().'\Foo\Bar.php', + $workingDir + ); + + $this->assertEquals("Foo\Bar.php", $file->getRelativePath()); + } +} From bae6bb2048b79c377b4db0ec42d998ad53f7bf8a Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Mon, 17 Jun 2013 10:57:30 +0200 Subject: [PATCH 03/17] behat: fix expecations on windows --- features/bootstrap/FeatureContext.php | 29 ++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/features/bootstrap/FeatureContext.php b/features/bootstrap/FeatureContext.php index 9dab76f..f04e4f8 100644 --- a/features/bootstrap/FeatureContext.php +++ b/features/bootstrap/FeatureContext.php @@ -99,7 +99,7 @@ public function iUseRefactoringWith($refactoringName, TableNode $table) public function thePhpFileShouldBeRefactored($file, PyStringNode $expectedPatch) { $output = array_map('trim', explode("\n", rtrim($this->output))); - $expectedPatch = array_map('trim', explode("\n", rtrim((string)$expectedPatch))); + $expectedPatch = $this->formatExpectedPatch((string)$expectedPatch); assertEquals( $expectedPatch, $output, @@ -107,4 +107,31 @@ public function thePhpFileShouldBeRefactored($file, PyStringNode $expectedPatch) "Diff:\n" . print_r(array_diff($expectedPatch, $output), true) ); } + + /** + * converts / paths in expectedPatch text to \ paths + * + * leaves the a/ b/ slashes untouched + * returns an array of lines + * @return array + */ + protected function formatExpectedPatch($patch) + { + if ('\\' === DIRECTORY_SEPARATOR) { + $formatLine = function ($line) { + if (0 === strpos($line, '---') || 0 === strpos($line, '+++')) { + $line = preg_replace('~/(? Date: Mon, 17 Jun 2013 11:12:37 +0200 Subject: [PATCH 04/17] update readme for new command --- README.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/README.md b/README.md index 4d4e848..ff09d2b 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,15 @@ the command to fix class and namespaces. php refactor.phar fix-class-names +### Optimize use statements + +Optimizes the use of Fully qualified names in a file so that FQN is imported with +"use" at the top of the file and the FQN is replaced with its classname. + +All other use statements will be untouched, only new ones will be added. + + php refactor.phar optimize-use + ## Roadmap Not prioritized. From 6bcdc90f21ef6ebe237fdd952f537bc6c26eb3f7 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Wed, 19 Jun 2013 20:31:18 +0200 Subject: [PATCH 05/17] ignore sublimes change acme in feature to bar --- .gitignore | 4 ++++ features/optimize_use.feature | 14 +++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 1cadc9a..996e6f6 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,7 @@ refactor.phar .pear build/ composer.phar + +/php-refactoring-browser.sublime-project + +/php-refactoring-browser.sublime-workspace diff --git a/features/optimize_use.feature b/features/optimize_use.feature index e31889a..6e4cd5a 100644 --- a/features/optimize_use.feature +++ b/features/optimize_use.feature @@ -8,9 +8,9 @@ Feature: Optimize use """ operation(new \ACME\Qux\Adapter($flag)); + $service->operation(new \Bar\Qux\Adapter($flag)); return $service; } @@ -36,10 +36,10 @@ Feature: Optimize use operation(new \ACME\Qux\Adapter($flag)); + - $service->operation(new \Bar\Qux\Adapter($flag)); + $service->operation(new Adapter($flag)); return $service; From 1a0f5b0db572add57aaff4591bf7048a4d5883f4 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Wed, 19 Jun 2013 21:14:50 +0200 Subject: [PATCH 06/17] advanced fix for windows vfs:// paths --- features/bootstrap/FeatureContext.php | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/features/bootstrap/FeatureContext.php b/features/bootstrap/FeatureContext.php index f04e4f8..5e4fb1e 100644 --- a/features/bootstrap/FeatureContext.php +++ b/features/bootstrap/FeatureContext.php @@ -119,9 +119,16 @@ protected function formatExpectedPatch($patch) { if ('\\' === DIRECTORY_SEPARATOR) { $formatLine = function ($line) { - if (0 === strpos($line, '---') || 0 === strpos($line, '+++')) { - $line = preg_replace('~/(? Date: Wed, 19 Jun 2013 22:39:27 +0200 Subject: [PATCH 07/17] fix PhpNameCollector to create correct FQCNs with PHPParser help --- .../PHPParser/Visitor/PhpNameCollector.php | 12 ++++---- .../PHPParser/ParserPhpNameScannerTest.php | 29 ++++++++++++++++++- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/main/QafooLabs/Refactoring/Adapters/PHPParser/Visitor/PhpNameCollector.php b/src/main/QafooLabs/Refactoring/Adapters/PHPParser/Visitor/PhpNameCollector.php index 00eceb7..0d751fa 100644 --- a/src/main/QafooLabs/Refactoring/Adapters/PHPParser/Visitor/PhpNameCollector.php +++ b/src/main/QafooLabs/Refactoring/Adapters/PHPParser/Visitor/PhpNameCollector.php @@ -49,7 +49,7 @@ public function enterNode(PHPParser_Node $node) $this->nameDeclarations[] = array( 'alias' => $usedAlias, - 'fqcn' => $this->fullyQualifiedNameFor($usedAlias), + 'fqcn' => $this->fullyQualifiedNameFor($usedAlias, $node->class->isFullyQualified()), 'line' => $node->getLine(), ); } @@ -59,7 +59,7 @@ public function enterNode(PHPParser_Node $node) $this->nameDeclarations[] = array( 'alias' => $usedAlias, - 'fqcn' => $this->fullyQualifiedNameFor($usedAlias), + 'fqcn' => $this->fullyQualifiedNameFor($usedAlias, $node->class->isFullyQualified()), 'line' => $node->getLine(), ); } @@ -70,7 +70,7 @@ public function enterNode(PHPParser_Node $node) $this->nameDeclarations[] = array( 'alias' => $usedAlias, - 'fqcn' => $this->fullyQualifiedNameFor($usedAlias), + 'fqcn' => $this->fullyQualifiedNameFor($usedAlias, $node->extends->isFullyQualified()), 'line' => $node->extends->getLine(), ); } @@ -80,7 +80,7 @@ public function enterNode(PHPParser_Node $node) $this->nameDeclarations[] = array( 'alias' => $usedAlias, - 'fqcn' => $this->fullyQualifiedNameFor($usedAlias), + 'fqcn' => $this->fullyQualifiedNameFor($usedAlias, $implement->isFullyQualified()), 'line' => $implement->getLine(), ); } @@ -92,11 +92,11 @@ public function enterNode(PHPParser_Node $node) } } - private function fullyQualifiedNameFor($alias) + private function fullyQualifiedNameFor($alias, $isFullyQualified) { $isAbsolute = $alias[0] === "\\"; - if ($isAbsolute) { + if ($isAbsolute || $isFullyQualified) { $class = $alias; } else if (isset($this->useStatements[$alias])) { $class = $this->useStatements[$alias]; diff --git a/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php b/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php index f29056a..584ea0a 100644 --- a/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php +++ b/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php @@ -17,12 +17,39 @@ public function testFindNames() array( new PhpName('QafooLabs\Refactoring\Domain\Model\File', 'QafooLabs\Refactoring\Domain\Model\File', $file, 5), new PhpName('QafooLabs\Refactoring\Domain\Model\PhpName', 'QafooLabs\Refactoring\Domain\Model\PhpName', $file, 6), - new PhpName('QafooLabs\Refactoring\Adapters\PHPParser\PHPUnit_Framework_TestCase', 'PHPUnit_Framework_TestCase', $file, 8), + new PhpName('PHPUnit_Framework_TestCase', 'PHPUnit_Framework_TestCase', $file, 8), new PhpName('QafooLabs\Refactoring\Domain\Model\File', 'File', $file, 12), new PhpName('QafooLabs\Refactoring\Adapters\PHPParser\ParserPhpNameScanner', 'ParserPhpNameScanner', $file, 13), ), array_slice($names, 0, 5) ); } + + public function testRegressionFindNamesDetectsFQCNCorrectly() { + $file = new File("Fqcn.php", <<<'PHP' +findNames($file); + + $this->assertEquals( + array( + new PhpName('Bar\Qux\Adapter', 'Bar\Qux\Adapter', $file, 9) + ), + $names + ); + } } From e859dc776170e244065bf265de7a6efc04fd9d73 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Wed, 19 Jun 2013 22:54:48 +0200 Subject: [PATCH 08/17] add isFullyQualified to PhpName --- .../Refactoring/Domain/Model/PhpName.php | 13 +++++++++++ .../Refactoring/Domain/Model/PhpNameTest.php | 22 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php b/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php index fd60c0e..6563d92 100644 --- a/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php +++ b/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php @@ -105,9 +105,22 @@ public function relativeName() return $this->relativeName; } + /** + * @return bool + */ public function equals(PhpName $other) { return $this->fullyQualifiedName === $other->fullyQualifiedName && $this->relativeName === $other->relativeName; } + + /** + * Is the relative name fully qualified ? + * + * @return bool + */ + public function isFullyQualified() + { + return $this->fullyQualifiedName === $this->relativeName; + } } diff --git a/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php b/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php index 715fb36..9a43a69 100644 --- a/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php +++ b/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php @@ -81,4 +81,26 @@ public function testRegression3() $this->assertEquals('Foo\\Bar\\Foo', $changed->fullyQualifiedName()); $this->assertEquals('Foo\\Bar\\Foo', $changed->relativeName()); } + + /** + * @dataProvider provideIsFullyQualified + */ + public function testIsFullyQualified($fqcn, $relativeName, $expected = TRUE) + { + $name = new PHPName($fqcn, $relativeName); + + $this->assertEquals($expected, $name->isFullyQualified()); + } + + public static function provideIsFullyQualified() { + $tests = array(); + + $tests[] = array('Foo', 'Foo', TRUE); + $tests[] = array('Foo\\Bar\\Foo', 'Foo\\Bar\\Foo', TRUE); + + $tests[] = array('Foo\\Bar\\Foo', 'Foo', FALSE); + $tests[] = array('Foo\\Bar\\Foo', 'Bar\\Foo', FALSE); + + return $tests; + } } From 814d6ed05c9442ede3a47e28587a4d9a6c16472e Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Wed, 19 Jun 2013 23:09:30 +0200 Subject: [PATCH 09/17] add shortName to PhpName --- src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php | 9 +++++++++ .../QafooLabs/Refactoring/Domain/Model/PhpNameTest.php | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php b/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php index 6563d92..74ca340 100644 --- a/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php +++ b/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php @@ -123,4 +123,13 @@ public function isFullyQualified() { return $this->fullyQualifiedName === $this->relativeName; } + + /** + * @return string + */ + public function shortName() + { + $parts = explode("\\", $this->fullyQualifiedName); + return end($parts); + } } diff --git a/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php b/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php index 9a43a69..b6893a1 100644 --- a/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php +++ b/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php @@ -103,4 +103,12 @@ public static function provideIsFullyQualified() { return $tests; } + + public function testGetShortNameReturnsLastPartForFQCN() { + $name = new PHPName('Foo\\Bar', "Foo\\Bar", null, null); + $short = new PhpName("Foo", "Foo", null, null); + + $this->assertEquals('Bar', $name->shortName()); + $this->assertEquals('Foo', $short->shortName()); + } } From 755e6f10b2c300572daf0e7119aad9bba0812dc6 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Sat, 29 Jun 2013 08:55:57 +0200 Subject: [PATCH 10/17] add a parent to PhpName to collect context of the PhpNames in Namescanner --- .../Refactoring/Domain/Model/PhpName.php | 15 ++++++++++++++- .../Refactoring/Domain/Model/PhpNameTest.php | 8 +++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php b/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php index 74ca340..b36e1d5 100644 --- a/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php +++ b/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php @@ -15,17 +15,20 @@ class PhpName { + private $fullyQualifiedName; private $relativeName; private $file; private $declaredLine; + private $parent; - public function __construct($fullyQualifiedName, $relativeName, File $file = null, $declaredLine = null) + public function __construct($fullyQualifiedName, $relativeName, File $file = null, $declaredLine = null, $parent = null) { $this->fullyQualifiedName = $fullyQualifiedName; $this->relativeName = $relativeName; $this->file = $file; $this->declaredLine = $declaredLine; + $this->parent = $parent; } /** @@ -124,6 +127,16 @@ public function isFullyQualified() return $this->fullyQualifiedName === $this->relativeName; } + /** + * Is the php name found in a use statement? + * + * @return bool + */ + public function isUse() + { + return $this->parent instanceof UseStatement; + } + /** * @return string */ diff --git a/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php b/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php index b6893a1..6a3cb5e 100644 --- a/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php +++ b/src/test/QafooLabs/Refactoring/Domain/Model/PhpNameTest.php @@ -105,10 +105,16 @@ public static function provideIsFullyQualified() { } public function testGetShortNameReturnsLastPartForFQCN() { - $name = new PHPName('Foo\\Bar', "Foo\\Bar", null, null); + $name = new PhpName('Foo\\Bar', "Foo\\Bar", null, null); $short = new PhpName("Foo", "Foo", null, null); $this->assertEquals('Bar', $name->shortName()); $this->assertEquals('Foo', $short->shortName()); } + + public function testIsUseStatementWhenParentIsAUseStatement() { + $name = new PhpName('Foo\\Bar', 'Foo\\Bar', null, null, new UseStatement()); + + $this->assertTrue($name->isUse()); + } } From f381cb39c1f24b1d662715e7151d9de8a0a3d8b6 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Sat, 29 Jun 2013 08:58:06 +0200 Subject: [PATCH 11/17] Model: add a UseStatement --- .../Refactoring/Domain/Model/UseStatement.php | 39 +++++++++++++++++++ .../Domain/Model/UseStatementTest.php | 20 ++++++++++ 2 files changed, 59 insertions(+) create mode 100644 src/main/QafooLabs/Refactoring/Domain/Model/UseStatement.php create mode 100644 src/test/QafooLabs/Refactoring/Domain/Model/UseStatementTest.php diff --git a/src/main/QafooLabs/Refactoring/Domain/Model/UseStatement.php b/src/main/QafooLabs/Refactoring/Domain/Model/UseStatement.php new file mode 100644 index 0000000..e9fee18 --- /dev/null +++ b/src/main/QafooLabs/Refactoring/Domain/Model/UseStatement.php @@ -0,0 +1,39 @@ +file = $file; + $this->declaredLines = $declaredLines; + } + + public function getEndLine() + { + return $this->declaredLines->getEnd(); + } +} diff --git a/src/test/QafooLabs/Refactoring/Domain/Model/UseStatementTest.php b/src/test/QafooLabs/Refactoring/Domain/Model/UseStatementTest.php new file mode 100644 index 0000000..6c0159e --- /dev/null +++ b/src/test/QafooLabs/Refactoring/Domain/Model/UseStatementTest.php @@ -0,0 +1,20 @@ +useStatement = new UseStatement($file, LineRange::fromLines(3,5)); + } + + public function testReturnsEndLineFromLineRange() { + $this->assertEquals(5, $this->useStatement->getEndLine()); + } +} From 7bf3f2f710e1b652a301d0f2ed0d32b8d9118ff3 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Sat, 29 Jun 2013 08:58:32 +0200 Subject: [PATCH 12/17] ParserPhpNameScanner: add parent scanning for use statements for PhpNames --- .../PHPParser/ParserPhpNameScanner.php | 23 +++++- .../PHPParser/Visitor/PhpNameCollector.php | 26 +++++-- .../PHPParser/ParserPhpNameScannerTest.php | 73 +++++++++++++++++-- 3 files changed, 106 insertions(+), 16 deletions(-) diff --git a/src/main/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScanner.php b/src/main/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScanner.php index 20e3606..1c87727 100644 --- a/src/main/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScanner.php +++ b/src/main/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScanner.php @@ -5,6 +5,8 @@ use QafooLabs\Refactoring\Adapters\PHPParser\Visitor\PhpNameCollector; use QafooLabs\Refactoring\Domain\Services\PhpNameScanner; use QafooLabs\Refactoring\Domain\Model\File; +use QafooLabs\Refactoring\Domain\Model\LineRange; +use QafooLabs\Refactoring\Domain\Model\UseStatement; use QafooLabs\Refactoring\Domain\Model\PhpName; use PHPParser_Parser; @@ -24,8 +26,25 @@ public function findNames(File $file) $traverser->addVisitor($collector); $traverser->traverse($stmts); - return array_map(function ($use) use ($file) { - return new PhpName($use['fqcn'], $use['alias'], $file, $use['line']); + return array_map(function ($name) use ($file) { + return new PhpName( + $name['fqcn'], + $name['alias'], + $file, + $name['line'], + isset($name['parent']) + ? $this->createParent($name['parent'], $file) + : null + ); + }, $collector->collectedNameDeclarations()); } + + private function createParent(Array $parent, File $file) + { + switch ($parent['type']) { + case 'use': + return new UseStatement($file, LineRange::fromLines($parent['lines'][0], $parent['lines'][1])); + } + } } diff --git a/src/main/QafooLabs/Refactoring/Adapters/PHPParser/Visitor/PhpNameCollector.php b/src/main/QafooLabs/Refactoring/Adapters/PHPParser/Visitor/PhpNameCollector.php index 0d751fa..bedc80b 100644 --- a/src/main/QafooLabs/Refactoring/Adapters/PHPParser/Visitor/PhpNameCollector.php +++ b/src/main/QafooLabs/Refactoring/Adapters/PHPParser/Visitor/PhpNameCollector.php @@ -33,17 +33,29 @@ class PhpNameCollector extends \PHPParser_NodeVisitorAbstract public function enterNode(PHPParser_Node $node) { - if ($node instanceof PHPParser_Node_Stmt_UseUse) { - $name = implode('\\', $node->name->parts); - $this->useStatements[$node->alias] = $name; - $this->nameDeclarations[] = array( - 'alias' => $name, - 'fqcn' => $name, - 'line' => $node->getLine() + if ($node instanceof PHPParser_Node_Stmt_Use) { + $parent = array( + 'type' => 'use', + 'lines' => array($node->getAttribute('startLine'), $node->getAttribute('endLine')) ); + + foreach ($node->uses as $use) { + if ($use instanceof PHPParser_Node_Stmt_UseUse) { + $name = implode('\\', $use->name->parts); + + $this->useStatements[$use->alias] = $name; + $this->nameDeclarations[] = array( + 'alias' => $name, + 'fqcn' => $name, + 'line' => $use->getLine(), + 'parent' => $parent + ); + } + } } + if ($node instanceof PHPParser_Node_Expr_New && $node->class instanceof PHPParser_Node_Name) { $usedAlias = implode('\\', $node->class->parts); diff --git a/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php b/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php index 584ea0a..3e03160 100644 --- a/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php +++ b/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php @@ -4,6 +4,8 @@ use QafooLabs\Refactoring\Domain\Model\File; use QafooLabs\Refactoring\Domain\Model\PhpName; +use QafooLabs\Refactoring\Domain\Model\UseStatement; +use QafooLabs\Refactoring\Domain\Model\LineRange; class ParserPhpNameScannerTest extends \PHPUnit_Framework_TestCase { @@ -13,15 +15,21 @@ public function testFindNames() $scanner = new ParserPhpNameScanner(); $names = $scanner->findNames($file); + $useStmt = function ($line) use ($file) { + return new UseStatement($file, LineRange::fromSingleLine($line)); + }; + $this->assertEquals( array( - new PhpName('QafooLabs\Refactoring\Domain\Model\File', 'QafooLabs\Refactoring\Domain\Model\File', $file, 5), - new PhpName('QafooLabs\Refactoring\Domain\Model\PhpName', 'QafooLabs\Refactoring\Domain\Model\PhpName', $file, 6), - new PhpName('PHPUnit_Framework_TestCase', 'PHPUnit_Framework_TestCase', $file, 8), - new PhpName('QafooLabs\Refactoring\Domain\Model\File', 'File', $file, 12), - new PhpName('QafooLabs\Refactoring\Adapters\PHPParser\ParserPhpNameScanner', 'ParserPhpNameScanner', $file, 13), + new PhpName('QafooLabs\Refactoring\Domain\Model\File', 'QafooLabs\Refactoring\Domain\Model\File', $file, 5, $useStmt(5)), + new PhpName('QafooLabs\Refactoring\Domain\Model\PhpName', 'QafooLabs\Refactoring\Domain\Model\PhpName', $file, 6, $useStmt(6)), + new PhpName('QafooLabs\Refactoring\Domain\Model\UseStatement', 'QafooLabs\Refactoring\Domain\Model\UseStatement', $file, 7, $useStmt(7)), + new PhpName('QafooLabs\Refactoring\Domain\Model\LineRange', 'QafooLabs\Refactoring\Domain\Model\LineRange', $file, 8, $useStmt(8)), + new PhpName('PHPUnit_Framework_TestCase', 'PHPUnit_Framework_TestCase', $file, 10), + new PhpName('QafooLabs\Refactoring\Domain\Model\File', 'File', $file, 14), + new PhpName('QafooLabs\Refactoring\Adapters\PHPParser\ParserPhpNameScanner', 'ParserPhpNameScanner', $file, 15), ), - array_slice($names, 0, 5) + array_slice($names, 0, 7) ); } @@ -51,5 +59,56 @@ public function main() $names ); } -} + + public function testFindNamesFindsParentForPhpNameInSingleLineUseStatement() { + $file = new File("Fqcn.php", <<<'PHP' +findNames($file); + + $this->assertEquals( + array( + new PhpName( + 'Bar\Qux\Adapter', + 'Bar\Qux\Adapter', + $file, + 3, + new UseStatement( + $file, + LineRange::fromSingleLine(3) + ) + ) + ), + $names + ); + } + + public function testFindNamesFindsParentForPhpNameInMultiLineUseStatement() { + $file = new File("Fqcn.php", <<<'PHP' +findNames($file); + + $use = new UseStatement($file, LineRange::fromLines(3, 4)); + + $this->assertEquals( + array( + new PhpName('Bar\Qux\Adapter', 'Bar\Qux\Adapter', $file, 3, $use), + new PhpName('Bar\Qux\Foo', 'Bar\Qux\Foo', $file, 4, $use), + ), + $names + ); + } +} From 3f43413a85d26791449b69854b8e07984495baeb Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Sat, 29 Jun 2013 09:30:10 +0200 Subject: [PATCH 13/17] behat: add optimize_use feature with a simple use case --- features/optimize_use.feature | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/features/optimize_use.feature b/features/optimize_use.feature index 6e4cd5a..4fb767c 100644 --- a/features/optimize_use.feature +++ b/features/optimize_use.feature @@ -32,27 +32,19 @@ Feature: Optimize use """ --- a/vfs://project/src/Foo.php +++ b/vfs://project/src/Foo.php - @@ -1,11 +1,13 @@ - operation(new \Bar\Qux\Adapter($flag)); + $service->operation(new Adapter($flag)); return $service; - } - } """ From f9e2b5f968589848f557932ddc082df846549bc2 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Sat, 29 Jun 2013 09:30:52 +0200 Subject: [PATCH 14/17] cli: create OptimizeUseCommand, add OptimizeUseCommand to CLI OptimizeUse: uses the new features from NameScanner to implement --- .../Adapters/Symfony/CliApplication.php | 1 + .../Symfony/Commands/OptimizeUseCommand.php | 84 +++++++++++++++++++ .../Refactoring/Application/OptimizeUse.php | 68 +++++++++++++++ 3 files changed, 153 insertions(+) create mode 100644 src/main/QafooLabs/Refactoring/Adapters/Symfony/Commands/OptimizeUseCommand.php create mode 100644 src/main/QafooLabs/Refactoring/Application/OptimizeUse.php diff --git a/src/main/QafooLabs/Refactoring/Adapters/Symfony/CliApplication.php b/src/main/QafooLabs/Refactoring/Adapters/Symfony/CliApplication.php index 5736832..99bdcc6 100644 --- a/src/main/QafooLabs/Refactoring/Adapters/Symfony/CliApplication.php +++ b/src/main/QafooLabs/Refactoring/Adapters/Symfony/CliApplication.php @@ -42,6 +42,7 @@ protected function getDefaultCommands() $commands[] = new Commands\RenameLocalVariableCommand(); $commands[] = new Commands\ConvertLocalToInstanceVariableCommand(); $commands[] = new Commands\FixClassNamesCommand(); + $commands[] = new Commands\OptimizeUseCommand(); return $commands; } diff --git a/src/main/QafooLabs/Refactoring/Adapters/Symfony/Commands/OptimizeUseCommand.php b/src/main/QafooLabs/Refactoring/Adapters/Symfony/Commands/OptimizeUseCommand.php new file mode 100644 index 0000000..f842898 --- /dev/null +++ b/src/main/QafooLabs/Refactoring/Adapters/Symfony/Commands/OptimizeUseCommand.php @@ -0,0 +1,84 @@ +setName('optimize-use') + ->setDescription('Optimize use statements of a file. Replace FQNs with imported aliases.') + ->addArgument('file', InputArgument::REQUIRED, 'File that contains the use statements to optimize') + ->setHelp(<<Operations: + +1. import found FQNs +2. replace FQNs with the imported classname + +Pre-Conditions: + +1. File has a single namespace defined + +Known issues: + +1. a FQN might be renamed with an conflicting name when the className of the renamend full qualified name is already in use +2. if there is no use statement in the whole file, new ones will be appended after the namespace + +Usage: + + php refactor.phar optimize-use file.php + +Will optimize the use statements in file.php. +HELP + ); + ; + } + + protected function execute(InputInterface $input, OutputInterface $output) + { + $file = File::createFromPath($input->getArgument('file'), getcwd()); + + $codeAnalysis = new StaticCodeAnalysis(); + $editor = new PatchEditor(new OutputPatchCommand($output)); + $phpNameScanner = new ParserPhpNameScanner(); + + $optimizeUse = new OptimizeUse($codeAnalysis, $editor, $phpNameScanner); + $optimizeUse->refactor($file); + } +} diff --git a/src/main/QafooLabs/Refactoring/Application/OptimizeUse.php b/src/main/QafooLabs/Refactoring/Application/OptimizeUse.php new file mode 100644 index 0000000..de486ad --- /dev/null +++ b/src/main/QafooLabs/Refactoring/Application/OptimizeUse.php @@ -0,0 +1,68 @@ +codeAnalysis = $codeAnalysis; + $this->editor = $editor; + $this->phpNameScanner = $phpNameScanner; + } + + public function refactor(File $file) + { + $classes = $this->codeAnalysis->findClasses($file); + $names = $this->phpNameScanner->findNames($file); + $class = $classes[0]; + + $lastUseStatement = null; + $usedNames = array(); + $fqcns = array(); + foreach ($names as $name) { + if ($name->isUse()) { + $lastUseStatement = $name->parent(); + $usedNames[] = $name->fullyQualifiedName(); + } elseif ($name->isFullyQualified()) { + $fqcns[] = $name; + } + } + $lastUseStatementLine = null !== $lastUseStatement ? $lastUseStatement->getEndLine() : $class->getNamespaceDeclarationLine()+2; + + if (count($fqcns) > 0) { + + $buffer = $this->editor->openBuffer($file); + foreach ($fqcns as $name) { + $buffer->replaceString($name->declaredLine(), '\\'.$name->fullyQualifiedName(), $name->shortname()); + + if (!in_array($name->fullyQualifiedName(), $usedNames)) { + $buffer->append($lastUseStatementLine, array(sprintf('use %s;', $name->fullyQualifiedName()))); + $lastUseStatementLine++; + } + } + + $this->editor->save(); + } + } +} From 87bb50517c94cb2dd84c89f5ed7df7ddbd4621f6 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Sat, 29 Jun 2013 09:31:15 +0200 Subject: [PATCH 15/17] PhpName: add getter for parent --- src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php b/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php index b36e1d5..efe5741 100644 --- a/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php +++ b/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php @@ -108,6 +108,10 @@ public function relativeName() return $this->relativeName; } + public function parent() { + return $this->parent; + } + /** * @return bool */ From a4b22088d51f7acd7aa8efd6e41b87c7d2b80cb2 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Mon, 1 Jul 2013 08:26:32 +0200 Subject: [PATCH 16/17] fix curly braces to new line for functions --- src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php | 3 ++- .../Adapters/PHPParser/ParserPhpNameScannerTest.php | 6 ++++-- .../QafooLabs/Refactoring/Domain/Model/PhpNameTest.php | 9 ++++++--- .../Refactoring/Domain/Model/UseStatementTest.php | 3 ++- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php b/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php index efe5741..0dff8fb 100644 --- a/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php +++ b/src/main/QafooLabs/Refactoring/Domain/Model/PhpName.php @@ -108,7 +108,8 @@ public function relativeName() return $this->relativeName; } - public function parent() { + public function parent() + { return $this->parent; } diff --git a/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php b/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php index 3e03160..540f78a 100644 --- a/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php +++ b/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php @@ -61,7 +61,8 @@ public function main() } - public function testFindNamesFindsParentForPhpNameInSingleLineUseStatement() { + public function testFindNamesFindsParentForPhpNameInSingleLineUseStatement() + { $file = new File("Fqcn.php", <<<'PHP' assertEquals($expected, $name->isFullyQualified()); } - public static function provideIsFullyQualified() { + public static function provideIsFullyQualified() + { $tests = array(); $tests[] = array('Foo', 'Foo', TRUE); @@ -104,7 +105,8 @@ public static function provideIsFullyQualified() { return $tests; } - public function testGetShortNameReturnsLastPartForFQCN() { + public function testGetShortNameReturnsLastPartForFQCN() + { $name = new PhpName('Foo\\Bar', "Foo\\Bar", null, null); $short = new PhpName("Foo", "Foo", null, null); @@ -112,7 +114,8 @@ public function testGetShortNameReturnsLastPartForFQCN() { $this->assertEquals('Foo', $short->shortName()); } - public function testIsUseStatementWhenParentIsAUseStatement() { + public function testIsUseStatementWhenParentIsAUseStatement() + { $name = new PhpName('Foo\\Bar', 'Foo\\Bar', null, null, new UseStatement()); $this->assertTrue($name->isUse()); diff --git a/src/test/QafooLabs/Refactoring/Domain/Model/UseStatementTest.php b/src/test/QafooLabs/Refactoring/Domain/Model/UseStatementTest.php index 6c0159e..9af3b7c 100644 --- a/src/test/QafooLabs/Refactoring/Domain/Model/UseStatementTest.php +++ b/src/test/QafooLabs/Refactoring/Domain/Model/UseStatementTest.php @@ -14,7 +14,8 @@ public function setUp() $this->useStatement = new UseStatement($file, LineRange::fromLines(3,5)); } - public function testReturnsEndLineFromLineRange() { + public function testReturnsEndLineFromLineRange() + { $this->assertEquals(5, $this->useStatement->getEndLine()); } } From e1962a904c194d28dd13b86bcc7cdfef285a3422 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Mon, 1 Jul 2013 08:26:32 +0200 Subject: [PATCH 17/17] fix curly braces to new line for functions --- .../Adapters/PHPParser/ParserPhpNameScannerTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php b/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php index 540f78a..a915180 100644 --- a/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php +++ b/src/test/QafooLabs/Refactoring/Adapters/PHPParser/ParserPhpNameScannerTest.php @@ -33,7 +33,8 @@ public function testFindNames() ); } - public function testRegressionFindNamesDetectsFQCNCorrectly() { + public function testRegressionFindNamesDetectsFQCNCorrectly() + { $file = new File("Fqcn.php", <<<'PHP'