ronantakizawa/codereview-bench huggingface dataset:
Repository: prettier/prettier
File: src/language-js/parentheses/needs-parentheses.js (JavaScript)

Code before:
    // fallthrough
    case "TSConditionalType":
    case "TSConstructorType":
    case "ConditionalTypeAnnotation":
      if (
        key === "extendsType" &&
        isConditionalType(node) &&
        parent.type === node.type
      ) {
        return true;
      }

      if (
        key === "constraint" &&
        isConditionalType(node) &&
        parent.type === "TSTypeParameter"
      ) {
        return true;
      }

      if (
        key === "typeAnnotation" &&
        isConditionalType(node) &&
        parent.type === "TypeAnnotation" &&
        path.grandparent.type === "TypeParameter" &&
        path.grandparent.bound === parent
      ) {
        return true;
      }

      if (key === "checkType" && isConditionalType(parent)) {
        return true;
      }

      if (key === "extendsType" && parent.type === "TSConditionalType") {
        let { typeAnnotation } = node.returnType || node.typeAnnotation;

        if (
          typeAnnotation.type === "TSTypePredicate" &&
          typeAnnotation.typeAnnotation
        ) {
          typeAnnotation = typeAnnotation.typeAnnotation.typeAnnotation;
        }

        if (
          typeAnnotation.type === "TSInferType" &&
          typeAnnotation.typeParameter.constraint
        ) {
          return true;
        }
      }

Review comment (bug):
I'm not familiar with this property, but can't be wrong to be more strict.


```suggestion
        path.grandparent.bound === parent &&
        path.grandparent.usesExtendsBound
```

Code after:
    // fallthrough
    case "TSConditionalType":
    case "TSConstructorType":
    case "ConditionalTypeAnnotation":
      if (
        key === "extendsType" &&
        isConditionalType(node) &&
        parent.type === node.type
      ) {
        return true;
      }

      // `const foo = <Foo extends (Bar extends Baz ? A : B)>() => true;`
      //                            ^^^^^^^^^^^^^^^^^^^^^^^
      if (
        (key === "constraint" &&
          node.type === "TSConditionalType" &&
          parent.type === "TSTypeParameter") ||
        (key === "typeAnnotation" &&
          node.type === "ConditionalTypeAnnotation" &&
          parent.type === "TypeAnnotation" &&
          path.grandparent.type === "TypeParameter" &&
          path.grandparent.bound === parent &&
          path.grandparent.usesExtendsBound)
      ) {
        return true;
      }

      if (key === "checkType" && isConditionalType(parent)) {
        return true;
      }

      if (key === "extendsType" && parent.type === "TSConditionalType") {
        let { typeAnnotation } = node.returnType || node.typeAnnotation;

        if (
          typeAnnotation.type === "TSTypePredicate" &&
          typeAnnotation.typeAnnotation
        ) {
          typeAnnotation = typeAnnotation.typeAnnotation.typeAnnotation;
        }

        if (
          typeAnnotation.type === "TSInferType" &&
          typeAnnotation.typeParameter.constraint
        ) {
          return true;
        }
      }

    // fallthrough
Repository: prettier/prettier
File: src/language-js/parentheses/needs-parentheses.js (JavaScript)

