ronantakizawa/codereview-bench huggingface dataset:
Repository: discordjs/discord.js
File: packages/core/src/api/channel.ts (TypeScript)

Code before:
	 * @param channelId - The id of the channel to add the recipient to
	 * @param userId - The id of the user to add as a recipient
	 * @param body - The data for adding the recipient
	 * @param options - The options for adding the recipient
	 */
	public async addGroupDMRecipient(
		channelId: Snowflake,
		userId: Snowflake,
		body: RESTPutAPIChannelRecipientJSONBody,
		{ auth, signal }: Pick<RequestData, 'auth' | 'signal'> = {},
	) {
		await this.rest.put(Routes.channelRecipient(channelId, userId), {
			auth,
			body,
			signal,
		});
	}

	/**
	 * Removes a recipient from a group DM channel
	 *
	 * @param channelId - The id of the channel to remove the recipient from
	 * @param userId - The id of the user to remove as a recipient
	 * @param options - The options for removing the recipient
	 */
	public async deleteGroupDMRecipient(
		channelId: Snowflake,
		userId: Snowflake,
		{ auth, signal }: Pick<RequestData, 'auth' | 'signal'> = {},
	) {
		await this.rest.delete(Routes.channelRecipient(channelId, userId), {
			auth,
			signal,
		});
	}
}


Review comment (suggestion):
```suggestion
	 * @see {@link https://discord.com/developers/docs/resources/channel#group-dm-remove-recipient}
	 * @param channelId - The id of the channel to remove the recipient from
	 * @param userId - The id of the user to remove as a recipient
	 * @param options - The options for removing the recipient
```

Code after:
	 * @see {@link https://discord.com/developers/docs/resources/channel#group-dm-add-recipient}
	 * @param channelId - The id of the channel to add the recipient to
	 * @param userId - The id of the user to add as a recipient
	 * @param body - The data for adding the recipient
	 * @param options - The options for adding the recipient
	 */
	public async addGroupDMRecipient(
		channelId: Snowflake,
		userId: Snowflake,
		body: RESTPutAPIChannelRecipientJSONBody,
		{ auth, signal }: Pick<RequestData, 'auth' | 'signal'> = {},
	) {
		await this.rest.put(Routes.channelRecipient(channelId, userId), {
			auth,
			body,
			signal,
		});
	}

	/**
	 * Removes a recipient from a group DM channel
	 *
	 * @see {@link https://discord.com/developers/docs/resources/channel#group-dm-remove-recipient}
	 * @param channelId - The id of the channel to remove the recipient from
	 * @param userId - The id of the user to remove as a recipient
	 * @param options - The options for removing the recipient
	 */
	public async removeGroupDMRecipient(
		channelId: Snowflake,
		userId: Snowflake,
		{ auth, signal }: Pick<RequestData, 'auth' | 'signal'> = {},
	) {
		await this.rest.delete(Routes.channelRecipient(channelId, userId), {
			auth,
			signal,
		});
	}
}
Repository: discordjs/discord.js
File: packages/core/scripts/find-returns-not-return-type.mts (TypeScript)

Code before:
import { glob, readFile } from 'node:fs/promises';

const cwd = new URL('../src/api/', import.meta.url);
const results: string[] = [];

for await (const file of glob('**/*.ts', { cwd })) {
	const content = await readFile(new URL(file, cwd), { encoding: 'utf-8' });

	const matches = content.matchAll(/as Promise<(?<returnType>\w+)>/g);

	for (const match of matches) {
		const returnType = match.groups!.returnType!;

		if (!returnType.startsWith('REST')) {
			results.push(`in file core/src/api/${file}: ${returnType}`);
		}
	}
}

if (results.length > 0) {
	console.warn('Found return types that are not REST return types:');

	for (const result of results) {
		console.warn(`  - ${result}`);
	}
} else {
	console.log('No return types that are not REST return types found');
}


Review comment (suggestion):
I think this makes sense too
```suggestion
		if (!returnType.startsWith('REST') || !returnType.includes("Result")) {
```

Code after:
import { glob, readFile } from 'node:fs/promises';

