ronantakizawa/codereview-bench huggingface dataset:
Repository: discordjs/discord.js
File: packages/discord.js/src/structures/Emoji.js (JavaScript)

Code before:
    this.name = emoji.name ?? null;

    /**
     * The emoji's id
     * @type {?Snowflake}
     */
    this.id = emoji.id ?? null;
  }

  /**
   * The identifier of this emoji, used for message reactions
   * @type {string}
   * @readonly
   */
  get identifier() {
    if (this.id) return `${this.animated ? 'a:' : ''}${this.name}:${this.id}`;
    return encodeURIComponent(this.name);
  }

  /**
   * Returns a URL for the emoji or `null` if this is not a custom emoji.
   * @param {BaseImageURLOptions} [options] Options for the image URL
   * @returns {?string}
   */
  imageURL(options) {
    return this.id && this.client.rest.cdn.emoji(this.id, this.animated ? { ...options, extension: 'gif' } : options);
  }

  /**
   * The timestamp the emoji was created at, or null if unicode
   * @type {?number}
   * @readonly
   */
  get createdTimestamp() {
    return this.id && DiscordSnowflake.timestampFrom(this.id);
  }

  /**
   * The time the emoji was created at, or null if unicode
   * @type {?Date}
   * @readonly
   */
  get createdAt() {
    return this.id && new Date(this.createdTimestamp);
  }

  /**
   * When concatenated with a string, this automatically returns the text required to form a graphical emoji on Discord
   * instead of the Emoji object.
   * @returns {string}
   * @example

Review comment (suggestion):
This change makes it so when an emoji is animated, passing `extension: 'png'` will still return a GIF.

However, if you swap the order, `...options` will be able to override the `extension` property:
```suggestion
    return this.id && this.client.rest.cdn.emoji(this.id, this.animated ? { extension: 'gif', ...options } : options);
```

Code after:
    this.name = emoji.name ?? null;

    /**
     * The emoji's id
     * @type {?Snowflake}
     */
    this.id = emoji.id ?? null;
  }

  /**
   * The identifier of this emoji, used for message reactions
   * @type {string}
   * @readonly
   */
  get identifier() {
    if (this.id) return `${this.animated ? 'a:' : ''}${this.name}:${this.id}`;
    return encodeURIComponent(this.name);
  }

  /**
   * Returns a URL for the emoji or `null` if this is not a custom emoji.
   * @param {ImageURLOptions} [options={}] Options for the image URL
   * @returns {?string}
   */
  imageURL(options = {}) {
    return this.id && this.client.rest.cdn.emoji(this.id, this.animated, options);
  }

  /**
   * The timestamp the emoji was created at, or null if unicode
   * @type {?number}
   * @readonly
   */
  get createdTimestamp() {
    return this.id && DiscordSnowflake.timestampFrom(this.id);
  }

  /**
   * The time the emoji was created at, or null if unicode
   * @type {?Date}
   * @readonly
   */
  get createdAt() {
    return this.id && new Date(this.createdTimestamp);
  }

  /**
   * When concatenated with a string, this automatically returns the text required to form a graphical emoji on Discord
   * instead of the Emoji object.
   * @returns {string}
   * @example
Repository: discordjs/discord.js
File: packages/discord.js/src/structures/Emoji.js (JavaScript)

Code before:
    this.name = emoji.name ?? null;

    /**
     * The emoji's id
     * @type {?Snowflake}
     */
    this.id = emoji.id ?? null;
  }

  /**
   * The identifier of this emoji, used for message reactions
   * @type {string}
   * @readonly
   */
  get identifier() {
    if (this.id) return `${this.animated ? 'a:' : ''}${this.name}:${this.id}`;
    return encodeURIComponent(this.name);
  }

  /**
   * Returns a URL for the emoji or `null` if this is not a custom emoji.
   * @param {BaseImageURLOptions} [options] Options for the image URL
   * @returns {?string}
   */
  imageURL(options) {
    return this.id && this.client.rest.cdn.emoji(this.id, this.animated ? { extension: 'gif', ...options } : options);
  }

  /**
   * The timestamp the emoji was created at, or null if unicode
   * @type {?number}
   * @readonly
   */
  get createdTimestamp() {
    return this.id && DiscordSnowflake.timestampFrom(this.id);
  }

  /**
   * The time the emoji was created at, or null if unicode
   * @type {?Date}
   * @readonly
   */
  get createdAt() {
    return this.id && new Date(this.createdTimestamp);
  }

  /**
   * When concatenated with a string, this automatically returns the text required to form a graphical emoji on Discord
   * instead of the Emoji object.
   * @returns {string}
   * @example

Review comment (suggestion):
Actually, while this works, it might be worth adding support for the `forceStatic` option like the other dynamic image URL methods

You'd have to change the parameter type to `ImageURLOptions` and handle the `forceStatic` option here as well

Code after:
    this.name = emoji.name ?? null;

    /**
     * The emoji's id
     * @type {?Snowflake}
     */
    this.id = emoji.id ?? null;
  }

  /**
   * The identifier of this emoji, used for message reactions
   * @type {string}
   * @readonly
   */
  get identifier() {
    if (this.id) return `${this.animated ? 'a:' : ''}${this.name}:${this.id}`;
    return encodeURIComponent(this.name);
  }

  /**
   * Returns a URL for the emoji or `null` if this is not a custom emoji.
   * @param {ImageURLOptions} [options={}] Options for the image URL
   * @returns {?string}
   */
  imageURL(options = {}) {
    return this.id && this.client.rest.cdn.emoji(this.id, this.animated, options);
  }

  /**
   * The timestamp the emoji was created at, or null if unicode
   * @type {?number}
   * @readonly
   */
  get createdTimestamp() {
    return this.id && DiscordSnowflake.timestampFrom(this.id);
  }

  /**
   * The time the emoji was created at, or null if unicode
   * @type {?Date}
   * @readonly
   */
  get createdAt() {
    return this.id && new Date(this.createdTimestamp);
  }

  /**
   * When concatenated with a string, this automatically returns the text required to form a graphical emoji on Discord
   * instead of the Emoji object.
   * @returns {string}
   * @example
Repository: discordjs/discord.js
File: packages/discord.js/src/structures/ThreadMember.js (JavaScript)

Code before:

  /**
   * The guild member associated with this thread member
   * @type {?GuildMember}
   * @readonly
   */
  get guildMember() {
    return this.member ?? this.thread.guild.members.cache.get(this.id) ?? null;
  }

  /**
   * The last time this member joined the thread
   * @type {?Date}
   * @readonly
   */
  get joinedAt() {
    return this.joinedTimestamp && new Date(this.joinedTimestamp);
  }

  /**
   * The user associated with this thread member
   * @type {?User}
   * @readonly
   */
  get user() {
    return this.client.users.cache.get(this.id);
  }

  /**
   * Whether the client user can manage this thread member
   * @type {boolean}
   * @readonly
   */
  get manageable() {
    return !this.thread.archived && this.thread.editable;
  }

  /**
   * Removes this member from the thread.
   * @returns {Promise<ThreadMember>}
   */
  async remove() {
    await this.thread.members.remove(this.id);
    return this;
  }
}