Code before:
      ) {
        return true;
      }
    // fallthrough
    case "TSConditionalType":
    case "TSConstructorType":
    case "ConditionalTypeAnnotation":
      if (
        key === "extendsType" &&
        isConditionalType(node) &&
        parent.type === node.type
      ) {
        return true;
      }

      if (
        key === "constraint" &&
        isConditionalType(node) &&
        parent.type === "TSTypeParameter"
      ) {
        return true;
      }

      if (
        key === "typeAnnotation" &&
        isConditionalType(node) &&
        parent.type === "TypeAnnotation" &&
        path.grandparent.type === "TypeParameter" &&
        path.grandparent.bound === parent
      ) {
        return true;
      }

      if (key === "checkType" && isConditionalType(parent)) {
        return true;
      }

      if (key === "extendsType" && parent.type === "TSConditionalType") {
        let { typeAnnotation } = node.returnType || node.typeAnnotation;

        if (
          typeAnnotation.type === "TSTypePredicate" &&
          typeAnnotation.typeAnnotation
        ) {
          typeAnnotation = typeAnnotation.typeAnnotation.typeAnnotation;
        }

        if (
          typeAnnotation.type === "TSInferType" &&
          typeAnnotation.typeParameter.constraint
        ) {

Review comment (suggestion):
```suggestion
        node.type === "ConditionalTypeAnnotation" &&
```

Code after:
      ) {
        return true;
      }
    // fallthrough
    case "TSConditionalType":
    case "TSConstructorType":
    case "ConditionalTypeAnnotation":
      if (
        key === "extendsType" &&
        isConditionalType(node) &&
        parent.type === node.type
      ) {
        return true;
      }

      // `const foo = <Foo extends (Bar extends Baz ? A : B)>() => true;`
      //                            ^^^^^^^^^^^^^^^^^^^^^^^
      if (
        (key === "constraint" &&
          node.type === "TSConditionalType" &&
          parent.type === "TSTypeParameter") ||
        (key === "typeAnnotation" &&
          node.type === "ConditionalTypeAnnotation" &&
          parent.type === "TypeAnnotation" &&
          path.grandparent.type === "TypeParameter" &&
          path.grandparent.bound === parent &&
          path.grandparent.usesExtendsBound)
      ) {
        return true;
      }

      if (key === "checkType" && isConditionalType(parent)) {
        return true;
      }

      if (key === "extendsType" && parent.type === "TSConditionalType") {
        let { typeAnnotation } = node.returnType || node.typeAnnotation;

        if (
          typeAnnotation.type === "TSTypePredicate" &&
          typeAnnotation.typeAnnotation
        ) {
          typeAnnotation = typeAnnotation.typeAnnotation.typeAnnotation;
        }

        if (
          typeAnnotation.type === "TSInferType" &&
          typeAnnotation.typeParameter.constraint
        ) {
          return true;
        }
Repository: prettier/prettier
File: src/language-js/print/miscellaneous.js (JavaScript)

Code before:
@returns {Doc}
*/
function printDanglingCommentsInList(path, options, filter) {
  const { node } = path;

  return hasComment(node, CommentCheckFlags.Dangling, filter)
    ? [
        indent([softline, printDanglingComments(path, options, { filter })]),
        hasComment(
          node,
          CommentCheckFlags.Dangling | CommentCheckFlags.Line,
          filter,
        )
          ? hardline
          : softline,
      ]
    : "";
}

export {
  adjustClause,
  printAbstractToken,
  printDanglingCommentsInList,
  printDeclareToken,
  printDefiniteToken,
  printIfOrWhileCondition as printDoWhileStatementCondition,
  printIfOrWhileCondition as printIfStatementCondition,
  printOptionalToken,
  printTypeScriptAccessibilityToken,
  printIfOrWhileCondition as printWhileStatementCondition,
};


Review comment (suggestion):
```suggestion
function printTrailingCommaIfNeeded(options, level = "es5") {
```
?

A bit more verbose, so not 100% about that.

Code after:
/**
@param {(comment: Comment) => boolean} [filter]
@returns {Doc}
*/
function printDanglingCommentsInList(path, options, filter) {
  const { node } = path;

  return hasComment(node, CommentCheckFlags.Dangling, filter)
    ? [
        indent([softline, printDanglingComments(path, options, { filter })]),
        hasComment(
          node,
          CommentCheckFlags.Dangling | CommentCheckFlags.Line,
          filter,
        )
          ? hardline
          : softline,
      ]
    : "";
}

/**
 * @param {("es5" | "all")} [level]
 * @returns {Doc}
 */
function printTrailingComma(options, level = "es5") {
  return shouldPrintTrailingComma(options, level) ? ifBreak(",") : "";
}

export {
  adjustClause,
  printAbstractToken,
  printDanglingCommentsInList,
  printDeclareToken,
  printDefiniteToken,
  printIfOrWhileCondition as printDoWhileStatementCondition,
  printIfOrWhileCondition as printIfStatementCondition,
  printOptionalToken,
  printTrailingComma,
  printTypeScriptAccessibilityToken,
  printIfOrWhileCondition as printWhileStatementCondition,
};
Repository: prisma/prisma
File: packages/adapter-mssql/src/connection-string.ts (TypeScript)

Code before:
      const tenantId = extractOption(paramParts, 'tenantId')
      if (clientId && clientSecret) {
        return {
          type: 'azure-active-directory-service-principal-secret',
          options: {
            clientId,
            clientSecret,
            tenantId: tenantId || '',
          },
        }
      } else {
        throw new Error(
          `Invalid authentication, ActiveDirectoryServicePrincipal requires userName (clientId), password (clientSecret)`,
        )
      }
    }
  }
  return undefined
}

