ronantakizawa/codereview-bench huggingface dataset:
Repository: square/leakcanary
File: shark/src/main/java/shark/internal/Strings.kt (Kotlin)

Code before:

import java.math.BigInteger
import java.nio.charset.Charset
import java.security.MessageDigest
import java.security.NoSuchAlgorithmException

@JvmField
internal val UTF_8: Charset = Charset.forName("UTF-8")

internal fun String.lastSegment(segmentingChar: Char): String {
  val separator = lastIndexOf(segmentingChar)
  return if (separator == -1) this else this.substring(separator + 1)
}

internal fun String.createSHA1Hash(): String = createHash(this, "SHA-1")

private fun createHash(
  text: String,
  algorithm: String
): String {
  try {
    // Create MD5 Hash.
    val digest = MessageDigest.getInstance(algorithm)
    digest.update(text.getBytes())

    return BigInteger(1, digest.digest()).toString(16).padStart(40, '0')
  } catch (e: NoSuchAlgorithmException) {
    throw AssertionError("Unable to construct MessageDigest for $algorithm")
  }
}

/** Gets the string as an array of UTF-8 bytes. */
internal fun String.getBytes(): ByteArray = toByteArray(UTF_8)

Review comment (refactor):
Thx! We should actually just remove this method alltogether and use `text.encodeUtf8().sha1().hex()` (from Okio) instead

Code after:
package shark.internal

import okio.ByteString.Companion.encodeUtf8

internal fun String.lastSegment(segmentingChar: Char): String {
  val separator = lastIndexOf(segmentingChar)
  return if (separator == -1) this else this.substring(separator + 1)
}

internal fun String.createSHA1Hash(): String = encodeUtf8().sha1().hex()
Repository: square/leakcanary
File: leakcanary-android-core/src/main/java/leakcanary/internal/tv/TvToast.kt (Kotlin)

Code before:
internal object TvToast {

  /**
   * Make an Android TV toast.
   * Don't forget to call [Toast.show] to display the toast!
   * @param activity Currently resumed [Activity] to display toast on. Note that it's not [Context]
   *        to prevent passing application context that could lead to crashes on older platforms.
   * @param text The text to show. Can be formatted text.
   */
  @SuppressLint("ShowToast")
  fun makeText(
    activity: Activity,
    text: CharSequence
  ): Toast {
    val inflater = LayoutInflater.from(activity)
    val toast = Toast(activity)

    val iconSize = activity.resources.getDimensionPixelSize(
      R.dimen.leak_canary_toast_icon_size
    )
    toast.apply {
      setGravity(Gravity.CENTER_VERTICAL, 0, -iconSize)
      duration = Toast.LENGTH_LONG
      val inflatedView = inflater.inflate(R.layout.leak_canary_heap_dump_toast, null)
      inflatedView.findViewById<TextView>(R.id.leak_canary_toast_text).text = text
      view = inflatedView
    }

    return toast
  }
}


Review comment (suggestion):
Just wondering if we can do fancy Kotlin's `also{}` block here 😃 
```
view = inflater.inflate(R.layout.leak_canary_heap_dump_toast, null).also {
  this.findViewById<TextView>(R.id.leak_canary_toast_text).text = text
}
```

Code after:
internal object TvToast {

  /**
   * Make an Android TV toast.
   * Don't forget to call [Toast.show] to display the toast!
   * @param activity Currently resumed [Activity] to display toast on. Note that it's not [Context]
   *        to prevent passing application context that could lead to crashes on older platforms.
   * @param text The text to show. Can be formatted text.
   */
  @SuppressLint("ShowToast")
  fun makeText(
    activity: Activity,
    text: CharSequence
  ): Toast {
    val inflater = LayoutInflater.from(activity)
    val toast = Toast(activity)

    toast.apply {
      setGravity(Gravity.CENTER_VERTICAL, 0, 0)
      duration = Toast.LENGTH_LONG
      view = inflater.inflate(R.layout.leak_canary_heap_dump_toast, null).also {
        it.findViewById<TextView>(R.id.leak_canary_toast_text).text = text
      }
    }

    return toast
  }
}
Repository: square/leakcanary
File: leakcanary-android-instrumentation/src/main/java/leakcanary/AndroidDetectLeaksAssert.kt (Kotlin)

