Merge pull request #4683 from paulbalandan/command-runner

Optimize CommandRunner and Commands

system/CLI/BaseCommand.php

@@ -16,7 +16,16 @@
 use Throwable;
 
 /**
- * Class BaseCommand
+ * BaseCommand is the base class used in creating CLI commands.
+ *
+ * @property string          $group
+ * @property string          $name
+ * @property string          $usage
+ * @property string          $description
+ * @property array           $options
+ * @property array           $arguments
+ * @property LoggerInterface $logger
+ * @property Commands        $commands
  */
 abstract class BaseCommand
 {
@@ -78,7 +87,6 @@ abstract class BaseCommand
 	 */
 	protected $commands;
 
-	//--------------------------------------------------------------------
 	/**
 	 * BaseCommand constructor.
 	 *
@@ -91,8 +99,6 @@ public function __construct(LoggerInterface $logger, Commands $commands)
 		$this->commands = $commands;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Actually execute a command.
 	 * This has to be over-ridden in any concrete implementation.
@@ -101,8 +107,6 @@ public function __construct(LoggerInterface $logger, Commands $commands)
 	 */
 	abstract public function run(array $params);
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Can be used by a command to run other commands.
 	 *
@@ -117,8 +121,6 @@ protected function call(string $command, array $params = [])
 		return $this->commands->run($command, $params);
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * A simple method to display an error with line/file, in child commands.
 	 *
@@ -132,8 +134,6 @@ protected function showError(Throwable $e)
 		require APPPATH . 'Views/errors/cli/error_exception.php';
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Show Help includes (Usage, Arguments, Description, Options).
 	 */
@@ -189,8 +189,6 @@ public function showHelp()
 		}
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Pads our string out so that all titles are the same length to nicely line up descriptions.
 	 *
@@ -208,8 +206,6 @@ public function setPad(string $item, int $max, int $extra = 2, int $indent = 0):
 		return str_pad(str_repeat(' ', $indent) . $item, $max);
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Get pad for $key => $value array output
 	 *
@@ -225,15 +221,15 @@ public function setPad(string $item, int $max, int $extra = 2, int $indent = 0):
 	public function getPad(array $array, int $pad): int
 	{
 		$max = 0;
+
 		foreach (array_keys($array) as $key)
 		{
 			$max = max($max, strlen($key));
 		}
+
 		return $max + $pad;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Makes it simple to access our protected properties.
 	 *
@@ -251,8 +247,6 @@ public function __get(string $key)
 		return null;
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Makes it simple to check our protected properties.
 	 *

system/CLI/CommandRunner.php

@@ -21,14 +21,12 @@
 class CommandRunner extends Controller
 {
 	/**
-	 * The Command Manager
+	 * Instance of class managing the collection of commands
 	 *
 	 * @var Commands
 	 */
 	protected $commands;
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Constructor
 	 */
@@ -58,8 +56,6 @@ public function _remap($method, ...$params)
 		return $this->index($params);
 	}
 
-	//--------------------------------------------------------------------
-
 	/**
 	 * Default command.
 	 *
@@ -70,14 +66,9 @@ public function _remap($method, ...$params)
 	 */
 	public function index(array $params)
 	{
-		$command = array_shift($params);
-
-		if (is_null($command))
-		{
-			$command = 'list';
-		}
+		$command = array_shift($params) ?? 'list';
 
-		return service('commands')->run($command, $params);
+		return $this->commands->run($command, $params);
 	}
 
 	/**

system/CLI/Commands.php

@@ -11,14 +11,12 @@
 
 namespace CodeIgniter\CLI;
 
+use CodeIgniter\Autoloader\FileLocator;
 use CodeIgniter\Log\Logger;
-use Config\Services;
 use ReflectionClass;
 use ReflectionException;
 
 /**
- * Class Commands
- *
  * Core functionality for running, listing, etc commands.
  */
 class Commands
@@ -44,7 +42,8 @@ class Commands
 	 */
 	public function __construct($logger = null)
 	{
-		$this->logger = $logger ?? Services::logger();
+		$this->logger = $logger ?? service('logger');
+		$this->discoverCommands();
 	}
 
 	/**
@@ -55,8 +54,6 @@ public function __construct($logger = null)
 	 */
 	public function run(string $command, array $params)
 	{
-		$this->discoverCommands();
-
 		if (! $this->verifyCommand($command, $this->commands))
 		{
 			return;
@@ -77,39 +74,39 @@ public function run(string $command, array $params)
 	 */
 	public function getCommands()
 	{
-		$this->discoverCommands();
-
 		return $this->commands;
 	}
 
 	/**
 	 * Discovers all commands in the framework and within user code,
 	 * and collects instances of them to work with.
+	 *
+	 * @return void
 	 */
 	public function discoverCommands()
 	{
-		if (! empty($this->commands))
+		if ($this->commands !== [])
 		{
 			return;
 		}
 
-		$files = service('locator')->listFiles('Commands/');
+		/** @var FileLocator $locator */
+		$locator = service('locator');
+		$files   = $locator->listFiles('Commands/');
 
 		// If no matching command files were found, bail
-		if (empty($files))
+		// This should never happen in unit testing.
+		if ($files === [])
 		{
-			// This should never happen in unit testing.
-			// if it does, we have far bigger problems!
-			// @codeCoverageIgnoreStart
-			return;
-			// @codeCoverageIgnoreEnd
+			return; // @codeCoverageIgnore
 		}
 
 		// Loop over each file checking to see if a command with that
-		// alias exists in the class. If so, return it. Otherwise, try the next.
+		// alias exists in the class.
 		foreach ($files as $file)
 		{
-			$className = Services::locator()->findQualifiedNameFromPath($file);
+			$className = $locator->findQualifiedNameFromPath($file);
+
 			if (empty($className) || ! class_exists($className))
 			{
 				continue;
@@ -124,10 +121,10 @@ public function discoverCommands()
 					continue;
 				}
 
+				/** @var BaseCommand $class */
 				$class = new $className($this->logger, $this);
 
-				// Store it!
-				if (! is_null($class->group))
+				if (isset($class->group))
 				{
 					$this->commands[$class->name] = [
 						'class'       => $className,
@@ -137,7 +134,6 @@ public function discoverCommands()
 					];
 				}
 
-				$class = null;
 				unset($class);
 			}
 			catch (ReflectionException $e)