/**
 * Return the value for the given option key, if present
 * @param paramParts
 * @param option
 */
function extractOption(paramParts: string[], option: string): string | undefined {
  const kv = paramParts.find((part) => part.startsWith(`${option}=`))?.split('=', 2)
  return kv && kv[1]?.trim()
}


Review comment (refactor):
not a blocker for this PR, but. I don't like that parameters are searched and parsed in multiple places, it could be worth first extracting all key-value pairs into a `Record<string, string>` first and then extract the config from a record

Code after:
    }
    case 'ActiveDirectoryManagedIdentity':
    case 'ActiveDirectoryMSI': {
      const clientId = firstKey(parameters, 'clientId')
      const msiEndpoint = firstKey(parameters, 'msiEndpoint')
      const msiSecret = firstKey(parameters, 'msiSecret')
      if (!msiEndpoint || !msiSecret) {
        throw new Error(`Invalid authentication, ActiveDirectoryManagedIdentity requires msiEndpoint, msiSecret`)
      }
      return {
        type: 'azure-active-directory-msi-app-service',
        options: {
          clientId: clientId || undefined,
          // @ts-expect-error TODO: tedious typings don't define msiEndpoint and msiSecret -- needs to be fixed upstream
          msiEndpoint,
          msiSecret,
        },
      }
    }
    case 'ActiveDirectoryServicePrincipal': {
      const clientId = firstKey(parameters, 'userName')
      const clientSecret = firstKey(parameters, 'password')
      const tenantId = firstKey(parameters, 'tenantId')
      if (clientId && clientSecret) {
        return {
          type: 'azure-active-directory-service-principal-secret',
          options: {
            clientId,
            clientSecret,
            tenantId: tenantId || '',
          },
        }
      } else {
        throw new Error(
          `Invalid authentication, ActiveDirectoryServicePrincipal requires userName (clientId), password (clientSecret)`,
        )
      }
    }
  }
  return undefined
}

/**
 * Return the value of the first key found in the parameters object
 * @param parameters
 * @param keys
 */