module.exports = ThreadMember;


Review comment (performance):
```suggestion
    return this.client.users.cache.get(this.id) ?? null;
```

Code after:

  /**
   * The guild member associated with this thread member
   * @type {?GuildMember}
   * @readonly
   */
  get guildMember() {
    return this.member ?? this.thread.guild.members.cache.get(this.id) ?? null;
  }

  /**
   * The last time this member joined the thread
   * @type {?Date}
   * @readonly
   */
  get joinedAt() {
    return this.joinedTimestamp && new Date(this.joinedTimestamp);
  }

  /**
   * The user associated with this thread member
   * @type {?User}
   * @readonly
   */
  get user() {
    return this.client.users.cache.get(this.id) ?? null;
  }

  /**
   * Whether the client user can manage this thread member
   * @type {boolean}
   * @readonly
   */
  get manageable() {
    return !this.thread.archived && this.thread.editable;
  }

  /**
   * Removes this member from the thread.
   * @returns {Promise<ThreadMember>}
   */
  async remove() {
    await this.thread.members.remove(this.id);
    return this;
  }
}

module.exports = ThreadMember;
Repository: discordjs/discord.js
File: packages/discord.js/src/structures/PrimaryEntryPointCommandInteraction.js (JavaScript)

Code before:
'use strict';

