ronantakizawa/codereview-bench huggingface dataset:
Repository: JetBrains/compose-multiplatform
File: gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/desktop/application/dsl/JvmApplicationDistributions.kt (Kotlin)

Code before:
)

abstract class JvmApplicationDistributions : AbstractDistributions() {
    var modules = arrayListOf(*DEFAULT_RUNTIME_MODULES)
    fun modules(vararg modules: String) {
        this.modules.addAll(modules.toList())
    }
    var includeAllModules: Boolean = false

    val linux: LinuxPlatformSettings = objects.newInstance(LinuxPlatformSettings::class.java)
    open fun linux(fn: Action<LinuxPlatformSettings>) {
        fn.execute(linux)
    }

    val macOS: JvmMacOSPlatformSettings = objects.newInstance(JvmMacOSPlatformSettings::class.java)
    open fun macOS(fn: Action<JvmMacOSPlatformSettings>) {
        fn.execute(macOS)
    }

    val windows: WindowsPlatformSettings = objects.newInstance(WindowsPlatformSettings::class.java)
    fun windows(fn: Action<WindowsPlatformSettings>) {
        fn.execute(windows)
    }
    
    internal val fileAssociations: MutableSet<FileAssociation> = mutableSetOf()
    fun fileAssociation(mimeType: String, extension: String, description: String) {
        fileAssociations.add(FileAssociation(mimeType, extension, description))
    }
}

Review comment (suggestion):
Let's add a doc for this function to https://github.com/JetBrains/compose-multiplatform/tree/master/tutorials/Native_distributions_and_local_execution#platform-specific-options

(1-2 points below `installationPath`

Code after:
    "java.base", "java.desktop", "java.logging", "jdk.crypto.ec"
)

abstract class JvmApplicationDistributions : AbstractDistributions() {
    var modules = arrayListOf(*DEFAULT_RUNTIME_MODULES)
    fun modules(vararg modules: String) {
        this.modules.addAll(modules.toList())
    }
    var includeAllModules: Boolean = false

    val linux: LinuxPlatformSettings = objects.newInstance(LinuxPlatformSettings::class.java)
    open fun linux(fn: Action<LinuxPlatformSettings>) {
        fn.execute(linux)
    }

    val macOS: JvmMacOSPlatformSettings = objects.newInstance(JvmMacOSPlatformSettings::class.java)
    open fun macOS(fn: Action<JvmMacOSPlatformSettings>) {
        fn.execute(macOS)
    }

    val windows: WindowsPlatformSettings = objects.newInstance(WindowsPlatformSettings::class.java)
    fun windows(fn: Action<WindowsPlatformSettings>) {
        fn.execute(windows)
    }
    
    @JvmOverloads
    fun fileAssociation(
        mimeType: String, extension: String, description: String,
        linuxIconFile: File? = null, windowsIconFile: File? = null, macOSIconFile: File? = null,
    ) {
        linux.fileAssociation(mimeType, extension, description, linuxIconFile)
        windows.fileAssociation(mimeType, extension, description, windowsIconFile)
        macOS.fileAssociation(mimeType, extension, description, macOSIconFile)
    }
}
Repository: JetBrains/compose-multiplatform
File: gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/desktop/application/dsl/FileAssociation.kt (Kotlin)

Code before:
package org.jetbrains.compose.desktop.application.dsl

import java.io.Serializable

data class FileAssociation(
    val mimeType: String,
    val extension: String,
    val description: String,
) : Serializable


Review comment (suggestion):
please, don't use kotlin data classes in the public API
https://kotlinlang.org/docs/api-guidelines-backward-compatibility.html#avoid-using-data-classes-in-your-api

Code after:
package org.jetbrains.compose.desktop.application.dsl

import java.io.File
import java.io.Serializable

internal data class FileAssociation(
    val mimeType: String,
    val extension: String,
    val description: String,
    val iconFile: File?,
) : Serializable
Repository: JetBrains/compose-multiplatform
File: gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/desktop/application/dsl/JvmApplicationDistributions.kt (Kotlin)