function firstKey(parameters: Record<string, string>, ...keys: string[]): string | null {
  for (const key of keys) {
    if (key in parameters) {
      return parameters[key]
Repository: prisma/prisma
File: packages/adapter-pg/src/conversion.ts (TypeScript)

Code before:
  // Parse timezone offset if present (in minutes)
  let offsetMinutes = 0
  if (timezone) {
    const tzMatch = timezone.match(/^([+-])(\d{2})(?::?(\d{2}))?$/)
    if (tzMatch) {
      const [, sign, hours, minutes = '0'] = tzMatch
      offsetMinutes = (parseInt(hours, 10) * 60 + parseInt(minutes, 10)) * (sign === '+' ? 1 : -1)
    }
  }

  // Use a safe year (2000) to avoid 2-digit year interpretation, then set the actual year
  const date = new Date(
    Date.UTC(
      2000,
      parseInt(month, 10) - 1,
      parseInt(day, 10),
      parseInt(hour, 10),
      parseInt(minute, 10),
      parseInt(second, 10),
      parseInt(ms, 10),
    ),
  )
  date.setUTCFullYear(parseInt(year, 10))
  date.setUTCMinutes(date.getUTCMinutes() - offsetMinutes)

  return date.toISOString().replace(/(\.000)?Z$/, '+00:00')
}

/*
 * TIME, TIMETZ, TIME_ARRAY - converts value (or value elements) to a string in the format HH:mm:ss.f
 */

function normalize_time(time: string): string {
  return time
}

function normalize_timez(time: string): string {
  // Although it might be controversial, UTC is assumed in consistency with the behavior of rust postgres driver
  // in quaint. See quaint/src/connector/postgres/conversion.rs
  return time.replace(/[+-]\d{2}(:\d{2})?$/, '')
}

/******************/
/* Money handling */
/******************/

function normalize_money(money: string): string {
  return money.slice(1)
}

/******************/

Review comment (suggestion):
I think things might work if we replace all of this with just `return time.replace(/[+-]\d{2}(:\d{2})?$/`

Code after:
/**
 * We hand off JSON handling entirely to engines, so we keep it
 * stringified here. This function needs to exist as otherwise
 * the default type parser attempts to deserialise it.
 */
function toJson(json: string): string {
  return json
}

/************************/
/* Binary data handling */
/************************/

/**
 * TODO:
 * 1. Check if using base64 would be more efficient than this encoding.
 * 2. Consider the possibility of eliminating re-encoding altogether
 *    and passing bytea hex format to the engine if that can be aligned
 *    with other adapters of the same database provider.
 */
function encodeBuffer(buffer: Buffer) {
  return Array.from(new Uint8Array(buffer))
}

/*
 * BYTEA - arbitrary raw binary strings
 */

const parsePgBytes = getTypeParser(ScalarColumnType.BYTEA) as (_: string) => Buffer

/*
 * BYTEA_ARRAY - arrays of arbitrary raw binary strings
 */

const parseBytesArray = getTypeParser(ArrayColumnType.BYTEA_ARRAY) as (_: string) => Buffer[]

function normalizeByteaArray(serializedBytesArray) {
  const buffers = parseBytesArray(serializedBytesArray)
  return buffers.map((buf) => (buf ? encodeBuffer(buf) : null))
}

/**
 * Convert bytes to a JSON-encodable representation since we can't
 * currently send a parsed Buffer or ArrayBuffer across JS to Rust
 * boundary.
 */
function convertBytes(serializedBytes: string): number[] {
  const buffer = parsePgBytes(serializedBytes)
  return encodeBuffer(buffer)
}
Repository: prisma/prisma
File: packages/client-generator-ts/src/utils/wasm.ts (TypeScript)

Code before:
}

function readSourceFile(sourceFile: string): Buffer {
  const bundledLocation = path.join(__dirname, sourceFile)
  const sourceLocation = path.join(__dirname, '..', '..', '..', 'cli', 'build', sourceFile)

  if (fs.existsSync(bundledLocation)) {
    debug('We are in the bundled Prisma CLI')
    // The artifact exists in __dirname, this means we are in the bundled Prisma CLI
    return fs.readFileSync(bundledLocation)
  } else if (fs.existsSync(sourceLocation)) {
    debug('We are in a dev/test environment')
    // The artifact exists in the source location, this means we are in the development environment or tests
    return fs.readFileSync(sourceLocation)
  } else {
    throw new Error(`Could not find ${sourceFile} in ${bundledLocation} or ${sourceLocation}`)
  }
}

export function buildWasmFileMap({ activeProvider, runtimeName }: BuildWasmFileMapOptions): FileMap {
  const fileMap: FileMap = {}
  debug('buildWasmFileMap with', { runtimeName })

  const component = 'compiler'
  if (!usesEdgeWasmRuntime(runtimeName)) {
    debug('Skipping component', component, 'for runtime', runtimeName)
    return fileMap
  }

  const fileNameBase = `query_compiler_bg.${activeProvider}` as const

  const files = {
    [`query_compiler_bg.wasm`]: `${fileNameBase}.wasm`,
    [`query_compiler_bg.js`]: `${fileNameBase}.mjs`,
  }

  for (const [targetFile, sourceFile] of Object.entries(files)) {
    fileMap[targetFile] = readSourceFile(sourceFile)
  }

  return fileMap
}


Review comment (bug):
```suggestion
    debug('Skipping query compiler for runtime', runtimeName)
```

Code after:
}

