ronantakizawa/codereview-bench huggingface dataset:
Repository: koajs/koa
File: __tests__/application/respond.test.js (JavaScript)

Code before:

        app.use(ctx => {
          ctx.length = 17
          ctx.type = 'json'
          ctx.set('foo', 'bar')
          ctx.res.flushHeaders()
          ctx.body = { hello: 'world' }
        })

        return request(app.callback())
          .get('/')
          .expect('Content-Type', 'application/json; charset=utf-8')
          .expect('Content-Length', '17')
          .expect('foo', 'bar')
          .expect('{"hello":"world"}')
      })
    })
  })

  describe('when an error occurs', () => {
    it('should emit "error" on the app', async () => {
      const app = new Koa()
      let errorCaught = false

      app.on('error', err => {
        errorCaught = true
        assert.strictEqual(err.message, 'test error')
      })

      app.use(ctx => {
        throw new Error('test error')
      })

      await request(app.callback())
        .get('/')
        .expect(500)

      assert.strictEqual(errorCaught, true)
    })

    describe('with an .expose property', () => {
      it('should expose the message', () => {
        const app = new Koa()

        app.use(ctx => {
          const err = new Error('sorry!')
          err.status = 403
          err.expose = true
          throw err
        })


Review comment (suggestion):
we should put the boolean after the assertion so we're sure it's true

Code after:

        app.use(ctx => {
          ctx.length = 17
          ctx.type = 'json'
          ctx.set('foo', 'bar')
          ctx.res.flushHeaders()
          ctx.body = { hello: 'world' }
        })

        return request(app.callback())
          .get('/')
          .expect('Content-Type', 'application/json; charset=utf-8')
          .expect('Content-Length', '17')
          .expect('foo', 'bar')
          .expect('{"hello":"world"}')
      })
    })
  })

  describe('when an error occurs', () => {
    it('should emit "error" on the app', async () => {
      const app = new Koa()
      let errorCaught = false

      app.on('error', err => {
        assert.strictEqual(err.message, 'test error')
        errorCaught = true
      })

      app.use(ctx => {
        throw new Error('test error')
      })

      await request(app.callback())
        .get('/')
        .expect(500)

      assert.strictEqual(errorCaught, true)
    })

    describe('with an .expose property', () => {
      it('should expose the message', () => {
        const app = new Koa()

        app.use(ctx => {
          const err = new Error('sorry!')
          err.status = 403
          err.expose = true
          throw err
        })
Repository: koajs/koa
File: __tests__/response/writable.test.js (JavaScript)

Code before:
      const datas = []

      client.on('data', data => datas.push(data))

      await once(client, 'connect')
      client.write(buf)
      client.write(buf)
      client.end()

      await once(client, 'end')
      client.destroy()

      const responses = Buffer.concat(datas).toString()
      assert.strictEqual(/request 1, writable: true/.test(responses), true)
      assert.strictEqual(/request 2, writable: true/.test(responses), true)

      server.close()
      await once(server, 'close')
    })
  })

  describe('when socket closed before response sent', () => {
    it('should not be writable', async () => {
      const app = new Koa()
      app.use(ctx => {
        assert(!ctx.writable)
      })

      const server = app.listen()
      const port = server.address().port
      const buf = Buffer.from('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n')

      const client = net.connect(port)
      await once(client, 'connect')
      client.write(buf)
      client.destroy()

      server.close()
      await once(server, 'close')
    })
  })

  describe('when response finished', () => {
    it('should not be writable', async () => {
      const app = new Koa()
      app.use(ctx => {
        ctx.res.end()
        assert(!ctx.writable)
      })

      const server = app.listen()

Review comment (suggestion):
should add a boolean to check that this assertion passed

Code after:
      const datas = []

      client.on('data', data => datas.push(data))

      await once(client, 'connect')
      client.write(buf)
      client.write(buf)
      client.end()

      await once(client, 'end')
      client.destroy()

      const responses = Buffer.concat(datas).toString()
      assert.strictEqual(/request 1, writable: true/.test(responses), true)
      assert.strictEqual(/request 2, writable: true/.test(responses), true)
      assert.strictEqual(count, 2) // Add assertion for request count

      server.close()
      await once(server, 'close')
    })
  })

  describe('when socket closed before response sent', () => {
    it('should not be writable', async () => {
      const app = new Koa()
      let ctx
      let assertionRan = false
      app.on('error', () => {})

      app.use(c => {
        ctx = c
        assertionRan = true
      })

      const server = app.listen()
      const port = server.address().port
      const buf = Buffer.from('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n')
      const client = net.connect(port)
      await once(client, 'connect')
      client.write(buf)

      await new Promise(resolve => setTimeout(resolve, 5))
      client.destroy()

      server.close()
      await once(server, 'close')

      assert(ctx)
      assert(assertionRan)
      assert(!ctx.writable)
    })
Repository: koajs/koa
File: __tests__/response/writable.test.js (JavaScript)

Code before:
      client.destroy()

      server.close()
      await once(server, 'close')
    })
  })

  describe('when response finished', () => {
    it('should not be writable', async () => {
      const app = new Koa()
      app.use(ctx => {
        ctx.res.end()
        assert(!ctx.writable)
      })

      const server = app.listen()
      const port = server.address().port
      const buf = Buffer.from('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n')

      const client = net.connect(port)
      await once(client, 'connect')
      client.write(buf)
      client.destroy()

      server.close()
      await once(server, 'close')
    })
  })
})