const { CommandInteraction } = require('./CommandInteraction.js');
/**
 * Represents a primary entry point command interaction.
 * @extends {CommandInteraction}
 */
class PrimaryEntryPointCommandInteraction extends CommandInteraction {}

exports.PrimaryEntryPointCommandInteraction = PrimaryEntryPointCommandInteraction;


Review comment (suggestion):
Isn’t this a somewhat breaking change in the sense that prior to this all instances of CommandInteraction automatically had an .options property (as evident by the needed change in typings)? Since this PR targets `main` anyway that wouldn’t be an issue, just curious about the semver label here.

Code after:
'use strict';

const { CommandInteraction } = require('./CommandInteraction.js');

/**
 * Represents a primary entry point command interaction.
 * @extends {CommandInteraction}
 */
class PrimaryEntryPointCommandInteraction extends CommandInteraction {}

exports.PrimaryEntryPointCommandInteraction = PrimaryEntryPointCommandInteraction;
Repository: discordjs/discord.js
File: packages/discord.js/src/structures/ApplicationCommand.js (JavaScript)

Code before:
          : false),
      autocomplete: option.autocomplete,
      choices: option.choices?.map(choice => ({
        name: choice.name,
        [nameLocalizedKey]: choice.nameLocalized ?? choice.name_localized,
        [nameLocalizationsKey]: choice.nameLocalizations ?? choice.name_localizations,
        value: choice.value,
      })),
      options: option.options?.map(opt => this.transformOption(opt, received)),
      [channelTypesKey]: option.channelTypes ?? option.channel_types,
      [minValueKey]: option.minValue ?? option.min_value,
      [maxValueKey]: option.maxValue ?? option.max_value,
      [minLengthKey]: option.minLength ?? option.min_length,
      [maxLengthKey]: option.maxLength ?? option.max_length,
    };
  }
}

exports.ApplicationCommand = ApplicationCommand;

/* eslint-disable max-len */
/**
 * @external ApplicationCommandOptionAllowedChannelTypes
 * @see {@link https://discord.js.org/docs/packages/builders/stable/ApplicationCommandOptionAllowedChannelTypes:TypeAlias}
 */

/**
 * @external EntryPointCommandHandlerType
 * @see {@link https://discord-api-types.dev/api/discord-api-types-v10/enum/EntryPointCommandHandlerType}
 */


Review comment (suggestion):
This goes in the [APITypes](https://github.com/discordjs/discord.js/blob/main/packages/discord.js/src/util/APITypes.js) file

Code after:
          : false),
      autocomplete: option.autocomplete,
      choices: option.choices?.map(choice => ({
        name: choice.name,
        [nameLocalizedKey]: choice.nameLocalized ?? choice.name_localized,
        [nameLocalizationsKey]: choice.nameLocalizations ?? choice.name_localizations,
        value: choice.value,
      })),
      options: option.options?.map(opt => this.transformOption(opt, received)),
      [channelTypesKey]: option.channelTypes ?? option.channel_types,
      [minValueKey]: option.minValue ?? option.min_value,
      [maxValueKey]: option.maxValue ?? option.max_value,
      [minLengthKey]: option.minLength ?? option.min_length,
      [maxLengthKey]: option.maxLength ?? option.max_length,
    };
  }
}

exports.ApplicationCommand = ApplicationCommand;

/* eslint-disable max-len */
/**
 * @external ApplicationCommandOptionAllowedChannelTypes
 * @see {@link https://discord.js.org/docs/packages/builders/stable/ApplicationCommandOptionAllowedChannelTypes:TypeAlias}
 */
Repository: discordjs/discord.js
File: packages/ws/__tests__/util/SimpleIdentifyThrottler.test.ts (TypeScript)

