Merge pull request #3 from parable-php/dev

Refactors and psalm
parable-php · Mar 11, 2021 · 58b0652 · 58b0652
2 parents 31258aa + b163a2d
commit 58b0652
Show file tree

Hide file tree

Showing 21 changed files with 131 additions and 70 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -32,3 +32,6 @@ jobs:
 
     - name: Run test suite
       run: vendor/bin/phpunit tests
+
+    - name: Run static analysis
+      run: vendor/bin/psalm
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,13 @@
 # Parable Console
 
+## 0.6.0
+
+_Changes_
+- Add static analysis using psalm.
+- `Output::writelns(string ...$lines)` now takes multiple string values instead of an array of those.
+- `Exception` has been renamed to `ConsoleException` for clarity.
+- Multiple small code changes to make it more php8.
+
 ## 0.5.1
 
 _Changes_

diff --git a/Makefile b/Makefile
@@ -4,16 +4,20 @@ dependencies:
 		--no-plugins \
 		--no-scripts
 
+psalm:
+	vendor/bin/psalm --clear-cache
+	vendor/bin/psalm
+
 tests: dependencies
 	vendor/bin/phpunit --verbose tests
 
 coverage: dependencies
 	rm -rf ./coverage
-	vendor/bin/phpunit --coverage-html ./coverage tests
+	XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-html ./coverage tests
 
 tests-clean:
 	vendor/bin/phpunit --verbose tests
 
 coverage-clean:
 	rm -rf ./coverage
-	vendor/bin/phpunit --coverage-html ./coverage tests
+	XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-html ./coverage tests
diff --git a/composer.json b/composer.json
@@ -17,7 +17,8 @@
     "parable-php/di": "^0.3.0"
   },
   "require-dev": {
-    "phpunit/phpunit": "^8.0"
+    "phpunit/phpunit": "^8.0",
+    "vimeo/psalm": "^4.6"
   },
   "autoload": {
     "psr-4": {

diff --git a/psalm.xml b/psalm.xml
@@ -0,0 +1,19 @@
+<?xml version="1.0"?>
+<psalm
+    errorLevel="4"
+    resolveFromConfigFile="true"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xmlns="https://getpsalm.org/schema/config"
+    xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
+>
+    <projectFiles>
+        <directory name="src" />
+        <ignoreFiles>
+            <directory name="vendor" />
+        </ignoreFiles>
+    </projectFiles>
+
+    <issueHandlers>
+        <UnresolvableInclude errorLevel="suppress" />
+    </issueHandlers>
+</psalm>
diff --git a/src/Application.php b/src/Application.php
@@ -8,7 +8,10 @@
 class Application
 {
     protected ?string $name = null;
+    /** @var Command[] */
     protected array $commands = [];
+    /** @var string[] */
+    protected array $commandNames = [];
     protected ?Command $activeCommand;
     protected ?string $defaultCommand = null;
     protected bool $onlyUseDefaultCommand = false;
@@ -23,7 +26,10 @@ public function __construct(
             $this->output->writeErrorBlock([$e->getMessage()]);
 
             if ($this->activeCommand) {
-                $this->output->writeln('<yellow>Usage</yellow>: ' . $this->getCommandUsage($this->activeCommand));
+                $this->output->writeln(sprintf(
+                    '<yellow>Usage</yellow>: %s',
+                    $this->getCommandUsage($this->activeCommand)
+                ));
             }
         });
     }
@@ -46,7 +52,7 @@ public function addCommand(Command $command): void
 
     public function addCommandByNameAndClass(string $commandName, string $className): void
     {
-        $this->commands[$commandName] = $className;
+        $this->commandNames[$commandName] = $className;
     }
 
     /**
@@ -82,7 +88,8 @@ public function shouldOnlyUseDefaultCommand(): bool
 
     public function hasCommand(string $commandName): bool
     {
-        return isset($this->commands[$commandName]);
+        return isset($this->commands[$commandName])
+            || isset($this->commandNames[$commandName]);
     }
 
     public function getCommand(string $commandName): ?Command
@@ -91,8 +98,10 @@ public function getCommand(string $commandName): ?Command
             return null;
         }
 
-        if (is_string($this->commands[$commandName])) {
-            $this->addCommand($this->container->get($this->commands[$commandName]));
+        if (isset($this->commandNames[$commandName]) && !isset($this->commands[$commandName])) {
+            /** @var Command $command */
+            $command = $this->container->get($this->commandNames[$commandName]);
+            $this->addCommand($command);
         }
 
         return $this->commands[$commandName];