Code before:
        objectInspectors = config.objectInspectors,
        proguardMapping = null
      )
    )
    val analysisResult = heapAnalyzer.analyze(heapDumpFile)
    val totalDurationMillis = SystemClock.uptimeMillis() - assertionStartUptimeMillis
    val heapAnalysisWithExtraDetails = analysisResult.let {
      when (it) {
        is HeapAnalysisSuccess -> it.copy(
          dumpDurationMillis = heapDumpDurationMillis,
          metadata = it.metadata + mapOf(
            ASSERTION_TAG to tag,
            WAIT_FOR_RETAINED to waitForRetainedDurationMillis.toString(),
            TOTAL_DURATION to totalDurationMillis.toString()
          ),
        )
        is HeapAnalysisFailure -> it.copy(dumpDurationMillis = heapDumpDurationMillis)
      }
    }
    heapAnalysisReporter.reportHeapAnalysis(heapAnalysisWithExtraDetails)
  }

  companion object {
    private val ASSERTION_TAG = "assertionTag"
    private val WAIT_FOR_RETAINED = "waitForRetainedDurationMillis"
    private val TOTAL_DURATION = "totalDurationMillis"
    private var totalVmDurationMillis = 0L

    val HeapAnalysisSuccess.assertionTag: String?
      get() = metadata[ASSERTION_TAG]

    val HeapAnalysisSuccess.waitForRetainedDurationMillis: Int?
      get() = metadata[WAIT_FOR_RETAINED]?.toInt()

    val HeapAnalysisSuccess.totalDurationMillis: Int?
      get() = metadata[TOTAL_DURATION]?.toInt()
  }
}


Review comment (suggestion):
```suggestion
    private const val ASSERTION_TAG = "assertionTag"
    private const val WAIT_FOR_RETAINED = "waitForRetainedDurationMillis"
    private const val TOTAL_DURATION = "totalDurationMillis"
```

Code after:
        objectInspectors = config.objectInspectors,
        proguardMapping = null
      )
    )
    val analysisResult = heapAnalyzer.analyze(heapDumpFile)
    val totalDurationMillis = SystemClock.uptimeMillis() - assertionStartUptimeMillis
    val heapAnalysisWithExtraDetails = analysisResult.let {
      when (it) {
        is HeapAnalysisSuccess -> it.copy(
          dumpDurationMillis = heapDumpDurationMillis,
          metadata = it.metadata + mapOf(
            ASSERTION_TAG to tag,
            WAIT_FOR_RETAINED to waitForRetainedDurationMillis.toString(),
            TOTAL_DURATION to totalDurationMillis.toString()
          ),
        )
        is HeapAnalysisFailure -> it.copy(dumpDurationMillis = heapDumpDurationMillis)
      }
    }
    heapAnalysisReporter.reportHeapAnalysis(heapAnalysisWithExtraDetails)
  }

  companion object {
    private const val ASSERTION_TAG = "assertionTag"
    private const val WAIT_FOR_RETAINED = "waitForRetainedDurationMillis"
    private const val TOTAL_DURATION = "totalDurationMillis"
    private var totalVmDurationMillis = 0L

    val HeapAnalysisSuccess.assertionTag: String?
      get() = metadata[ASSERTION_TAG]

    val HeapAnalysisSuccess.waitForRetainedDurationMillis: Int?
      get() = metadata[WAIT_FOR_RETAINED]?.toInt()

    val HeapAnalysisSuccess.totalDurationMillis: Int?
      get() = metadata[TOTAL_DURATION]?.toInt()
  }
}
Repository: square/okhttp
File: okhttp-testing-support/src/main/kotlin/okhttp3/RecordingEventListener.kt (Kotlin)

