Skip to content

Commit

Permalink
refactor: resolve several sonar issues (#823)
Browse files Browse the repository at this point in the history
* refactor: resolve several sonar issues

Mostly code smells and 2 deprecations

* refactor: use `If` to improve overloads

* fix: build errors and Vlad's request

* chore: commit committed

* chore: bugger

* chore: fixed what was broken.

* refactor: target is object

* refactor: `const Required` -> `Required`

* refactor: fewer array index accessors

---------

Co-authored-by: Aura Román <kyradiscord@gmail.com>
  • Loading branch information
favna and kyranet authored Nov 2, 2024
1 parent bb6c889 commit ba915f9
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 77 deletions.
2 changes: 1 addition & 1 deletion packages/async-queue/src/lib/AsyncQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export class AsyncQueue {
/**
* The promises array
*/
private promises: AsyncQueueEntry[] = [];
private readonly promises: AsyncQueueEntry[] = [];

/**
* Waits for last promise and queues a new one
Expand Down
7 changes: 3 additions & 4 deletions packages/decorators/src/base-decorators.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import type { NonNullObject } from '@sapphire/utilities';
import { createMethodDecorator } from './utils';

/**
* Decorator that sets the enumerable property of a class field to the desired value.
* @param value Whether the property should be enumerable or not
*/
export function Enumerable(value: boolean) {
return (target: unknown, key: string) => {
Reflect.defineProperty(target as NonNullObject, key, {
return (target: object, key: string) => {
Reflect.defineProperty(target, key, {
enumerable: value,
set(this: unknown, val: unknown) {
Reflect.defineProperty(this as NonNullObject, key, {
Reflect.defineProperty(this as object, key, {
configurable: true,
enumerable: value,
value: val,
Expand Down
94 changes: 41 additions & 53 deletions packages/discord-utilities/src/lib/InteractionOptionResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ import {
type APIApplicationCommandInteractionDataOption,
type APIApplicationCommandInteractionDataStringOption,
type APIAttachment,
type APIChatInputApplicationCommandInteractionDataResolved,
type APIInteractionDataResolved,
type APIInteractionDataResolvedChannel,
type APIInteractionDataResolvedGuildMember,
type APIMessage,
type APIMessageApplicationCommandInteractionDataResolved,
type APIModalSubmitInteraction,
type APIRole,
type APIUser,
type APIUserApplicationCommandInteractionDataResolved
type APIUserInteractionDataResolved
} from 'discord-api-types/v10';

/**
Expand All @@ -37,8 +37,8 @@ export class InteractionOptionResolver {
* The interaction resolved data
*/
private readonly resolved:
| APIChatInputApplicationCommandInteractionDataResolved
| APIUserApplicationCommandInteractionDataResolved
| APIInteractionDataResolved
| APIUserInteractionDataResolved
| APIMessageApplicationCommandInteractionDataResolved
| null = null;

Expand Down Expand Up @@ -80,14 +80,13 @@ export class InteractionOptionResolver {
}
}

public get(name: string, required?: boolean | false): APIApplicationCommandInteractionDataOption | null;
public get(name: string, required: true): APIApplicationCommandInteractionDataOption;

/**
* Gets an option by its name
* @param name The name of the option
* @param required Whether to throw an error if the option is not found
*/
public get<Required extends boolean = false>(name: string, required?: Required): RequiredIf<Required, APIApplicationCommandInteractionDataOption>;

public get(name: string, required = false): APIApplicationCommandInteractionDataOption | null {
const option = this.hoistedOptions?.find((opt) => opt.name === name);
if (!option) {
Expand All @@ -101,13 +100,11 @@ export class InteractionOptionResolver {
return option;
}

public getSubcommand(required?: boolean | false): string | null;
public getSubcommand(required: true): string;

/**
* Gets the selected subcommand
* @param required Whether to throw an error if there is no subcommand
*/
public getSubcommand<Required extends boolean = false>(required?: Required): RequiredIf<Required, string>;
public getSubcommand(required = true): string | null {
if (required && !this.subcommand) {
throw new Error('A subcommand was not selected');
Expand All @@ -116,13 +113,11 @@ export class InteractionOptionResolver {
return this.subcommand;
}

public getSubcommandGroup(required?: boolean | false): string | null;
public getSubcommandGroup(required: true): string;

/**
* Gets the selected subcommand group
* @param required Whether to throw an error if there is no subcommand group
*/
public getSubcommandGroup<Required extends boolean = false>(required?: Required): RequiredIf<Required, string>;
public getSubcommandGroup(required = true): string | null {
if (required && !this.group) {
throw new Error('A subcommand group was not selected');
Expand All @@ -131,131 +126,120 @@ export class InteractionOptionResolver {
return this.group;
}

public getBoolean(name: string, required?: boolean | false): boolean | null;
public getBoolean(name: string, required: true): boolean;

/**
* Gets a boolean option
* @param name The name of the option
* @param required Whether to throw an error if the option is not found
*/
public getBoolean<Required extends boolean = false>(name: string, required?: Required): RequiredIf<Required, boolean>;
public getBoolean(name: string, required = false): boolean | null {
const option = this.getTypedOption(name, ApplicationCommandOptionType.Boolean, required);
return option?.value ?? null;
}

public getChannel(name: string, required?: boolean | false): APIInteractionDataResolvedChannel | null;
public getChannel(name: string, required: true): APIInteractionDataResolvedChannel;

/**
* Gets a channel option
* @param name The name of the option
* @param required Whether to throw an error if the option is not found
*/
public getChannel<Required extends boolean = false>(name: string, required?: Required): RequiredIf<Required, APIInteractionDataResolvedChannel>;

public getChannel(name: string, required = false): APIInteractionDataResolvedChannel | null {
const option = this.getTypedOption(name, ApplicationCommandOptionType.Channel, required);
return option && this.resolved && 'channels' in this.resolved ? (this.resolved.channels?.[option.value] ?? null) : null;
}

public getString(name: string, required?: boolean | false): string | null;
public getString(name: string, required: true): string;

/**
* Gets a string option
* @param name The name of the option
* @param required Whether to throw an error if the option is not found
*/
public getString<Required extends boolean = false>(name: string, required?: Required): RequiredIf<Required, string>;
public getString(name: string, required = false): string | null {
const option = this.getTypedOption(name, ApplicationCommandOptionType.String, required);
return option?.value ?? null;
}

public getInteger(name: string, required?: boolean | false): number | null;
public getInteger(name: string, required: true): number;

/**
* Gets an integer option
* @param name The name of the option
* @param required Whether to throw an error if the option is not found
*/
public getInteger<Required extends boolean = false>(name: string, required?: Required): RequiredIf<Required, number>;
public getInteger(name: string, required = false): number | null {
const option = this.getTypedOption(name, ApplicationCommandOptionType.Integer, required);
return option?.value ?? null;
}

public getNumber(name: string, required?: boolean | false): number | null;
public getNumber(name: string, required: true): number;

/**
* Gets a number option
* @param name The name of the option
* @param required Whether to throw an error if the option is not found
*/
public getNumber<Required extends boolean = false>(name: string, required?: Required): RequiredIf<Required, number>;
public getNumber(name: string, required = false): number | null {
const option = this.getTypedOption(name, ApplicationCommandOptionType.Number, required);
return option?.value ?? null;
}

public getUser(name: string, required?: boolean | false): APIUser | null;
public getUser(name: string, required: true): APIUser;

/**
* Gets a user option
* @param name The name of the option
* @param required Whether to throw an error if the option is not found
*/
public getUser<Required extends boolean = false>(name: string, required?: Required): RequiredIf<Required, APIUser>;
public getUser(name: string, required = false): APIUser | null {
const option = this.getTypedOption(name, ApplicationCommandOptionType.User, required);
return option && this.resolved && 'users' in this.resolved ? (this.resolved.users?.[option.value] ?? null) : null;
}

public getMember(name: string, required?: boolean | false): APIInteractionDataResolvedGuildMember | null;
public getMember(name: string, required: true): APIInteractionDataResolvedGuildMember;

/**
* Gets a member option
* @param name The name of the option
* @param required Whether to throw an error if the option is not found
*/
public getMember<Required extends boolean = false>(
name: string,
required?: Required
): RequiredIf<Required, APIInteractionDataResolvedGuildMember>;

public getMember(name: string, required = false): APIInteractionDataResolvedGuildMember | null {
const option = this.getTypedOption(name, ApplicationCommandOptionType.User, required);
return option && this.resolved && 'members' in this.resolved ? (this.resolved.members?.[option.value] ?? null) : null;
}

public getRole(name: string, required?: boolean | false): APIRole | null;
public getRole(name: string, required: true): APIRole;

/**
* Gets a role option
* @param name The name of the option
* @param required Whether to throw an error if the option is not found
*/
public getRole<Required extends boolean = false>(name: string, required?: Required): RequiredIf<Required, APIRole>;
public getRole(name: string, required = false): APIRole | null {
const option = this.getTypedOption(name, ApplicationCommandOptionType.Role, required);
return option && this.resolved && 'roles' in this.resolved ? (this.resolved.roles?.[option.value] ?? null) : null;
}

public getAttachment(name: string, required?: boolean | false): APIAttachment | null;
public getAttachment(name: string, required: true): APIAttachment;

/**
* Gets an attachment option
* @param name The name of the option
* @param required Whether to throw an error if the option is not found
*/
public getAttachment<Required extends boolean = false>(name: string, required?: Required): RequiredIf<Required, APIAttachment>;
public getAttachment(name: string, required = false): APIAttachment | null {
const option = this.getTypedOption(name, ApplicationCommandOptionType.Attachment, required);
return option && this.resolved && 'attachments' in this.resolved ? (this.resolved.attachments?.[option.value] ?? null) : null;
}

public getMentionable(name: string, required?: boolean | false): APIUser | APIInteractionDataResolvedGuildMember | APIRole | null;
public getMentionable(name: string, required: true): APIUser | APIInteractionDataResolvedGuildMember | APIRole;

/**
* Gets a mentionable option
* @param name The name of the option
* @param required Whether to throw an error if the option is not found
*/
public getMentionable<Required extends boolean = false>(
name: string,
required?: Required
): RequiredIf<Required, APIUser | APIInteractionDataResolvedGuildMember | APIRole>;

public getMentionable(name: string, required = false): APIUser | APIInteractionDataResolvedGuildMember | APIRole | null {
const option = this.getTypedOption(name, ApplicationCommandOptionType.Mentionable, required);

Expand Down Expand Up @@ -286,22 +270,20 @@ export class InteractionOptionResolver {
throw new Error('This method can only be used on user context menu interactions');
}

return (this.resolved as APIUserApplicationCommandInteractionDataResolved).users[this.interaction.data.target_id];
return (this.resolved as APIUserInteractionDataResolved).users[this.interaction.data.target_id];
}

public getTargetMember(required?: boolean | false): APIInteractionDataResolvedGuildMember | null;
public getTargetMember(required: true): APIInteractionDataResolvedGuildMember;

/**
* Gets the target member for a context menu interaction
* @param required Whether to throw an error if the member data is not present
*/
public getTargetMember<Required extends boolean = false>(required?: Required): RequiredIf<Required, APIInteractionDataResolvedGuildMember>;
public getTargetMember(required = false): APIInteractionDataResolvedGuildMember | null {
if (this.interaction.type !== InteractionType.ApplicationCommand || this.interaction.data.type !== ApplicationCommandType.User) {
throw new Error('This method can only be used on user context menu interactions');
}

const member = (this.resolved as APIUserApplicationCommandInteractionDataResolved).members?.[this.interaction.data.target_id] ?? null;
const member = (this.resolved as APIUserInteractionDataResolved).members?.[this.interaction.data.target_id] ?? null;

if (!member && required) {
throw new Error('Member data is not present');
Expand Down Expand Up @@ -345,13 +327,11 @@ export class InteractionOptionResolver {
return option;
}

private getTypedOption<Option extends BasicApplicationCommandOptionType>(
private getTypedOption<Option extends BasicApplicationCommandOptionType, Required extends boolean = false>(
name: string,
type: Option,
required?: boolean | false
): TypeToOptionMap[Option] | null;

private getTypedOption<Option extends BasicApplicationCommandOptionType>(name: string, type: Option, required: true): TypeToOptionMap[Option];
required: Required
): RequiredIf<Required, TypeToOptionMap[Option]>;

private getTypedOption<Option extends BasicApplicationCommandOptionType>(
name: string,
Expand Down Expand Up @@ -379,3 +359,11 @@ type _TypeToOptionMap = {
type TypeToOptionMap = {
[Option in keyof _TypeToOptionMap]: _TypeToOptionMap[Option];
};

type If<Value extends boolean, TrueResult, FalseResult> = Value extends true
? TrueResult
: Value extends false
? FalseResult
: TrueResult | FalseResult;

type RequiredIf<Value extends boolean, ValueType, FallbackType = null> = If<Value, ValueType, ValueType | FallbackType>;
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export class MessagePrompter<S extends keyof StrategyReturns = 'confirm'> {
/**
* The available strategies
*/
public static strategies: Map<
public static readonly strategies: Map<
keyof StrategyReturns,
Ctor<
| ConstructorParameters<typeof MessagePrompterConfirmStrategy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ export class PaginatedMessage {

// #region private class fields
/** The response we send when someone gets into an invalid flow */
#thisMazeWasNotMeantForYouContent = { content: "This maze wasn't meant for you...what did you do." };
readonly #thisMazeWasNotMeantForYouContent = { content: "This maze wasn't meant for you...what did you do." };
// #endregion

/**
Expand Down Expand Up @@ -1202,16 +1202,16 @@ export class PaginatedMessage {
public addPageAction(action: PaginatedMessageAction, index: number): this {
if (index < 0 || index > this.pages.length - 1) throw new Error('Provided index is out of bounds');

if (!this.pageActions[index]) {
this.pageActions[index] = new Map<string, PaginatedMessageAction>();
}
const pageActionAtIndex = this.pageActions[index] ?? new Map<string, PaginatedMessageAction>();

if (actionIsLinkButton(action)) {
this.pageActions[index]!.set(action.url, action);
pageActionAtIndex.set(action.url, action);
} else if (actionIsButtonOrMenu(action)) {
this.pageActions[index]!.set(action.customId, action);
pageActionAtIndex.set(action.customId, action);
}

this.pageActions[index] = pageActionAtIndex;

return this;
}
// #endregion
Expand Down Expand Up @@ -1328,7 +1328,7 @@ export class PaginatedMessage {
if (this.collector) {
this.collector.once('end', () => {
PaginatedMessage.messages.delete(messageId);
PaginatedMessage.handlers.delete(target!.id);
PaginatedMessage.handlers.delete(target.id);
});

PaginatedMessage.messages.set(messageId, this);
Expand Down Expand Up @@ -1467,8 +1467,7 @@ export class PaginatedMessage {
filter: (interaction) => {
if (!isNullish(this.response) && interaction.isMessageComponent()) {
const customIdValidation =
this.actions.has(interaction.customId) ||
this.pageActions.some((actions) => actions && actions.has(interaction.customId));
this.actions.has(interaction.customId) || this.pageActions.some((actions) => actions?.has(interaction.customId));

if (isAnyInteraction(messageOrInteraction) && messageOrInteraction.ephemeral) {
return interaction.user.id === targetUser.id && customIdValidation;
Expand Down Expand Up @@ -1602,7 +1601,7 @@ export class PaginatedMessage {
handler: this,
author: targetUser,
channel,
response: this.response!,
response: this.response,
collector: this.collector!
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ export interface PaginatedMessageInternationalizationContext {
* Represents the parameters for safely replying to an interaction.
* @template T - The type of message method ('edit', 'reply', or never).
*/
export interface SafeReplyToInteractionParameters<T extends 'edit' | 'reply' | never = never> {
export interface SafeReplyToInteractionParameters<T extends 'edit' | 'reply' = never> {
/**
* The message or interaction to reply to.
*/
Expand Down
Loading

0 comments on commit ba915f9

Please sign in to comment.