@@ -174,7 +183,7 @@ public function run(): void
         $command = $command ?: $defaultCommand;
 
         if (!$command) {
-            throw Exception::fromMessage('No valid commands found.');
+            throw ConsoleException::fromMessage('No valid commands found.');
         }
 
         if (!$command->isPrepared()) {

diff --git a/src/Command.php b/src/Command.php
@@ -110,10 +110,16 @@ public function getArguments(): array
 
     public function run(): void
     {
-        $callable = $this->getCallable();
-        if (is_callable($callable)) {
-            $callable($this->application, $this->output, $this->input, $this->parameter);
+        if ($this->getCallable() === null) {
+            return;
         }
+
+        ($this->getCallable())(
+            $this->application,
+            $this->output,
+            $this->input,
+            $this->parameter
+        );
     }
 
     /**

diff --git a/src/Commands/HelpCommand.php b/src/Commands/HelpCommand.php
@@ -37,6 +37,7 @@ protected function showGeneralHelp(): void
         $longestName = 0;
         foreach ($this->application->getCommands() as $command) {
             $strlen = strlen($command->getName());
+
             if ($strlen > $longestName) {
                 $longestName = $strlen;
             }

diff --git a/src/Exception.php → src/ConsoleException.php b/src/Exception.php → src/ConsoleException.php
@@ -2,7 +2,7 @@
 
 namespace Parable\Console;
 
-class Exception extends \Exception
+class ConsoleException extends \Exception
 {
     public static function fromMessage(string $message, ...$replacements): self
     {

diff --git a/src/Environment.php b/src/Environment.php
@@ -13,6 +13,7 @@ public function getTerminalWidth(): int
             return self::TERMINAL_DEFAULT_WIDTH;
         }
 
+        /** @psalm-suppress ForbiddenCode */
         return (int)shell_exec('TERM=ansi tput cols');
     }
 
@@ -22,6 +23,7 @@ public function getTerminalHeight(): int
             return self::TERMINAL_DEFAULT_HEIGHT;
         }
 
+        /** @psalm-suppress ForbiddenCode */
         return (int)shell_exec('TERM=ansi tput lines');
     }
 

diff --git a/src/Input.php b/src/Input.php
@@ -6,8 +6,7 @@ class Input
 {
     public function __construct(
         protected Environment $environment
-    ) {
-    }
+    ) {}
 
     public function get(): string
     {
@@ -36,7 +35,7 @@ public function disableShowInput(): void
     public function getHidden(): string
     {
         if ($this->environment->isWindows()) {
-            throw Exception::fromMessage("Hidden input is not supported on windows.");
+            throw ConsoleException::fromMessage("Hidden input is not supported on windows.");
         }
 
         $this->disableShowInput();

diff --git a/src/Output.php b/src/Output.php
@@ -9,8 +9,7 @@ class Output
     public function __construct(
         protected Environment $environment,
         protected Tags $tags
-    ) {
-    }
+    ) {}
 
     public function write(string $string): void
     {
@@ -27,7 +26,7 @@ public function writeln(string $line): void
         $this->newline();
     }
 
-    public function writelns(array $lines): void
+    public function writelns(string ...$lines): void
     {
         foreach ($lines as $line) {
             $this->writeln($line);
@@ -142,9 +141,11 @@ public function writeBlock(array $lines, array $tags = []): void
 
         $actualLines = [];
         foreach ($lines as $line) {
-            $actualLines = array_merge($actualLines, explode("\n", $line));
+            $actualLines[] = explode("\n", $line);
         }
 
+        $actualLines = array_merge([], ...$actualLines);
+
         foreach ($actualLines as $line) {
             $strlen = max($strlen, mb_strlen($line));
         }
@@ -188,6 +189,6 @@ public function writeBlock(array $lines, array $tags = []): void
         );
         $outputLines[] = "";
 
-        $this->writelns($outputLines);
+        $this->writelns(...$outputLines);
     }
 }
diff --git a/src/Parameter.php b/src/Parameter.php
@@ -48,7 +48,7 @@ public function getParameters(): array
         return $this->parameters;
     }
 
