From 0beec8a5aa95a2dffbdfc154a32a0b9dccfdef2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Haso=C5=88?= Date: Wed, 7 Aug 2013 08:55:13 +0200 Subject: [PATCH] [Console] Improved searching commands --- src/Symfony/Component/Console/Application.php | 196 ++++++------------ .../Console/Tests/ApplicationTest.php | 62 ++++-- .../Console/Tests/Fixtures/FoobarCommand.php | 25 +++ 3 files changed, 128 insertions(+), 155 deletions(-) create mode 100644 src/Symfony/Component/Console/Tests/Fixtures/FoobarCommand.php diff --git a/src/Symfony/Component/Console/Application.php b/src/Symfony/Component/Console/Application.php index a7b120cfeb..4d70a4e998 100644 --- a/src/Symfony/Component/Console/Application.php +++ b/src/Symfony/Component/Console/Application.php @@ -494,51 +494,31 @@ class Application public function findNamespace($namespace) { $allNamespaces = $this->getNamespaces(); - $found = ''; - foreach (explode(':', $namespace) as $i => $part) { - // select sub-namespaces matching the current namespace we found - $namespaces = array(); - foreach ($allNamespaces as $n) { - if ('' === $found || 0 === strpos($n, $found)) { - $namespaces[$n] = explode(':', $n); - } - } + $expr = preg_replace_callback('{([^:]+|)}', function ($matches) { return preg_quote($matches[1]).'[^:]*'; }, $namespace); + $namespaces = preg_grep('{^'.$expr.'}', $allNamespaces); - $abbrevs = static::getAbbreviations(array_unique(array_values(array_filter(array_map(function ($p) use ($i) { return isset($p[$i]) ? $p[$i] : ''; }, $namespaces))))); + if (empty($namespaces)) { + $message = sprintf('There are no commands defined in the "%s" namespace.', $namespace); - if (!isset($abbrevs[$part])) { - $message = sprintf('There are no commands defined in the "%s" namespace.', $namespace); - - if (1 <= $i) { - $part = $found.':'.$part; + if ($alternatives = $this->findAlternatives($namespace, $allNamespaces, array())) { + if (1 == count($alternatives)) { + $message .= "\n\nDid you mean this?\n "; + } else { + $message .= "\n\nDid you mean one of these?\n "; } - if ($alternatives = $this->findAlternativeNamespace($part, $abbrevs)) { - if (1 == count($alternatives)) { - $message .= "\n\nDid you mean this?\n "; - } else { - $message .= "\n\nDid you mean one of these?\n "; - } - - $message .= implode("\n ", $alternatives); - } - - throw new \InvalidArgumentException($message); + $message .= implode("\n ", $alternatives); } - // there are multiple matches, but $part is an exact match of one of them so we select it - if (in_array($part, $abbrevs[$part])) { - $abbrevs[$part] = array($part); - } - - if (count($abbrevs[$part]) > 1) { - throw new \InvalidArgumentException(sprintf('The namespace "%s" is ambiguous (%s).', $namespace, $this->getAbbreviationSuggestions($abbrevs[$part]))); - } - - $found .= $found ? ':' . $abbrevs[$part][0] : $abbrevs[$part][0]; + throw new \InvalidArgumentException($message); } - return $found; + $exact = in_array($namespace, $namespaces, true); + if (1 < count($namespaces) && !$exact) { + throw new \InvalidArgumentException(sprintf('The namespace "%s" is ambiguous (%s).', $namespace, $this->getAbbreviationSuggestions($namespaces))); + } + + return $exact ? $namespace : reset($namespaces); } /** @@ -557,58 +537,19 @@ class Application */ public function find($name) { - // namespace - $namespace = ''; - $searchName = $name; - if (false !== $pos = strrpos($name, ':')) { - $namespace = $this->findNamespace(substr($name, 0, $pos)); - $searchName = $namespace.substr($name, $pos); - } + $allCommands = array_keys($this->commands); + $expr = preg_replace_callback('{([^:]+|)}', function ($matches) { return preg_quote($matches[1]).'[^:]*'; }, $name); + $commands = preg_grep('{^'.$expr.'}', $allCommands); - // name - $commands = array(); - foreach ($this->commands as $command) { - $extractedNamespace = $this->extractNamespace($command->getName()); - if ($extractedNamespace === $namespace - || !empty($namespace) && 0 === strpos($extractedNamespace, $namespace) - ) { - $commands[] = $command->getName(); + if (empty($commands) || count(preg_grep('{^'.$expr.'$}', $commands)) < 1) { + if (false !== $pos = strrpos($name, ':')) { + // check if a namespace exists and contains commands + $this->findNamespace(substr($name, 0, $pos)); } - } - $abbrevs = static::getAbbreviations(array_unique($commands)); - if (isset($abbrevs[$searchName]) && 1 == count($abbrevs[$searchName])) { - return $this->get($abbrevs[$searchName][0]); - } - - if (isset($abbrevs[$searchName]) && in_array($searchName, $abbrevs[$searchName])) { - return $this->get($searchName); - } - - if (isset($abbrevs[$searchName]) && count($abbrevs[$searchName]) > 1) { - $suggestions = $this->getAbbreviationSuggestions($abbrevs[$searchName]); - - throw new \InvalidArgumentException(sprintf('Command "%s" is ambiguous (%s).', $name, $suggestions)); - } - - // aliases - $aliases = array(); - foreach ($this->commands as $command) { - foreach ($command->getAliases() as $alias) { - $extractedNamespace = $this->extractNamespace($alias); - if ($extractedNamespace === $namespace - || !empty($namespace) && 0 === strpos($extractedNamespace, $namespace) - ) { - $aliases[] = $alias; - } - } - } - - $aliases = static::getAbbreviations(array_unique($aliases)); - if (!isset($aliases[$searchName])) { $message = sprintf('Command "%s" is not defined.', $name); - if ($alternatives = $this->findAlternativeCommands($searchName, $abbrevs)) { + if ($alternatives = $this->findAlternatives($name, $allCommands, array())) { if (1 == count($alternatives)) { $message .= "\n\nDid you mean this?\n "; } else { @@ -620,11 +561,14 @@ class Application throw new \InvalidArgumentException($message); } - if (count($aliases[$searchName]) > 1) { - throw new \InvalidArgumentException(sprintf('Command "%s" is ambiguous (%s).', $name, $this->getAbbreviationSuggestions($aliases[$searchName]))); + $exact = in_array($name, $commands, true); + if (count($commands) > 1 && !$exact) { + $suggestions = $this->getAbbreviationSuggestions(array_values($commands)); + + throw new \InvalidArgumentException(sprintf('Command "%s" is ambiguous (%s).', $name, $suggestions)); } - return $this->get($aliases[$searchName][0]); + return $this->get($exact ? $name : reset($commands)); } /** @@ -1093,72 +1037,50 @@ class Application } /** - * Finds alternative commands of $name - * - * @param string $name The full name of the command - * @param array $abbrevs The abbreviations - * - * @return array A sorted array of similar commands - */ - private function findAlternativeCommands($name, $abbrevs) - { - $callback = function($item) { - return $item->getName(); - }; - - return $this->findAlternatives($name, $this->commands, $abbrevs, $callback); - } - - /** - * Finds alternative namespace of $name - * - * @param string $name The full name of the namespace - * @param array $abbrevs The abbreviations - * - * @return array A sorted array of similar namespace - */ - private function findAlternativeNamespace($name, $abbrevs) - { - return $this->findAlternatives($name, $this->getNamespaces(), $abbrevs); - } - - /** - * Finds alternative of $name among $collection, - * if nothing is found in $collection, try in $abbrevs + * Finds alternative of $name among $collection * * @param string $name The string * @param array|Traversable $collection The collection - * @param array $abbrevs The abbreviations - * @param Closure|string|array $callback The callable to transform collection item before comparison * * @return array A sorted array of similar string */ - private function findAlternatives($name, $collection, $abbrevs, $callback = null) + private function findAlternatives($name, $collection) { + $threshold = 1e3; $alternatives = array(); + $collectionParts = array(); foreach ($collection as $item) { - if (null !== $callback) { - $item = call_user_func($callback, $item); - } - - $lev = levenshtein($name, $item); - if ($lev <= strlen($name) / 3 || false !== strpos($item, $name)) { - $alternatives[$item] = $lev; - } + $collectionParts[$item] = explode(':', $item); } - if (!$alternatives) { - foreach ($abbrevs as $key => $values) { - $lev = levenshtein($name, $key); - if ($lev <= strlen($name) / 3 || false !== strpos($key, $name)) { - foreach ($values as $value) { - $alternatives[$value] = $lev; - } + foreach (explode(':', $name) as $i => $subname) { + foreach ($collectionParts as $collectionName => $parts) { + $exists = isset($alternatives[$collectionName]); + if (!isset($parts[$i]) && $exists) { + $alternatives[$collectionName] += $threshold; + continue; + } elseif (!isset($parts[$i])) { + continue; + } + + $lev = levenshtein($subname, $parts[$i]); + if ($lev <= strlen($subname) / 3 || false !== strpos($parts[$i], $subname)) { + $alternatives[$collectionName] = $exists ? $alternatives[$collectionName] + $lev : $lev; + } elseif ($exists) { + $alternatives[$collectionName] += $threshold; } } } + foreach ($collection as $item) { + $lev = levenshtein($name, $item); + if ($lev <= strlen($name) / 3 || false !== strpos($item, $name)) { + $alternatives[$item] = isset($alternatives[$item]) ? $alternatives[$item] - $lev : $lev; + } + } + + $alternatives = array_filter($alternatives, function ($lev) use ($threshold) { return $lev < 2*$threshold; }); asort($alternatives); return array_keys($alternatives); diff --git a/src/Symfony/Component/Console/Tests/ApplicationTest.php b/src/Symfony/Component/Console/Tests/ApplicationTest.php index 3075261649..ebe2266d5d 100644 --- a/src/Symfony/Component/Console/Tests/ApplicationTest.php +++ b/src/Symfony/Component/Console/Tests/ApplicationTest.php @@ -40,6 +40,7 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase require_once self::$fixturesPath.'/Foo2Command.php'; require_once self::$fixturesPath.'/Foo3Command.php'; require_once self::$fixturesPath.'/Foo4Command.php'; + require_once self::$fixturesPath.'/FoobarCommand.php'; } protected function normalizeLineBreaks($text) @@ -208,6 +209,20 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase $application->findNamespace('bar'); } + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Command "foo1" is not defined + */ + public function testFindUniqueNameButNamespaceName() + { + $application = new Application(); + $application->add(new \FooCommand()); + $application->add(new \Foo1Command()); + $application->add(new \Foo2Command()); + + $application->find($commandName = 'foo1'); + } + public function testFind() { $application = new Application(); @@ -240,7 +255,7 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase return array( array('f', 'Command "f" is not defined.'), array('a', 'Command "a" is ambiguous (afoobar, afoobar1 and 1 more).'), - array('foo:b', 'Command "foo:b" is ambiguous (foo:bar, foo:bar1).') + array('foo:b', 'Command "foo:b" is ambiguous (foo:bar, foo:bar1 and 1 more).') ); } @@ -254,6 +269,23 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase $this->assertInstanceOf('Foo4Command', $application->find('foo3:bar:toh'), '->find() returns a command even if its namespace equals another command name'); } + public function testFindCommandWithAmbiguousNamespacesButUniqueName() + { + $application = new Application(); + $application->add(new \FooCommand()); + $application->add(new \FoobarCommand()); + + $this->assertInstanceOf('FoobarCommand', $application->find('f:f')); + } + + public function testFindCommandWithMissingNamespace() + { + $application = new Application(); + $application->add(new \Foo4Command()); + + $this->assertInstanceOf('Foo4Command', $application->find('f::t')); + } + /** * @dataProvider provideInvalidCommandNamesSingle * @expectedException \InvalidArgumentException @@ -262,15 +294,15 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase public function testFindAlternativeExceptionMessageSingle($name) { $application = new Application(); - $application->add(new \FooCommand()); + $application->add(new \Foo3Command()); $application->find($name); } public function provideInvalidCommandNamesSingle() { return array( - array('foo:baR'), - array('foO:bar') + array('foo3:baR'), + array('foO3:bar') ); } @@ -288,6 +320,8 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase } catch (\Exception $e) { $this->assertInstanceOf('\InvalidArgumentException', $e, '->find() throws an \InvalidArgumentException if command does not exist, with alternatives'); $this->assertRegExp('/Did you mean one of these/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternatives'); + $this->assertRegExp('/foo1:bar/', $e->getMessage()); + $this->assertRegExp('/foo:bar/', $e->getMessage()); } // Namespace + plural @@ -297,6 +331,7 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase } catch (\Exception $e) { $this->assertInstanceOf('\InvalidArgumentException', $e, '->find() throws an \InvalidArgumentException if command does not exist, with alternatives'); $this->assertRegExp('/Did you mean one of these/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternatives'); + $this->assertRegExp('/foo1/', $e->getMessage()); } $application->add(new \Foo3Command()); @@ -329,26 +364,17 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase $this->assertEquals(sprintf('Command "%s" is not defined.', $commandName), $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, without alternatives'); } + // Test if "bar1" command throw an "\InvalidArgumentException" and does not contain + // "foo:bar" as alternative because "bar1" is too far from "foo:bar" try { - $application->find($commandName = 'foo'); + $application->find($commandName = 'bar1'); $this->fail('->find() throws an \InvalidArgumentException if command does not exist'); } catch (\Exception $e) { $this->assertInstanceOf('\InvalidArgumentException', $e, '->find() throws an \InvalidArgumentException if command does not exist'); $this->assertRegExp(sprintf('/Command "%s" is not defined./', $commandName), $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternatives'); - $this->assertRegExp('/foo:bar/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternative : "foo:bar"'); - $this->assertRegExp('/foo1:bar/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternative : "foo1:bar"'); + $this->assertRegExp('/afoobar1/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternative : "afoobar1"'); $this->assertRegExp('/foo:bar1/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternative : "foo:bar1"'); - } - - // Test if "foo1" command throw an "\InvalidArgumentException" and does not contain - // "foo:bar" as alternative because "foo1" is too far from "foo:bar" - try { - $application->find($commandName = 'foo1'); - $this->fail('->find() throws an \InvalidArgumentException if command does not exist'); - } catch (\Exception $e) { - $this->assertInstanceOf('\InvalidArgumentException', $e, '->find() throws an \InvalidArgumentException if command does not exist'); - $this->assertRegExp(sprintf('/Command "%s" is not defined./', $commandName), $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternatives'); - $this->assertFalse(strpos($e->getMessage(), 'foo:bar'), '->find() throws an \InvalidArgumentException if command does not exist, without "foo:bar" alternative'); + $this->assertNotRegExp('/foo:bar(?>!1)/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, without "foo:bar" alternative'); } } diff --git a/src/Symfony/Component/Console/Tests/Fixtures/FoobarCommand.php b/src/Symfony/Component/Console/Tests/Fixtures/FoobarCommand.php new file mode 100644 index 0000000000..968162804c --- /dev/null +++ b/src/Symfony/Component/Console/Tests/Fixtures/FoobarCommand.php @@ -0,0 +1,25 @@ +setName('foobar:foo') + ->setDescription('The foobar:foo command') + ; + } + + protected function execute(InputInterface $input, OutputInterface $output) + { + $this->input = $input; + $this->output = $output; + } +}