From ce76807261d55252ed057843c9f532a77ce10862 Mon Sep 17 00:00:00 2001 From: Erin Date: Mon, 30 Aug 2021 18:01:05 -0400 Subject: [PATCH 1/2] Improve type handling/docs in command constructor --- src/Command.ts | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/Command.ts b/src/Command.ts index b134b03..e3f9f89 100644 --- a/src/Command.ts +++ b/src/Command.ts @@ -116,7 +116,7 @@ export class Command { names: string[]; /** The function executed when the command is triggered. */ - process: CommandProcess | GuildCommandProcess | PrivateCommandProcess + process: CommandProcess; /** The requirements for the command being triggered. */ requirements: CommandRequirements; @@ -127,13 +127,19 @@ export class Command { /** Subcommands of this command. */ subcommands: Command[] = []; - // For some reason, I cannot get TS to recognize that `CommandProcess` is a - // superset of `GuildCommandProcess` and `PrivateCommandProcess`, so for - // now we have one more override than we should really need. Oh well. - // TODO: Does microsoft/typescript#31023 fix this? - constructor(names: string | string[], process: CommandProcess, requirements?: CommandRequirements); - constructor(names: string | string[], process: GuildCommandProcess, requirements: CommandRequirements & { guildOnly: true; dmOnly?: false }); - constructor(names: string | string[], process: PrivateCommandProcess, requirements: CommandRequirements & { dmOnly: true; guildOnly?: false }) + /** Creates a command restricted to use in guilds. */ + constructor (names: string | string[], process: GuildCommandProcess, requirements: CommandRequirements & { guildOnly: true; dmOnly?: false }); + /** Creates a command restricted to use in DMs. */ + constructor (names: string | string[], process: PrivateCommandProcess, requirements: CommandRequirements & { dmOnly: true; guildOnly?: false }); + /** Creates a command. */ + constructor (names: string | string[], process: CommandProcess, requirements?: CommandRequirements); + + /** + * This implememtation signature is really messy to account for all the + * different forms the constructor can take. It doesn't need to be exposed + * in documentation or code suggestions. + * @internal + */ constructor (names: string | string[], process: CommandProcess | GuildCommandProcess | PrivateCommandProcess, requirements?: CommandRequirements) { if (Array.isArray(names)) { this.names = names; @@ -141,8 +147,15 @@ export class Command { this.names = [names]; } if (!this.names[0]) throw new TypeError('At least one name is required'); - this.process = process; + + // NOTE: This cast discards away type information related to the + // channels we expect this command to be executed in. The only + // thing preventing e.g. private channel messages from being + // passed to a guild-only command process are the runtime checks + // in Command#execute below. + this.process = process as CommandProcess; if (!this.process) throw new TypeError('Process is required'); + this.requirements = {}; if (requirements) { if (requirements.owner) { @@ -210,10 +223,8 @@ export class Command { // We have no subcommand, so call this command's process // NOTE: By calling checkPermissions and returning early if it returns // false, we guarantee that messages will be the correct type for - // the stored process, so this call is always safe. Restructuring - // this to properly use TS type guards would be very messy and - // would result in duplicate safety checks that we want to avoid. - // @ts-ignore + // the stored process, even though we no longer have type info + // about the process. this.process(msg, args, ctx); return true; } From 61e070d9b467706f2beba51d7973722d9b2b9bc6 Mon Sep 17 00:00:00 2001 From: Erin Date: Mon, 30 Aug 2021 18:25:40 -0400 Subject: [PATCH 2/2] Add options-first signatures, deprecate old ones The signature with no options at all is still around --- src/Command.ts | 58 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/src/Command.ts b/src/Command.ts index e3f9f89..138fe61 100644 --- a/src/Command.ts +++ b/src/Command.ts @@ -127,12 +127,31 @@ export class Command { /** Subcommands of this command. */ subcommands: Command[] = []; + /** Creates a command. */ + constructor (names: string | string[], process: CommandProcess); + /** Creates a command restricted to use in guilds. */ - constructor (names: string | string[], process: GuildCommandProcess, requirements: CommandRequirements & { guildOnly: true; dmOnly?: false }); + constructor (names: string | string[], requirements: CommandRequirements & { guildOnly: true; dmOnly?: false }, process: GuildCommandProcess); /** Creates a command restricted to use in DMs. */ - constructor (names: string | string[], process: PrivateCommandProcess, requirements: CommandRequirements & { dmOnly: true; guildOnly?: false }); + constructor (names: string | string[], requirements: CommandRequirements & { dmOnly: true; guildOnly?: false }, process: PrivateCommandProcess); /** Creates a command. */ - constructor (names: string | string[], process: CommandProcess, requirements?: CommandRequirements); + constructor (names: string | string[], requirements: CommandRequirements, process: CommandProcess); + + /** + * Creates a command restricted to use in guilds. + * @deprecated Use the `new Command(names, requirements, process)` form. + */ + constructor (names: string | string[], process: GuildCommandProcess, requirements: CommandRequirements & { guildOnly: true; dmOnly?: false }); + /** + * Creates a command restricted to use in DMs. + * @deprecated Use the `new Command(names, requirements, process)` form. + */ + constructor (names: string | string[], process: PrivateCommandProcess, requirements: CommandRequirements & { dmOnly: true; guildOnly?: false }); + /** + * Creates a command. + * @deprecated Use the `new Command(names, requirements, process)` form. + */ + constructor (names: string | string[], process: CommandProcess, requirements: CommandRequirements); /** * This implememtation signature is really messy to account for all the @@ -140,7 +159,21 @@ export class Command { * in documentation or code suggestions. * @internal */ - constructor (names: string | string[], process: CommandProcess | GuildCommandProcess | PrivateCommandProcess, requirements?: CommandRequirements) { + // TODO: This can be simplified once process-first forms are removed for v3 + constructor (...args: [ + names: string | string[], + requirements: CommandRequirements, + process: CommandProcess | GuildCommandProcess | PrivateCommandProcess, + ] | [ + names: string | string[], + process: CommandProcess | GuildCommandProcess | PrivateCommandProcess, + requirements: CommandRequirements, + ] | [ + names: string | string[], + process: CommandProcess, + ]) { + // Names is always the first argument + const names = args[0]; if (Array.isArray(names)) { this.names = names; } else { @@ -148,6 +181,23 @@ export class Command { } if (!this.names[0]) throw new TypeError('At least one name is required'); + // Figure out what order the user passed requirements and process + let requirements: CommandRequirements | undefined; + let process: CommandProcess; + if (!args[2]) { + // (name, process) + requirements = undefined; + process = args[1] as CommandProcess; + } else if (typeof args[2] === 'function') { + // (name, requirements, process) + requirements = args[1] as CommandRequirements; + process = args[2] as CommandProcess; + } else { + // (name, process, requirements) + requirements = args[2]; + process = args[1] as CommandProcess; + } + // NOTE: This cast discards away type information related to the // channels we expect this command to be executed in. The only // thing preventing e.g. private channel messages from being