From 2b65a3df3fb089e35592dfb64b6b47e5ba6ef130 Mon Sep 17 00:00:00 2001 From: "Vladyslav G." Date: Wed, 2 Sep 2026 13:19:38 +0200 Subject: [PATCH] feat(commands): take a backed enum as a command option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fixed set of values had to be written as a string option with a choice list beside it — two places to change and one to forget, and the handler still received a bare string it had to validate itself. The framework already resolved backed enums for a component's arguments; a command's options were the one place it did not. Now an enum typed option becomes a string or integer option depending on its backing, every case is offered as a choice, and the handler is given the case. Cases are labelled by their own name, which is a PHP identifier and rarely what a bot wants shown, so an enum may implement Choosable to say how each one reads. A value that is not a case can only come from a client that made one up, since Discord checks a choice against the list it was given. That names the option and the value rather than surfacing a ValueError from deep inside from(). --- docs/README.md | 2 + docs/index.json | 18 ++++ docs/reference/index.md | 4 + docs/reference/options/choosable.md | 16 ++++ phpstan.neon | 4 + src/Compiler/CommandCompiler.php | 64 ++++++++++++- src/Interfaces/Choosable.php | 32 +++++++ src/Runtime/ArgumentResolver.php | 6 +- src/Runtime/OptionValueResolver.php | 32 +++++++ tests/Fixtures/Platform.php | 21 ++++ tests/Fixtures/PlatformCommand.php | 26 +++++ tests/Fixtures/Region.php | 12 +++ tests/Unit/Compiler/EnumOptionTest.php | 127 +++++++++++++++++++++++++ tools/src/ApiReflector.php | 3 + 14 files changed, 362 insertions(+), 5 deletions(-) create mode 100644 docs/reference/options/choosable.md create mode 100644 src/Interfaces/Choosable.php create mode 100644 tests/Fixtures/Platform.php create mode 100644 tests/Fixtures/PlatformCommand.php create mode 100644 tests/Fixtures/Region.php create mode 100644 tests/Unit/Compiler/EnumOptionTest.php diff --git a/docs/README.md b/docs/README.md index dfc6a5d..3ee6439 100644 --- a/docs/README.md +++ b/docs/README.md @@ -21,6 +21,8 @@ Generated from the source, so it describes what the framework actually does. **Attributes** — [Command](reference/attributes/command.md), [SubcommandGroup](reference/attributes/subcommand-group.md), [Subcommand](reference/attributes/subcommand.md), [Option](reference/attributes/option.md), [Event](reference/attributes/event.md), [Autocomplete](reference/attributes/autocomplete.md), [Button](reference/attributes/button.md), [SelectMenu](reference/attributes/select-menu.md), [ModalSubmit](reference/attributes/modal-submit.md) +**Options** — [Choosable](reference/options/choosable.md) + **Autocomplete** — [Autocomplete](reference/autocomplete/autocomplete.md), [ArrayAutocomplete](reference/autocomplete/array-autocomplete.md) **Cache** — [Cache](reference/cache/cache.md) diff --git a/docs/index.json b/docs/index.json index 33b506c..86606b1 100644 --- a/docs/index.json +++ b/docs/index.json @@ -326,6 +326,24 @@ "methods": [] } ], + "options": [ + { + "name": "Choosable", + "fqcn": "Tempcord\\Interfaces\\Choosable", + "kind": "interface", + "target": null, + "summary": "An enum that says how each of its cases should read in Discord.", + "slug": "reference/options/choosable", + "parameters": [], + "cases": [], + "methods": [ + { + "signature": "label(): string", + "summary": "What a member reads when picking this case." + } + ] + } + ], "autocomplete": [ { "name": "Autocomplete", diff --git a/docs/reference/index.md b/docs/reference/index.md index 67dc016..5d80e45 100644 --- a/docs/reference/index.md +++ b/docs/reference/index.md @@ -14,6 +14,10 @@ - [SelectMenu](attributes/select-menu.md) — Declares a class or method as the handler for a select menu choice. - [ModalSubmit](attributes/modal-submit.md) — Declares a class or method as the handler for a submitted modal. +## Options + +- [Choosable](options/choosable.md) — An enum that says how each of its cases should read in Discord. + ## Autocomplete - [Autocomplete](autocomplete/autocomplete.md) diff --git a/docs/reference/options/choosable.md b/docs/reference/options/choosable.md new file mode 100644 index 0000000..f68ac9a --- /dev/null +++ b/docs/reference/options/choosable.md @@ -0,0 +1,16 @@ + + +# Choosable + +An enum that says how each of its cases should read in Discord. + +```php +use Tempcord\Interfaces\Choosable; +``` + +## Methods + +### `label(): string` + +What a member reads when picking this case. + diff --git a/phpstan.neon b/phpstan.neon index b35a2d9..e1cd1f0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -6,3 +6,7 @@ parameters: - tools excludePaths: - tests/Fixtures + # Fixtures are not analysed — they exist to be malformed in interesting ways + # — but a test naming one still has to resolve to something. + scanDirectories: + - tests/Fixtures diff --git a/src/Compiler/CommandCompiler.php b/src/Compiler/CommandCompiler.php index 0f5a762..766a0da 100644 --- a/src/Compiler/CommandCompiler.php +++ b/src/Compiler/CommandCompiler.php @@ -24,6 +24,8 @@ use Tempcord\Interfaces\Autocomplete; use Tempcord\Localization\LocalizationProvider; use Tempcord\Localization\NullLocalizations; +use ReflectionEnum; +use Tempcord\Interfaces\Choosable; use Tempest\Reflection\ClassReflector; use Tempest\Reflection\MethodReflector; use Tempest\Reflection\ParameterReflector; @@ -242,7 +244,7 @@ private function optionsOf(ClassReflector $class, MethodReflector $method, ?stri isRequired: !$parameter->isOptional(), autocomplete: $this->autocompleteFor($option, $completers[$name] ?? null), parameter: $parameter, - choices: $this->choicesOf($option), + choices: $this->choicesOf($option, $parameter), minValue: $option->minValue, maxValue: $option->maxValue, minLength: $option->minLength, @@ -312,10 +314,10 @@ private function completersOf(ClassReflector $class): array * * @return array */ - private function choicesOf(Option $option): array + private function choicesOf(Option $option, ParameterReflector $parameter): array { if ($option->choices === []) { - return []; + return $this->casesOf($parameter); } if (!array_is_list($option->choices)) { @@ -331,16 +333,70 @@ private function choicesOf(Option $option): array return $choices; } + /** + * Every case of an enum typed option, labelled the way the enum labels + * itself. + * + * Naming them is the whole reason to reach for an enum here rather than a + * string with a hand written choice list that has to be kept in step with + * it. An enum implementing Choosable says how each case reads; otherwise + * the case name is used, which is at least a name someone chose. + * + * @return array + */ + private function casesOf(ParameterReflector $parameter): array + { + if (!$parameter->getReflection()->hasType()) { + return []; + } + + $name = $parameter->getType()->getName(); + + if (!self::isBackedEnum($name)) { + return []; + } + + $choices = []; + + foreach ($name::cases() as $case) { + $label = $case instanceof Choosable ? $case->label() : $case->name; + $choices[$label] = $case->value; + } + + return $choices; + } + private function typeOf(ParameterReflector $parameter): ApplicationCommandOptionType { if (!$parameter->getReflection()->hasType()) { throw new LogicException('Command option does not have type'); } - return self::OPTION_TYPES[$parameter->getType()->getName()] + $name = $parameter->getType()->getName(); + + if (self::isBackedEnum($name)) { + /* + * Discord has no enum of its own; a backed enum is a fixed set of + * values, which is a string or an integer option whose choices + * happen to be all of them. + */ + return (new ReflectionEnum($name))->getBackingType()?->getName() === 'int' + ? ApplicationCommandOptionType::INTEGER + : ApplicationCommandOptionType::STRING; + } + + return self::OPTION_TYPES[$name] ?? throw new LogicException('Command option type not supported'); } + /** + * @phpstan-assert-if-true class-string $name + */ + private static function isBackedEnum(string $name): bool + { + return is_subclass_of($name, BackedEnum::class); + } + /** * Extends a translation key one step down the command tree. Null stays * null, so a command that declares no key localizes nothing. diff --git a/src/Interfaces/Choosable.php b/src/Interfaces/Choosable.php new file mode 100644 index 0000000..804a50d --- /dev/null +++ b/src/Interfaces/Choosable.php @@ -0,0 +1,32 @@ + 'PC', + * self::PlayStation => 'PlayStation', + * }; + * } + * } + */ +interface Choosable +{ + /** + * What a member reads when picking this case. + */ + public function label(): string; +} diff --git a/src/Runtime/ArgumentResolver.php b/src/Runtime/ArgumentResolver.php index 43651dc..b1a7e3c 100644 --- a/src/Runtime/ArgumentResolver.php +++ b/src/Runtime/ArgumentResolver.php @@ -46,7 +46,11 @@ public function resolve(HandlerDefinition $handler, CommandInteraction $interact continue; } - $supplied[$option->parameter->getName()] = $this->values->resolve($structure, $interaction); + $supplied[$option->parameter->getName()] = $this->values->resolve( + $structure, + $interaction, + $option->parameter, + ); } $arguments = []; diff --git a/src/Runtime/OptionValueResolver.php b/src/Runtime/OptionValueResolver.php index f7ea79a..a33241f 100644 --- a/src/Runtime/OptionValueResolver.php +++ b/src/Runtime/OptionValueResolver.php @@ -6,7 +6,9 @@ use Tempcord\Discord\Enums\ApplicationCommandOptionType; use Tempcord\Discord\Interaction\CommandInteraction; use Tempcord\Discord\Parts\ApplicationCommandInteractionDataOptionStructure; +use BackedEnum; use RuntimeException; +use Tempest\Reflection\ParameterReflector; use Throwable; use function React\Async\await; @@ -30,11 +32,16 @@ public function __construct( public function resolve( ?ApplicationCommandInteractionDataOptionStructure $option, CommandInteraction $interaction, + ?ParameterReflector $parameter = null, ): mixed { if ($option === null) { return null; } + if ($parameter !== null && $this->wantsEnum($parameter)) { + return $this->toEnum($option, $parameter); + } + return match ($option->type) { ApplicationCommandOptionType::USER => await( $this->discord->rest->user->get($option->value), @@ -55,4 +62,29 @@ public function resolve( default => $option->value, }; } + + private function wantsEnum(ParameterReflector $parameter): bool + { + return $parameter->getReflection()->hasType() + && is_subclass_of($parameter->getType()->getName(), BackedEnum::class); + } + + /** + * Discord validates a choice against the list it was given, so a value that + * is not a case can only come from a client that made one up. Saying which + * option and which value beats a ValueError from deep inside from(). + */ + private function toEnum( + ApplicationCommandInteractionDataOptionStructure $option, + ParameterReflector $parameter, + ): BackedEnum { + /** @var class-string $enum */ + $enum = $parameter->getType()->getName(); + $backing = new \ReflectionEnum($enum)->getBackingType()?->getName(); + $value = $backing === 'int' ? (int) $option->value : (string) $option->value; + + return $enum::tryFrom($value) ?? throw new RuntimeException( + 'Option [' . $option->name . '] was sent "' . $option->value . '", which is not a case of ' . $enum, + ); + } } diff --git a/tests/Fixtures/Platform.php b/tests/Fixtures/Platform.php new file mode 100644 index 0000000..ced3cd2 --- /dev/null +++ b/tests/Fixtures/Platform.php @@ -0,0 +1,21 @@ + 'PC', + self::PlayStation => 'PlayStation', + self::Xbox => 'Xbox', + }; + } +} diff --git a/tests/Fixtures/PlatformCommand.php b/tests/Fixtures/PlatformCommand.php new file mode 100644 index 0000000..a03cc41 --- /dev/null +++ b/tests/Fixtures/PlatformCommand.php @@ -0,0 +1,26 @@ +definition(PlatformCommand::class)->options[$name]; + } + + public function test_a_string_backed_enum_is_a_string_option(): void + { + $this->assertSame(ApplicationCommandOptionType::STRING, $this->option('platform')->type); + } + + public function test_an_int_backed_enum_is_an_integer_option(): void + { + $this->assertSame(ApplicationCommandOptionType::INTEGER, $this->option('region')->type); + } + + /** + * The whole point of reaching for an enum: the cases are the choices, so + * there is no second list to keep in step with it. + */ + public function test_every_case_is_offered_as_a_choice(): void + { + $this->assertSame( + ['PC' => 'PC', 'PlayStation' => 'PS4', 'Xbox' => 'X1'], + $this->option('platform')->choices, + ); + } + + /** + * A case name is a PHP identifier and rarely what a bot wants shown, so an + * enum can say how each case reads. + */ + public function test_a_choosable_enum_names_its_own_cases(): void + { + $this->assertContains(Choosable::class, class_implements(Platform::class)); + $this->assertArrayHasKey('PlayStation', $this->option('platform')->choices); + } + + /** + * Without that, the case name is used — at least a name somebody chose. + */ + public function test_a_plain_enum_falls_back_to_its_case_names(): void + { + $this->assertSame( + ['Europe' => 1, 'NorthAmerica' => 2], + $this->option('region')->choices, + ); + } + + private function resolve(string $value, string $option = 'platform'): mixed + { + $structure = new ApplicationCommandInteractionDataOptionStructure(); + $structure->name = $option; + $structure->type = $this->option($option)->type; + $structure->value = $value; + + $data = new InteractionData(); + $data->name = 'platform'; + $data->options = [$structure]; + + $interaction = new InteractionCreate(); + $interaction->id = '1'; + $interaction->token = 'token'; + $interaction->data = $data; + + return new OptionValueResolver(new FakeDiscord(new RecordingHttp()))->resolve( + $structure, + new CommandInteraction($interaction, new FakeDiscord(new RecordingHttp())), + $this->option($option)->parameter, + ); + } + + public function test_the_handler_is_given_the_case_rather_than_the_value(): void + { + $this->assertSame(Platform::PlayStation, $this->resolve('PS4')); + } + + public function test_an_int_backed_case_is_found_by_its_number(): void + { + $this->assertSame(Region::NorthAmerica, $this->resolve('2', 'region')); + } + + /** + * Discord checks a choice against the list it was given, so a value that is + * not a case can only come from a client that made one up. Saying which + * option and which value beats a ValueError from deep inside from(). + */ + public function test_a_value_that_is_not_a_case_is_reported_clearly(): void + { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('is not a case of'); + + $this->resolve('Nintendo'); + } +} diff --git a/tools/src/ApiReflector.php b/tools/src/ApiReflector.php index e24ec06..649e0df 100644 --- a/tools/src/ApiReflector.php +++ b/tools/src/ApiReflector.php @@ -35,6 +35,9 @@ \Tempcord\Attributes\SelectMenu::class, \Tempcord\Attributes\ModalSubmit::class, ], + 'options' => [ + \Tempcord\Interfaces\Choosable::class, + ], 'autocomplete' => [ \Tempcord\Interfaces\Autocomplete::class, \Tempcord\AutoCompletes\ArrayAutocomplete::class,