Code before:
    }
    var includeAllModules: Boolean = false

    val linux: LinuxPlatformSettings = objects.newInstance(LinuxPlatformSettings::class.java)
    open fun linux(fn: Action<LinuxPlatformSettings>) {
        fn.execute(linux)
    }

    val macOS: JvmMacOSPlatformSettings = objects.newInstance(JvmMacOSPlatformSettings::class.java)
    open fun macOS(fn: Action<JvmMacOSPlatformSettings>) {
        fn.execute(macOS)
    }

    val windows: WindowsPlatformSettings = objects.newInstance(WindowsPlatformSettings::class.java)
    fun windows(fn: Action<WindowsPlatformSettings>) {
        fn.execute(windows)
    }
    
    fun fileAssociation(
        mimeType: String, extension: String, description: String,
        linuxIcon: File? = null, windowsIcon: File? = null, macOSIcon: File? = null,
    ) {
        linux.fileAssociation(mimeType, extension, description, linuxIcon)
        windows.fileAssociation(mimeType, extension, description, windowsIcon)
        macOS.fileAssociation(mimeType, extension, description, macOSIcon)
    }
}

Review comment (suggestion):
```suggestion
    fun fileAssociation(
        mimeType: String, extension: String, description: String,
        linuxIconFile: File? = null, windowsIconFile: File? = null, macOSIconFile: File? = null,
    ) {
        linux.fileAssociation(mimeType, extension, description, linuxIconFile)
        windows.fileAssociation(mimeType, extension, description, windowsIconFile)
        macOS.fileAssociation(mimeType, extension, description, macOSIconFile)
    }
```

Code after:
    }
    var includeAllModules: Boolean = false

    val linux: LinuxPlatformSettings = objects.newInstance(LinuxPlatformSettings::class.java)
    open fun linux(fn: Action<LinuxPlatformSettings>) {
        fn.execute(linux)
    }

    val macOS: JvmMacOSPlatformSettings = objects.newInstance(JvmMacOSPlatformSettings::class.java)
    open fun macOS(fn: Action<JvmMacOSPlatformSettings>) {
        fn.execute(macOS)
    }

    val windows: WindowsPlatformSettings = objects.newInstance(WindowsPlatformSettings::class.java)
    fun windows(fn: Action<WindowsPlatformSettings>) {
        fn.execute(windows)
    }
    
    @JvmOverloads
    fun fileAssociation(
        mimeType: String, extension: String, description: String,
        linuxIconFile: File? = null, windowsIconFile: File? = null, macOSIconFile: File? = null,
    ) {
        linux.fileAssociation(mimeType, extension, description, linuxIconFile)
        windows.fileAssociation(mimeType, extension, description, windowsIconFile)
        macOS.fileAssociation(mimeType, extension, description, macOSIconFile)
    }
}
Repository: JetBrains/compose-multiplatform
File: components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/ImageDecoders.kt (Kotlin)

Code before:
package org.jetbrains.compose.resources

import androidx.compose.ui.graphics.vector.ImageVector
import androidx.compose.ui.unit.Density
import org.jetbrains.compose.resources.vector.toImageVector

/**
 * Decodes a byte array of a vector XML file to an ImageVector.
 *
 * @return The converted ImageVector.
 */
@ExperimentalResourceApi
fun ByteArray.decodeToImageVector(density: Density): ImageVector {
    return this.toXmlElement().toImageVector(density)
}


Review comment (suggestion):
```suggestion
 * @param sourceDensity density to apply during converting the source units to the [ImageVector] units (px -> dp, dp -> px).
 * @return The converted ImageVector.
 */
@ExperimentalResourceApi
fun ByteArray.decodeToImageVector(sourceDensity: Density): ImageVector {
```


Code after:
import androidx.compose.ui.graphics.vector.ImageVector
import androidx.compose.ui.unit.Density
import org.jetbrains.compose.resources.vector.toImageVector

