ronantakizawa/codereview-bench huggingface dataset:
Repository: honojs/hono
File: src/utils/body.ts (TypeScript)

Code before:

async function parseFormData<T extends BodyData = BodyData>(
  request: HonoRequest | Request,
  options: ParseBodyOptions
): Promise<T> {
  const formData = await (request as Request).formData()

  if (formData) {
    return convertFormDataToBodyData<T>(formData, options)
  }

  return {} as T
}

function convertFormDataToBodyData<T extends BodyData = BodyData>(
  formData: FormData,
  options: ParseBodyOptions
): T {
  const form: BodyData = {}

  formData.forEach((value, key) => {
    const shouldParseAllValues = options.all || key.endsWith('[]')

    if (!shouldParseAllValues) {
      if (options.dot && key.includes('.')) {
        setNestedValue(form, key.split('.'), value)
      } else {
        form[key] = value
      }
    } else {
      handleParsingAllValues(form, key, value, options.dot)
    }
  })

  return form as T
}

const handleParsingAllValues = (
  form: BodyData,
  key: string,
  value: FormDataEntryValue,
  dot?: boolean
): void => {
  const formKey = form[key]

  if (formKey && Array.isArray(formKey)) {
    ;(form[key] as (string | File)[]).push(value)
  } else if (formKey) {
    form[key] = [formKey, value]
  } else {
    if (dot) {

Review comment (suggestion):
Maybe don't split the keys here to make the API look similar to `handleParsingAllValues`. You can do it inside the func too.

Code after:
  const contentType = headers.get('Content-Type')

  if (
    (contentType !== null && contentType.startsWith('multipart/form-data')) ||
    (contentType !== null && contentType.startsWith('application/x-www-form-urlencoded'))
  ) {
    return parseFormData<T>(request, { all, dot })
  }

  return {} as T
}

/**
 * Parses form data from a request.
 *
 * @template T - The type of the parsed body data.
 * @param {HonoRequest | Request} request - The request object containing form data.
 * @param {ParseBodyOptions} options - Options for parsing the form data.
 * @returns {Promise<T>} The parsed body data.
 */
async function parseFormData<T extends BodyData = BodyData>(
  request: HonoRequest | Request,
  options: ParseBodyOptions
): Promise<T> {
  const formData = await (request as Request).formData()

  if (formData) {
    return convertFormDataToBodyData<T>(formData, options)
  }

  return {} as T
}

/**
 * Converts form data to body data based on the provided options.
 *
 * @template T - The type of the parsed body data.
 * @param {FormData} formData - The form data to convert.
 * @param {ParseBodyOptions} options - Options for parsing the form data.
 * @returns {T} The converted body data.
 */
function convertFormDataToBodyData<T extends BodyData = BodyData>(
  formData: FormData,
  options: ParseBodyOptions
): T {
  const form: BodyData = Object.create(null)

  formData.forEach((value, key) => {
    const shouldParseAllValues = options.all || key.endsWith('[]')

    if (shouldParseAllValues) {
Repository: honojs/hono
File: src/utils/body.ts (TypeScript)

Code before:
import { HonoRequest } from '../request'

export type BodyData = Record<string, string | object | File | (string | File)[]>
export type ParseBodyOptions = {
  /**
   * Determines whether all fields with multiple values should be parsed as arrays.
   * @default false
   * @example
   * const data = new FormData()
   * data.append('file', 'aaa')
   * data.append('file', 'bbb')
   * data.append('message', 'hello')
   *
   * If all is false:
   * parseBody should return { file: 'bbb', message: 'hello' }
   *
   * If all is true:
   * parseBody should return { file: ['aaa', 'bbb'], message: 'hello' }
   */
  all?: boolean
  /**
   * Determines whether all fields with dot notation should be parsed as nested objects.
   * @default false
   * @example
   * const data = new FormData()
   * data.append('obj.key1', 'value1')
   * data.append('obj.key2', 'value2')
   *

Review comment (suggestion):
In place of an `object` type, we should now have a nested generic type that would be more helpful

Code after:
import { HonoRequest } from '../request'

type BodyDataValue = string | File | (string | File)[] | { [key: string]: BodyDataValue }
export type BodyData = Record<string, BodyDataValue>
export type ParseBodyOptions = {
  /**
   * Determines whether all fields with multiple values should be parsed as arrays.
   * @default false
   * @example
   * const data = new FormData()
   * data.append('file', 'aaa')
   * data.append('file', 'bbb')
   * data.append('message', 'hello')
   *
   * If all is false:
   * parseBody should return { file: 'bbb', message: 'hello' }
   *
   * If all is true:
   * parseBody should return { file: ['aaa', 'bbb'], message: 'hello' }
   */
  all: boolean
  /**
   * Determines whether all fields with dot notation should be parsed as nested objects.
   * @default false
   * @example
   * const data = new FormData()
   * data.append('obj.key1', 'value1')
   * data.append('obj.key2', 'value2')
Repository: honojs/hono
File: src/utils/body.ts (TypeScript)

Code before:

  formData.forEach((value, key) => {
    const shouldParseAllValues = options.all || key.endsWith('[]')

    if (!shouldParseAllValues) {
      handleNestedValues(form, key, value, options.dot)
    } else {
      handleParsingAllValues(form, key, value, options.dot)
    }
  })

  return form as T
}

const handleParsingAllValues = (
  form: BodyData,
  key: string,
  value: FormDataEntryValue,
  dot?: boolean
): void => {
  if (dot && key.includes('.')) {
    handleNestedValues(form, key, value, dot, true)
  } else {
    if (form[key] !== undefined) {
      if (Array.isArray(form[key])) {
        ;(form[key] as (string | File)[]).push(value)
      } else {
        form[key] = [form[key] as string | File, value]
      }
    } else {
      form[key] = value
    }
  }
}

const handleNestedValues = (
  form: BodyData,
  key: string,
  value: FormDataEntryValue,
  dot?: boolean,
  parseAllValues?: boolean
): void => {
  if (dot && key.includes('.')) {
    let nestedForm = form
    const keys = key.split('.')

    keys.forEach((key, index) => {
      if (index === keys.length - 1) {
        if (parseAllValues) {
          if (nestedForm[key] !== undefined) {
            if (Array.isArray(nestedForm[key])) {

Review comment (suggestion):
I have updated this loop

```ts
formData.forEach((value, key) => {
  const shouldParseAllValues = options.all || key.endsWith('[]')
  const shouldParseDotValues = options.dot && key.includes('.')

  if (shouldParseDotValues) {
    handleParsingNestedValues(form, key, value, shouldParseAllValues)
  } else if (shouldParseAllValues) {
    handleParsingAllValues(form, key, value)
  } else {
    form[key] = value
  }
})
```

Code after:
  options: ParseBodyOptions
): Promise<T> {
  const formData = await (request as Request).formData()

  if (formData) {
    return convertFormDataToBodyData<T>(formData, options)
  }

  return {} as T
}

/**
 * Converts form data to body data based on the provided options.
 *
 * @template T - The type of the parsed body data.
 * @param {FormData} formData - The form data to convert.
 * @param {ParseBodyOptions} options - Options for parsing the form data.
 * @returns {T} The converted body data.
 */
function convertFormDataToBodyData<T extends BodyData = BodyData>(
  formData: FormData,
  options: ParseBodyOptions
): T {
  const form: BodyData = Object.create(null)

  formData.forEach((value, key) => {
    const shouldParseAllValues = options.all || key.endsWith('[]')

    if (shouldParseAllValues) {
      handleParsingAllValues(form, key, value)
    } else {
      form[key] = value
    }
  })

  if (options.dot) {
    const nestedForm: BodyData = Object.create(null)

    Object.entries(form).forEach(([key, value]) => {
      const shouldParseDotValues = key.includes('.')

      if (shouldParseDotValues) {
        handleParsingNestedValues(nestedForm, key, value)
      } else {
        nestedForm[key] = value
      }
    })

    return nestedForm as T
  }
Repository: honojs/hono
File: src/utils/body.ts (TypeScript)

Code before:
/**
 * Handles parsing all values for a given key, supporting multiple values as arrays.
 *
 * @param {BodyData} form - The form data object.
 * @param {string} key - The key to parse.
 * @param {FormDataEntryValue} value - The value to assign.
 */
const handleParsingAllValues = (form: BodyData, key: string, value: FormDataEntryValue): void => {
  if (form[key] !== undefined) {
    if (Array.isArray(form[key])) {
      ;(form[key] as (string | File)[]).push(value)
    } else {
      form[key] = [form[key] as string | File, value]
    }
  } else {
    form[key] = value
  }
}

/**
 * Handles parsing nested values using dot notation keys.
 *
 * @param {BodyData} form - The form data object.
 * @param {string} key - The dot notation key.
 * @param {FormDataEntryValue} value - The value to assign.
 * @param {boolean} parseAllValues - Whether to parse all values as arrays.
 */
const handleParsingNestedValues = (form: BodyData, key: string, value: BodyDataValue): void => {
  let nestedForm = form
  const keys = key.split('.')

  keys.forEach((key, index) => {
    if (index === keys.length - 1) {
      nestedForm[key] = value
    } else {
      if (
        !nestedForm[key] ||
        typeof nestedForm[key] !== 'object' ||
        Array.isArray(nestedForm[key]) ||
        nestedForm[key] instanceof File
      ) {
        nestedForm[key] = Object.create(null)
      }
      nestedForm = nestedForm[key] as BodyData
    }
  })
}


Review comment (refactor):
Sorry for the nit-picking but now you can remove this line for `parseAllValues` props from the TSDocs and then do the denofy thing

Code after:
/**
 * Handles parsing all values for a given key, supporting multiple values as arrays.
 *
 * @param {BodyData} form - The form data object.
 * @param {string} key - The key to parse.
 * @param {FormDataEntryValue} value - The value to assign.
 */
const handleParsingAllValues = (form: BodyData, key: string, value: FormDataEntryValue): void => {
  if (form[key] !== undefined) {
    if (Array.isArray(form[key])) {
      ;(form[key] as (string | File)[]).push(value)
    } else {
      form[key] = [form[key] as string | File, value]
    }
  } else {
    form[key] = value
  }
}

/**
 * Handles parsing nested values using dot notation keys.
 *
 * @param {BodyData} form - The form data object.
 * @param {string} key - The dot notation key.
 * @param {BodyDataValue} value - The value to assign.
 */
const handleParsingNestedValues = (form: BodyData, key: string, value: BodyDataValue): void => {
  let nestedForm = form
  const keys = key.split('.')

  keys.forEach((key, index) => {
    if (index === keys.length - 1) {
      nestedForm[key] = value
    } else {
      if (
        !nestedForm[key] ||
        typeof nestedForm[key] !== 'object' ||
        Array.isArray(nestedForm[key]) ||
        nestedForm[key] instanceof File
      ) {
        nestedForm[key] = Object.create(null)
      }
      nestedForm = nestedForm[key] as BodyData
    }
  })
}
Repository: honojs/hono
File: deno_dist/middleware/csrf/index.ts (TypeScript)

Code before:
import type { Context } from '../../context.ts'
import { HTTPException } from '../../http-exception.ts'
import type { MiddlewareHandler } from '../../types.ts'

type IsAllowedOriginHandler = (origin: string, context: Context) => boolean
interface CSRFOptions {
  origin?: string | string[] | IsAllowedOriginHandler
}

const isSafeMethodRe = /^(GET|HEAD)$/
const isRequestedByFormElementRe =
  /^\b(application\/x-www-form-urlencoded|multipart\/form-data|text\/plain)\b/

/**
 * CSRF protection middleware for Hono.
 *
 * @see {@link https://hono.dev/middleware/builtin/csrf}
 *
 * @param {CSRFOptions} [options] - The options for the CSRF protection middleware.
 * @param {string|string[]|IsAllowedOriginHandler} [options.origin] - Specify origins.
 * @returns {MiddlewareHandler} The middleware handler function.
 *
 * @example
 * ```ts
 * const app = new Hono()
 *
 * app.use(csrf())
 *
 * // Specifying origins with using `origin` option
 * // string
 * app.use(csrf({ origin: 'myapp.example.com' }))
 *
 * // string[]
 * app.use(
 *   csrf({
 *     origin: ['myapp.example.com', 'development.myapp.example.com'],
 *   })
 * )
 *
 * // Function
 * // It is strongly recommended that the protocol be verified to ensure a match to `$`.
 * // You should *never* do a forward match.
 * app.use(
 *   '*',
 *   csrf({

Review comment (suggestion):
Writing the following instead of `IsAllowedOriginHandler` would be more helpful.

```ts
(origin: string, context: Context) => boolean
```

Code after:
import type { Context } from '../../context.ts'
import { HTTPException } from '../../http-exception.ts'
import type { MiddlewareHandler } from '../../types.ts'

type IsAllowedOriginHandler = (origin: string, context: Context) => boolean
interface CSRFOptions {
  origin?: string | string[] | IsAllowedOriginHandler
}

const isSafeMethodRe = /^(GET|HEAD)$/
const isRequestedByFormElementRe =
  /^\b(application\/x-www-form-urlencoded|multipart\/form-data|text\/plain)\b/

/**
 * CSRF protection middleware for Hono.
 *
 * @see {@link https://hono.dev/middleware/builtin/csrf}
 *
 * @param {CSRFOptions} [options] - The options for the CSRF protection middleware.
 * @param {string|string[]|(origin: string, context: Context) => boolean} [options.origin] - Specify origins.
 * @returns {MiddlewareHandler} The middleware handler function.
 *
 * @example
 * ```ts
 * const app = new Hono()
 *
 * app.use(csrf())
 *
 * // Specifying origins with using `origin` option
 * // string
 * app.use(csrf({ origin: 'myapp.example.com' }))
 *
 * // string[]
 * app.use(
 *   csrf({
 *     origin: ['myapp.example.com', 'development.myapp.example.com'],
 *   })
 * )
 *
 * // Function
 * // It is strongly recommended that the protocol be verified to ensure a match to `$`.
 * // You should *never* do a forward match.
 * app.use(
 *   '*',
 *   csrf({
Repository: honojs/hono
File: src/utils/body.test.ts (TypeScript)

Code before:
import { parseBody } from './body'
import type { BodyData } from './body'

type RecursiveRecord<K extends string, T> = {
  [key in K]: T | RecursiveRecord<K, T>
}

describe('Parse Body Util', () => {
  const FORM_URL = 'https://localhost/form'
  const SEARCH_URL = 'https://localhost/search'

  const createRequest = (
    url: string,
    method: 'POST',
    body: BodyInit,
    headers?: { [key: string]: string }
  ) => {
    return new Request(url, {
      method,
      body,
      headers,
    })
  }

  it('should parse `multipart/form-data`', async () => {
    const data = new FormData()
    data.append('message', 'hello')

Review comment (suggestion):
This is cool PR!

I think you can just merge the imports in one import like this

```ts
import { parseBody, type BodyData } from './body'
```

I think we need to setup eslint to avoid imports to the same file @usualoma  @yusukebe 


Code after:
import { parseBody, type BodyData } from './body'

type RecursiveRecord<K extends string, T> = {
  [key in K]: T | RecursiveRecord<K, T>
}

describe('Parse Body Util', () => {
  const FORM_URL = 'https://localhost/form'
  const SEARCH_URL = 'https://localhost/search'

  const createRequest = (
    url: string,
    method: 'POST',
    body: BodyInit,
    headers?: { [key: string]: string }
  ) => {
    return new Request(url, {
      method,
      body,
      headers,
    })
  }

  it('should parse `multipart/form-data`', async () => {
    const data = new FormData()
    data.append('message', 'hello')
Repository: honojs/hono
File: src/middleware/ip-restriction/index.test.ts (TypeScript)

Code before:
import { Hono } from '../../hono'
import { Context } from '../../context'
import { HonoRequest } from '../../request'
import type { GetConnInfo } from '../../helper/conninfo'
import { ipRestriction, isMatchForRule } from '.'

describe('ipLimit middleware', () => {
  it('Should limit', async () => {
    const getConnInfo: GetConnInfo = (c) => {
      return {
        remote: {
          address: c.env.ip,
        },
      }
    }
    const app = new Hono<{
      Bindings: {
        ip: string
      }
    }>()
    app.use(
      '/basic',
      ipRestriction(getConnInfo, {
        allowList: ['192.168.1.0', '192.168.2.0/24'],
        denyList: ['192.168.2.10'],
      })
    )
    app.get('/basic', (c) => c.text('Hello World!'))

    app.use(
      '/allow-empty',
      ipRestriction(getConnInfo, {

Review comment (suggestion):
```suggestion
describe('ipRestriction middleware', () => {
```

we should also replace s/ipLimit/ipRestriction/

Code after:
import { Hono } from '../../hono'
import { Context } from '../../context'
import type { AddressType, GetConnInfo } from '../../helper/conninfo'
import { ipRestriction } from '.'
import type { IPRestrictionRule } from '.'

describe('ipRestriction middleware', () => {
  it('Should restrict', async () => {
    const getConnInfo: GetConnInfo = (c) => {
      return {
        remote: {
          address: c.env.ip,
        },
      }
    }
    const app = new Hono<{
      Bindings: {
        ip: string
      }
    }>()
    app.use(
      '/basic',
      ipRestriction(getConnInfo, {
        allowList: ['192.168.1.0', '192.168.2.0/24'],
        denyList: ['192.168.2.10'],
      })
    )
    app.get('/basic', (c) => c.text('Hello World!'))

    app.use(
      '/allow-empty',
      ipRestriction(getConnInfo, {
Repository: honojs/hono
File: src/middleware/ip-restriction/index.ts (TypeScript)

Code before:
export interface IPRestrictRules {
  denyList?: IPRestrictRule[]
  allowList?: IPRestrictRule[]
}

/**
 * IP Limit Middleware
 *
 * @param getIP function to get IP Address
 */
export const ipRestriction = (
  getIP: GetIPAddr,
  { denyList = [], allowList = [] }: IPRestrictRules,
  onError?: (remote: { addr: string; type: AddressType }) => Response | Promise<Response>
): MiddlewareHandler => {
  const denyLength = denyList.length
  const allowLength = allowList.length

  const blockError = (): HTTPException =>
    new HTTPException(403, {
      res: new Response('Unauthorized', {
        status: 403,
      }),
    })

  return createMiddleware(async (c, next) => {
    const connInfo = getIP(c)
    const addr = typeof connInfo === 'string' ? connInfo : connInfo.remote.address
    if (!addr) {
      throw blockError()
    }
    const type =
      (typeof connInfo !== 'string' && connInfo.remote.addressType) || distinctRemoteAddr(addr)

    for (let i = 0; i < denyLength; i++) {
      const isValid = isMatchForRule({ type, addr }, denyList[i])
      if (isValid) {
        if (onError) {
          return onError({ type, addr })
        }
        throw blockError()
      }
    }
    for (let i = 0; i < allowLength; i++) {
      const isValid = isMatchForRule({ type, addr }, allowList[i])
      if (isValid) {
        return await next()
      }
    }

    if (allowLength === 0) {

Review comment (bug):
It is sometimes helpful to be able to display the middleware name when debugging, so we use named functions for functions included in the hono repository.

```suggestion
  return async function ipRestriction(c, next) {
```

Code after:
    type: AddressType
    isIPv4: boolean
    binaryAddr?: bigint
  }): boolean => {
    if (staticRules.has(remote.addr)) {
      return true
    }
    for (const [isIPv4, addr, mask] of cidrRules) {
      if (isIPv4 !== remote.isIPv4) {
        continue
      }
      const remoteAddr = (remote.binaryAddr ||= (
        isIPv4 ? convertIPv4ToBinary : convertIPv6ToBinary
      )(remote.addr))
      if ((remoteAddr & mask) === addr) {
        return true
      }
    }
    for (const rule of functionRules) {
      if (rule({ addr: remote.addr, type: remote.type })) {
        return true
      }
    }
    return false
  }
}

/**
 * Rules for IP Limit Middleware
 */
export interface IPRestrictionRules {
  denyList?: IPRestrictionRule[]
  allowList?: IPRestrictionRule[]
}

/**
 * IP Limit Middleware
 *
 * @param getIP function to get IP Address
 */
export const ipRestriction = (
  getIP: GetIPAddr,
  { denyList = [], allowList = [] }: IPRestrictionRules,
  onError?: (
    remote: { addr: string; type: AddressType },
    c: Context
  ) => Response | Promise<Response>
): MiddlewareHandler => {
  const allowLength = allowList.length

  const denyMatcher = buildMatcher(denyList)
Repository: honojs/hono
File: src/middleware/ip-restriction/index.ts (TypeScript)

Code before:
import { HTTPException } from '../../http-exception'
import {
  convertIPv4ToBinary,
  convertIPv6ToBinary,
  distinctRemoteAddr,
  expandIPv6,
} from '../../utils/ipaddr'

/**
 * Function to get IP Address
 */
type GetIPAddr = GetConnInfo | ((c: Context) => string)

/**
 * ### IPv4 and IPv6
 * - `*` match all
 *
 * ### IPv4
 * - `192.168.2.0` static
 * - `192.168.2.0/24` CIDR Notation
 *
 * ### IPv6
 * - `::1` static
 * - `::1/10` CIDR Notation
 */
export type IPRestrictRule = string | ((addr: { addr: string; type: AddressType }) => boolean)

const IS_CIDR_NOTATION_REGEX = /\/[0-9]{0,3}$/
export const isMatchForRule = (
  remote: {
    addr: string
    type: AddressType
  },
  rule: IPRestrictRule
): boolean => {
  if (rule === '*') {
    // Match all
    return true
  }
  if (typeof rule === 'function') {
    return rule(remote)
  }
  if (IS_CIDR_NOTATION_REGEX.test(rule) && (remote.type === 'IPv4' || remote.type === 'IPv6')) {
    const isIPv4 = remote.type === 'IPv4'

    const splitedRule = rule.split('/')
    // CIDR
    const baseRuleAddr = splitedRule[0]
    if (distinctRemoteAddr(baseRuleAddr) !== remote.type) {
      return false
    }

Review comment (suggestion):
```suggestion
export type IPRestrictionRule = string | ((addr: { addr: string; type: AddressType }) => boolean)
```
IPRestrictRule -> IPRestrictionRule is better.
The name is not common.
https://www.google.com/search?q=IPRestrictRule

Code after:
import { HTTPException } from '../../http-exception'
import {
  convertIPv4ToBinary,
  convertIPv6BinaryToString,
  convertIPv6ToBinary,
  distinctRemoteAddr,
} from '../../utils/ipaddr'

/**
 * Function to get IP Address
 */
type GetIPAddr = GetConnInfo | ((c: Context) => string)

/**
 * ### IPv4 and IPv6
 * - `*` match all
 *
 * ### IPv4
 * - `192.168.2.0` static
 * - `192.168.2.0/24` CIDR Notation
 *
 * ### IPv6
 * - `::1` static
 * - `::1/10` CIDR Notation
 */
type IPRestrictionRuleFunction = (addr: { addr: string; type: AddressType }) => boolean
export type IPRestrictionRule = string | ((addr: { addr: string; type: AddressType }) => boolean)

const IS_CIDR_NOTATION_REGEX = /\/[0-9]{0,3}$/
const buildMatcher = (
  rules: IPRestrictionRule[]
): ((addr: { addr: string; type: AddressType; isIPv4: boolean }) => boolean) => {
  const functionRules: IPRestrictionRuleFunction[] = []
  const staticRules: Set<string> = new Set()
  const cidrRules: [boolean, bigint, bigint][] = []

  for (let rule of rules) {
    if (rule === '*') {
      return () => true
    } else if (typeof rule === 'function') {
      functionRules.push(rule)
    } else {
      if (IS_CIDR_NOTATION_REGEX.test(rule)) {
        const splittedRule = rule.split('/')

        const addrStr = splittedRule[0]
        const type = distinctRemoteAddr(addrStr)
        if (type === undefined) {
          throw new TypeError(`Invalid rule: ${rule}`)
        }