ronantakizawa/codereview-bench huggingface dataset:
Repository: Kotlin/kotlinx.coroutines
File: buildSrc/src/main/kotlin/animalsniffer-convention.gradle.kts (Kotlin)

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

import ru.vyarus.gradle.plugin.animalsniffer.*

subprojects {
    // Skip benchmarks and bom -- not interested
    if (!shouldSniff()) return@subprojects
    apply(plugin = "ru.vyarus.animalsniffer")
    configure<AnimalSnifferExtension> {
        // TODO use project.sourceSets on the newer gradle
        sourceSets = listOf((project.extensions.getByName("sourceSets") as SourceSetContainer).getByName("main"))
    }
    val signature: Configuration by configurations
    dependencies {
        signature("net.sf.androidscents.signature:android-api-level-14:4.0_r4@signature")
        signature("org.codehaus.mojo.signature:java17:1.0@signature")
    }
}

fun Project.shouldSniff(): Boolean {
    // Skip all non-JVM projects
    if (platformOf(project) != "jvm") return false
    val name = project.name
    if (name in unpublished || name in sourceless || name in androidNonCompatibleProjects) return false
    return true
}


Review comment (suggestion):
Looks like this is outdated? `shouldSniff` returns `false` on much more than that.

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

import ru.vyarus.gradle.plugin.animalsniffer.*

subprojects {
    // Skip JDK 8 projects or unpublished ones
    if (!shouldSniff()) return@subprojects
    apply(plugin = "ru.vyarus.animalsniffer")
    configure<AnimalSnifferExtension> {
        sourceSets = listOf((project.extensions.getByName("sourceSets") as SourceSetContainer).getByName("main"))
    }
    val signature: Configuration by configurations
    dependencies {
        signature("net.sf.androidscents.signature:android-api-level-14:4.0_r4@signature")
        signature("org.codehaus.mojo.signature:java17:1.0@signature")
    }
}

fun Project.shouldSniff(): Boolean {
    // Skip all non-JVM projects
    if (platformOf(project) != "jvm") return false
    val name = project.name
    if (name in unpublished || name in sourceless || name in androidNonCompatibleProjects) return false
    return true
}
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-core/jvm/src/internal/ExceptionsConstructor.kt (Kotlin)

Code before:

private fun Class<*>.fieldsCountOrDefault(defaultValue: Int) =
    kotlin.runCatching { fieldsCount() }.getOrDefault(defaultValue)

private tailrec fun Class<*>.fieldsCount(accumulator: Int = 0): Int {
    val fieldsCount = declaredFields.count { !Modifier.isStatic(it.modifiers) }
    val totalFields = accumulator + fieldsCount
    val superClass = superclass ?: return totalFields
    return superClass.fieldsCount(totalFields)
}

internal abstract class CtorCache {
    abstract fun get(key: Class<out Throwable>): Ctor
}

private object WeakMapCtorCache : CtorCache() {
    private val cacheLock = ReentrantReadWriteLock()
    private val exceptionCtors: WeakHashMap<Class<out Throwable>, Ctor> = WeakHashMap()

    override fun get(key: Class<out Throwable>): Ctor {
        cacheLock.read { exceptionCtors[key]?.let { return it } }
        cacheLock.write {
            exceptionCtors[key]?.let { return it }
            val result = createConstructor(key)
            exceptionCtors[key] = result
            return result
        }
    }
}

@IgnoreJreRequirement
private object ClassValueCtorCache : CtorCache() {
    private val cache = object : ClassValue<Ctor>() {
        override fun computeValue(type: Class<*>?): Ctor {
            @Suppress("UNCHECKED_CAST")
            return createConstructor(type as Class<out Throwable>)
        }
    }

    override fun get(key: Class<out Throwable>): Ctor = cache.get(key)
}


Review comment (suggestion):
`result` doesn't sound to me like a name worth binding.
```suggestion
            return createConstructor(key).also { exceptionCtors[key] = it }
```

Code after:

private fun Class<*>.fieldsCountOrDefault(defaultValue: Int) =
    kotlin.runCatching { fieldsCount() }.getOrDefault(defaultValue)

private tailrec fun Class<*>.fieldsCount(accumulator: Int = 0): Int {
    val fieldsCount = declaredFields.count { !Modifier.isStatic(it.modifiers) }
    val totalFields = accumulator + fieldsCount
    val superClass = superclass ?: return totalFields
    return superClass.fieldsCount(totalFields)
}