const cwd = new URL('../src/api/', import.meta.url);
const results: string[] = [];

for await (const file of glob('**/*.ts', { cwd })) {
	const content = await readFile(new URL(file, cwd), { encoding: 'utf-8' });

	const matches = content.matchAll(/as Promise<(?<returnType>\w+)>/g);

	for (const match of matches) {
		const returnType = match.groups!.returnType!;

		if (!returnType.startsWith('REST') || !returnType.includes('Result')) {
			results.push(`in file core/src/api/${file}: ${returnType}`);
		}
	}
}

if (results.length > 0) {
	console.warn('Found return types that are not REST return types:');

	for (const result of results) {
		console.warn(`  - ${result}`);
	}
} else {
	console.log('No return types that are not REST return types found');
}
Repository: discordjs/discord.js
File: apps/guide/src/middleware.ts (TypeScript)

Code before:
import { NextResponse, type NextRequest } from 'next/server';

export function middleware(request: NextRequest) {
	// TODO: Remove this eventually
	if (request.nextUrl.pathname.startsWith('/guide/')) {
		const newUrl = request.nextUrl.clone();
		newUrl.pathname = newUrl.pathname.replace('/guide/', '/');
		return NextResponse.redirect(new URL(newUrl.pathname, request.url));
	}

	// Redirect old urls to /legacy
	if (!request.nextUrl.pathname.startsWith('/legacy') && !request.nextUrl.pathname.startsWith('/voice')) {
		const newUrl = request.nextUrl.clone();
		newUrl.pathname = `/legacy${newUrl.pathname}`;
		return NextResponse.redirect(new URL(newUrl.pathname, request.url));
	}

	return NextResponse.next();
}

export const config = {
	matcher: ['/((?!_next|api|og|.*\\..*|_static).*)'],
};


Review comment (suggestion):
This should be enough (i know i was the one that introduced this above... 😅)
```suggestion
		return NextResponse.redirect(newUrl);
```

Code after:
import { NextResponse, type NextRequest } from 'next/server';

export function middleware(request: NextRequest) {
	// TODO: Remove this eventually
	if (request.nextUrl.pathname.startsWith('/guide/')) {
		const newUrl = request.nextUrl.clone();
		newUrl.pathname = newUrl.pathname.replace('/guide/', '/');
		return NextResponse.redirect(newUrl);
	}

	// Redirect old urls to /legacy
	if (!request.nextUrl.pathname.startsWith('/legacy') && !request.nextUrl.pathname.startsWith('/voice')) {
		const newUrl = request.nextUrl.clone();
		newUrl.pathname = `/legacy${newUrl.pathname}`;
		return NextResponse.redirect(newUrl);
	}

	return NextResponse.next();
}

export const config = {
	matcher: ['/((?!_next|api|og|.*\\..*|_static).*)'],
};
Repository: discordjs/discord.js
File: apps/guide/src/middleware.ts (TypeScript)

Code before:
import { NextResponse, type NextRequest } from 'next/server';

export function middleware(request: NextRequest) {
	// TODO: Remove this eventually
	if (request.nextUrl.pathname.startsWith('/guide/')) {
		const newUrl = request.nextUrl.clone();
		newUrl.pathname = newUrl.pathname.replace('/guide/', '/');
		return NextResponse.redirect(new URL(newUrl.pathname, request.url));
	}

	// Redirect old urls to /legacy
	if (!request.nextUrl.pathname.startsWith('/legacy') && !request.nextUrl.pathname.startsWith('/voice')) {
		const newUrl = request.nextUrl.clone();
		newUrl.pathname = `/legacy${newUrl.pathname}`;
		return NextResponse.redirect(new URL(newUrl.pathname, request.url));
	}

	return NextResponse.next();
}

export const config = {
	matcher: ['/((?!_next|api|og|.*\\..*|_static).*)'],
};


Review comment (suggestion):
```suggestion
		return NextResponse.redirect(newUrl);
```

Code after:
import { NextResponse, type NextRequest } from 'next/server';