Review comment (suggestion):
should add a boolean to check that this assertion passed

Code after:
      const port = server.address().port
      const buf = Buffer.from('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n')
      const client = net.connect(port)
      await once(client, 'connect')
      client.write(buf)

      await new Promise(resolve => setTimeout(resolve, 5))
      client.destroy()

      server.close()
      await once(server, 'close')

      assert(ctx)
      assert(assertionRan)
      assert(!ctx.writable)
    })
  })

  describe('when response finished', () => {
    it('should not be writable', async () => {
      const app = new Koa()
      let assertionRan = false

      app.use(ctx => {
        ctx.res.end()
        assert(!ctx.writable)
        assertionRan = true
      })

      const server = app.listen()
      const port = server.address().port
      const buf = Buffer.from('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n')

      const client = net.connect(port)
      await once(client, 'connect')
      client.write(buf)

      await new Promise(resolve => setTimeout(resolve, 5))

      client.destroy()

      server.close()
      await once(server, 'close')

      assert(assertionRan)
    })
  })
})
Repository: koajs/koa
File: __tests__/response/writable.test.js (JavaScript)

Code before:
      await once(client, 'connect')
      client.write(buf)
      client.write(buf)
      client.end()

      await once(client, 'end')
      client.destroy()

      const responses = Buffer.concat(datas).toString()
      assert.strictEqual(/request 1, writable: true/.test(responses), true)
      assert.strictEqual(/request 2, writable: true/.test(responses), true)
      assert.strictEqual(count, 2) // Add assertion for request count

      server.close()
      await once(server, 'close')
    })
  })

  describe('when socket closed before response sent', () => {
    it('should not be writable', async () => {
      const app = new Koa()
      app.use(ctx => {
        let assertionRan = false
        assert(!ctx.writable)
        assertionRan = true
        assert(assertionRan)
      })

      const server = app.listen()
      const port = server.address().port
      const buf = Buffer.from('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n')

      const client = net.connect(port)
      await once(client, 'connect')
      client.write(buf)
      client.destroy()

      server.close()
      await once(server, 'close')
    })
  })

  describe('when response finished', () => {
    it('should not be writable', async () => {
      const app = new Koa()
      app.use(ctx => {
        ctx.res.end()
        let assertionRan = false
        assert(!ctx.writable)
        assertionRan = true
        assert(assertionRan)

Review comment (refactor):
we should move this assertion to the end of the test

Code after:
      await once(client, 'connect')
      client.write(buf)
      client.write(buf)
      client.end()

      await once(client, 'end')
      client.destroy()

      const responses = Buffer.concat(datas).toString()
      assert.strictEqual(/request 1, writable: true/.test(responses), true)
      assert.strictEqual(/request 2, writable: true/.test(responses), true)
      assert.strictEqual(count, 2) // Add assertion for request count

      server.close()
      await once(server, 'close')
    })
  })

  describe('when socket closed before response sent', () => {
    it('should not be writable', async () => {
      const app = new Koa()
      let ctx
      let assertionRan = false
      app.on('error', () => {})

      app.use(c => {
        ctx = c
        assertionRan = true
      })

      const server = app.listen()
      const port = server.address().port
      const buf = Buffer.from('GET / HTTP/1.1\r\nHost: localhost:' + port + '\r\nConnection: keep-alive\r\n\r\n')
      const client = net.connect(port)
      await once(client, 'connect')
      client.write(buf)

      await new Promise(resolve => setTimeout(resolve, 5))
      client.destroy()

      server.close()
      await once(server, 'close')

      assert(ctx)
      assert(assertionRan)
      assert(!ctx.writable)
    })
  })

  describe('when response finished', () => {
    it('should not be writable', async () => {
Repository: koajs/koa
File: lib/response.js (JavaScript)

Code before:

    // set the status
    if (!this._explicitStatus) this.status = 200

    // set the content-type only if not yet set
    const setType = !this.has('Content-Type')

    // string
    if (typeof val === 'string') {
      if (setType) this.type = /^\s*</.test(val) ? 'html' : 'text'
      this.length = Buffer.byteLength(val)
      return
    }

    // buffer
    if (Buffer.isBuffer(val)) {
      if (setType) this.type = 'bin'
      this.length = val.length
      return
    }

    // stream
    if (isStream(val)) {
      onFinish(this.res, destroy.bind(null, val))
      if (original !== val) {
        val.once('error', err => this.ctx.onerror(err))
        // overwriting
        if (original != null) this.remove('Content-Length')
      }

      if (setType) this.type = 'bin'
      return
    }

    // ReadableStream
    if (val instanceof ReadableStream) {
      if (setType) this.type = 'bin'
      return
    }

    // blob
    if (val instanceof Blob) {
      if (setType) this.type = 'bin'
      this.length = val.size
      return
    }

    // Response
    if (val instanceof Response) {
      this.status = val.status
      if (setType) this.type = 'bin'

Review comment (suggestion):
@yowainwright I consecutively set 2 streams that will cause exceptions, deleting here is problematic.

```js
ctx.body = fs.createReadStream('does not exist');
ctx.body = fs.createReadStream('does not exist');
```

will throw unhandled error: `Error: ENOENT: no such file or directory, open 'does not exist'`

Code after:
      if (shouldDestroyOriginal) {
        // Ignore errors during cleanup to prevent unhandled exceptions when destroying the stream
        original.once('error', () => {})
        destroy(original)
      }
      return
    }

    // set the status
    if (!this._explicitStatus) this.status = 200

    // set the content-type only if not yet set
    const setType = !this.has('Content-Type')

    // string
    if (typeof val === 'string') {
      if (setType) this.type = /^\s*</.test(val) ? 'html' : 'text'
      this.length = Buffer.byteLength(val)
      return
    }

    // buffer
    if (Buffer.isBuffer(val)) {
      if (setType) this.type = 'bin'
      this.length = val.length
      return
    }

    // stream
    if (isStream(val)) {
      onFinish(this.res, destroy.bind(null, val))
      if (original !== val) {
        // overwriting
        if (original != null) this.remove('Content-Length')
      }

      if (setType) this.type = 'bin'
      return
    }

    // ReadableStream
    if (val instanceof ReadableStream) {
      if (setType) this.type = 'bin'
      return
    }

    // blob
    if (val instanceof Blob) {
      if (setType) this.type = 'bin'
      this.length = val.size
      return
Repository: ktorio/ktor
File: ktor-server/ktor-server-plugins/ktor-server-i18n/jvm/src/io/ktor/i18n/Translate.kt (Kotlin)

Code before:
/*
 * Copyright 2014-2020 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
 */

package io.ktor.i18n

import io.ktor.server.routing.*
import io.ktor.utils.io.charsets.*
import java.util.*

/**
 * Translate a message key to an accepted language specified in HTTP request
 */
public fun RoutingContext.i18n(key: String): String {
    val acceptedLanguages = call.attributes[ACCEPTED_LANGUAGES]
    val availableLanguages = call.attributes[AVAILABLE_LANGUAGES]
    val defaultLanguage = call.attributes[DEFAULT_LANGUAGE]

    val bestMatchLanguage = acceptedLanguages.firstOrNull {
        availableLanguages.contains(it.value)
    }?.value ?: defaultLanguage

    val locale = Locale.forLanguageTag(bestMatchLanguage)

    val bundle = ResourceBundle.getBundle("messages/messages", locale)
    val value = bundle.getString(key)

    I18N_LOGGER.debug("translating to $locale - acceptedLanguages=${acceptedLanguages}: $key=$value")
    return value
}


Review comment (bug):
I think we ought to be setting the `Content-Language` header somewhere in the response.  It'd make it easier for debugging and provide some browser support.

Code after:
/*
 * Copyright 2014-2020 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
 */

package io.ktor.i18n

import io.ktor.server.routing.*
import java.util.*

/**
 * Translate a message key to an accepted language specified in HTTP request
 */
public fun RoutingContext.i18n(key: String): String {
    val bestMatchLanguage = call.attributes[REQUIRED_RESPONSE_LANGUAGE]

    val locale = Locale.forLanguageTag(bestMatchLanguage)

    val bundle = ResourceBundle.getBundle("messages/messages", locale)
    val value = bundle.getString(key)

    I18N_LOGGER.debug("translating to $locale: $key=$value")
    return value
}
Repository: ktorio/ktor
File: gradle-settings-conventions/src/main/kotlin/conventions-develocity.settings.gradle.kts (Kotlin)

Code before:
    id("com.gradle.common-custom-user-data-gradle-plugin")
}

develocity {
    val startParameter = gradle.startParameter
    val scanJournal = File(settingsDir, "scan-journal.log")

    server = DEVELOCITY_SERVER

    buildScan {
        uploadInBackground = !isCIRun

        // obfuscate NIC since we don't want to expose user real IP (will be relevant without VPN)
        obfuscation {
            ipAddresses { addresses -> addresses.map { _ -> "0.0.0.0" } }
        }

        capture {
            fileFingerprints = true
        }

        buildScanPublished {
            scanJournal.appendText("${Date()} — $buildScanUri — $startParameter\n")
        }

        publishing.onlyIf { true }
    }
}

buildCache {
    if (isCIRun) {
        local {
            isEnabled = false
        }
    }

    remote(develocity.buildCache) {
        isPush = isCIRun
        isEnabled = true
    }
}

enrichTeamCityData()
enrichGitData()


Review comment (suggestion):
```suggestion
        publishing.onlyIf { isCIRun }
```

Code after:
    id("com.gradle.common-custom-user-data-gradle-plugin")
}

develocity {
    val startParameter = gradle.startParameter
    val scanJournal = File(settingsDir, "scan-journal.log")

    server = DEVELOCITY_SERVER

    buildScan {
        uploadInBackground = !isCIRun

        // obfuscate NIC since we don't want to expose user real IP (will be relevant without VPN)
        obfuscation {
            ipAddresses { addresses -> addresses.map { _ -> "0.0.0.0" } }
        }

        capture {
            fileFingerprints = true
        }

        buildScanPublished {
            scanJournal.appendText("${Date()} — $buildScanUri — $startParameter\n")
        }

        val skipBuildScans = settings.providers.gradleProperty("ktor.develocity.skipBuildScans")
            .getOrElse("false")
            .toBooleanStrict()

        publishing.onlyIf { !skipBuildScans }
    }
}

buildCache {
    if (isCIRun) {
        local {
            isEnabled = false
        }
    }

    remote(develocity.buildCache) {
        isPush = isCIRun
        isEnabled = true
    }
}

enrichTeamCityData()
enrichGitData()
Repository: ktorio/ktor
File: ktor-server/ktor-server-core/jvm/src/io/ktor/server/engine/EmbeddedServerJvm.kt (Kotlin)

Code before:
    private fun avoidingDoubleStartupFor(fqName: String, block: () -> Unit) {
        val modules = currentStartupModules.getOrSet { ArrayList(1) }
        check(!modules.contains(fqName)) {
            "Module startup is already in progress for function $fqName (recursive module startup from module main?)"
        }

        modules.add(fqName)
        try {
            block()
        } finally {
            modules.remove(fqName)
        }
    }

    private fun cleanupWatcher() {
        try {
            watcher?.close()
        } catch (_: NoClassDefFoundError) {
        }
    }
}

internal fun checkUrlMatches(url: URL, pattern: String): Boolean {
    val urlPath = url.path?.replace(File.separatorChar, '/') ?: return false
    val normalizedPattern = pattern.replace(File.separatorChar, '/')
    return urlPath?.contains(normalizedPattern, ignoreCase = true) == true
}


Review comment (suggestion):
One more minor change
```suggestion
    return urlPath.contains(normalizedPattern, ignoreCase = true)
```

Code after:
    private fun avoidingDoubleStartupFor(fqName: String, block: () -> Unit) {
        val modules = currentStartupModules.getOrSet { ArrayList(1) }
        check(!modules.contains(fqName)) {
            "Module startup is already in progress for function $fqName (recursive module startup from module main?)"
        }

        modules.add(fqName)
        try {
            block()
        } finally {
            modules.remove(fqName)
        }
    }

    private fun cleanupWatcher() {
        try {
            watcher?.close()
        } catch (_: NoClassDefFoundError) {
        }
    }
}

internal fun checkUrlMatches(url: URL, pattern: String): Boolean {
    val urlPath = url.path?.replace(File.separatorChar, '/') ?: return false
    val normalizedPattern = pattern.replace(File.separatorChar, '/')
    return urlPath.contains(normalizedPattern, ignoreCase = true)
}
Repository: ktorio/ktor
File: ktor-utils/jvm/src/io/ktor/util/reflect/TypeInfoJvm.kt (Kotlin)

Code before:
/*
 * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
 */

package io.ktor.util.reflect

import kotlin.reflect.*

public actual typealias Type = java.lang.reflect.Type

@OptIn(ExperimentalStdlibApi::class)
public actual inline fun <reified T> typeInfo(): TypeInfo {
    val kType = typeOfOrNull<T>()
    val reifiedType = try {
        // kType != null always returns false, so we try to get javaType anyway
        kType!!.javaType
    } catch (_: NullPointerException) {
        // Fallback to a type without generics if we can't get KType for the given T
        T::class.java
    }
    return TypeInfo(T::class, reifiedType, kType)
}

@Deprecated("Use TypeInfo constructor instead.", ReplaceWith("TypeInfo(kClass, reifiedType, kType)"))
public fun typeInfoImpl(reifiedType: Type, kClass: KClass<*>, kType: KType?): TypeInfo =
    TypeInfo(kClass, reifiedType, kType)

/**
 * Check [this] is instance of [type].
 */
public actual fun Any.instanceOf(type: KClass<*>): Boolean = type.java.isInstance(this)

@OptIn(ExperimentalStdlibApi::class)
public actual val KType.platformType: Type
    get() = javaType


Review comment (suggestion):
Now the hack looks much better. For some reason, the problem with null check doesn't exist for a class field.

Code after:
/*
 * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
 */

package io.ktor.util.reflect

import kotlin.reflect.*

@Deprecated("Not used anymore in common code as it was needed only for JVM target.")
public actual typealias Type = java.lang.reflect.Type

/** Type with substituted generics. */
@OptIn(ExperimentalStdlibApi::class)
public val TypeInfo.reifiedType: java.lang.reflect.Type
    // Fallback to a type without generics if we couldn't get KType.
    // For example, when class signature was stripped by ProGuard/R8
    get() = kotlinType?.javaType ?: type.java

@Suppress("DEPRECATION")
@Deprecated("Use TypeInfo constructor instead.", ReplaceWith("TypeInfo(kClass, kType)"))
public fun typeInfoImpl(reifiedType: Type, kClass: KClass<*>, kType: KType?): TypeInfo = TypeInfo(kClass, kType)

/**
 * Check [this] is instance of [type].
 */
public actual fun Any.instanceOf(type: KClass<*>): Boolean = type.java.isInstance(this)

@Suppress("DEPRECATION")
@Deprecated("Use KType.javaType instead.", ReplaceWith("this.javaType", "kotlin.reflect.javaType"))
@OptIn(ExperimentalStdlibApi::class)
public actual val KType.platformType: Type
    get() = javaType