Code before:
	// Those shouldn't wait since they're in different keys

	await throttler.waitForIdentify(0);
	expect(sleep).not.toHaveBeenCalled();

	await throttler.waitForIdentify(1);
	expect(sleep).not.toHaveBeenCalled();

	// Those should wait

	await throttler.waitForIdentify(2);
	expect(sleep).toHaveBeenCalledTimes(1);

	await throttler.waitForIdentify(3);
	expect(sleep).toHaveBeenCalledTimes(2);
});

test('does not call sleep with a negative time', async () => {
	await throttler.waitForIdentify(0);
	expect(sleep).not.toHaveBeenCalled();

	await throttler.waitForIdentify(0);
	expect(sleep).toHaveBeenCalledTimes(1);

	// By overshooting, we're simulating a bug that existed prior to this test, where-in by enough time
	// passing before the shard tried to identify for a subseuent time, the passed value would end up being negative
	// (and this was unchecked). Node simply treats that as 1ms, so it wasn't particularly harmful, but they
	// recently introduced a warning for it, so we want to avoid that.
	NOW.mockReturnValueOnce(TIME + 10_000);
	await throttler.waitForIdentify(0);
	expect(sleep).toHaveBeenCalledTimes(1);
});


Review comment (suggestion):
```suggestion
	// passing before the shard tried to identify for a subsequent time, the passed value would end up being negative
```


Code after:
afterEach(() => {
	sleep.mockClear();
});

test('basic case', async () => {
	// Those shouldn't wait since they're in different keys
	await throttler.waitForIdentify(0, controller.signal);
	expect(sleep).not.toHaveBeenCalled();

	await throttler.waitForIdentify(1, controller.signal);
	expect(sleep).not.toHaveBeenCalled();

	// Those should wait
	await throttler.waitForIdentify(2, controller.signal);
	expect(sleep).toHaveBeenCalledTimes(1);

	await throttler.waitForIdentify(3, controller.signal);
	expect(sleep).toHaveBeenCalledTimes(2);
});

test('does not call sleep with a negative time', async () => {
	await throttler.waitForIdentify(0, controller.signal);
	expect(sleep).not.toHaveBeenCalled();

	await throttler.waitForIdentify(0, controller.signal);
	expect(sleep).toHaveBeenCalledTimes(1);

	// By overshooting, we're simulating a bug that existed prior to this test, where-in by enough time
	// passing before the shard tried to identify for a subsequent time, the passed value would end up being negative
	// (and this was unchecked). Node simply treats that as 1ms, so it wasn't particularly harmful, but they
	// recently introduced a warning for it, so we want to avoid that.
	NOW.mockReturnValueOnce(TIME + 10_000);
	await throttler.waitForIdentify(0, controller.signal);
	expect(sleep).toHaveBeenCalledTimes(1);
});
Repository: discordjs/discord.js
File: packages/rest/src/lib/utils/types.ts (TypeScript)

Code before:
	name: string;
}

export interface AuthData {
	/**
	 * The authorization prefix to use for this request, useful if you use this with bearer tokens
	 *
	 * @defaultValue `REST.options.authPrefix`
	 */
	prefix?: 'Bearer' | 'Bot';
	/**
	 * The authorization token to use for this request
	 */
	token: string;
}

/**
 * Represents possible data to be given to an endpoint
 */
export interface RequestData {
	/**
	 * Whether to append JSON data to form data instead of `payload_json` when sending files
	 */
	appendToFormData?: boolean;
	/**
	 * Alternate authorization data to use for this request only, or `null` to disable the Authorizaiton header
	 *
	 * @defaultValue `true`
	 */
	auth?: AuthData | boolean;
	/**
	 * The body to send to this request.
	 * If providing as BodyInit, set `passThroughBody: true`
	 */
	body?: BodyInit | unknown;
	/**
	 * The {@link https://undici.nodejs.org/#/docs/api/Agent | Agent} to use for the request.
	 */
	dispatcher?: Agent;
	/**
	 * Files to be attached to this request
	 */
	files?: RawFile[] | undefined;
	/**
	 * Additional headers to add to this request
	 */
	headers?: Record<string, string>;
	/**
	 * Whether to pass-through the body property directly to `fetch()`.
	 * <warn>This only applies when files is NOT present</warn>
	 */

Review comment (security):
```suggestion
	 * Alternate authorization data to use for this request only, or `null` to disable the Authorization header
```

Code after:
	name: string;
}