export function middleware(request: NextRequest) {
	// TODO: Remove this eventually
	if (request.nextUrl.pathname.startsWith('/guide/')) {
		const newUrl = request.nextUrl.clone();
		newUrl.pathname = newUrl.pathname.replace('/guide/', '/');
		return NextResponse.redirect(newUrl);
	}

	// Redirect old urls to /legacy
	if (!request.nextUrl.pathname.startsWith('/legacy') && !request.nextUrl.pathname.startsWith('/voice')) {
		const newUrl = request.nextUrl.clone();
		newUrl.pathname = `/legacy${newUrl.pathname}`;
		return NextResponse.redirect(newUrl);
	}

	return NextResponse.next();
}

export const config = {
	matcher: ['/((?!_next|api|og|.*\\..*|_static).*)'],
};
Repository: discordjs/discord.js
File: packages/discord.js/src/client/websocket/handlers/READY.js (JavaScript)

Code before:

let ClientUser;

module.exports = async (client, { d: data }, shardId) => {
  if (client.user) {
    client.user._patch(data.user);
  } else {
    ClientUser ??= require('../../../structures/ClientUser.js').ClientUser;
    client.user = new ClientUser(client, data.user);
    client.users.cache.set(client.user.id, client.user);
  }

  for (const guild of data.guilds) {
    client.expectedGuilds.add(guild.id);
    guild.shardId = shardId;
    client.guilds._add(guild);
  }

  if (client.application) {
    client.application._patch(data.application);
  } else {
    client.application = new ClientApplication(client, data.application);
  }

  client.status = Status.WaitingForGuilds;
  await client._checkReady();
};


Review comment (suggestion):
If we await the checkReady it should be done in `Client#handleMessage()`; the READY handler should not be async.

Code after:

let ClientUser;

module.exports = (client, { d: data }, shardId) => {
  if (client.user) {
    client.user._patch(data.user);
  } else {
    ClientUser ??= require('../../../structures/ClientUser.js').ClientUser;
    client.user = new ClientUser(client, data.user);
    client.users.cache.set(client.user.id, client.user);
  }

  for (const guild of data.guilds) {
    client.expectedGuilds.add(guild.id);
    guild.shardId = shardId;
    client.guilds._add(guild);
  }

  if (client.application) {
    client.application._patch(data.application);
  } else {
    client.application = new ClientApplication(client, data.application);
  }

  client.status = Status.WaitingForGuilds;
};
Repository: discordjs/discord.js
File: packages/builders/src/messages/Attachment.ts (TypeScript)

Code before:

	/**
	 * Sets the filename of this attachment.
	 *
	 * @param filename - The filename of the attachment
	 */
	public setFilename(filename: string): this {
		this.data.filename = filename;
		return this;
	}

	/**
	 * Clears the filename of this attachment.
	 */
	public clearFilename(): this {
		this.data.filename = undefined;
		return this;
	}

	/**
	 * Sets the file data to upload with this attachment.
	 *
	 * @param data - The file data (Buffer, Uint8Array, or string)
	 * @remarks Note that this data is NOT included in the `toJSON()` output. To retrieve, use {@link getRawFile}.
	 */
	public setFileData(data: Buffer | Uint8Array | boolean | number | string): this {
		this.fileData.data = data;
		return this;
	}

	/**
	 * Clears the file data from this attachment.
	 */
	public clearFileData(): this {
		this.fileData.data = undefined;
		return this;
	}

	/**
	 * Sets the content type of the file data to upload with this attachment.
	 *
	 * @remarks Note that this data is NOT included in the `toJSON()` output. To retrieve, use {@link getRawFile}.
	 */
	public setFileContentType(contentType: string): this {
		this.fileData.contentType = contentType;
		return this;
	}

	/**
	 * Clears the content type of the file data from this attachment.
	 */

Review comment (suggestion):
From what I can tell from docs it seems like support for `boolean | number` in RawFile in `@discordjs/rest` is for when not using `payload_json` but instead sending all properties as form data. So this should probably not be supported as parameters here.
```suggestion
	public setFileData(data: Buffer | Uint8Array | string): this {
```