Code before:
    }

    if (elapsedMs != -1L) {
      assertThat(
          TimeUnit.NANOSECONDS.toMillis(actualElapsedNs)
              .toDouble()
      )
          .isCloseTo(elapsedMs.toDouble(), Offset.offset(100.0))
    }

    return result
  }

  fun recordedEventTypes() = eventSequence.map { it.name }

  fun clearAllEvents() {
    while (eventSequence.isNotEmpty()) {
      takeEvent()
    }
  }

  private fun logEvent(e: CallEvent) {
    for (lock in forbiddenLocks) {
      assertThat(Thread.holdsLock(lock))
          .overridingErrorMessage(lock.toString())
          .isFalse
    }

    val startEvent = e.closes(-1L)
    if (startEvent != null) {
      assertTrue(eventSequence.any { it == e.closes(it.timestampNs) })
    }

    eventSequence.offer(e)
  }

  override fun proxySelectStart(
    call: Call,
    url: HttpUrl
  ) = logEvent(ProxySelectStart(System.nanoTime(), call, url))

  override fun proxySelectEnd(
    call: Call,
    url: HttpUrl,
    proxies: List<Proxy>
  ) = logEvent(ProxySelectEnd(System.nanoTime(), call, url, proxies))

  override fun dnsStart(
    call: Call,
    domainName: String
  ) = logEvent(DnsStart(System.nanoTime(), call, domainName))

Review comment (suggestion):
The IntelliJ waning irks me here, and I prefer the quotes. The is false() call has a critical side-effect, but accessing a property should not have side-effects.

I'd prefer to leave these as functions.

Code after:
    }

    if (elapsedMs != -1L) {
      assertThat(
          TimeUnit.NANOSECONDS.toMillis(actualElapsedNs)
              .toDouble()
      )
          .isCloseTo(elapsedMs.toDouble(), Offset.offset(100.0))
    }

    return result
  }

  fun recordedEventTypes() = eventSequence.map { it.name }

  fun clearAllEvents() {
    while (eventSequence.isNotEmpty()) {
      takeEvent()
    }
  }

  private fun logEvent(e: CallEvent) {
    for (lock in forbiddenLocks) {
      assertThat(Thread.holdsLock(lock))
          .overridingErrorMessage(lock.toString())
          .isFalse()
    }

    val startEvent = e.closes(-1L)
    if (startEvent != null) {
      assertTrue(eventSequence.any { it == e.closes(it.timestampNs) })
    }

    eventSequence.offer(e)
  }

  override fun proxySelectStart(
    call: Call,
    url: HttpUrl
  ) = logEvent(ProxySelectStart(System.nanoTime(), call, url))

  override fun proxySelectEnd(
    call: Call,
    url: HttpUrl,
    proxies: List<Proxy>
  ) = logEvent(ProxySelectEnd(System.nanoTime(), call, url, proxies))

  override fun dnsStart(
    call: Call,
    domainName: String
  ) = logEvent(DnsStart(System.nanoTime(), call, domainName))
Repository: square/okhttp
File: samples/guide/src/main/java/okhttp3/recipes/kt/WiresharkExample.kt (Kotlin)

Code before:
      sendTestRequest(twitterRequest)
      Thread.sleep(1000)
      sendTestRequest(googleRequest)
      Thread.sleep(2000)
    }
  }

  private fun sendTestRequest(twitterRequest: Request) {
    try {
      client.newCall(twitterRequest)
          .execute()
          .use {
            val firstLine = it.body!!.string()
                .lines()
                .first()
            println("${it.code} ${it.request.url.host} $firstLine")
            Unit
          }
    } catch (e: IOException) {
      System.err.println(e)
    }
  }
}

fun main() {
  // Call this before anything else initialises the JSSE stack.
  WireSharkKeyLoggerListener.register()

  // TLSv1.2 works without additional setup.
  val example = WiresharkExample(tlsVersions = listOf(TlsVersion.TLS_1_2))
  example.run()
}


Review comment (suggestion):
Passing tlsVersions here is a bit clumsy... But makes the copy and paste keep the warning messages on.  I could react to the negotiated connection instead.

Code after:
          }
          .get(this) as? SecretKey

    val randomRegex = "\"random\"\\s+:\\s+\"([^\"]+)\"".toRegex()

    fun register() {
      // Enable JUL logging for SSL events, must be activated early or via -D option.
      System.setProperty("javax.net.debug", "")
      logger = Logger.getLogger("javax.net.ssl")
          .apply {
            level = Level.FINEST
            useParentHandlers = false
          }
    }
  }
}

