Skip to content

Create OptionsResolver normalizers #1455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
valzargaming wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Create OptionsResolver normalizers #1455

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9e13b6c
Create normalizers
valzargaming Jan 9, 2026
c95cc5e
PHPDocs
valzargaming Jan 9, 2026
f52d883
Defaults for Loop and Logger
valzargaming Jan 10, 2026
ecb9b81
Tweak comment
valzargaming Jan 10, 2026
1d9cb2b
Update src/Discord/Discord.php
valzargaming Jan 11, 2026
3e25411
Merge branch 'master' into normalize-options
valzargaming Apr 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 70 additions & 8 deletions src/Discord/Discord.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
use Ratchet\Client\Connector;
use Ratchet\Client\WebSocket;
use Ratchet\RFC6455\Messaging\Message;
use React\Dns\Config\Config as DnsConfig;
use React\EventLoop\Loop;
use React\EventLoop\LoopInterface;
use React\EventLoop\TimerInterface;
Expand Down Expand Up @@ -100,6 +101,29 @@
* @property SoundRepository $sounds
* @property StickerPackRepository $sticker_packs
* @property UserRepository $users
*
* @property array $options
* @property string $options['token']
* @property LoggerInterface $options['logger']
* @property LoopInterface $options['loop']
* @property array|bool $options['loadAllMembers']
* @property array $options['disabledEvents']
* @property bool $options['storeMessages']
* @property array|bool $options['retrieveBans']
* @property int|null $options['large_threshold']
* @property array|null $options['shard']
* @property int|null $options['shard_id']
* @property int|null $options['num_shards']
* @property int|null $options['shardId']
* @property int|null $options['shardCount']
* @property array|null $options['presence']
* @property int $options['intents']
* @property array $options['socket_options']
* @property DnsConfig|string $options['dnsConfig']
* @property array $options['cache']
* @property string $options['collection']
* @property bool $options['useTransportCompression']
Comment on lines +105 to +125
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PHPDoc uses @property tags with array offsets (e.g. $options['token']). This syntax isn’t understood by most PHPDoc parsers (including phpDocumentor), so it’s likely to be ignored or mis-parsed. If the goal is to document the options array shape for tooling, consider using an @var array{...} shape (or a @phpstan-type/@psalm-type) instead of @property entries for offsets.

Suggested change
* @property array $options
* @property string $options['token']
* @property LoggerInterface $options['logger']
* @property LoopInterface $options['loop']
* @property array|bool $options['loadAllMembers']
* @property array $options['disabledEvents']
* @property bool $options['storeMessages']
* @property array|bool $options['retrieveBans']
* @property int|null $options['large_threshold']
* @property array|null $options['shard']
* @property int|null $options['shard_id']
* @property int|null $options['num_shards']
* @property int|null $options['shardId']
* @property int|null $options['shardCount']
* @property array|null $options['presence']
* @property int $options['intents']
* @property array $options['socket_options']
* @property DnsConfig|string $options['dnsConfig']
* @property array $options['cache']
* @property string $options['collection']
* @property bool $options['useTransportCompression']
* @property array{
* token: string,
* logger: LoggerInterface,
* loop: LoopInterface,
* loadAllMembers: array|bool,
* disabledEvents: array,
* storeMessages: bool,
* retrieveBans: array|bool,
* large_threshold: int|null,
* shard: array|null,
* shard_id: int|null,
* num_shards: int|null,
* shardId: int|null,
* shardCount: int|null,
* presence: array|null,
* intents: int,
* socket_options: array,
* dnsConfig: DnsConfig|string,
* cache: array,
* collection: string,
* useTransportCompression: bool
* } $options