Code after:

	/**
	 * Sets the filename of this attachment.
	 *
	 * @param filename - The filename of the attachment
	 */
	public setFilename(filename: string): this {
		this.data.filename = filename;
		return this;
	}

	/**
	 * Clears the filename of this attachment.
	 */
	public clearFilename(): this {
		this.data.filename = undefined;
		return this;
	}

	/**
	 * Sets the file data to upload with this attachment.
	 *
	 * @param data - The file data
	 * @remarks Note that this data is NOT included in the {@link toJSON} output. To retrieve it, use {@link getRawFile}.
	 */
	public setFileData(data: Buffer | Uint8Array | string): this {
		this.fileData.data = data;
		return this;
	}

	/**
	 * Clears the file data from this attachment.
	 */
	public clearFileData(): this {
		this.fileData.data = undefined;
		return this;
	}

	/**
	 * Sets the content type of the file data to upload with this attachment.
	 *
	 * @remarks Note that this data is NOT included in the {@link toJSON} output. To retrieve it, use {@link getRawFile}.
	 */
	public setFileContentType(contentType: string): this {
		this.fileData.contentType = contentType;
		return this;
	}

	/**
	 * Clears the content type of the file data from this attachment.
	 */
Repository: discordjs/discord.js
File: packages/builders/src/messages/Assertions.ts (TypeScript)

Code before:
import { Buffer } from 'node:buffer';
import { AllowedMentionsTypes, ComponentType, MessageFlags, MessageReferenceType } from 'discord-api-types/v10';
import { z } from 'zod';
import { embedPredicate } from './embed/Assertions.js';
import { pollPredicate } from './poll/Assertions.js';

const fileKeyRegex = /^files\[(?<placeholder>.+?)]$/;

export const rawFilePredicate = z.object({
	data: z.union([z.instanceof(Buffer), z.instanceof(Uint8Array), z.boolean(), z.number(), z.string()]),
	name: z.string().min(1),
	contentType: z.string().optional(),
	key: z.string().regex(fileKeyRegex).optional(),
});

export const attachmentPredicate = z.object({
	// As a string it only makes sense for edits when we do have an attachment snowflake
	id: z.union([z.string(), z.number()]),
	description: z.string().max(1_024).optional(),
	duration_secs: z
		.number()
		.max(2 ** 31 - 1)
		.optional(),
	filename: z.string().max(1_024).optional(),
	title: z.string().max(1_024).optional(),
	waveform: z.string().max(400).optional(),
});

export const allowedMentionPredicate = z
	.object({
		parse: z.enum(AllowedMentionsTypes).array().optional(),
		roles: z.string().array().max(100).optional(),
		users: z.string().array().max(100).optional(),
		replied_user: z.boolean().optional(),
	})

Review comment (suggestion):
```suggestion
	data: z.union([z.instanceof(Buffer), z.instanceof(Uint8Array), z.string()]),
```

Code after:
import { Buffer } from 'node:buffer';
import { AllowedMentionsTypes, ComponentType, MessageFlags, MessageReferenceType } from 'discord-api-types/v10';
import { z } from 'zod';
import { embedPredicate } from './embed/Assertions.js';
import { pollPredicate } from './poll/Assertions.js';

const fileKeyRegex = /^files\[(?<placeholder>\d+?)]$/;

export const rawFilePredicate = z.object({
	data: z.union([z.instanceof(Buffer), z.instanceof(Uint8Array), z.string()]),
	name: z.string().min(1),
	contentType: z.string().optional(),
	key: z.string().regex(fileKeyRegex).optional(),
});

export const attachmentPredicate = z.object({
	// As a string it only makes sense for edits when we do have an attachment snowflake
	id: z.union([z.string(), z.number()]),
	description: z.string().max(1_024).optional(),
	duration_secs: z
		.number()
		.max(2 ** 31 - 1)
		.optional(),
	filename: z.string().max(1_024).optional(),
	title: z.string().max(1_024).optional(),
	waveform: z.string().max(400).optional(),
});

export const allowedMentionPredicate = z
	.object({
		parse: z.enum(AllowedMentionsTypes).array().optional(),
		roles: z.string().array().max(100).optional(),
		users: z.string().array().max(100).optional(),
		replied_user: z.boolean().optional(),
	})