/**
 * Decodes a byte array of a Bitmap to an ImageBitmap. Supports JPEG, PNG, BMP, WEBP
 *
 * Different platforms can support additional formats.
 *
 * @return The converted ImageBitmap.
 */
@ExperimentalResourceApi
fun ByteArray.decodeToImageBitmap(): ImageBitmap {
    val dumbDensity = 0 //any equal source and target density disable scaling here
    return this.toImageBitmap(dumbDensity, dumbDensity)
}

/**
 * Decodes a byte array of a vector XML file to an ImageVector.
 *
 * @param density density to apply during converting the source units to the [ImageVector] units.
 *
 * @return The converted ImageVector.
 */
@ExperimentalResourceApi
fun ByteArray.decodeToImageVector(density: Density): ImageVector {
    return this.toXmlElement().toImageVector(density)
}
Repository: JetBrains/compose-multiplatform
File: components/resources/library/src/skikoMain/kotlin/org/jetbrains/compose/resources/ImageDecoders.skiko.kt (Kotlin)

Code before:
package org.jetbrains.compose.resources

import androidx.compose.ui.graphics.painter.Painter
import androidx.compose.ui.unit.Density

/**
 * Decodes a byte array of an SVG file to a compose Painter.
 *
 * @return The converted Painter.
 */
@ExperimentalResourceApi
fun ByteArray.decodeToSvgPainter(density: Density): Painter {
    return this.toSvgElement().toSvgPainter(density)
}

Review comment (suggestion):
```suggestion
 * @param sourceDensity density to apply during converting the source units to the [Painter] units (px -> dp, dp -> px).
 * @return The converted Painter.
 */
@ExperimentalResourceApi
fun ByteArray.decodeToSvgPainter(sourceDensity: Density): Painter {
```


Code after:
package org.jetbrains.compose.resources

import androidx.compose.ui.graphics.painter.Painter
import androidx.compose.ui.graphics.vector.ImageVector
import androidx.compose.ui.unit.Density

/**
 * Decodes a byte array of an SVG file to a compose Painter.
 *
 * @param density density to apply during converting the source units to the [Painter] units.
 *
 * @return The converted Painter.
 */
@ExperimentalResourceApi
fun ByteArray.decodeToSvgPainter(density: Density): Painter {
    return this.toSvgElement().toSvgPainter(density)
}
Repository: JetBrains/compose-multiplatform
File: benchmarks/multiplatform/benchmarks/src/commonMain/kotlin/MeasureComposable.kt (Kotlin)

Code before:
            frames
        )
    } finally {
        scene.close()
        surface.close()
        runGC() // cleanup for next benchmarks
    }
}

private val pictureRecorder = PictureRecorder()

/**
 * Mimic Skiko render logic from https://github.com/JetBrains/skiko/blob/eb1f04ec99d50ff0bdb2f592fdf49711a9251aa7/skiko/src/awtMain/kotlin/org/jetbrains/skiko/SkiaLayer.awt.kt#L531
 *
 * This is very simplified logic, and it still can differ from the real cases.
 *
 * Though one main function - rendering into picture - was affecting performance.
 *
 * Benchmarks showed an improvement by 10%, but there was a regression by 10%.
 *
 * Beware that this logic can be changed in some new version of Skiko.
 *
 * If Skiko stops using `picture`, we need to remove it here too.
 */
@OptIn(InternalComposeUiApi::class)
fun ComposeScene.mimicSkikoRender(surface: Surface, time: Long, width: Int, height: Int) {
    val pictureCanvas = pictureRecorder.beginRecording(Rect(0f, 0f, width.toFloat(), height.toFloat()))
    render(pictureCanvas.asComposeCanvas(), time)
    val picture = pictureRecorder.finishRecordingAsPicture()

    surface.canvas.clear(Color.TRANSPARENT)
    surface.canvas.drawPicture(picture)
    picture.close()
}