-    /**
+    /*
      * Split the parameters into script name, command name, options and arguments.
      *
      * Flag options can be passed in a single set preceded by a dash:
@@ -95,7 +95,7 @@ protected function parseOption(string $optionString): void
         $this->options[$key] = $value;
     }
 
-    /**
+    /*
      * Parse a flag option string (-a or -abc, this last version
      * is parsed as a concatenated string of one char per option).
      */
@@ -117,7 +117,7 @@ protected function parseFlagOption(string $optionString): void
         }
     }
 
-    /**
+    /*
      * Parse argument. If no command name set and commands are enabled,
      * interpret as command name. Otherwise, add to argument list.
      */
@@ -142,12 +142,13 @@ public function getCommandName(): ?string
 
     /**
      * @param OptionParameter[] $options
+     * @throws ConsoleException
      */
     public function setCommandOptions(array $options): void
     {
         foreach ($options as $name => $option) {
             if ((!$option instanceof OptionParameter)) {
-                throw Exception::fromMessage(
+                throw ConsoleException::fromMessage(
                     "Options must be instances of Parameter\\Option. %s is not.",
                     $name
                 );
@@ -169,12 +170,13 @@ public function checkCommandOptions(): void
             } else {
                 $parameters = $this->options;
             }
+
             $option->addParameters($parameters);
 
             if ($option->isValueRequired() && $option->hasBeenProvided() && !$option->getValue()) {
                 $dashes = $option->isFlagOption() ? '-' : '--';
 
-                throw Exception::fromMessage(
+                throw ConsoleException::fromMessage(
                     "Option '%s%s' requires a value, which is not provided.",
                     $dashes,
                     $option->getName()
@@ -183,8 +185,9 @@ public function checkCommandOptions(): void
         }
     }
 
-    /**
-     * Returns null if the value doesn't exist. Otherwise, it's whatever was passed to it or set
+    /*
+     * Returns null if the value doesn't exist. Returns true if the option was provided
+     * but no value was provided. Otherwise, it's whatever was passed to it or set
      * as a default value.
      */
     public function getOption(string $name)
@@ -195,7 +198,10 @@ public function getOption(string $name)
 
         $option = $this->commandOptions[$name];
 
-        if ($option->hasBeenProvided() && $option->getProvidedValue() === null && $option->getDefaultValue() === null) {
+        if ($option->hasBeenProvided()
+            && $option->getProvidedValue() === null
+            && $option->getDefaultValue() === null
+        ) {
             return true;
         }
 
@@ -205,25 +211,29 @@ public function getOption(string $name)
     public function getOptions(): array
     {
         $returnArray = [];
+
         foreach ($this->commandOptions as $option) {
             $optionName = $option->getName();
 
             $returnArray[$optionName] = $this->getOption($optionName);
         }
+
         return $returnArray;
     }
 
     /**
      * Set the arguments from a command.
      *
      * @param ArgumentParameter[] $arguments
+     * @throws ConsoleException
      */
     public function setCommandArguments(array $arguments): void
     {
         $orderedArguments = [];
+
         foreach ($arguments as $index => $argument) {
             if (!($argument instanceof ArgumentParameter)) {
-                throw Exception::fromMessage(
+                throw ConsoleException::fromMessage(
                     "Arguments must be instances of Parameter\\Argument. The item at index %d is not.",
                     $index
                 );
@@ -232,6 +242,7 @@ public function setCommandArguments(array $arguments): void
             $argument->setOrder($index);
             $orderedArguments[$index] = $argument;
         }
+
         $this->commandArguments = $orderedArguments;
     }
 
@@ -245,7 +256,7 @@ public function checkCommandArguments(): void
             $argument->addParameters($this->arguments);
 
             if ($argument->isRequired() && !$argument->hasBeenProvided()) {
-                throw Exception::fromMessage(
+                throw ConsoleException::fromMessage(
                     "Required argument with index #%d '%s' not provided.",
                     $index,
                     $argument->getName()
@@ -318,6 +329,7 @@ public function disableCommandName(): void
         if ($this->commandNameEnabled && $this->commandName) {
             array_unshift($this->arguments, $this->commandName);
         }
+
         $this->commandNameEnabled = false;
     }
 }