Repository: discordjs/discord.js
File: packages/builders/src/messages/Attachment.ts (TypeScript)

Code before:

	/**
	 * Sets the content type of the file data to upload with this attachment.
	 *
	 * @remarks Note that this data is NOT included in the `toJSON()` output. To retrieve, use {@link getRawFile}.
	 */
	public setFileContentType(contentType: string): this {
		this.fileData.contentType = contentType;
		return this;
	}

	/**
	 * Clears the content type of the file data from this attachment.
	 */
	public clearFileContentType(): this {
		this.fileData.contentType = undefined;
		return this;
	}

	/**
	 * Converts this attachment to a RawFile for uploading.
	 *
	 * @returns A RawFile object, or undefined if no file data is set
	 */
	public getRawFile(): Partial<RawFile> | undefined {
		if (!this.fileData) {
			return;
		}

		return {
			...this.fileData,
			name: this.data.filename,
			key: this.data.id ? `files[${this.data.id}]` : undefined,
		};
	}

	/**
	 * Sets the title of this attachment.
	 *
	 * @param title - The title of the attachment
	 */
	public setTitle(title: string): this {
		this.data.title = title;
		return this;
	}

	/**
	 * Clears the title of this attachment.
	 */
	public clearTitle(): this {
		this.data.title = undefined;

Review comment (suggestion):
This should probably check if fileData.data is undefined as well, since you aren't deleting keys when you clear.

(I do see you check this in the message builder but why not do it here)

Code after:

	/**
	 * Sets the content type of the file data to upload with this attachment.
	 *
	 * @remarks Note that this data is NOT included in the {@link toJSON} output. To retrieve it, use {@link getRawFile}.
	 */
	public setFileContentType(contentType: string): this {
		this.fileData.contentType = contentType;
		return this;
	}

	/**
	 * Clears the content type of the file data from this attachment.
	 */
	public clearFileContentType(): this {
		this.fileData.contentType = undefined;
		return this;
	}

	/**
	 * Converts this attachment to a {@link RawFile} for uploading.
	 *
	 * @returns A {@link RawFile} object, or `undefined` if no file data is set
	 */
	public getRawFile(): Partial<RawFile> | undefined {
		if (!this.fileData?.data) {
			return;
		}

		return {
			...this.fileData,
			name: this.data.filename,
			key: this.data.id ? `files[${this.data.id}]` : undefined,
		};
	}

	/**
	 * Sets the title of this attachment.
	 *
	 * @param title - The title of the attachment
	 */
	public setTitle(title: string): this {
		this.data.title = title;
		return this;
	}

	/**
	 * Clears the title of this attachment.
	 */
	public clearTitle(): this {
		this.data.title = undefined;
Repository: discordjs/discord.js
File: packages/util/src/RawFile.ts (TypeScript)

Code before:
import type { Buffer } from 'node:buffer';

/**
 * Represents a file to be added to a request with multipart/form-data encoding
 */
export interface RawFile {
	/**
	 * Content-Type of the file (e.g., 'image/png', 'application/pdf')
	 * If not provided, it will be inferred from the file data when possible
	 */
	contentType?: string;
	/**
	 * The actual data for the file
	 */
	data: Buffer | Uint8Array | boolean | number | string;
	/**
	 * An explicit key to use for the formdata field for this file.
	 * When not provided, the index of the file in the files array is used in the form `files[${index}]`.
	 * If you wish to alter the placeholder snowflake, you must provide this property in the same form (`files[${placeholder}]`)
	 */
	key?: string;
	/**
	 * The name of the file. This is the actual filename that will be used when uploading to Discord.
	 * This is also the name you'll use to reference the file with attachment:// URLs.
	 *
	 * @example 'image.png', 'document.pdf', 'SPOILER_secret.jpg'
	 */
	name: string;
}


Review comment (security):
```suggestion
	 * @example 'image.png'
	 * @example 'document.pdf'
	 * @example 'SPOILER_secret.jpeg'
```

Code after:
import type { Buffer } from 'node:buffer';

/**
 * Represents a file to be added to a request with multipart/form-data encoding
 */
export interface RawFile {
	/**
	 * Content-Type of the file.
	 * If not provided, it will be inferred from the file data when possible
	 *
	 * @example 'image/png'
	 * @example 'application/pdf'
	 */
	contentType?: string;
	/**
	 * The actual data for the file
	 */
	data: Buffer | Uint8Array | boolean | number | string;
	/**
	 * An explicit key to use for the formdata field for this file.
	 * When not provided, the index of the file in the files array is used in the form `files[${index}]`.
	 * If you wish to alter the placeholder snowflake, you must provide this property in the same form (`files[${placeholder}]`)
	 */
	key?: string;
	/**
	 * The name of the file. This is the actual filename that will be used when uploading to Discord.
	 * This is also the name you'll use to reference the file with attachment:// URLs.
	 *
	 * @example 'image.png'
	 * @example 'document.pdf'
	 * @example 'SPOILER_secret.jpeg'
	 */
	name: string;
}
Repository: discordjs/discord.js
File: packages/util/src/encodables.ts (TypeScript)

Code before:
 * Objects implementing this interface can separate JSON body data from binary file data,
 * which is necessary for multipart/form-data requests.
 *
 * @typeParam BodyValue - The JSON body type
 */
export interface FileBodyEncodable<BodyValue> extends JSONEncodable<BodyValue> {
	/**
	 * Transforms this object to its file body format, separating the JSON body from file attachments.
	 *
	 * This method should generate the JSON extract file data into the RawFile format.
	 */
	toFileBody(): FileBodyEncodableResult<BodyValue>;
}

/**
 * Indicates if an object is file body encodable or not.
 *
 * @param maybeEncodable - The object to check against
 */
export function isFileBodyEncodable(maybeEncodable: unknown): maybeEncodable is FileBodyEncodable<unknown> {
	return (
		maybeEncodable !== null &&
		typeof maybeEncodable === 'object' &&
		'toJSON' in maybeEncodable &&
		'toFileBody' in maybeEncodable
	);
}


Review comment (suggestion):
```suggestion
	return isJSONEncodable(maybeEncodable) && 'toFileBody' in maybeEncodable;
```

Code after:
import type { RawFile } from './RawFile.js';

/**
 * Represents an object capable of representing itself as a JSON object
 *
 * @typeParam Value - The JSON type corresponding to {@link JSONEncodable.toJSON} outputs.
 */
export interface JSONEncodable<Value> {
	/**
	 * Transforms this object to its JSON format
	 */
	toJSON(): Value;
}

/**
 * Indicates if an object is encodable or not.
 *
 * @param maybeEncodable - The object to check against
 */
export function isJSONEncodable(maybeEncodable: unknown): maybeEncodable is JSONEncodable<unknown> {
	return maybeEncodable !== null && typeof maybeEncodable === 'object' && 'toJSON' in maybeEncodable;
}

/**
 * Result of encoding an object that includes file attachments
 *
 * @typeParam BodyValue - The JSON body type
 */
export interface FileBodyEncodableResult<BodyValue> {
	/**
	 * The JSON body to send with the request
	 */
	body: BodyValue;
	/**
	 * The files to attach to the request
	 */
	files: RawFile[];
}

/**
 * Represents an object capable of representing itself as a request body with file attachments.
 * Objects implementing this interface can separate JSON body data from binary file data,
 * which is necessary for multipart/form-data requests.
 *
 * @typeParam BodyValue - The JSON body type
 */
export interface FileBodyEncodable<BodyValue> {
	/**
	 * Transforms this object to its file body format, separating the JSON body from file attachments.
	 */
	toFileBody(): FileBodyEncodableResult<BodyValue>;
}

/**
 * Indicates if an object is file body encodable or not.
 *
 * @param maybeEncodable - The object to check against
 */
export function isFileBodyEncodable(maybeEncodable: unknown): maybeEncodable is FileBodyEncodable<unknown> {
	return maybeEncodable !== null && typeof maybeEncodable === 'object' && 'toFileBody' in maybeEncodable;
}