Review comment (suggestion):
I would rename it to reflect what it actually does , f.i `renderViaPicture`. 

Code after:
            frames
        )
    } finally {
        scene.close()
        surface.close()
        runGC() // cleanup for next benchmarks
    }
}

private val pictureRecorder = PictureRecorder()

/**
 * Mimic Skiko render logic from
 * https://github.com/JetBrains/skiko/blob/eb1f04ec99d50ff0bdb2f592fdf49711a9251aa7/skiko/src/awtMain/kotlin/org/jetbrains/skiko/SkiaLayer.awt.kt#L531
 *
 * This is a simplified logic and it still can differ from the real cases:
 * - Rendering into an intermediate picture. Benchmarks can show incorrect results without it.
 *   For example, we had a case when they showed an improvement by 10%,
 *   but there was a regression by 10%
 * - Clearing the canvas each frame
 *
 * Beware that this logic can be changed in some new version of Skiko.
 * If Skiko stops using `picture`, we need to remove it here too.
 */
@OptIn(InternalComposeUiApi::class)
fun ComposeScene.mimicSkikoRender(surface: Surface, time: Long, width: Int, height: Int) {
    val pictureCanvas = pictureRecorder.beginRecording(Rect(0f, 0f, width.toFloat(), height.toFloat()))
    render(pictureCanvas.asComposeCanvas(), time)
    val picture = pictureRecorder.finishRecordingAsPicture()

    surface.canvas.clear(Color.TRANSPARENT)
    surface.canvas.drawPicture(picture)
    picture.close()
}
Repository: JetBrains/compose-multiplatform
File: components/ui-tooling-preview/library/build.gradle.kts (Kotlin)

Code before:
    macosX64()
    macosArm64()
}

android {
    compileSdk = 35
    namespace = "org.jetbrains.compose.ui.tooling.preview"
    defaultConfig {
        minSdk = 21
        testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
    }
    compileOptions {
        sourceCompatibility = JavaVersion.VERSION_11
        targetCompatibility = JavaVersion.VERSION_11
    }
}

configureMavenPublication(
    groupId = "org.jetbrains.compose.components",
    artifactId = "components-ui-tooling-preview",
    name = "Experimental Compose Multiplatform tooling library API. This library provides the API required to declare " +
            "@Preview composables in user apps."
)

dependencies {
    compileOnly(libs.androidx.ui.tooling.preview.android)
}


Review comment (bug):
Doesn't this mean, that this will crash:
```
class SampleUserProvider: PreviewParameterProvider<User> {
    override val values = sequenceOf(User("Jens",31),User("Jim",44))
}
```
If users don't include it explicitly?

If so, we have to fix it. The common preview ideally should require including only it

Code after:
    macosArm64()

    applyDefaultHierarchyTemplate()

    sourceSets {
        val commonMain by getting

        androidMain.dependencies {
            api(libs.androidx.ui.tooling.preview)
        }

        val nonAndroidMain by creating {
            dependsOn(commonMain)
        }

        val appleMain by getting
        appleMain.dependsOn(nonAndroidMain)

        val desktopMain by getting
        desktopMain.dependsOn(nonAndroidMain)

        val jsMain by getting
        jsMain.dependsOn(nonAndroidMain)

        val wasmJsMain by getting
        wasmJsMain.dependsOn(nonAndroidMain)
    }
}

android {
    compileSdk = 35
    namespace = "org.jetbrains.compose.ui.tooling.preview"
    defaultConfig {
        minSdk = 21
        testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
    }
    compileOptions {
        sourceCompatibility = JavaVersion.VERSION_11
        targetCompatibility = JavaVersion.VERSION_11
    }
}

configureMavenPublication(
    groupId = "org.jetbrains.compose.components",
    artifactId = "components-ui-tooling-preview",
    name = "Experimental Compose Multiplatform tooling library API. This library provides the API required to declare " +
            "@Preview composables in user apps."
)
Repository: JetBrains/compose-multiplatform
File: components/resources/library/src/desktopTest/kotlin/org/jetbrains/compose/resources/CustomClassloaderTest.kt (Kotlin)