internal abstract class CtorCache {
    abstract fun get(key: Class<out Throwable>): Ctor
}

private object WeakMapCtorCache : CtorCache() {
    private val cacheLock = ReentrantReadWriteLock()
    private val exceptionCtors: WeakHashMap<Class<out Throwable>, Ctor> = WeakHashMap()

    override fun get(key: Class<out Throwable>): Ctor {
        cacheLock.read { exceptionCtors[key]?.let { return it } }
        cacheLock.write {
            exceptionCtors[key]?.let { return it }
            return createConstructor(key).also { exceptionCtors[key] = it }
        }
    }
}

@IgnoreJreRequirement
private object ClassValueCtorCache : CtorCache() {
    private val cache = object : ClassValue<Ctor>() {
        override fun computeValue(type: Class<*>?): Ctor {
            @Suppress("UNCHECKED_CAST")
            return createConstructor(type as Class<out Throwable>)
        }
    }

    override fun get(key: Class<out Throwable>): Ctor = cache.get(key)
}
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-test/common/src/TestBuilders.kt (Kotlin)

Code before:
@DelicateCoroutinesApi
public fun TestDispatcher.runTest(
    dispatchTimeoutMs: Long = DEFAULT_DISPATCH_TIMEOUT_MS,
    block: suspend TestCoroutineScope.() -> Unit
): TestResult =
    runTest(this, dispatchTimeoutMs, block)

/** A coroutine context element indicating that the coroutine is running inside `runTest`. */
private class RunningInRunTest: AbstractCoroutineContextElement(RunningInRunTest), CoroutineContext.Element {
    companion object Key : CoroutineContext.Key<RunningInRunTest>
}

/** The default timeout to use when waiting for asynchronous completions of the coroutines managed by
 * a [TestCoroutineScheduler]. */
private const val DEFAULT_DISPATCH_TIMEOUT_MS = 10_000L

private class TestBodyCoroutine<T>(
    private val testScope: TestCoroutineScope,
    active: Boolean
) : AbstractCoroutine<T>(testScope.coroutineContext, true, active = active),
    SelectClause1<T>, TestCoroutineScope
{
    val onAwait: SelectClause1<T> get() = this

    @Suppress("INVISIBLE_MEMBER")
    override fun <R> registerSelectClause1(select: SelectInstance<R>, block: suspend (T) -> R) =
        registerSelectClause1Internal2(select, block)

    override val testScheduler get() = testScope.testScheduler

    override fun cleanupTestCoroutines() = testScope.cleanupTestCoroutines()

    override fun reportException(throwable: Throwable) = testScope.reportException(throwable)
}


Review comment (suggestion):
Suppressing `internal` access in a cross-module environment is actually UB. It works on JVM because we know some implementation details, but not on other platforms.

A stable way to fix it is to mark the internal declaration with `@PublishedApi`.

Code after:
public fun TestDispatcher.runTest(
    dispatchTimeoutMs: Long = DEFAULT_DISPATCH_TIMEOUT_MS,
    block: suspend TestCoroutineScope.() -> Unit
): TestResult =
    runTest(this, dispatchTimeoutMs, block)

/** A coroutine context element indicating that the coroutine is running inside `runTest`. */
private class RunningInRunTest: AbstractCoroutineContextElement(RunningInRunTest), CoroutineContext.Element {
    companion object Key : CoroutineContext.Key<RunningInRunTest>
}

/** The default timeout to use when waiting for asynchronous completions of the coroutines managed by
 * a [TestCoroutineScheduler]. */
private const val DEFAULT_DISPATCH_TIMEOUT_MS = 10_000L

private class TestBodyCoroutine<T>(
    private val testScope: TestCoroutineScope,
) : AbstractCoroutine<T>(testScope.coroutineContext, initParentJob = true, active = true), TestCoroutineScope
{
    override val testScheduler get() = testScope.testScheduler

    override fun cleanupTestCoroutines() = testScope.cleanupTestCoroutines()

    override fun reportException(throwable: Throwable) = testScope.reportException(throwable)
}
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-test/common/test/TestDispatchersTest.kt (Kotlin)

Code before:
/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */
package kotlinx.coroutines.test

import kotlinx.coroutines.*
import kotlinx.coroutines.test.internal.*
import kotlin.coroutines.*
import kotlin.test.*