Copilot uses AI. Check for mistakes.
* @property bool $options['usePayloadCompression']
*/
class Discord
{
Expand Down Expand Up @@ -1703,7 +1727,8 @@ protected function resolveOptions(array $options = []): array
'usePayloadCompression',
])
->setDefaults([
'logger' => null,
'loop' => Loop::get(),
'logger' => new Monolog('DiscordPHP', [(new StreamHandler('php://stdout', Level::Debug))->setFormatter(new LineFormatter(null, null, true, true))]),
Comment on lines +1730 to +1731
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loop and logger now have concrete defaults set in the resolver, but the function still contains post-resolve() fallback logic for these options later on. Keeping both paths makes the resolution behavior harder to reason about and risks future divergence; consider removing the redundant post-processing and letting the resolver defaults/normalizers be the single source of truth.

Suggested change
'loop' => Loop::get(),
'logger' => new Monolog('DiscordPHP', [(new StreamHandler('php://stdout', Level::Debug))->setFormatter(new LineFormatter(null, null, true, true))]),
'loop' => null,
'logger' => null,

Copilot uses AI. Check for mistakes.
'loadAllMembers' => false,
'disabledEvents' => [],
'storeMessages' => false,
Expand Down Expand Up @@ -1738,9 +1763,51 @@ protected function resolveOptions(array $options = []): array
->setAllowedTypes('shardCount', ['null', 'int'])
->setAllowedTypes('presence', ['null', 'array'])
->setAllowedTypes('intents', ['array', 'int'])
->setAllowedTypes('capabilities', ['null', 'array', 'int'])
->setNormalizer('intents', function ($options, $value) {
if (is_array($value)) {
$intent = 0;
$validIntents = Intents::getValidIntents();

foreach ($value as $idx => $i) {
if (! in_array($i, $validIntents)) {
throw new IntentException('Given intent at index '.$idx.' is invalid.');
}

$intent |= $i;
}

$value = $intent;
}

return (int) $value;
})
->setAllowedTypes('socket_options', 'array')
->setAllowedTypes('dnsConfig', ['string', \React\Dns\Config\Config::class])
->setNormalizer('socket_options', function ($options, $value) {
// Discord doesn't currently support IPv6. This prevents xdebug from catching exceptions when trying to fetch IPv6 for Discord.
$value['happy_eyeballs'] = false;

return $value;
})
->setAllowedTypes('dnsConfig', ['string', DnsConfig::class])
->setNormalizer('dnsConfig', function ($options, $value) {
if (null === $value) {
$value = DnsConfig::loadSystemConfigBlocking();
if (! $value->nameservers) {
$value->nameservers[] = '8.8.8.8';
}
Comment on lines +1791 to +1797
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dnsConfig normalizer only runs when the caller explicitly sets the option; since dnsConfig has no default in the resolver, omitting it will bypass this normalizer and rely on separate manual initialization later in resolveOptions(). To fully centralize normalization, consider setting a default (e.g. null) so the normalizer always executes, and then remove the later manual dnsConfig fallback block.

Copilot uses AI. Check for mistakes.
}

return $value;
})
Comment thread
valzargaming marked this conversation as resolved.
->setAllowedTypes('collection', 'string')
->setNormalizer('collection', function ($options, $value) {
if (is_string($value) && class_exists($value) && is_subclass_of($value, ExCollectionInterface::class)) {
return $value;
}

return Collection::class;
})
Comment on lines +1802 to +1809
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newly added collection allowed-types + normalizer block duplicates an existing collection configuration later in this method (there’s already another setAllowedTypes('collection', ...) + normalizer below). This results in the same option being configured twice and makes future changes error-prone; keep only a single collection configuration block.

Copilot uses AI. Check for mistakes.
->setAllowedTypes('capabilities', ['null', 'array', 'int'])
->setAllowedTypes('cache', ['array', CacheConfig::class, \React\Cache\CacheInterface::class, \Psr\SimpleCache\CacheInterface::class])
->setNormalizer('cache', function ($options, $value) {
if (! is_array($value)) {
Expand Down Expand Up @@ -1819,11 +1886,6 @@ protected function resolveOptions(array $options = []): array
'See the documentation on the `loadAllMembers` property for more information: http://discord-php.github.io/DiscordPHP/#basics');
}

// Discord doesn't currently support IPv6
// This prevents xdebug from catching exceptions when trying to fetch IPv6
// for Discord
$options['socket_options']['happy_eyeballs'] = false;

return $options;
}

Expand Down