From ab933a763f069203c16992f11350afb151cd8e19 Mon Sep 17 00:00:00 2001 From: Robin de Graaf Date: Thu, 11 Mar 2021 13:01:54 +0000 Subject: [PATCH 1/3] Refactors and psalm --- .github/workflows/tests.yml | 0 Makefile | 8 ++++-- composer.json | 3 +- psalm.xml | 19 ++++++++++++ src/Application.php | 21 ++++++++++---- src/Command.php | 12 ++++++-- src/Commands/HelpCommand.php | 1 + src/{Exception.php => ConsoleException.php} | 2 +- src/Environment.php | 2 ++ src/Input.php | 5 ++-- src/Output.php | 11 +++---- src/Parameter.php | 32 ++++++++++++++------- src/Parameters/AbstractParameter.php | 6 +--- src/Parameters/ArgumentParameter.php | 4 +-- src/Parameters/OptionParameter.php | 7 +++-- src/Tags.php | 23 +++++++-------- tests/ApplicationTest.php | 6 ++-- tests/InputTest.php | 4 +-- tests/OutputTest.php | 6 ++-- tests/ParameterTest.php | 18 ++++++------ 20 files changed, 120 insertions(+), 70 deletions(-) mode change 100755 => 100644 .github/workflows/tests.yml create mode 100644 psalm.xml rename src/{Exception.php => ConsoleException.php} (87%) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml old mode 100755 new mode 100644 diff --git a/Makefile b/Makefile index 79c8f34..289792f 100755 --- 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 \ No newline at end of file + XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-html ./coverage tests diff --git a/composer.json b/composer.json index 97b6d0a..eac92b6 100755 --- 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 new file mode 100644 index 0000000..8679d7a --- /dev/null +++ b/psalm.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + diff --git a/src/Application.php b/src/Application.php index 11e396f..7f4a7c2 100755 --- 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('Usage: ' . $this->getCommandUsage($this->activeCommand)); + $this->output->writeln(sprintf( + 'Usage: %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 index 7a7106f..bd9423e 100755 --- 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 index 2eb1b28..c60ee94 100755 --- 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 b/src/ConsoleException.php similarity index 87% rename from src/Exception.php rename to src/ConsoleException.php index 742a2ed..c7db13e 100755 --- a/src/Exception.php +++ b/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 index 0773c2c..e4973b2 100755 --- 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 index b99749b..92dda95 100755 --- 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 index 78c8ddf..6a1650d 100755 --- 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 index f934d67..17e7e7d 100755 --- 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,11 +211,13 @@ 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; } @@ -217,13 +225,15 @@ public function getOptions(): array * 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; } } diff --git a/src/Parameters/AbstractParameter.php b/src/Parameters/AbstractParameter.php index 8fd02c9..f84b7cb 100755 --- a/src/Parameters/AbstractParameter.php +++ b/src/Parameters/AbstractParameter.php @@ -51,11 +51,7 @@ public function getProvidedValue(): ?string public function getValue(): mixed { - if ($this->getProvidedValue() !== null) { - return $this->getProvidedValue(); - } - - return $this->getDefaultValue(); + return $this->getProvidedValue() ?? $this->getDefaultValue(); } /** diff --git a/src/Parameters/ArgumentParameter.php b/src/Parameters/ArgumentParameter.php index 3c63b46..aa444c4 100755 --- a/src/Parameters/ArgumentParameter.php +++ b/src/Parameters/ArgumentParameter.php @@ -2,7 +2,7 @@ namespace Parable\Console\Parameters; -use Parable\Console\Exception; +use Parable\Console\ConsoleException; use Parable\Console\Parameter; class ArgumentParameter extends AbstractParameter @@ -30,7 +30,7 @@ public function setRequired(int $required): void ], true )) { - throw Exception::fromMessage('Required must be one of the PARAMETER_* constants.'); + throw ConsoleException::fromMessage('Required must be one of the PARAMETER_* constants.'); } $this->required = $required; diff --git a/src/Parameters/OptionParameter.php b/src/Parameters/OptionParameter.php index 950cf63..590cf22 100755 --- a/src/Parameters/OptionParameter.php +++ b/src/Parameters/OptionParameter.php @@ -2,7 +2,7 @@ namespace Parable\Console\Parameters; -use Parable\Console\Exception; +use Parable\Console\ConsoleException; use Parable\Console\Parameter; class OptionParameter extends AbstractParameter @@ -32,7 +32,7 @@ public function setValueType(int $valueType): void ], true )) { - throw Exception::fromMessage('Value type must be one of the OPTION_* constants.'); + throw ConsoleException::fromMessage('Value type must be one of the OPTION_* constants.'); } $this->valueType = $valueType; @@ -46,8 +46,9 @@ public function isValueRequired(): bool public function setFlagOption(bool $enabled): void { if ($enabled && mb_strlen($this->getName()) > 1) { - throw Exception::fromMessage("Flag options can only have a single-letter name."); + throw ConsoleException::fromMessage("Flag options can only have a single-letter name."); } + $this->flagOption = $enabled; } diff --git a/src/Tags.php b/src/Tags.php index ab898ae..ad8b9da 100755 --- a/src/Tags.php +++ b/src/Tags.php @@ -74,6 +74,7 @@ protected function getTagsFromString(string $string): array preg_match_all('/<(?!\/)(.|\n)*?>/', $string, $matches); $tags = []; + foreach ($matches[0] as $tag) { $tags[] = trim($tag, '<>'); } @@ -85,29 +86,27 @@ protected function getCodeFor(string $tag): string { try { return $this->getCodeForPredefined($tag); - } catch (Throwable) { - } + } catch (Throwable) {} try { $tags = $this->getTagsForSet($tag); + } catch (Throwable) { + throw ConsoleException::fromMessage('No predefined or tag set found for <%s>.', $tag); + } - $codes = ''; - - foreach ($tags as $tagFound) { - $codes .= $this->getCodeForPredefined($tagFound); - } + $codes = ''; - return $codes; - } catch (Throwable) { + foreach ($tags as $tagFound) { + $codes .= $this->getCodeForPredefined($tagFound); } - throw Exception::fromMessage('No predefined or tag set found for <%s>.', $tag); + return $codes; } protected function getCodeForPredefined(string $tag): string { if (!isset($this->predefinedTags[$tag])) { - throw Exception::fromMessage('Predefined tag <%s> not found.', $tag); + throw ConsoleException::fromMessage('Predefined tag <%s> not found.', $tag); } return $this->predefinedTags[$tag]; @@ -116,7 +115,7 @@ protected function getCodeForPredefined(string $tag): string protected function getTagsForSet(string $tag): array { if (!isset($this->tagSets[$tag])) { - throw Exception::fromMessage('Tag set <%s> not found.', $tag); + throw ConsoleException::fromMessage('Tag set <%s> not found.', $tag); } return $this->tagSets[$tag]; diff --git a/tests/ApplicationTest.php b/tests/ApplicationTest.php index b58b8ec..4bd04f0 100755 --- a/tests/ApplicationTest.php +++ b/tests/ApplicationTest.php @@ -4,7 +4,7 @@ use Parable\Console\Application; use Parable\Console\Command; -use Parable\Console\Exception; +use Parable\Console\ConsoleException; use Parable\Console\Input; use Parable\Console\Output; use Parable\Console\Parameter; @@ -294,7 +294,7 @@ public function testOptionalOptionWithRequiredValueThrowsExceptionIfNoValue(): v $application->setDefaultCommand($this->command1); - $this->expectException(Exception::class); + $this->expectException(ConsoleException::class); $this->expectExceptionMessage("Option '--option' requires a value, which is not provided."); $application->run(); @@ -339,7 +339,7 @@ public function testOptionWithDefaultValueWorksProperly(): void public function testThrowsExceptionWhenRanWithoutCommand(): void { $this->expectExceptionMessage("No valid commands found."); - $this->expectException(Exception::class); + $this->expectException(ConsoleException::class); $application = $this->container->buildAll(Application::class); $application->run(); diff --git a/tests/InputTest.php b/tests/InputTest.php index be7ba25..9f08bf3 100755 --- a/tests/InputTest.php +++ b/tests/InputTest.php @@ -3,7 +3,7 @@ namespace Parable\Console\Tests; use Parable\Console\Environment; -use Parable\Console\Exception; +use Parable\Console\ConsoleException; use Parable\Console\Input; use PHPUnit\Framework\MockObject\MockObject; @@ -87,7 +87,7 @@ public function testGetHidden(): void public function testGetHiddenThrowsOnWindows(): void { - $this->expectException(Exception::class); + $this->expectException(ConsoleException::class); $this->expectExceptionMessage('Hidden input is not supported on windows.'); $this->createInput(true); diff --git a/tests/OutputTest.php b/tests/OutputTest.php index 3e71c33..85f4b7d 100755 --- a/tests/OutputTest.php +++ b/tests/OutputTest.php @@ -46,12 +46,12 @@ public function testWriteln(): void $this->assertSameWithTag("OK\n", $content); } - public function testWritelnWithArray(): void + public function testWritelnWithMultipleLines(): void { - $this->output->writelns([ + $this->output->writelns( 'line1', 'line2' - ]); + ); $content = $this->getActualOutputAndClean(); $this->assertSameWithTag("line1\nline2\n", $content); diff --git a/tests/ParameterTest.php b/tests/ParameterTest.php index 7657975..c71de02 100755 --- a/tests/ParameterTest.php +++ b/tests/ParameterTest.php @@ -2,7 +2,7 @@ namespace Parable\Console\Tests; -use Parable\Console\Exception; +use Parable\Console\ConsoleException; use Parable\Console\Parameter; use Parable\Console\Parameters\ArgumentParameter; use Parable\Console\Parameters\OptionParameter; @@ -95,7 +95,7 @@ public function testCommandNameIsNullIfNotGivenButThereIsAnOptionGiven(): void public function testThrowsExceptionWhenOptionIsGivenButValueRequiredNotGiven(): void { - $this->expectException(Exception::class); + $this->expectException(ConsoleException::class); $this->expectExceptionMessage("Option '--option' requires a value, which is not provided."); $this->parameter->setParameters([ @@ -116,7 +116,7 @@ public function testThrowsExceptionWhenOptionIsGivenButValueRequiredNotGiven(): public function testThrowsExceptionWhenFlagOptionIsGivenButValueRequiredNotGiven(): void { - $this->expectException(Exception::class); + $this->expectException(ConsoleException::class); $this->expectExceptionMessage("Option '-a' requires a value, which is not provided."); $this->parameter->setParameters([ @@ -158,7 +158,7 @@ public function testOptionIsGivenAndValueRequiredAlsoGivenWorksProperly(): void public function testRequiredArgumentThrowsException(): void { - $this->expectException(Exception::class); + $this->expectException(ConsoleException::class); $this->expectExceptionMessage("Required argument with index #1 'numero2' not provided."); $this->parameter->setParameters([ @@ -278,7 +278,7 @@ public function testArgumentsWorkProperly(): void public function testSetCommandOptionsWithArrayThrowsException(): void { - $this->expectException(Exception::class); + $this->expectException(ConsoleException::class); $this->expectExceptionMessage("Options must be instances of Parameter\Option. invalid_option is not."); $this->parameter->setCommandOptions(["invalid_option" => []]); @@ -286,7 +286,7 @@ public function testSetCommandOptionsWithArrayThrowsException(): void public function testSetCommandArgumentsWithArrayThrowsException(): void { - $this->expectException(Exception::class); + $this->expectException(ConsoleException::class); $this->expectExceptionMessage("Arguments must be instances of Parameter\Argument. The item at index 0 is not."); $this->parameter->setCommandArguments([[]]); @@ -340,7 +340,7 @@ public function testEnableDisableCommandNameKeepsArgumentOrderValid(): void public function testParameterRequiredOnlyAcceptConstantValues(): void { - $this->expectException(Exception::class); + $this->expectException(ConsoleException::class); $this->expectExceptionMessage("Required must be one of the PARAMETER_* constants."); new ArgumentParameter("test", 418); @@ -348,7 +348,7 @@ public function testParameterRequiredOnlyAcceptConstantValues(): void public function testParameterValueRequiredOnlyAcceptConstantValues(): void { - $this->expectException(Exception::class); + $this->expectException(ConsoleException::class); $this->expectExceptionMessage("Value type must be one of the OPTION_* constants."); new OptionParameter( @@ -553,7 +553,7 @@ public function testSkippingUndefinedOptions(): void public function testFlagOptionCanOnlyHaveSingleLetterName(): void { - $this->expectException(Exception::class); + $this->expectException(ConsoleException::class); $this->expectExceptionMessage('Flag options can only have a single-letter name.'); new OptionParameter("test", Parameter::OPTION_VALUE_OPTIONAL, null, true); From 884718cbcf7c8cca95378c769a4d03488c6e5569 Mon Sep 17 00:00:00 2001 From: Robin de Graaf Date: Thu, 11 Mar 2021 13:04:10 +0000 Subject: [PATCH 2/3] Add psalm to github workflow --- .github/workflows/tests.yml | 3 +++ psalm.xml | 0 2 files changed, 3 insertions(+) mode change 100644 => 100755 .github/workflows/tests.yml mode change 100644 => 100755 psalm.xml diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml old mode 100644 new mode 100755 index 5d3a185..d181c60 --- 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/psalm.xml b/psalm.xml old mode 100644 new mode 100755 From b163a2d53ce00681abfb26ca8363c1f509f75523 Mon Sep 17 00:00:00 2001 From: Robin de Graaf Date: Thu, 11 Mar 2021 13:08:33 +0000 Subject: [PATCH 3/3] Update CHANGELOG for 0.6.0 --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13bc10f..e753c07 100755 --- 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_