Code before:
@file:OptIn(ExperimentalTestApi::class)

package org.jetbrains.compose.resources

import androidx.compose.ui.test.ExperimentalTestApi
import kotlinx.coroutines.test.runTest
import java.net.URLClassLoader
import kotlin.io.path.Path
import kotlin.io.path.isDirectory
import kotlin.test.Test
import kotlin.test.assertEquals


class CustomClassloaderTest {

    val RESOURCES_PATH
        get() = System.getenv("RESOURCES_PATH")
            ?.let { Path(it) }
            ?.takeIf { it.isDirectory() }
            ?: error("RESOURCES_PATH environment variable is not set or is not a directory")

    @Test
    fun testCustomClassloader() = runTest {
        val actualResourceText =
            CustomClassloaderTest::class
                .java
                .classLoader
                .getResourceAsStream("hello.txt")
                ?.readAllBytes()
                ?.toString(Charsets.UTF_8)

        val classloader = URLClassLoader(arrayOf(RESOURCES_PATH.toUri().toURL()), null)
        val reader = JvmResourceReader(classloader)
        assertEquals(
            expected = actualResourceText,
            actual = reader.read("hello.txt").toString(Charsets.UTF_8)
        )
    }
}



Review comment (suggestion):
Do you need `runTest`? I don't see any coroutine APIs being used in the test

Code after:
@file:OptIn(ExperimentalTestApi::class)

package org.jetbrains.compose.resources

import androidx.compose.ui.test.ExperimentalTestApi
import kotlinx.coroutines.test.runTest
import java.net.URLClassLoader
import kotlin.io.path.Path
import kotlin.io.path.isDirectory
import kotlin.test.Test
import kotlin.test.assertEquals


class CustomClassloaderTest {

    val RESOURCES_PATH
        get() = System.getenv("RESOURCES_PATH")
            ?.let { Path(it) }
            ?.takeIf { it.isDirectory() }
            ?: error("RESOURCES_PATH environment variable is not set or is not a directory")

    @Test
    fun testCustomClassloader() = runTest {
        val actualResourceText =
            CustomClassloaderTest::class
                .java
                .classLoader
                .getResourceAsStream("hello.txt")
                ?.readAllBytes()
                ?.toString(Charsets.UTF_8)

        val classloader = URLClassLoader(arrayOf(RESOURCES_PATH.toUri().toURL()), null)
        val reader = JvmResourceReader(classloader)
        assertEquals(
            expected = actualResourceText,
            actual = reader.read("hello.txt").toString(Charsets.UTF_8)
        )
    }
}
Repository: JetBrains/compose-multiplatform
File: components/resources/library/src/desktopMain/kotlin/org/jetbrains/compose/resources/ResourceReader.desktop.kt (Kotlin)

Code before:
package org.jetbrains.compose.resources

import java.io.InputStream

actual fun getDefaultResourceReader(): ResourceReader =
    JvmResourceReader.DEFAULT