function readSourceFile(sourceFile: string): Buffer {
  const bundledLocation = path.join(__dirname, sourceFile)
  const sourceLocation = path.join(__dirname, '..', '..', '..', 'cli', 'build', sourceFile)

  if (fs.existsSync(bundledLocation)) {
    debug('We are in the bundled Prisma CLI')
    // The artifact exists in __dirname, this means we are in the bundled Prisma CLI
    return fs.readFileSync(bundledLocation)
  } else if (fs.existsSync(sourceLocation)) {
    debug('We are in a dev/test environment')
    // The artifact exists in the source location, this means we are in the development environment or tests
    return fs.readFileSync(sourceLocation)
  } else {
    throw new Error(`Could not find ${sourceFile} in ${bundledLocation} or ${sourceLocation}`)
  }
}

export function buildWasmFileMap({ activeProvider, runtimeName }: BuildWasmFileMapOptions): FileMap {
  const fileMap: FileMap = {}
  debug('buildWasmFileMap with', { runtimeName })

  if (!usesEdgeWasmRuntime(runtimeName)) {
    debug('Skipping component compiler for runtime', runtimeName)
    return fileMap
  }

  const fileNameBase = `query_compiler_bg.${activeProvider}` as const

  const files = {
    [`query_compiler_bg.wasm`]: `${fileNameBase}.wasm`,
    [`query_compiler_bg.js`]: `${fileNameBase}.mjs`,
  }

  for (const [targetFile, sourceFile] of Object.entries(files)) {
    fileMap[targetFile] = readSourceFile(sourceFile)
  }

  return fileMap
}
Repository: prisma/prisma
File: packages/client/tests/e2e/enum-import-in-edge/_steps.ts (TypeScript)

Code before:
    throw new Error('Wrangler stdout closed before reporting readiness')
  } catch (error) {
    if (controller.signal.aborted) {
      throw controller.signal.reason as Error
    }

    throw error
  } finally {
    clearTimeout(timeoutId)
    rl.close()
    stdout.resume()
  }
}

async function stopProcess(processPromise: ProcessPromise) {
  try {
    await processPromise.kill('SIGINT')
  } catch {
    // ignore – process might have already exited
  }

  try {
    await processPromise
  } catch {
    // swallow to keep cleanup best-effort
  }
}


Review comment (suggestion):
I'd probably just add `.catch(() => {})` on the callsite, that way the error handling logic is in one function instead of being scattered and we don't want to handle any stopProcess errors anyway

Code after:
import { createInterface } from 'node:readline/promises'
import timers from 'node:timers/promises'

import type { ProcessPromise } from 'zx'
import { $ } from 'zx'

import { executeSteps } from '../_utils/executeSteps'
import { retry } from '../_utils/retry'

void executeSteps({
  setup: async () => {
    await $`pnpm install`
    await $`pnpm prisma generate`
  },
  test: async () => {
    const { stdout } = await retry(async () => {
      const wranglerProcess = $`pnpm wrangler dev --ip 127.0.0.1 --port 8787 src/index.ts`.nothrow()

      try {
        await waitForWranglerReady(wranglerProcess)

        await timers.setTimeout(1000)

        return await $`curl http://localhost:8787/ -s`
      } finally {
        await stopProcess(wranglerProcess).catch(() => {})
      }
    }, 3)

    const expected =
      '{"Role":{"USER":"USER","ADMIN":"ADMIN"},"ModelName":{"User":"User","Post":"Post","Profile":"Profile"}}'

    if (!stdout.includes(expected)) {
      throw new Error('Expected to fetch the enum values')
    } else {
      console.log('Success!')
    }
  },
  finish: async () => {
    await $`echo "done"`
  },
})