export interface AuthData {
	/**
	 * The authorization prefix to use for this request, useful if you use this with bearer tokens
	 *
	 * @defaultValue `REST.options.authPrefix`
	 */
	prefix?: 'Bearer' | 'Bot';
	/**
	 * The authorization token to use for this request
	 */
	token: string;
}

/**
 * Represents possible data to be given to an endpoint
 */
export interface RequestData {
	/**
	 * Whether to append JSON data to form data instead of `payload_json` when sending files
	 */
	appendToFormData?: boolean;
	/**
	 * Alternate authorization data to use for this request only, or `false` to disable the Authorization header
	 *
	 * @defaultValue `true`
	 */
	auth?: AuthData | boolean;
	/**
	 * The body to send to this request.
	 * If providing as BodyInit, set `passThroughBody: true`
	 */
	body?: BodyInit | unknown;
	/**
	 * The {@link https://undici.nodejs.org/#/docs/api/Agent | Agent} to use for the request.
	 */
	dispatcher?: Agent;
	/**
	 * Files to be attached to this request
	 */
	files?: RawFile[] | undefined;
	/**
	 * Additional headers to add to this request
	 */
	headers?: Record<string, string>;
	/**
	 * Whether to pass-through the body property directly to `fetch()`.
	 * <warn>This only applies when files is NOT present</warn>
	 */
Repository: discordjs/discord.js
File: packages/rest/src/lib/utils/types.ts (TypeScript)

Code before:
	name: string;
}

export interface AuthData {
	/**
	 * The authorization prefix to use for this request, useful if you use this with bearer tokens
	 *
	 * @defaultValue `REST.options.authPrefix`
	 */
	prefix?: 'Bearer' | 'Bot';
	/**
	 * The authorization token to use for this request
	 */
	token: string;
}

/**
 * Represents possible data to be given to an endpoint
 */
export interface RequestData {
	/**
	 * Whether to append JSON data to form data instead of `payload_json` when sending files
	 */
	appendToFormData?: boolean;
	/**
	 * Alternate authorization data to use for this request only, or `null` to disable the Authorization header
	 *
	 * @defaultValue `true`
	 */
	auth?: AuthData | boolean;
	/**
	 * The body to send to this request.
	 * If providing as BodyInit, set `passThroughBody: true`
	 */
	body?: BodyInit | unknown;
	/**
	 * The {@link https://undici.nodejs.org/#/docs/api/Agent | Agent} to use for the request.
	 */
	dispatcher?: Agent;
	/**
	 * Files to be attached to this request
	 */
	files?: RawFile[] | undefined;
	/**
	 * Additional headers to add to this request
	 */
	headers?: Record<string, string>;
	/**
	 * Whether to pass-through the body property directly to `fetch()`.
	 * <warn>This only applies when files is NOT present</warn>
	 */

Review comment (security):
```suggestion
	 * Alternate authorization data to use for this request only, or `false` to disable the Authorization header
```

Code after:
	name: string;
}

export interface AuthData {
	/**
	 * The authorization prefix to use for this request, useful if you use this with bearer tokens
	 *
	 * @defaultValue `REST.options.authPrefix`
	 */
	prefix?: 'Bearer' | 'Bot';
	/**
	 * The authorization token to use for this request
	 */
	token: string;
}

/**
 * Represents possible data to be given to an endpoint
 */
