Skip to content

Commit

Permalink
Disallow open @composable functions with default params
Browse files Browse the repository at this point in the history 
Old version of Compose compiler allowed open functions with default params without a proper wrapper around it. We need to add some feature detection for precompiled artifacts to make sure we handle those functions correctly before enabling it.

Fixes: 357878245
Relnote: Disallow open @composable functions with default params to fix binary compatibility issues.
  • Loading branch information
@ShikaSD
ShikaSD authored and Space Cloud committed Aug 8, 2024
1 parent 9a4c77e commit ca9fb23
Show file tree
Hide file tree
Showing 18 changed files with 1,179 additions and 47 deletions.
34 changes: 30 additions & 4 deletions ...src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/ComposeBytecodeCodegenTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package androidx.compose.compiler.plugins.kotlin

import org.junit.Assume.assumeFalse
import org.junit.Test
import kotlin.test.Ignore
import kotlin.test.assertFalse
import kotlin.test.assertTrue

Expand Down Expand Up @@ -623,25 +624,50 @@ class ComposeBytecodeCodegenTest(useFir: Boolean) : AbstractCodegenTest(useFir)
)

@Test
fun testDefaultParametersInVirtualFunctions() = validateBytecode(
fun testDefaultParametersInAbstractFunctions() = validateBytecode(
"""
import androidx.compose.runtime.*
interface Test {
@Composable fun foo(param: Int = remember { 0 })
@Composable fun bar(param: Int = remember { 0 }): Int = param
}
class TestImpl : Test {
@Composable override fun foo(param: Int) {}
}
@Composable fun CallWithDefaults(test: Test) {
test.foo()
test.foo(0)
}
""",
validate = {
assertTrue(
it.contains(
"INVOKESTATIC test/Test%ComposeDefaultImpls.foo%default (ILtest/Test;Landroidx/compose/runtime/Composer;II)V"
),
"default static functions should be generated in ComposeDefaultsImpl class"
)
}
)

@Ignore("b/357878245")
@Test
fun testDefaultParametersInOpenFunctions() = validateBytecode(
"""
import androidx.compose.runtime.*
interface Test {
@Composable fun bar(param: Int = remember { 0 }): Int = param
}
class TestImpl : Test {
@Composable override fun bar(param: Int): Int {
return super.bar(param)
}
}
@Composable fun CallWithDefaults(test: Test) {
test.foo()
test.foo(0)
test.bar()
test.bar(0)
}
Expand Down
4 changes: 3 additions & 1 deletion ...ts/src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/ComposeCrossModuleTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import org.junit.Test
import org.junit.rules.TemporaryFolder
import org.junit.runner.RunWith
import org.junit.runners.Parameterized
import kotlin.test.Ignore

@RunWith(Parameterized::class)
class ComposeCrossModuleTests(useFir: Boolean) : AbstractCodegenTest(useFir) {
Expand Down Expand Up @@ -1224,8 +1225,9 @@ class ComposeCrossModuleTests(useFir: Boolean) : AbstractCodegenTest(useFir) {
)
}

@Ignore("b/357878245")
@Test
fun defaultParametersInFakeOverrideVirtualComposableFunctions() {
fun defaultParametersInFakeOverrideOpenComposableFunctions() {
compile(
mapOf(
"Base" to mapOf(
Expand Down
94 changes: 78 additions & 16 deletions ...src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/DefaultParamTransformTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package androidx.compose.compiler.plugins.kotlin

import org.intellij.lang.annotations.Language
import org.junit.Ignore
import org.junit.Test

class DefaultParamTransformTests(useFir: Boolean) : AbstractIrTransformTest(useFir) {
Expand Down Expand Up @@ -405,16 +406,49 @@ class DefaultParamTransformTests(useFir: Boolean) : AbstractIrTransformTest(useF
)

@Test
fun testDefaultParamOnInterface() = defaultParams(
fun testAbstractDefaultParamOnInterface() = defaultParams(
unchecked = """""",
checked = """
interface Test {
@Composable fun foo(param: Int = remember { 0 })
@Composable fun bar(param: Int = remember { 0 }): Int = param
}
interface TestBetween : Test {
@Composable fun betweenFoo(param: Int = remember { 0 })
}
class TestImpl : TestBetween {
@Composable override fun foo(param: Int) {}
@Composable override fun betweenFoo(param: Int) {}
}
@Composable fun CallWithDefaults(test: Test, testBetween: TestBetween, testImpl: TestImpl) {
test.foo()
test.foo(0)
testBetween.foo()
testBetween.foo(0)
testBetween.betweenFoo()
testBetween.betweenFoo(0)
testImpl.foo()
testImpl.foo(0)
testImpl.betweenFoo()
testImpl.betweenFoo(0)
}
"""
)

@Ignore("b/357878245")
@Test
fun testOpenDefaultParamOnInterface() = defaultParams(
unchecked = """""",
checked = """
interface Test {
@Composable fun bar(param: Int = remember { 0 }): Int = param
}
interface TestBetween : Test {
@Composable fun betweenFooDefault(param: Int = remember { 0 }) {}
@Composable fun betweenBar(param: Int = remember { 0 }): Int = param
}
Expand All @@ -428,28 +462,18 @@ class DefaultParamTransformTests(useFir: Boolean) : AbstractIrTransformTest(useF
}
@Composable fun CallWithDefaults(test: Test, testBetween: TestBetween, testImpl: TestImpl) {
test.foo()
test.foo(0)
test.bar()
test.bar(0)
testBetween.foo()
testBetween.foo(0)
testBetween.bar()
testBetween.bar(0)
testBetween.betweenFoo()
testBetween.betweenFoo(0)
testBetween.betweenFooDefault()
testBetween.betweenFooDefault(0)
testBetween.betweenBar()
testBetween.betweenBar(0)
testImpl.foo()
testImpl.foo(0)
testImpl.bar()
testImpl.bar(0)
testImpl.betweenFoo()
testImpl.betweenFoo(0)
testImpl.betweenFooDefault()
testImpl.betweenFooDefault(0)
testImpl.betweenBar()
Expand All @@ -458,6 +482,7 @@ class DefaultParamTransformTests(useFir: Boolean) : AbstractIrTransformTest(useF
"""
)

@Ignore("b/357878245")
@Test
fun testDefaultParamOverrideOpenFunction() = defaultParams(
unchecked = """""",
Expand All @@ -484,30 +509,49 @@ class DefaultParamTransformTests(useFir: Boolean) : AbstractIrTransformTest(useF
)

@Test
fun testDefaultParamOverrideExtensionReceiver() = defaultParams(
fun testAbstractDefaultParamOverrideExtensionReceiver() = defaultParams(
unchecked = "",
checked = """
interface Test {
@Composable fun Int.foo(param: Int = remember { 0 })
@Composable fun Int.bar(param: Int = remember { 0 }): Int = param
}
class TestImpl : Test {
@Composable override fun Int.foo(param: Int) {}
@Composable override fun Int.bar(param: Int): Int = 0
}
@Composable fun CallWithDefaults(test: Test) {
with(test) {
42.foo()
42.foo(0)
}
}
"""
)

@Ignore("b/357878245")
@Test
fun testOpenDefaultParamOverrideExtensionReceiver() = defaultParams(
unchecked = "",
checked = """
interface Test {
@Composable fun Int.bar(param: Int = remember { 0 }): Int = param
}
class TestImpl : Test {
@Composable override fun Int.bar(param: Int): Int = 0
}
@Composable fun CallWithDefaults(test: Test) {
with(test) {
42.bar()
42.bar(0)
}
}
"""
)

@Ignore("b/357878245")
@Test
fun testDefaultParamFakeOverride() = defaultParams(
unchecked = "",
Expand All @@ -531,12 +575,30 @@ class DefaultParamTransformTests(useFir: Boolean) : AbstractIrTransformTest(useF
)

@Test
fun testDefaultParamComposableLambda() = defaultParams(
fun testAbstractDefaultParamComposableLambda() = defaultParams(
unchecked = """
@Composable fun Text(value: String) {}
""",
checked = """
private interface DefaultParamInterface {
@Composable fun Content(
content: @Composable () -> Unit = @Composable { ComposedContent { Text("default") } }
)
@Composable fun ComposedContent(
content: @Composable () -> Unit = @Composable { Text("default") }
)
}
""",
)

@Ignore("b/357878245")
@Test
fun testOpenDefaultParamComposableLambda() = defaultParams(
unchecked = """
@Composable fun Text(value: String) {}
""",
checked = """
private interface DefaultParamInterface {
@Composable fun Content(
content: @Composable () -> Unit = @Composable { ComposedContent { Text("default") } }
)
Expand Down
4 changes: 2 additions & 2 deletions ...in/androidx/compose/compiler/plugins/kotlin/analysis/ComposableDeclarationCheckerTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ class ComposableDeclarationCheckerTests(useFir: Boolean) : AbstractComposeDiagno
"""
import androidx.compose.runtime.Composable
interface A {
@Composable fun foo(x: Int = 0) {}
@Composable fun foo(x: Int = <!ABSTRACT_COMPOSABLE_DEFAULT_PARAMETER_VALUE!>0<!>) {}
}
"""
)
Expand All @@ -314,7 +314,7 @@ class ComposableDeclarationCheckerTests(useFir: Boolean) : AbstractComposeDiagno
"""
import androidx.compose.runtime.Composable
open class A {
@Composable open fun foo(x: Int = 0) {}
@Composable open fun foo(x: Int = <!ABSTRACT_COMPOSABLE_DEFAULT_PARAMETER_VALUE!>0<!>) {}
}
"""
)
Expand Down
Loading

0 comments on commit ca9fb23

Please sign in to comment.