@SuppressSignatureCheck
class WiresharkExample(tlsVersions: List<TlsVersion>, private val launch: Launch? = null) {
  private val connectionSpec =
    ConnectionSpec.Builder(ConnectionSpec.RESTRICTED_TLS)
        .tlsVersions(*tlsVersions.toTypedArray())
        .build()

  private val eventListenerFactory = WireSharkListenerFactory(
      logFile = File("/tmp/key.log"), tlsVersions = tlsVersions, launch = launch)

  val client = OkHttpClient.Builder()
      .connectionSpecs(listOf(connectionSpec))
      .eventListenerFactory(eventListenerFactory)
      .build()

  fun run() {
    // Launch wireshark in the background
    val process = eventListenerFactory.launchWireShark()

    val fbRequest = Request.Builder()
        .url("https://graph.facebook.com/robots.txt?s=fb")
        .build()
    val twitterRequest = Request.Builder()
        .url("https://api.twitter.com/robots.txt?s=tw")
        .build()
    val googleRequest = Request.Builder()
        .url("https://www.google.com/robots.txt?s=g")
        .build()

    try {
      for (i in 1..2) {
        // Space out traffic to make it easier to demarcate.
        sendTestRequest(fbRequest)
        Thread.sleep(1000)
Repository: square/okhttp
File: okhttp/src/main/kotlin/okhttp3/internal/Util.kt (Kotlin)

Code before:
  for (i in startIndex until endIndex) {
    if (this[i] == delimiter) return i
  }
  return endIndex
}

/**
 * Returns the index of the first character in this string that is either a control character (like
 * `\u0000` or `\n`) or a non-ASCII character. Returns -1 if this string has no such characters.
 */
fun String.indexOfControlOrNonAscii(): Int {
  for (i in 0 until length) {
    val c = this[i]
    if (c <= '\u001f' || c >= '\u007f') {
      return i
    }
  }
  return -1
}

/** Returns true if this string is not a host name and might be an IP address. */
fun String.canParseAsIpAddress(): Boolean {
  return VERIFY_AS_IP_ADDRESS.matches(this)
}

/** Returns a [Locale.US] formatted [String]. */
fun format(format: String, vararg args: Any): String {
  return String.format(Locale.US, format, *args)
}

@Throws(IOException::class)
fun BufferedSource.readBomAsCharset(default: Charset): Charset {
  return when (select(UNICODE_BOMS)) {
    0 -> UTF_8
    1 -> UTF_16BE
    2 -> UTF_16LE
    3 -> UTF_32BE
    4 -> UTF_32LE
    -1 -> default
    else -> throw AssertionError()
  }
}

fun checkDuration(name: String, duration: Long, unit: TimeUnit?): Int {
  check(duration >= 0L) { "$name < 0" }
  check(unit != null) { "unit == null" }
  val millis = unit.toMillis(duration)
  require(millis <= Integer.MAX_VALUE) { "$name too large." }
  require(millis != 0L || duration <= 0L) { "$name too small." }
  return millis.toInt()
}

Review comment (suggestion):
```suggestion
/** Returns true if we should void putting this header in an exception or toString(). */
```

Code after:
  for (i in startIndex until endIndex) {
    if (this[i] == delimiter) return i
  }
  return endIndex
}

/**
 * Returns the index of the first character in this string that is either a control character (like
 * `\u0000` or `\n`) or a non-ASCII character. Returns -1 if this string has no such characters.
 */
fun String.indexOfControlOrNonAscii(): Int {
  for (i in 0 until length) {
    val c = this[i]
    if (c <= '\u001f' || c >= '\u007f') {
      return i
    }
  }
  return -1
}

/** Returns true if this string is not a host name and might be an IP address. */
fun String.canParseAsIpAddress(): Boolean {
  return VERIFY_AS_IP_ADDRESS.matches(this)
}

/** Returns true if we should void putting this this header in an exception or toString(). */
fun isSensitiveHeader(name: String): Boolean {
  return name.equals("Authorization", ignoreCase = true) ||
      name.equals("Cookie", ignoreCase = true) ||
      name.equals("Proxy-Authorization", ignoreCase = true) ||
      name.equals("Set-Cookie", ignoreCase = true)
}

/** Returns a [Locale.US] formatted [String]. */
fun format(format: String, vararg args: Any): String {
  return String.format(Locale.US, format, *args)
}

@Throws(IOException::class)
fun BufferedSource.readBomAsCharset(default: Charset): Charset {
  return when (select(UNICODE_BOMS)) {
    0 -> UTF_8
    1 -> UTF_16BE
    2 -> UTF_16LE
    3 -> UTF_32BE
    4 -> UTF_32LE
    -1 -> default
    else -> throw AssertionError()
  }
}
Repository: square/okhttp
File: okhttp/src/test/java/okhttp3/ConscryptTest.kt (Kotlin)

Code before:

  @Test
  fun testTrustManager() {
    assertThat(Conscrypt.isConscrypt(Platform.get().platformTrustManager())).isTrue()
  }

  @ParameterizedTest(name = "{displayName}({arguments})")
  @ValueSource(strings = ["TLSv1.2", "TLSv1.3"])
  fun testSessionReuse(tlsVersion: String) {
    val sessionIds = mutableListOf<String>()

    enableTls()

    val tlsVersion = TlsVersion.forJavaName(tlsVersion)
    val spec = ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS)
      .tlsVersions(tlsVersion)
      .build()

    var reuseSession = false

    val sslContext = handshakeCertificates.sslContext()
    val systemSslSocketFactory = sslContext.socketFactory
    val sslSocketFactory = object : DelegatingSSLSocketFactory(systemSslSocketFactory) {
      override fun configureSocket(sslSocket: SSLSocket): SSLSocket {
        return sslSocket.apply {
          if (reuseSession) {
            this.enableSessionCreation = false
          }
        }
      }
    }

    client = client.newBuilder().connectionSpecs(listOf(spec))
      .eventListenerFactory(clientTestRule.wrap(object : EventListener() {
        override fun connectionAcquired(call: Call, connection: Connection) {
          val sslSocket = connection.socket() as SSLSocket

          sessionIds.add(sslSocket.session.id.toByteString().hex())
        }
      }))
      .sslSocketFactory(sslSocketFactory, handshakeCertificates.trustManager)
      .build()

    server.enqueue(MockResponse().setBody("abc1"))
    server.enqueue(MockResponse().setBody("abc2"))

    val request = Request.Builder().url(server.url("/")).build()

    client.newCall(request).execute().use { response ->
      assertEquals(200, response.code)
    }

Review comment (suggestion):
I find this somewhat hard to follow ... capturing a mutable local. Maybe I’m too used to how it works in Java and not Kotlin!

Code after:
    assumeNetwork()

    val request = Request.Builder().url("https://google.com/robots.txt").build()

    client.newCall(request).execute().use {
      assertThat(it.protocol).isEqualTo(Protocol.HTTP_2)
      if (it.handshake!!.tlsVersion != TlsVersion.TLS_1_3) {
        System.err.println("Flaky TLSv1.3 with google")
//    assertThat(it.handshake()!!.tlsVersion).isEqualTo(TlsVersion.TLS_1_3)
      }
    }
  }