class JvmResourceReader(
    private val classLoader: ClassLoader = JvmResourceReader::class.java.classLoader
) : ResourceReader {

    companion object {
        val DEFAULT = JvmResourceReader()
    }

    override suspend fun read(path: String): ByteArray =
        getResourceAsStream(path)
            .use { input -> input.readBytes() }

    override suspend fun readPart(path: String, offset: Long, size: Long): ByteArray {
        val resource = getResourceAsStream(path)
        val result = ByteArray(size.toInt())
        resource.use { input ->
            input.skipBytes(offset)
            input.readNBytes(result, 0, size.toInt())
        }
        return result
    }

    //skipNBytes requires API 12
    private fun InputStream.skipBytes(offset: Long) {
        var skippedBytes = 0L
        while (skippedBytes < offset) {
            val count = skip(offset - skippedBytes)
            if (count == 0L) break
            skippedBytes += count
        }
    }

Review comment (suggestion):
```suggestion
        val Default = JvmResourceReader()
```
the Compose style guidelines are different from Kotlin guidelines

(cc @terrakok, we should take this into account for all future API, even if we already used another style)

Code after:
package org.jetbrains.compose.resources

import java.io.InputStream

@ExperimentalResourceApi
internal actual fun getPlatformResourceReader(): ResourceReader =
    JvmResourceReader.Default

@ExperimentalResourceApi
class JvmResourceReader(
    private val classLoader: ClassLoader
) : ResourceReader {

    companion object {
        internal val Default = JvmResourceReader(JvmResourceReader::class.java.classLoader)
    }

    override suspend fun read(path: String): ByteArray {
        val resource = getResourceAsStream(path)
        return resource.use { input -> input.readBytes() }
    }

    override suspend fun readPart(path: String, offset: Long, size: Long): ByteArray {
        val resource = getResourceAsStream(path)
        val result = ByteArray(size.toInt())
        resource.use { input ->
            input.skipBytes(offset)
            input.readNBytes(result, 0, size.toInt())
        }
        return result
    }

    //skipNBytes requires API 12
    private fun InputStream.skipBytes(offset: Long) {
        var skippedBytes = 0L
        while (skippedBytes < offset) {
            val count = skip(offset - skippedBytes)
            if (count == 0L) break
Repository: JetBrains/compose-multiplatform
File: components/resources/library/src/desktopMain/kotlin/org/jetbrains/compose/resources/ResourceReader.desktop.kt (Kotlin)

Code before:
package org.jetbrains.compose.resources

import java.io.InputStream

actual fun getDefaultResourceReader(): ResourceReader =
    JvmResourceReader.DEFAULT

class JvmResourceReader(
    private val classLoader: ClassLoader = JvmResourceReader::class.java.classLoader
) : ResourceReader {

    companion object {
        val DEFAULT = JvmResourceReader()
    }

    override suspend fun read(path: String): ByteArray =
        getResourceAsStream(path)
            .use { input -> input.readBytes() }

    override suspend fun readPart(path: String, offset: Long, size: Long): ByteArray {
        val resource = getResourceAsStream(path)
        val result = ByteArray(size.toInt())
        resource.use { input ->
            input.skipBytes(offset)
            input.readNBytes(result, 0, size.toInt())
        }
        return result
    }

    //skipNBytes requires API 12
    private fun InputStream.skipBytes(offset: Long) {
        var skippedBytes = 0L
        while (skippedBytes < offset) {
            val count = skip(offset - skippedBytes)
            if (count == 0L) break
            skippedBytes += count
        }
    }

Review comment (suggestion):
Consider making a top-level `val DefaultJvmResourceReader` to be consistent with the other targets

Code after:
package org.jetbrains.compose.resources

import java.io.InputStream

@ExperimentalResourceApi
internal actual fun getPlatformResourceReader(): ResourceReader =
    JvmResourceReader.Default

@ExperimentalResourceApi
class JvmResourceReader(
    private val classLoader: ClassLoader
) : ResourceReader {

    companion object {
        internal val Default = JvmResourceReader(JvmResourceReader::class.java.classLoader)
    }

    override suspend fun read(path: String): ByteArray {
        val resource = getResourceAsStream(path)
        return resource.use { input -> input.readBytes() }
    }

    override suspend fun readPart(path: String, offset: Long, size: Long): ByteArray {
        val resource = getResourceAsStream(path)
        val result = ByteArray(size.toInt())
        resource.use { input ->
            input.skipBytes(offset)
            input.readNBytes(result, 0, size.toInt())
        }
        return result
    }

    //skipNBytes requires API 12
    private fun InputStream.skipBytes(offset: Long) {
        var skippedBytes = 0L
        while (skippedBytes < offset) {
            val count = skip(offset - skippedBytes)
            if (count == 0L) break