merged branch hason/console (PR #8800)

This PR was merged into the master branch.

Discussion
----------

[Console] Improved searching commands

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

I was very annoyed when I typed the command ``app/console a:d`` and I saw this exception ``The namespace "a" is ambiguous (assets, assetic)``. However, the shortcut ``a:d`` is unambiguous (``assets:install`` vs ``assetic:dump``).

Commits
-------

0beec8a [Console] Improved searching commands

src/Symfony/Component/Console/Application.php

@@ -494,51 +494,31 @@ class Application
     public function findNamespace($namespace)
     {
         $allNamespaces = $this->getNamespaces();
-        $found = '';
+        $expr = preg_replace_callback('{([^:]+|)}', function ($matches) { return preg_quote($matches[1]).'[^:]*'; }, $namespace);
-        foreach (explode(':', $namespace) as $i => $part) {
+        $namespaces = preg_grep('{^'.$expr.'}', $allNamespaces);
-            // 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);
-                }
-            }
 
-            $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])) {
+            if ($alternatives = $this->findAlternatives($namespace, $allNamespaces, array())) {
-                $message = sprintf('There are no commands defined in the "%s" namespace.', $namespace);
+                if (1 == count($alternatives)) {
-
+                    $message .= "\n\nDid you mean this?\n    ";
-                if (1 <= $i) {
+                } else {
-                    $part = $found.':'.$part;
+                    $message .= "\n\nDid you mean one of these?\n    ";
                 }
 
-                if ($alternatives = $this->findAlternativeNamespace($part, $abbrevs)) {
+                $message .= implode("\n    ", $alternatives);
-                    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);
             }
 
-            // there are multiple matches, but $part is an exact match of one of them so we select it
+            throw new \InvalidArgumentException($message);
-            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];
         }
 
-        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
+        $allCommands = array_keys($this->commands);
-        $namespace = '';
+        $expr = preg_replace_callback('{([^:]+|)}', function ($matches) { return preg_quote($matches[1]).'[^:]*'; }, $name);
-        $searchName = $name;
+        $commands = preg_grep('{^'.$expr.'}', $allCommands);
-        if (false !== $pos = strrpos($name, ':')) {
-            $namespace = $this->findNamespace(substr($name, 0, $pos));
-            $searchName = $namespace.substr($name, $pos);
-        }
 
-        // name
+        if (empty($commands) || count(preg_grep('{^'.$expr.'$}', $commands)) < 1) {
-        $commands = array();
+            if (false !== $pos = strrpos($name, ':')) {
-        foreach ($this->commands as $command) {
+                // check if a namespace exists and contains commands
-            $extractedNamespace = $this->extractNamespace($command->getName());
+                $this->findNamespace(substr($name, 0, $pos));
-            if ($extractedNamespace === $namespace
-               || !empty($namespace) && 0 === strpos($extractedNamespace, $namespace)
-            ) {
-                $commands[] = $command->getName();
             }
-        }
 
-        $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) {
+        $exact = in_array($name, $commands, true);
-            throw new \InvalidArgumentException(sprintf('Command "%s" is ambiguous (%s).', $name, $this->getAbbreviationSuggestions($aliases[$searchName])));
+        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
+     * Finds alternative of $name among $collection
-     *
-     * @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
      *
      * @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) {
+            $collectionParts[$item] = explode(':', $item);
-                $item = call_user_func($callback, $item);
-            }
-
-            $lev = levenshtein($name, $item);
-            if ($lev <= strlen($name) / 3 || false !== strpos($item, $name)) {
-                $alternatives[$item] = $lev;
-            }
         }
 
-        if (!$alternatives) {
+        foreach (explode(':', $name) as $i => $subname) {
-            foreach ($abbrevs as $key => $values) {
+            foreach ($collectionParts as $collectionName => $parts) {
-                $lev = levenshtein($name, $key);
+                $exists = isset($alternatives[$collectionName]);
-                if ($lev <= strlen($name) / 3 || false !== strpos($key, $name)) {
+                if (!isset($parts[$i]) && $exists) {
-                    foreach ($values as $value) {
+                    $alternatives[$collectionName] += $threshold;
-                        $alternatives[$value] = $lev;
+                    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);

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('foo3:baR'),
-            array('foO: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('/afoobar1/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternative : "afoobar1"');
-            $this->assertRegExp('/foo1:bar/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternative : "foo1:bar"');
             $this->assertRegExp('/foo:bar1/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternative : "foo:bar1"');
-        }
+            $this->assertNotRegExp('/foo:bar(?>!1)/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, without "foo:bar" alternative');
-
-        // 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');
         }
     }
 

src/Symfony/Component/Console/Tests/Fixtures/FoobarCommand.php

@@ -0,0 +1,25 @@
+<?php
+
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class FoobarCommand extends Command
+{
+    public $input;
+    public $output;
+
+    protected function configure()
+    {
+        $this
+            ->setName('foobar:foo')
+            ->setDescription('The foobar:foo command')
+        ;
+    }
+
+    protected function execute(InputInterface $input, OutputInterface $output)
+    {
+        $this->input = $input;
+        $this->output = $output;
+    }
+}