export interface RequestData {
	/**
	 * Whether to append JSON data to form data instead of `payload_json` when sending files
	 */
	appendToFormData?: boolean;
	/**
	 * Alternate authorization data to use for this request only, or `false` to disable the Authorization header
	 *
	 * @defaultValue `true`
	 */
	auth?: AuthData | boolean;
	/**
	 * The body to send to this request.
	 * If providing as BodyInit, set `passThroughBody: true`
	 */
	body?: BodyInit | unknown;
	/**
	 * The {@link https://undici.nodejs.org/#/docs/api/Agent | Agent} to use for the request.
	 */
	dispatcher?: Agent;
	/**
	 * Files to be attached to this request
	 */
	files?: RawFile[] | undefined;
	/**
	 * Additional headers to add to this request
	 */
	headers?: Record<string, string>;
	/**
	 * Whether to pass-through the body property directly to `fetch()`.
	 * <warn>This only applies when files is NOT present</warn>
	 */
Repository: discordjs/discord.js
File: packages/discord.js/typings/index.d.ts (TypeScript)

Code before:

export type PresenceStatusData = ClientPresenceStatus | 'invisible';

export type PresenceStatus = PresenceStatusData | 'offline';

export interface ReactionCollectorOptions extends CollectorOptions<[MessageReaction, User]> {
  max?: number;
  maxEmojis?: number;
  maxUsers?: number;
}

export interface ReplyOptions {
  messageReference: MessageResolvable;
  failIfNotExists?: boolean;
}

export interface BaseForwardOptions {
  messageId: MessageResolvable;
  channelId?: Exclude<TextBasedChannelResolvable, PartialGroupDMChannel>;
  guildId?: GuildResolvable;
}

export type ForwardOptionsWithMandatoryChannel = BaseForwardOptions & Required<Pick<BaseForwardOptions, 'channelId'>>;

export interface ForwardOptionsWithOptionalChannel extends BaseForwardOptions {
  messageId: Exclude<MessageResolvable, Snowflake>;
}

export type ForwardOptions = ForwardOptionsWithMandatoryChannel | ForwardOptionsWithOptionalChannel;

export interface MessageReplyOptions extends Omit<MessageCreateOptions, 'reply' | 'forward'> {
  failIfNotExists?: boolean;
}

export interface ResolvedOverwriteOptions {
  allow: PermissionsBitField;
  deny: PermissionsBitField;
}

export interface RoleData {
  name?: string;
  color?: ColorResolvable;
  hoist?: boolean;
  position?: number;
  permissions?: PermissionResolvable;
  mentionable?: boolean;
  icon?: BufferResolvable | Base64Resolvable | EmojiResolvable | null;
  unicodeEmoji?: string | null;
}

export type RoleMention = '@everyone' | `<@&${Snowflake}>`;

Review comment (suggestion):
So if I'm reading this right, passing in `channel.send({ forward: { messageId: message } })` works? Or is supposed to work?? Because the JS code does not default channel_id from message

Code after:
}

export interface PartialRoleData extends RoleData {
  id?: Snowflake | number;
}

export enum Partials {
  User,
  Channel,
  GuildMember,
  Message,
  Reaction,
  GuildScheduledEvent,
  ThreadMember,
}

export interface PartialUser extends Partialize<User, 'username' | 'tag' | 'discriminator'> {}

export type PresenceStatusData = ClientPresenceStatus | 'invisible';

export type PresenceStatus = PresenceStatusData | 'offline';

export interface ReactionCollectorOptions extends CollectorOptions<[MessageReaction, User]> {
  max?: number;
  maxEmojis?: number;
  maxUsers?: number;
}

export interface ReplyOptions {
  messageReference: MessageResolvable;
  failIfNotExists?: boolean;
}

export interface BaseForwardOptions {
  message: MessageResolvable;
  channel?: Exclude<TextBasedChannelResolvable, PartialGroupDMChannel>;
  guild?: GuildResolvable;
}

export type ForwardOptionsWithMandatoryChannel = BaseForwardOptions & Required<Pick<BaseForwardOptions, 'channel'>>;

export interface ForwardOptionsWithOptionalChannel extends BaseForwardOptions {
  message: Exclude<MessageResolvable, Snowflake>;
}

export type ForwardOptions = ForwardOptionsWithMandatoryChannel | ForwardOptionsWithOptionalChannel;

export interface MessageReplyOptions extends Omit<MessageCreateOptions, 'reply' | 'forward'> {
  failIfNotExists?: boolean;
}