  @Test
  fun testBuildIfSupported() {
    val actual = ConscryptPlatform.buildIfSupported()
    assertThat(actual).isNotNull
  }

  @Test
  fun testVersion() {
    val version = Conscrypt.version()

    assertTrue(ConscryptPlatform.atLeastVersion(1, 4, 9))
    assertTrue(ConscryptPlatform.atLeastVersion(version.major()))
    assertTrue(ConscryptPlatform.atLeastVersion(version.major(), version.minor()))
    assertTrue(ConscryptPlatform.atLeastVersion(version.major(), version.minor(), version.patch()))
    assertFalse(ConscryptPlatform.atLeastVersion(version.major(), version.minor(), version.patch() + 1))
    assertFalse(ConscryptPlatform.atLeastVersion(version.major(), version.minor() + 1))
    assertFalse(ConscryptPlatform.atLeastVersion(version.major() + 1))
  }
}
Repository: square/okhttp
File: okhttp-sse/src/main/kotlin/okhttp3/sse/internal/RealEventSource.kt (Kotlin)

Code before:
        .eventListener(EventListener.NONE)
        .build()
    call = client.newCall(request) as RealCall
    call.enqueue(this)
  }

  override fun onResponse(call: Call, response: Response) {
    processResponse(response)
  }

  fun processResponse(response: Response) {
    response.use {
      if (!response.isSuccessful) {
        listener.onFailure(this, null, response)
        return
      }

      val body = response.body!!

      if (!body.isEventStream()) {
        listener.onFailure(this,
            IllegalStateException("Invalid content-type: ${body.contentType()}"), response)
        return
      }

      if (this::call.isInitialized) {
        // This is a long-lived response. Cancel full-call timeouts.
        call.timeoutEarlyExit()
      }

      // Replace the body with an empty one so the callbacks can't see real data.
      val response = response.newBuilder()
          .body(EMPTY_RESPONSE)
          .build()

      val reader = ServerSentEventReader(body.source(), this)
      try {
        listener.onOpen(this, response)
        while (reader.processNextEvent()) {
        }
      } catch (e: Exception) {
        listener.onFailure(this, e, response)
        return
      }
      listener.onClosed(this)
    }
  }

  private fun ResponseBody.isEventStream(): Boolean {
    val contentType = contentType() ?: return false
    return contentType.type == "text" && contentType.subtype == "event-stream"

Review comment (suggestion):
We should probably change this to be a simple null check.  The lateinit doesn't hold if it can be null.

Code after:
        .eventListener(EventListener.NONE)
        .build()
    val realCall = client.newCall(request) as RealCall
    call = realCall
    realCall.enqueue(this)
  }

  override fun onResponse(call: Call, response: Response) {
    processResponse(response)
  }

  fun processResponse(response: Response) {
    response.use {
      if (!response.isSuccessful) {
        listener.onFailure(this, null, response)
        return
      }

      val body = response.body!!

      if (!body.isEventStream()) {
        listener.onFailure(this,
            IllegalStateException("Invalid content-type: ${body.contentType()}"), response)
        return
      }

      // This is a long-lived response. Cancel full-call timeouts.
      call?.timeoutEarlyExit()

      // Replace the body with an empty one so the callbacks can't see real data.
      val response = response.newBuilder()
          .body(EMPTY_RESPONSE)
          .build()

      val reader = ServerSentEventReader(body.source(), this)
      try {
        listener.onOpen(this, response)
        while (reader.processNextEvent()) {
        }
      } catch (e: Exception) {
        listener.onFailure(this, e, response)
        return
      }
      listener.onClosed(this)
    }
  }

  private fun ResponseBody.isEventStream(): Boolean {
    val contentType = contentType() ?: return false
    return contentType.type == "text" && contentType.subtype == "event-stream"
  }
