Skip to content

Commit

Permalink
feat(*): enforce strings (#4880)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Removes all Resolvables for only string inputs

Co-authored-by: SpaceEEC <spaceeec@yahoo.com>
  • Loading branch information
almostSouji and SpaceEEC committed Jun 1, 2021
1 parent 66a6a1f commit 7b85a72
Show file tree
Hide file tree
Showing 14 changed files with 97 additions and 153 deletions.
2 changes: 1 addition & 1 deletion esm/discord.mjs
Expand Up @@ -49,7 +49,7 @@ export const {
escapeMarkdown,
fetchRecommendedShards,
resolveColor,
resolveString,
verifyString,
splitMessage,
Application,
ApplicationCommand,
Expand Down
8 changes: 8 additions & 0 deletions src/errors/Messages.js
Expand Up @@ -37,6 +37,13 @@ const Messages = {
COLOR_RANGE: 'Color must be within the range 0 - 16777215 (0xFFFFFF).',
COLOR_CONVERT: 'Unable to convert color to a number.',

EMBED_TITLE: 'MessageEmbed title must be a string.',
EMBED_FIELD_NAME: 'MessageEmbed field names must be non-empty strings.',
EMBED_FIELD_VALUE: 'MessageEmbed field values must be non-empty strings.',
EMBED_FOOTER_TEXT: 'MessageEmbed footer text must be a string.',
EMBED_DESCRIPTION: 'MessageEmbed description must be a string.',
EMBED_AUTHOR_NAME: 'MessageEmbed author name must be a string.',

FILE_NOT_FOUND: file => `File could not be found: ${file}`,

USER_NO_DMCHANNEL: 'No DM Channel exists!',
Expand Down Expand Up @@ -72,6 +79,7 @@ const Messages = {

MESSAGE_BULK_DELETE_TYPE: 'The messages must be an Array, Collection, or number.',
MESSAGE_NONCE_TYPE: 'Message nonce must be an integer or a string.',
MESSAGE_CONTENT_TYPE: 'Message content must be a non-empty string.',

TYPING_COUNT: 'Count must be at least 1',

Expand Down
2 changes: 1 addition & 1 deletion src/index.js
Expand Up @@ -56,7 +56,7 @@ module.exports = {
escapeMarkdown: Util.escapeMarkdown,
fetchRecommendedShards: Util.fetchRecommendedShards,
resolveColor: Util.resolveColor,
resolveString: Util.resolveString,
verifyString: Util.verifyString,
splitMessage: Util.splitMessage,

// Structures
Expand Down
6 changes: 3 additions & 3 deletions src/structures/APIMessage.js
Expand Up @@ -92,7 +92,7 @@ class APIMessage {
if (this.options.content === null) {
content = '';
} else if (typeof this.options.content !== 'undefined') {
content = Util.resolveString(this.options.content);
content = Util.verifyString(this.options.content, RangeError, 'MESSAGE_CONTENT_TYPE', false);
}

if (typeof content !== 'string') return content;
Expand Down Expand Up @@ -322,7 +322,7 @@ class APIMessage {
/**
* Transforms the user-level arguments into a final options object. Passing a transformed options object alone into
* this method will keep it the same, allowing for the reuse of the final options object.
* @param {StringResolvable} [content] Content to send
* @param {string} [content] Content to send
* @param {MessageOptions|WebhookMessageOptions|MessageAdditions} [options={}] Options to use
* @param {MessageOptions|WebhookMessageOptions} [extra={}] Extra options to add onto transformed options
* @param {boolean} [isWebhook=false] Whether or not to use WebhookMessageOptions as the result
Expand Down Expand Up @@ -358,7 +358,7 @@ class APIMessage {
/**
* Creates an `APIMessage` from user-level arguments.
* @param {MessageTarget} target Target to send to
* @param {StringResolvable} [content] Content to send
* @param {string} [content] Content to send
* @param {MessageOptions|WebhookMessageOptions|MessageAdditions} [options={}] Options to use
* @param {MessageOptions|WebhookMessageOptions} [extra={}] - Extra options to add onto transformed options
* @returns {MessageOptions|WebhookMessageOptions}
Expand Down
2 changes: 1 addition & 1 deletion src/structures/Message.js
Expand Up @@ -486,7 +486,7 @@ class Message extends Base {

/**
* Edits the content of the message.
* @param {StringResolvable|APIMessage} [content] The new content for the message
* @param {string|APIMessage} [content] The new content for the message
* @param {MessageEditOptions|MessageEmbed} [options] The options to provide
* @returns {Promise<Message>}
* @example
Expand Down
39 changes: 19 additions & 20 deletions src/structures/MessageEmbed.js
Expand Up @@ -265,8 +265,8 @@ class MessageEmbed {

/**
* Adds a field to the embed (max 25).
* @param {StringResolvable} name The name of this field
* @param {StringResolvable} value The value of this field
* @param {string} name The name of this field
* @param {string} value The value of this field
* @param {boolean} [inline=false] If this field will be displayed inline
* @returns {MessageEmbed}
*/
Expand Down Expand Up @@ -309,13 +309,13 @@ class MessageEmbed {

/**
* Sets the author of this embed.
* @param {StringResolvable} name The name of the author
* @param {string} name The name of the author
* @param {string} [iconURL] The icon URL of the author
* @param {string} [url] The URL of the author
* @returns {MessageEmbed}
*/
setAuthor(name, iconURL, url) {
this.author = { name: Util.resolveString(name), iconURL, url };
this.author = { name: Util.verifyString(name, RangeError, 'EMBED_AUTHOR_NAME'), iconURL, url };
return this;
}

Expand All @@ -331,24 +331,22 @@ class MessageEmbed {

/**
* Sets the description of this embed.
* @param {StringResolvable} description The description
* @param {string} description The description
* @returns {MessageEmbed}
*/
setDescription(description) {
description = Util.resolveString(description);
this.description = description;
this.description = Util.verifyString(description, RangeError, 'EMBED_DESCRIPTION');
return this;
}

/**
* Sets the footer of this embed.
* @param {StringResolvable} text The text of the footer
* @param {string} text The text of the footer
* @param {string} [iconURL] The icon URL of the footer
* @returns {MessageEmbed}
*/
setFooter(text, iconURL) {
text = Util.resolveString(text);
this.footer = { text, iconURL };
this.footer = { text: Util.verifyString(text, RangeError, 'EMBED_FOOTER_TEXT'), iconURL };
return this;
}

Expand Down Expand Up @@ -385,12 +383,11 @@ class MessageEmbed {

/**
* Sets the title of this embed.
* @param {StringResolvable} title The title
* @param {string} title The title
* @returns {MessageEmbed}
*/
setTitle(title) {
title = Util.resolveString(title);
this.title = title;
this.title = Util.verifyString(title, RangeError, 'EMBED_TITLE');
return this;
}

Expand Down Expand Up @@ -437,21 +434,23 @@ class MessageEmbed {

/**
* Normalizes field input and resolves strings.
* @param {StringResolvable} name The name of the field
* @param {StringResolvable} value The value of the field
* @param {string} name The name of the field
* @param {string} value The value of the field
* @param {boolean} [inline=false] Set the field to display inline
* @returns {EmbedField}
*/
static normalizeField(name, value, inline = false) {
name = Util.resolveString(name);
value = Util.resolveString(value);
return { name, value, inline };
return {
name: Util.verifyString(name, RangeError, 'EMBED_FIELD_NAME', false),
value: Util.verifyString(value, RangeError, 'EMBED_FIELD_VALUE', false),
inline,
};
}

/**
* @typedef {Object} EmbedFieldData
* @property {StringResolvable} name The name of this field
* @property {StringResolvable} value The value of this field
* @property {string} name The name of this field
* @property {string} value The value of this field
* @property {boolean} [inline] If this field will be displayed inline
*/

Expand Down
2 changes: 1 addition & 1 deletion src/structures/Webhook.js
Expand Up @@ -105,7 +105,7 @@ class Webhook {

/**
* Sends a message with this webhook.
* @param {StringResolvable|APIMessage} [content=''] The content to send
* @param {string|APIMessage} [content=''] The content to send
* @param {WebhookMessageOptions|MessageAdditions} [options={}] The options to provide
* @returns {Promise<Message|Object>}
* @example
Expand Down
2 changes: 1 addition & 1 deletion src/structures/interfaces/TextBasedChannel.js
Expand Up @@ -114,7 +114,7 @@ class TextBasedChannel {

/**
* Sends a message to this channel.
* @param {StringResolvable|APIMessage} [content=''] The content to send
* @param {string|APIMessage} [content=''] The content to send
* @param {MessageOptions|MessageAdditions} [options={}] The options to provide
* @returns {Promise<Message|Message[]>}
* @example
Expand Down
32 changes: 16 additions & 16 deletions src/util/Util.js
Expand Up @@ -57,12 +57,12 @@ class Util {

/**
* Splits a string into multiple chunks at a designated character that do not exceed a specific length.
* @param {StringResolvable} text Content to split
* @param {string} text Content to split
* @param {SplitOptions} [options] Options controlling the behavior of the split
* @returns {string[]}
*/
static splitMessage(text, { maxLength = 2000, char = '\n', prepend = '', append = '' } = {}) {
text = Util.resolveString(text);
text = Util.verifyString(text, RangeError, 'MESSAGE_CONTENT_TYPE', false);
if (text.length <= maxLength) return [text];
const splitText = text.split(char);
if (splitText.some(chunk => chunk.length > maxLength)) throw new RangeError('SPLIT_MAX_LEN');
Expand Down Expand Up @@ -347,22 +347,22 @@ class Util {
}

/**
* Data that can be resolved to give a string. This can be:
* * A string
* * An array (joined with a new line delimiter to give a string)
* * Any value
* @typedef {string|Array|*} StringResolvable
*/

/**
* Resolves a StringResolvable to a string.
* @param {StringResolvable} data The string resolvable to resolve
* Verifies the provided data is a string, otherwise throws provided error.
* @param {string} data The string resolvable to resolve
* @param {Function} [error] The Error constructor to instantiate. Defaults to Error
* @param {string} [errorMessage] The error message to throw with. Defaults to "Expected string, got <data> instead."
* @param {boolean} [allowEmpty=true] Whether an empty string should be allowed
* @returns {string}
*/
static resolveString(data) {
if (typeof data === 'string') return data;
if (Array.isArray(data)) return data.join('\n');
return String(data);
static verifyString(
data,
error = Error,
errorMessage = `Expected a string, got ${data} instead.`,
allowEmpty = true,
) {
if (typeof data !== 'string') throw new error(errorMessage);
if (!allowEmpty && data.length === 0) throw new error(errorMessage);
return data;
}

/**
Expand Down
33 changes: 2 additions & 31 deletions test/sendtest.js
Expand Up @@ -9,7 +9,6 @@ const { Client, Intents, MessageAttachment, MessageEmbed } = require('../src');

const client = new Client({ intents: [Intents.FLAGS.GUILDS, Intents.FLAGS.GUILD_MESSAGES] });

const fill = c => Array(4).fill(c.repeat(1000));
const buffer = l => fetch(l).then(res => res.buffer());
const read = util.promisify(fs.readFile);
const readStream = fs.createReadStream;
Expand All @@ -24,16 +23,11 @@ const attach = (attachment, name) => new MessageAttachment(attachment, name);

const tests = [
m => m.channel.send('x'),
m => m.channel.send(['x', 'y']),

m => m.channel.send('x', { code: true }),
m => m.channel.send('1', { code: 'js' }),
m => m.channel.send('x', { code: '' }),

m => m.channel.send(fill('x'), { split: true }),
m => m.channel.send(fill('1'), { code: 'js', split: true }),
m => m.channel.send(fill('xyz '), { split: { char: ' ' } }),

m => m.channel.send('x', { embed: { description: 'a' } }),
m => m.channel.send({ embed: { description: 'a' } }),
m => m.channel.send({ files: [{ attachment: linkA }] }),
Expand Down Expand Up @@ -68,8 +62,8 @@ const tests = [
m => m.channel.send({ embed: { description: 'a' } }).then(m2 => m2.edit({ embed: null })),
m => m.channel.send(embed().setDescription('a')).then(m2 => m2.edit({ embed: null })),

m => m.channel.send(['x', 'y'], [embed().setDescription('a'), attach(linkB)]),
m => m.channel.send(['x', 'y'], [attach(linkA), attach(linkB)]),
m => m.channel.send('x', [embed().setDescription('a'), attach(linkB)]),
m => m.channel.send('x', [attach(linkA), attach(linkB)]),

m => m.channel.send([embed().setDescription('a'), attach(linkB)]),
m =>
Expand All @@ -83,37 +77,14 @@ const tests = [
.setImage('attachment://two.png')
.attachFiles([attach(linkB, 'two.png')]),
}),
async m =>
m.channel.send(['x', 'y', 'z'], {
code: 'js',
embed: embed()
.setImage('attachment://two.png')
.attachFiles([attach(linkB, 'two.png')]),
files: [{ attachment: await buffer(linkA) }],
}),

m => m.channel.send('x', attach(fileA)),
m => m.channel.send({ files: [fileA] }),
m => m.channel.send(attach(fileA)),
async m => m.channel.send({ files: [await read(fileA)] }),
async m =>
m.channel.send(fill('x'), {
code: 'js',
split: true,
embed: embed().setImage('attachment://zero.png'),
files: [attach(await buffer(linkA), 'zero.png')],
}),

m => m.channel.send('x', attach(readStream(fileA))),
m => m.channel.send({ files: [readStream(fileA)] }),
m => m.channel.send({ files: [{ attachment: readStream(fileA) }] }),
async m =>
m.channel.send(fill('xyz '), {
code: 'js',
split: { char: ' ', prepend: 'hello! ', append: '!!!' },
embed: embed().setImage('attachment://zero.png'),
files: [linkB, attach(await buffer(linkA), 'zero.png'), readStream(fileA)],
}),

m => m.channel.send('Done!'),
];
Expand Down
2 changes: 1 addition & 1 deletion test/voice.js
Expand Up @@ -58,7 +58,7 @@ client.on('message', m => {
m.channel.send(com, { code: true });
} catch (e) {
console.log(e);
m.channel.send(e, { code: true });
m.channel.send(String(e), { code: true });
}
}
});
26 changes: 2 additions & 24 deletions test/webhooktest.js
Expand Up @@ -9,7 +9,6 @@ const { Client, Intents, MessageAttachment, MessageEmbed, WebhookClient } = requ

const client = new Client({ intents: [Intents.FLAGS.GUILDS, Intents.FLAGS.GUILD_MESSAGES] });

const fill = c => Array(4).fill(c.repeat(1000));
const buffer = l => fetch(l).then(res => res.buffer());
const read = util.promisify(fs.readFile);
const readStream = fs.createReadStream;
Expand All @@ -24,16 +23,10 @@ const attach = (attachment, name) => new MessageAttachment(attachment, name);

const tests = [
(m, hook) => hook.send('x'),
(m, hook) => hook.send(['x', 'y']),

(m, hook) => hook.send('x', { code: true }),
(m, hook) => hook.send('1', { code: 'js' }),
(m, hook) => hook.send('x', { code: '' }),

(m, hook) => hook.send(fill('x'), { split: true }),
(m, hook) => hook.send(fill('1'), { code: 'js', split: true }),
(m, hook) => hook.send(fill('xyz '), { split: { char: ' ' } }),

(m, hook) => hook.send({ embeds: [{ description: 'a' }] }),
(m, hook) => hook.send({ files: [{ attachment: linkA }] }),
(m, hook) =>
Expand Down Expand Up @@ -61,8 +54,8 @@ const tests = [
(m, hook) => hook.send({ embeds: [{ description: 'a' }] }),
(m, hook) => hook.send(embed().setDescription('a')),

(m, hook) => hook.send(['x', 'y'], [embed().setDescription('a'), attach(linkB)]),
(m, hook) => hook.send(['x', 'y'], [attach(linkA), attach(linkB)]),
(m, hook) => hook.send('x', [embed().setDescription('a'), attach(linkB)]),
(m, hook) => hook.send('x', [attach(linkA), attach(linkB)]),

(m, hook) => hook.send([embed().setDescription('a'), attach(linkB)]),
(m, hook) =>
Expand Down Expand Up @@ -93,25 +86,10 @@ const tests = [
(m, hook) => hook.send({ files: [fileA] }),
(m, hook) => hook.send(attach(fileA)),
async (m, hook) => hook.send({ files: [await read(fileA)] }),
async (m, hook) =>
hook.send(fill('x'), {
code: 'js',
split: true,
embeds: [embed().setImage('attachment://zero.png')],
files: [attach(await buffer(linkA), 'zero.png')],
}),

(m, hook) => hook.send('x', attach(readStream(fileA))),
(m, hook) => hook.send({ files: [readStream(fileA)] }),
(m, hook) => hook.send({ files: [{ attachment: readStream(fileA) }] }),
async (m, hook) =>
hook.send(fill('xyz '), {
code: 'js',
split: { char: ' ', prepend: 'hello! ', append: '!!!' },
embeds: [embed().setImage('attachment://zero.png')],
files: [linkB, attach(await buffer(linkA), 'zero.png'), readStream(fileA)],
}),

(m, hook) => hook.send('Done!'),
];

Expand Down

2 comments on commit 7b85a72

@Koyamie
Copy link
Contributor

@Koyamie Koyamie commented on 7b85a72 Jun 6, 2021

Choose a reason for hiding this comment

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

What's up with removing arrays? I don't get it, killing useful features

@monbrey
Copy link
Member

@monbrey monbrey commented on 7b85a72 Jun 7, 2021

Choose a reason for hiding this comment

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

"Features" that make things work in ways that are unpredictable make the library more unreliable and overall a less consistent user interface.

Enforcing strings improves the consistency and expected behaviour of the library. You can always join an array yourself.

Please sign in to comment.