async function waitForWranglerReady(processPromise: ProcessPromise) {
  const stdout = processPromise.stdout

  if (!stdout) {
    throw new Error('Wrangler stdout is not available; cannot detect readiness')
  }

  const controller = new AbortController()
  const timeoutId = setTimeout(() => {
    controller.abort(new Error('Timed out waiting for wrangler to report readiness'))
  }, 30_000)

  const rl = createInterface({ input: stdout, crlfDelay: Infinity })

  try {
    controller.signal.throwIfAborted()

    for await (const line of rl) {
      controller.signal.throwIfAborted()

      if (line.includes('Ready')) {
        return
      }
    }

    throw new Error('Wrangler stdout closed before reporting readiness')
  } catch (error) {
    if (controller.signal.aborted) {
      throw controller.signal.reason as Error
    }

    throw error
  } finally {
    clearTimeout(timeoutId)
    rl.close()
    stdout.resume()
  }
}

async function stopProcess(processPromise: ProcessPromise) {
  await processPromise.kill('SIGINT')
  await processPromise
}
Repository: prisma/prisma
File: packages/client-engine-runtime/src/interpreter/data-mapper.ts (TypeScript)

Code before:
import { Decimal } from '@prisma/client-runtime-utils'

import { FieldScalarType, FieldType, ResultNode } from '../query-plan'
import { UserFacingError } from '../user-facing-error'
import { assertNever, safeJsonStringify } from '../utils'
import { PrismaObject, Value } from './scope'

export class DataMapperError extends UserFacingError {
  name = 'DataMapperError'

  constructor(message: string, options?: { cause?: unknown }) {
    super(message, 'P2006', options)
  }
}

export function applyDataMap(data: Value, structure: ResultNode, enums: Record<string, Record<string, string>>): Value {
  switch (structure.type) {
    case 'affectedRows':
      if (typeof data !== 'number') {
        throw new DataMapperError(`Expected an affected rows count, got: ${typeof data} (${data})`)
      }
      return { count: data }

    case 'object':
      return mapArrayOrObject(data, structure.fields, enums, structure.skipNulls)

    case 'field':
      return mapValue(data, '<result>', structure.fieldType, enums)

    default:
      assertNever(structure, `Invalid data mapping type: '${(structure as ResultNode).type}'`)
  }
}

function mapArrayOrObject(
  data: Value,
  fields: Record<string, ResultNode>,

Review comment (suggestion):
```suggestion
    super(message, 'P2023', options)
```

Code after:
import { Decimal } from '@prisma/client-runtime-utils'

import { FieldScalarType, FieldType, ResultNode } from '../query-plan'
import { UserFacingError } from '../user-facing-error'
import { assertNever, safeJsonStringify } from '../utils'
import { PrismaObject, Value } from './scope'

export class DataMapperError extends UserFacingError {
  name = 'DataMapperError'

  constructor(message: string, options?: { cause?: unknown }) {
    super(message, 'P2023', options)
  }
}

export function applyDataMap(data: Value, structure: ResultNode, enums: Record<string, Record<string, string>>): Value {
  switch (structure.type) {
    case 'affectedRows':
      if (typeof data !== 'number') {
        throw new DataMapperError(`Expected an affected rows count, got: ${typeof data} (${data})`)
      }
      return { count: data }

    case 'object':
      return mapArrayOrObject(data, structure.fields, enums, structure.skipNulls)

    case 'field':
      return mapValue(data, '<result>', structure.fieldType, enums)

    default:
      assertNever(structure, `Invalid data mapping type: '${(structure as ResultNode).type}'`)
  }
}

function mapArrayOrObject(
  data: Value,
  fields: Record<string, ResultNode>,