Repository: square/okhttp
File: mockwebserver-junit5/build.gradle.kts (Kotlin)

Code before:

tasks {
  jar {
    manifest {
      attributes("Automatic-Module-Name" to "mockwebserver3.junit5")
    }
  }
  test {
    useJUnitPlatform()
    systemProperty("junit.jupiter.extensions.autodetection.enabled", "true")
  }
}

dependencies {
  api(project(":mockwebserver3"))
  api(Dependencies.junit5Api)
  compileOnly(Dependencies.animalSniffer)

  testRuntimeOnly(Dependencies.junit5JupiterEngine)
  testImplementation(Dependencies.assertj)
  testImplementation(Dependencies.kotlinJunit5)
}

configure<MavenPublishBaseExtension> {
  configure(KotlinJvm(javadocJar = JavadocJar.Dokka("dokkaGfm"))
  )
}


Review comment (suggestion):
```suggestion
  configure(KotlinJvm(javadocJar = JavadocJar.Dokka("dokkaGfm")))
```

Code after:

tasks {
  jar {
    manifest {
      attributes("Automatic-Module-Name" to "mockwebserver3.junit5")
    }
  }
  test {
    useJUnitPlatform()
    systemProperty("junit.jupiter.extensions.autodetection.enabled", "true")
  }
}

dependencies {
  api(project(":mockwebserver3"))
  api(Dependencies.junit5Api)
  compileOnly(Dependencies.animalSniffer)

  testRuntimeOnly(Dependencies.junit5JupiterEngine)
  testImplementation(Dependencies.assertj)
  testImplementation(Dependencies.kotlinJunit5)
}

configure<MavenPublishBaseExtension> {
  configure(KotlinJvm(javadocJar = JavadocJar.Dokka("dokkaGfm")))
}