Updates UnnecessaryLambdaCreationDetector to handle lambda subtypes
E.g:
interface Foo
class FooImpl : Foo
val onFooImpl: (Foo) -> Unit = {}
fun foo(onFoo: (FooImpl) -> Unit) {}
foo(onFooImpl) is valid
Also fixes a bug with the lint check not handling lambdas returned by a function, such as:
{ foo()() }
where fun foo(): () -> Unit
Test: UnnecessaryLambdaCreationDetectorTest
Change-Id: Ibea2208e35a6dfe4075e99b7f81c6f8a7dfa0ed2
diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/input/internal/LegacyAdaptingPlatformTextInputModifierNode.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/input/internal/LegacyAdaptingPlatformTextInputModifierNode.kt
index 000655e..57c3a12 100644
--- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/input/internal/LegacyAdaptingPlatformTextInputModifierNode.kt
+++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/input/internal/LegacyAdaptingPlatformTextInputModifierNode.kt
@@ -108,9 +108,7 @@
): Job? {
if (!isAttached) return null
return coroutineScope.launch(start = CoroutineStart.UNDISPATCHED) {
- establishTextInputSession {
- block()
- }
+ establishTextInputSession(block)
}
}
}
diff --git a/compose/lint/common/src/main/java/androidx/compose/lint/ComposableUtils.kt b/compose/lint/common/src/main/java/androidx/compose/lint/ComposableUtils.kt
index cc8ef54..0cdf01c 100644
--- a/compose/lint/common/src/main/java/androidx/compose/lint/ComposableUtils.kt
+++ b/compose/lint/common/src/main/java/androidx/compose/lint/ComposableUtils.kt
@@ -263,7 +263,7 @@
/**
* Returns whether this type reference is @Composable or not
*/
-private val UTypeReferenceExpression.isComposable: Boolean
+val UTypeReferenceExpression.isComposable: Boolean
get() {
if (type.hasAnnotation(Names.Runtime.Composable.javaFqn)) return true
diff --git a/compose/lint/internal-lint-checks/src/main/java/androidx/compose/lint/UnnecessaryLambdaCreationDetector.kt b/compose/lint/internal-lint-checks/src/main/java/androidx/compose/lint/UnnecessaryLambdaCreationDetector.kt
index 70c0df9..41c356b 100644
--- a/compose/lint/internal-lint-checks/src/main/java/androidx/compose/lint/UnnecessaryLambdaCreationDetector.kt
+++ b/compose/lint/internal-lint-checks/src/main/java/androidx/compose/lint/UnnecessaryLambdaCreationDetector.kt
@@ -39,8 +39,8 @@
import org.jetbrains.uast.UCallExpression
import org.jetbrains.uast.UElement
import org.jetbrains.uast.ULambdaExpression
+import org.jetbrains.uast.UMethod
import org.jetbrains.uast.UReturnExpression
-import org.jetbrains.uast.UUnknownExpression
import org.jetbrains.uast.UVariable
import org.jetbrains.uast.skipParenthesizedExprDown
import org.jetbrains.uast.toUElement
@@ -73,18 +73,13 @@
* This handler visits every lambda expression and reports an issue if the following criteria
* (in order) hold true:
*
- * 1. There is only one expression inside the lambda.
- * 2. The expression is a function call
- * 3. The lambda is being invoked as part of a function call, and not as a property assignment
+ * 1. There is only one expression inside the lambda
+ * 2. The lambda literal is created as part of a function call, and not as a property assignment
* such as val foo = @Composable {}
- * 4. The receiver type of the function call is `Function0` (i.e, we are invoking something
- * that matches `() -> Unit` - this both avoids non-lambda invocations but also makes sure
- * that we don't warn for lambdas that have parameters, such as @Composable (Int) -> Unit
- * - this cannot be inlined.)
- * 5. The outer function call that contains this lambda is not a call to a `LayoutNode`
- * (because these are technically constructor invocations that we just intercept calls to
- * there is no way to avoid using a trailing lambda for this)
- * 6. The lambda is not being passed as a parameter, for example `Foo { lambda -> lambda() }`
+ * 3. The expression is an invoke() call
+ * 4. The receiver type of the invoke call is a functional type, and it is a subtype of (i.e
+ * compatible to cast to) the lambda parameter functional type
+ * 5. The lambda parameter and literal have matching composability
*/
class UnnecessaryLambdaCreationHandler(private val context: JavaContext) : UElementHandler() {
@@ -97,7 +92,7 @@
is UCallExpression -> expr
is UReturnExpression -> {
if (expr.sourcePsi == null) { // implicit return
- expr.returnExpression as? UCallExpression
+ expr.returnExpression?.skipParenthesizedExprDown() as? UCallExpression
} else null
}
else -> null
@@ -126,9 +121,7 @@
analyze(expressionSourcePsi) {
val functionType = dispatchReceiverType(expressionSourcePsi) ?: return
val argumentType = toLambdaFunctionalType(node) ?: return
- // TODO enable subType checking
-// if (!(functionType isSubTypeOf argumentType)) return
- if (!(functionType isEqualTo argumentType)) return
+ if (!(functionType isSubTypeOf argumentType)) return
}
val expectedComposable = node.isComposable
@@ -144,10 +137,17 @@
val isComposable = when (resolvedLambdaSource) {
is UVariable -> resolvedLambdaSource.isComposable
- // TODO: if the resolved source is a parameter in a local function, it
- // incorrectly returns an UnknownKotlinExpression instead of a UParameter
- // https://youtrack.jetbrains.com/issue/KTIJ-19125
- is UUnknownExpression -> return
+ // If the source is a method, then the lambda is the return type of the method, so
+ // check the return type
+ is UMethod -> resolvedLambdaSource.returnTypeReference?.isComposable == true
+ // Safe return if we failed to resolve. This can happen for implicit `it` parameters
+ // that are lambdas, but this should only happen exceptionally for lambdas with
+ // an `Any` parameter, such as { any: Any -> }.let { it(Any()) }, since this passes
+ // the isSubTypeOf check above. In this case it isn't possible to inline this call,
+ // so no need to handle these implicit parameters.
+ null -> return
+ // Throw since this is an internal check, and we want to fix this for unknown types.
+ // If making this check public, it's safer to return instead without throwing.
else -> error(parentExpression.asSourceString())
}
diff --git a/compose/lint/internal-lint-checks/src/test/java/androidx/compose/lint/UnnecessaryLambdaCreationDetectorTest.kt b/compose/lint/internal-lint-checks/src/test/java/androidx/compose/lint/UnnecessaryLambdaCreationDetectorTest.kt
index c512441..1c026c7 100644
--- a/compose/lint/internal-lint-checks/src/test/java/androidx/compose/lint/UnnecessaryLambdaCreationDetectorTest.kt
+++ b/compose/lint/internal-lint-checks/src/test/java/androidx/compose/lint/UnnecessaryLambdaCreationDetectorTest.kt
@@ -363,5 +363,59 @@
"""
).expectClean()
}
+
+ @Test
+ fun warnsForFunctionsReturningALambda() {
+ check(
+ """
+ package test
+
+ import androidx.compose.runtime.Composable
+
+ fun returnsLambda(): () -> Unit = {}
+ fun returnsComposableLambda(): @Composable () -> Unit = {}
+
+ @Composable
+ fun Test() {
+ ComposableFunction {
+ returnsLambda()()
+ }
+
+ InlineComposableFunction {
+ returnsLambda()()
+ }
+
+ ReifiedComposableFunction<Any> {
+ returnsLambda()()
+ }
+
+ ComposableFunction {
+ returnsComposableLambda()()
+ }
+
+ InlineComposableFunction {
+ returnsComposableLambda()()
+ }
+
+ ReifiedComposableFunction<Any> {
+ returnsComposableLambda()()
+ }
+ }
+ """
+ ).expect(
+ """
+src/test/test.kt:23: Error: Creating an unnecessary lambda to emit a captured lambda [UnnecessaryLambdaCreation]
+ returnsComposableLambda()()
+ ~
+src/test/test.kt:27: Error: Creating an unnecessary lambda to emit a captured lambda [UnnecessaryLambdaCreation]
+ returnsComposableLambda()()
+ ~
+src/test/test.kt:31: Error: Creating an unnecessary lambda to emit a captured lambda [UnnecessaryLambdaCreation]
+ returnsComposableLambda()()
+ ~
+3 errors, 0 warnings
+ """
+ )
+ }
}
/* ktlint-enable max-line-length */
diff --git a/glance/glance-appwidget/src/main/java/androidx/glance/appwidget/lazy/LazyList.kt b/glance/glance-appwidget/src/main/java/androidx/glance/appwidget/lazy/LazyList.kt
index 2430387..f2faf26 100644
--- a/glance/glance-appwidget/src/main/java/androidx/glance/appwidget/lazy/LazyList.kt
+++ b/glance/glance-appwidget/src/main/java/androidx/glance/appwidget/lazy/LazyList.kt
@@ -126,7 +126,7 @@
?: (ReservedItemIdRangeEnd - index)
check(id != LazyListScope.UnspecifiedItemId) { "Implicit list item ids exhausted." }
LazyListItem(id, alignment) {
- object : LazyItemScope { }.apply { composable() }
+ object : LazyItemScope { }.composable()
}
}
}
diff --git a/glance/glance-appwidget/src/main/java/androidx/glance/appwidget/lazy/LazyVerticalGrid.kt b/glance/glance-appwidget/src/main/java/androidx/glance/appwidget/lazy/LazyVerticalGrid.kt
index 53a41ce..71d0724 100644
--- a/glance/glance-appwidget/src/main/java/androidx/glance/appwidget/lazy/LazyVerticalGrid.kt
+++ b/glance/glance-appwidget/src/main/java/androidx/glance/appwidget/lazy/LazyVerticalGrid.kt
@@ -135,7 +135,7 @@
"Implicit list item ids exhausted."
}
LazyVerticalGridItem(id, alignment) {
- object : LazyItemScope { }.apply { composable() }
+ object : LazyItemScope { }.composable()
}
}
}
diff --git a/glance/glance-wear-tiles/src/main/java/androidx/glance/wear/tiles/curved/CurvedRow.kt b/glance/glance-wear-tiles/src/main/java/androidx/glance/wear/tiles/curved/CurvedRow.kt
index 58489b1..33afc1f 100644
--- a/glance/glance-wear-tiles/src/main/java/androidx/glance/wear/tiles/curved/CurvedRow.kt
+++ b/glance/glance-wear-tiles/src/main/java/androidx/glance/wear/tiles/curved/CurvedRow.kt
@@ -124,7 +124,7 @@
return {
curvedChildList.forEach { composable ->
- object : CurvedChildScope {}.apply { composable() }
+ object : CurvedChildScope {}.composable()
}
}
}