class TestDispatchersTest: OrderedExecutionTestBase() {

    @BeforeTest
    fun setUp() {
        Dispatchers.setMain(StandardTestDispatcher())
    }

    @AfterTest
    fun tearDown() {
        Dispatchers.resetMain()
    }

    /** Tests that asynchronous execution of tests does not happen concurrently with [AfterTest] (in fact, it does). */
    @Ignore // fails on JS.
    @Test
    fun testMainMocking() = runTest {
        val mainAtStart = mainTestDispatcher
        assertNotNull(mainAtStart)
        withContext(Dispatchers.Main) {
            delay(10)
        }
        withContext(Dispatchers.Default) {
            delay(10)
        }
        withContext(Dispatchers.Main) {
            delay(10)
        }
        assertSame(mainAtStart, mainTestDispatcher)
    }

    /** Tests that the mocked [Dispatchers.Main] correctly forwards [Delay] methods. */
    @Test
    fun testMockedMainImplementsDelay() = runTest {
        val main = Dispatchers.Main
        withContext(main) {
            delay(10)
        }
        withContext(Dispatchers.Default) {
            delay(10)

Review comment (refactor):
For such kludges, we typically introduce helpers like [`noJs`](https://github.com/Kotlin/kotlinx.serialization/blob/5ce8e2cbd8d324ed5e0447f133137ccacc9f6614/formats/json/commonTest/src/kotlinx/serialization/test/TestHelpers.kt#L27)

Code after:
/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */
package kotlinx.coroutines.test

import kotlinx.coroutines.*
import kotlinx.coroutines.test.internal.*
import kotlin.coroutines.*
import kotlin.test.*

class TestDispatchersTest: OrderedExecutionTestBase() {

    @BeforeTest
    fun setUp() {
        Dispatchers.setMain(StandardTestDispatcher())
    }

    @AfterTest
    fun tearDown() {
        Dispatchers.resetMain()
    }

    /** Tests that asynchronous execution of tests does not happen concurrently with [AfterTest]. */
    @NoJs
    @Test
    fun testMainMocking() = runTest {
        val mainAtStart = mainTestDispatcher
        assertNotNull(mainAtStart)
        withContext(Dispatchers.Main) {
            delay(10)
        }
        withContext(Dispatchers.Default) {
            delay(10)
        }
        withContext(Dispatchers.Main) {
            delay(10)
        }
        assertSame(mainAtStart, mainTestDispatcher)
    }

    /** Tests that the mocked [Dispatchers.Main] correctly forwards [Delay] methods. */
    @Test
    fun testMockedMainImplementsDelay() = runTest {
        val main = Dispatchers.Main
        withContext(main) {
            delay(10)
        }
        withContext(Dispatchers.Default) {
            delay(10)
Repository: Kotlin/kotlinx.coroutines
File: buildSrc/build.gradle.kts (Kotlin)

Code before:
    // Intercept reading from properties file
    if (target == "kotlin") {
        val snapshotVersion = properties["kotlin_snapshot_version"]
        if (snapshotVersion != null) return snapshotVersion.toString()
    }
    return props.getProperty("${target}_version")
}

dependencies {
    implementation(kotlin("gradle-plugin", version("kotlin")))
    /*
     * Dokka is compiled with language level = 1.4, but depends on Kotlin 1.6.0, while
     * our version of Gradle bundles Kotlin 1.4.x and can read metadata only up to 1.5.x,
     * thus we're excluding stdlib compiled with 1.6.0 from dependencies.
     */
    implementation("org.jetbrains.dokka:dokka-gradle-plugin:${version("dokka")}") {
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib-jdk8")
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib-jdk7")
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib")
    }
    implementation("org.jetbrains.dokka:dokka-core:${version("dokka")}") {
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib-jdk8")
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib-jdk7")
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib")
    }
    implementation("ru.vyarus:gradle-animalsniffer-plugin:1.5.3") // Android API check implementation("ru.vyarus:gradle-animalsniffer-plugin:1.5.3") // Android API check
}


Review comment (suggestion):
```suggestion
    implementation("ru.vyarus:gradle-animalsniffer-plugin:1.5.3") // Android API check
```

Code after:
    // Intercept reading from properties file
    if (target == "kotlin") {
        val snapshotVersion = properties["kotlin_snapshot_version"]
        if (snapshotVersion != null) return snapshotVersion.toString()
    }
    return props.getProperty("${target}_version")
}

dependencies {
    implementation(kotlin("gradle-plugin", version("kotlin")))
    /*
     * Dokka is compiled with language level = 1.4, but depends on Kotlin 1.6.0, while
     * our version of Gradle bundles Kotlin 1.4.x and can read metadata only up to 1.5.x,
     * thus we're excluding stdlib compiled with 1.6.0 from dependencies.
     */
    implementation("org.jetbrains.dokka:dokka-gradle-plugin:${version("dokka")}") {
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib-jdk8")
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib-jdk7")
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib")
    }
    implementation("org.jetbrains.dokka:dokka-core:${version("dokka")}") {
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib-jdk8")
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib-jdk7")
        exclude(group = "org.jetbrains.kotlin", module = "kotlin-stdlib")
    }
    implementation("ru.vyarus:gradle-animalsniffer-plugin:1.5.3") // Android API check
}
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-core/common/src/internal/StackTraceRecovery.common.kt (Kotlin)

Code before:
internal expect fun <E: Throwable> recoverStackTrace(exception: E, continuation: Continuation<*>): E

/**
 * initCause on JVM, nop on other platforms
 */
@Suppress("EXTENSION_SHADOWED_BY_MEMBER")
internal expect fun Throwable.initCause(cause: Throwable)

/**
 * Tries to recover stacktrace for given [exception]. Used in non-suspendable points of awaiting.
 * Stacktrace recovery tries to instantiate exception of given type with original exception as a cause.
 * Wrapping exception will have proper stacktrace as it's instantiated in the right context.
 *
 * Works only on JVM with enabled debug-mode.
 */
internal expect fun <E: Throwable> recoverStackTrace(exception: E): E

// Name conflict with recoverStackTrace
@Suppress("NOTHING_TO_INLINE")
internal expect suspend inline fun recoverAndThrow(exception: Throwable): Nothing

/**
 * The opposite of [recoverStackTrace].
 * It is guaranteed that `unwrap(recoverStackTrace(e)) === e`
 */
@PublishedApi
internal expect fun <E: Throwable> unwrap(exception: E): E

internal expect class StackTraceElement

internal expect interface CoroutineStackFrame {
    public val callerFrame: CoroutineStackFrame?
    public fun getStackTraceElement(): StackTraceElement?
}


Review comment (suggestion):
Please leave a comment that it is only used from other module tests, so when the time comes we know it can be safely deleted/reverted

Code after:
internal expect fun <E: Throwable> recoverStackTrace(exception: E, continuation: Continuation<*>): E

/**
 * initCause on JVM, nop on other platforms
 */
@Suppress("EXTENSION_SHADOWED_BY_MEMBER")
internal expect fun Throwable.initCause(cause: Throwable)

/**
 * Tries to recover stacktrace for given [exception]. Used in non-suspendable points of awaiting.
 * Stacktrace recovery tries to instantiate exception of given type with original exception as a cause.
 * Wrapping exception will have proper stacktrace as it's instantiated in the right context.
 *
 * Works only on JVM with enabled debug-mode.
 */
internal expect fun <E: Throwable> recoverStackTrace(exception: E): E

// Name conflict with recoverStackTrace
@Suppress("NOTHING_TO_INLINE")
internal expect suspend inline fun recoverAndThrow(exception: Throwable): Nothing

/**
 * The opposite of [recoverStackTrace].
 * It is guaranteed that `unwrap(recoverStackTrace(e)) === e`
 */
@PublishedApi // only published for the multiplatform tests in our own code
internal expect fun <E: Throwable> unwrap(exception: E): E

internal expect class StackTraceElement

internal expect interface CoroutineStackFrame {
    public val callerFrame: CoroutineStackFrame?
    public fun getStackTraceElement(): StackTraceElement?
}
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-core/jvm/src/internal/ResizableAtomicArray.kt (Kotlin)

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

package kotlinx.coroutines.internal

import kotlinx.coroutines.*
import java.util.concurrent.atomic.*

/**
 * Atomic array with lock-free reads and synchronized modifications. It is logically has unbounded size,
 * is implicitly filled with nulls, and is resized on updates as needed to grow.
 */
internal class ResizableAtomicArray<T>(initialLength: Int) {
    @Volatile
    private var array = AtomicReferenceArray<T>(initialLength)

    public fun length(): Int = array.length()

    public operator fun get(index: Int): T? {
        assert { index >= 0 }
        val array = this.array // volatile read
        return if (index < array.length()) array[index] else null
    }

    @Synchronized
    operator fun set(index: Int, value: T?) {
        assert { index >= 0 }
        val curArray = this.array
        val curLen = curArray.length()
        if (index < curLen) {
            curArray[index] = value
        } else {
            val newArray = AtomicReferenceArray<T>((index + 1).coerceAtLeast(2 * curLen))
            for (i in 0 until curLen) newArray[i] = curArray[i]
            newArray[index] = value
            array = newArray // copy done
        }
    }
}


Review comment (bug):
(Minor) This operation is only used for debug output and looks out-of-place in a logically unbounded structure. Maybe call this something like `currentLength` instead?

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

package kotlinx.coroutines.internal

import java.util.concurrent.atomic.*

/**
 * Atomic array with lock-free reads and synchronized modifications. It logically has an unbounded size,
 * is implicitly filled with nulls, and is resized on updates as needed to grow.
 */
internal class ResizableAtomicArray<T>(initialLength: Int) {
    @Volatile
    private var array = AtomicReferenceArray<T>(initialLength)

    // for debug output
    public fun currentLength(): Int = array.length()

    public operator fun get(index: Int): T? {
        val array = this.array // volatile read
        return if (index < array.length()) array[index] else null
    }

    // Must not be called concurrently, e.g. always use synchronized(this) to call this function
    fun setSynchronized(index: Int, value: T?) {
        val curArray = this.array
        val curLen = curArray.length()
        if (index < curLen) {
            curArray[index] = value
        } else {
            val newArray = AtomicReferenceArray<T>((index + 1).coerceAtLeast(2 * curLen))
            for (i in 0 until curLen) newArray[i] = curArray[i]
            newArray[index] = value
            array = newArray // copy done
        }
    }
}
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-core/jvm/src/internal/ResizableAtomicArray.kt (Kotlin)

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

package kotlinx.coroutines.internal

import kotlinx.coroutines.*
import java.util.concurrent.atomic.*

/**
 * Atomic array with lock-free reads and synchronized modifications. It is logically has unbounded size,
 * is implicitly filled with nulls, and is resized on updates as needed to grow.
 */
internal class ResizableAtomicArray<T>(initialLength: Int) {
    @Volatile
    private var array = AtomicReferenceArray<T>(initialLength)

    public fun length(): Int = array.length()

    public operator fun get(index: Int): T? {
        assert { index >= 0 }
        val array = this.array // volatile read
        return if (index < array.length()) array[index] else null
    }

    @Synchronized
    operator fun set(index: Int, value: T?) {
        assert { index >= 0 }
        val curArray = this.array
        val curLen = curArray.length()
        if (index < curLen) {
            curArray[index] = value
        } else {
            val newArray = AtomicReferenceArray<T>((index + 1).coerceAtLeast(2 * curLen))
            for (i in 0 until curLen) newArray[i] = curArray[i]
            newArray[index] = value

Review comment (suggestion):
```suggestion
 * Atomic array with lock-free reads and synchronized modifications. It logically has an unbounded size,
```

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

package kotlinx.coroutines.internal

import java.util.concurrent.atomic.*

/**
 * Atomic array with lock-free reads and synchronized modifications. It logically has an unbounded size,
 * is implicitly filled with nulls, and is resized on updates as needed to grow.
 */
internal class ResizableAtomicArray<T>(initialLength: Int) {
    @Volatile
    private var array = AtomicReferenceArray<T>(initialLength)

    // for debug output
    public fun currentLength(): Int = array.length()

    public operator fun get(index: Int): T? {
        val array = this.array // volatile read
        return if (index < array.length()) array[index] else null
    }

    // Must not be called concurrently, e.g. always use synchronized(this) to call this function
    fun setSynchronized(index: Int, value: T?) {
        val curArray = this.array
        val curLen = curArray.length()
        if (index < curLen) {
            curArray[index] = value
        } else {
            val newArray = AtomicReferenceArray<T>((index + 1).coerceAtLeast(2 * curLen))
            for (i in 0 until curLen) newArray[i] = curArray[i]
            newArray[index] = value
            array = newArray // copy done
        }