From 474781a92e0e7f99fe259a9f5416349f8746da6d Mon Sep 17 00:00:00 2001 From: Renegade334 Date: Fri, 6 Sep 2024 12:35:12 +0100 Subject: [PATCH 1/3] refactor(PermissionOverwrites): cache-independent resolve --- packages/discord.js/src/errors/ErrorCodes.js | 4 +++ packages/discord.js/src/errors/Messages.js | 4 +++ .../src/structures/PermissionOverwrites.js | 32 ++++++++++++------- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/packages/discord.js/src/errors/ErrorCodes.js b/packages/discord.js/src/errors/ErrorCodes.js index c1552392aa90..d43aeda1a491 100644 --- a/packages/discord.js/src/errors/ErrorCodes.js +++ b/packages/discord.js/src/errors/ErrorCodes.js @@ -180,6 +180,8 @@ * @property {'BulkBanUsersOptionEmpty'} BulkBanUsersOptionEmpty * @property {'PollAlreadyExpired'} PollAlreadyExpired + + * @property {'PermissionOverwritesTypeMismatch'} PermissionOverwritesTypeMismatch */ const keys = [ @@ -337,6 +339,8 @@ const keys = [ 'BulkBanUsersOptionEmpty', 'PollAlreadyExpired', + + 'PermissionOverwritesTypeMismatch', ]; // JSDoc for IntelliSense purposes diff --git a/packages/discord.js/src/errors/Messages.js b/packages/discord.js/src/errors/Messages.js index 234718c50c79..e1b72cd0bd2b 100644 --- a/packages/discord.js/src/errors/Messages.js +++ b/packages/discord.js/src/errors/Messages.js @@ -173,6 +173,10 @@ const Messages = { [DjsErrorCodes.BulkBanUsersOptionEmpty]: 'Option "users" array or collection is empty', [DjsErrorCodes.PollAlreadyExpired]: 'This poll has already expired.', + + [DjsErrorCodes.PermissionOverwritesTypeMismatch]: expected => + `"overwrite.id" is a ${expected.toLowerCase()} object, ` + + `but "overwrite.type" is not equal to OverwriteType.${expected}.`, }; module.exports = Messages; diff --git a/packages/discord.js/src/structures/PermissionOverwrites.js b/packages/discord.js/src/structures/PermissionOverwrites.js index 3f3c286ff798..45b3561ef5b6 100644 --- a/packages/discord.js/src/structures/PermissionOverwrites.js +++ b/packages/discord.js/src/structures/PermissionOverwrites.js @@ -160,7 +160,7 @@ class PermissionOverwrites extends Base { * @property {GuildMemberResolvable|RoleResolvable} id Member or role this overwrite is for * @property {PermissionResolvable} [allow] The permissions to allow * @property {PermissionResolvable} [deny] The permissions to deny - * @property {OverwriteType} [type] The type of this OverwriteData + * @property {OverwriteType} [type] The type of this OverwriteData (mandatory if `id` is a Snowflake) */ /** @@ -171,21 +171,29 @@ class PermissionOverwrites extends Base { */ static resolve(overwrite, guild) { if (overwrite instanceof this) return overwrite.toJSON(); - if (typeof overwrite.id === 'string' && overwrite.type in OverwriteType) { - return { - id: overwrite.id, - type: overwrite.type, - allow: PermissionsBitField.resolve(overwrite.allow ?? PermissionsBitField.DefaultBit).toString(), - deny: PermissionsBitField.resolve(overwrite.deny ?? PermissionsBitField.DefaultBit).toString(), - }; + + const id = guild.roles.resolveId(overwrite.id) ?? guild.client.users.resolveId(overwrite.id); + if (!id) { + throw new DiscordjsTypeError(ErrorCodes.InvalidType, 'overwrite.id', 'GuildMemberResolvable or RoleResolvable'); } - const userOrRole = guild.roles.resolve(overwrite.id) ?? guild.client.users.resolve(overwrite.id); - if (!userOrRole) throw new DiscordjsTypeError(ErrorCodes.InvalidType, 'parameter', 'User nor a Role'); - const type = userOrRole instanceof Role ? OverwriteType.Role : OverwriteType.Member; + let type; + if (typeof overwrite.id === 'string') { + if (typeof overwrite.type === 'number' && overwrite.type in OverwriteType) { + type = overwrite.type; + } else { + throw new DiscordjsTypeError(ErrorCodes.InvalidType, 'overwrite.type', 'OverwriteType', true); + } + } else { + type = overwrite.id instanceof Role ? OverwriteType.Role : OverwriteType.Member; + // eslint-disable-next-line eqeqeq + if (overwrite.type != null && type !== overwrite.type) { + throw new DiscordjsTypeError(ErrorCodes.PermissionOverwriteTypeMismatch, OverwriteType[type]); + } + } return { - id: userOrRole.id, + id, type, allow: PermissionsBitField.resolve(overwrite.allow ?? PermissionsBitField.DefaultBit).toString(), deny: PermissionsBitField.resolve(overwrite.deny ?? PermissionsBitField.DefaultBit).toString(), From 75dc3f402ef70f1dbaaee182e63570ae3991ed2d Mon Sep 17 00:00:00 2001 From: Renegade334 Date: Tue, 1 Oct 2024 23:23:07 +0100 Subject: [PATCH 2/3] refactor: improve errors --- packages/discord.js/src/errors/ErrorCodes.js | 2 ++ packages/discord.js/src/errors/Messages.js | 3 ++- .../src/structures/PermissionOverwrites.js | 13 ++++++++----- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/discord.js/src/errors/ErrorCodes.js b/packages/discord.js/src/errors/ErrorCodes.js index d43aeda1a491..edc54d21477e 100644 --- a/packages/discord.js/src/errors/ErrorCodes.js +++ b/packages/discord.js/src/errors/ErrorCodes.js @@ -181,6 +181,7 @@ * @property {'PollAlreadyExpired'} PollAlreadyExpired + * @property {'PermissionOverwritesTypeMandatory'} PermissionOverwritesTypeMandatory * @property {'PermissionOverwritesTypeMismatch'} PermissionOverwritesTypeMismatch */ @@ -340,6 +341,7 @@ const keys = [ 'PollAlreadyExpired', + 'PermissionOverwritesTypeMandatory', 'PermissionOverwritesTypeMismatch', ]; diff --git a/packages/discord.js/src/errors/Messages.js b/packages/discord.js/src/errors/Messages.js index e1b72cd0bd2b..bd8973488732 100644 --- a/packages/discord.js/src/errors/Messages.js +++ b/packages/discord.js/src/errors/Messages.js @@ -174,9 +174,10 @@ const Messages = { [DjsErrorCodes.PollAlreadyExpired]: 'This poll has already expired.', + [DjsErrorCodes.PermissionOverwritesTypeMandatory]: '"overwrite.type" is mandatory if "overwrite.id" is a Snowflake', [DjsErrorCodes.PermissionOverwritesTypeMismatch]: expected => `"overwrite.id" is a ${expected.toLowerCase()} object, ` + - `but "overwrite.type" is not equal to OverwriteType.${expected}.`, + `but "overwrite.type" is defined and not equal to OverwriteType.${expected}`, }; module.exports = Messages; diff --git a/packages/discord.js/src/structures/PermissionOverwrites.js b/packages/discord.js/src/structures/PermissionOverwrites.js index 45b3561ef5b6..d5febf5ae1bf 100644 --- a/packages/discord.js/src/structures/PermissionOverwrites.js +++ b/packages/discord.js/src/structures/PermissionOverwrites.js @@ -177,18 +177,21 @@ class PermissionOverwrites extends Base { throw new DiscordjsTypeError(ErrorCodes.InvalidType, 'overwrite.id', 'GuildMemberResolvable or RoleResolvable'); } + if (overwrite.type !== undefined && (typeof overwrite.type !== 'number' || !(overwrite.type in OverwriteType))) { + throw new DiscordjsTypeError(ErrorCodes.InvalidType, 'overwrite.type', 'OverwriteType', true); + } + let type; if (typeof overwrite.id === 'string') { - if (typeof overwrite.type === 'number' && overwrite.type in OverwriteType) { + if (overwrite.type !== undefined) { type = overwrite.type; } else { - throw new DiscordjsTypeError(ErrorCodes.InvalidType, 'overwrite.type', 'OverwriteType', true); + throw new DiscordjsTypeError(ErrorCodes.PermissionOverwritesTypeMandatory); } } else { type = overwrite.id instanceof Role ? OverwriteType.Role : OverwriteType.Member; - // eslint-disable-next-line eqeqeq - if (overwrite.type != null && type !== overwrite.type) { - throw new DiscordjsTypeError(ErrorCodes.PermissionOverwriteTypeMismatch, OverwriteType[type]); + if (overwrite.type !== undefined && type !== overwrite.type) { + throw new DiscordjsTypeError(ErrorCodes.PermissionOverwritesTypeMismatch, OverwriteType[type]); } } From 1a4943cf48c0f3adc3fa52f5a3dcbb511ad78c30 Mon Sep 17 00:00:00 2001 From: Renegade334 Date: Tue, 1 Oct 2024 23:23:42 +0100 Subject: [PATCH 3/3] types: enforce type when id may be a Snowflake --- packages/discord.js/typings/index.d.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/discord.js/typings/index.d.ts b/packages/discord.js/typings/index.d.ts index 2ac27e78a990..314c618a7d01 100644 --- a/packages/discord.js/typings/index.d.ts +++ b/packages/discord.js/typings/index.d.ts @@ -6590,13 +6590,23 @@ export interface MultipleShardSpawnOptions { timeout?: number; } -export interface OverwriteData { +export interface BaseOverwriteData { allow?: PermissionResolvable; deny?: PermissionResolvable; id: GuildMemberResolvable | RoleResolvable; type?: OverwriteType; } +export interface OverwriteDataWithMandatoryType extends BaseOverwriteData { + type: OverwriteType; +} + +export interface OverwriteDataWithOptionalType extends BaseOverwriteData { + id: Exclude; +} + +export type OverwriteData = OverwriteDataWithMandatoryType | OverwriteDataWithOptionalType; + export type OverwriteResolvable = PermissionOverwrites | OverwriteData; export type PermissionFlags = Record;