Add LambdaParameterEventTrailing rule

docs/detekt.md

@@ -51,6 +51,12 @@ Compose:
     # treatAsComposableLambda: MyComposableLambdaType
   DefaultsVisibility:
     active: true
+  LambdaParameterEventTrailing:
+    active: true
+    # -- You can optionally add your own composables here
+    # contentEmitters: MyComposable,MyOtherComposable
+    # -- You can add composables here that you don't want to count as content emitters (e.g. custom dialogs or modals)
+    # contentEmittersDenylist: MyNonEmitterComposable
   LambdaParameterInRestartableEffect:
     active: true
     # -- You can optionally have a list of types to be treated as lambdas (e.g. typedefs or fun interfaces not picked up automatically)

docs/rules.md

@@ -20,7 +20,7 @@ More information: [State and Jetpack Compose](https://developer.android.com/jetp
 
 ### State should be remembered in composables
 
-Be careful when using `mutableStateOf` (or any of the other state builders) to make sure that you `remember` the instance. If you don't `remember` the state instance, a new state instance will be created when the function is recomposed.
+Be careful when using `mutableStateOf` (or any of the other `State<T>` builders) to make sure that you `remember` the instance. If you don't `remember` the state instance, a new state instance will be created when the function is recomposed.
 
 !!! info ""
 
@@ -236,6 +236,45 @@ fun Profile(user: User, modifier: Modifier = Modifier) {
 
     :material-chevron-right-box: [compose:content-trailing-lambda](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ContentTrailingLambda.kt) ktlint :material-chevron-right-box: [ContentTrailingLambda](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ContentTrailingLambda.kt) detekt
 
+### Avoid using the trailing lambda for event lambdas in UI Composables
+
+In Compose, trailing lambdas in composable functions are typically used for content slots. To avoid confusion and maintain consistency, event lambdas (e.g., `onClick`, `onValueChange`) should generally not be placed in the trailing position.
+
+Recommendations:
+
+- **Required** Event Lambdas: Place required event lambdas before the `Modifier` parameter. This clearly distinguishes them from content slots.
+- **Optional** Event Lambdas: When possible, avoid placing optional event lambdas as the last parameter. If an optional event lambda must be positioned at the end, consider adding a clarifying comment to the function definition.
+
+```kotlin
+// ❌ Using an event lambda (like onClick) as the trailing lambda when in a composable makes it error prone and awkward to read
+@Composable
+fun MyButton(modifier: Modifier = Modifier, onClick: () -> Unit) { /* ... */ }
+
+@Composable
+fun SomeUI(modifier: Modifier = Modifier) {
+    MyButton {
+        // This is an onClick, but by reading it people would assume it's a content slot
+    }
+}
+
+// ✅ By moving the event lambda to be before Modifier, we avoid confusion
+@Composable
+fun MyBetterButton(onClick: () -> Unit, modifier: Modifier = Modifier) { /* ... */ }
+
+@Composable
+fun SomeUI(modifier: Modifier = Modifier) {
+    MyBetterButton(
+        onClick = {
+            // Now this param is straightforward to understand
+        },
+    )
+}
+```
+
+!!! info ""
+
+    :material-chevron-right-box: [compose:lambda-param-event-trailing](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ContentTrailingLambda.kt) ktlint :material-chevron-right-box: [LambdaParameterEventTrailing](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/LambdaParameterEventTrailing.kt) detekt
+
 ### Naming CompositionLocals properly
 
 `CompositionLocal`s should be named by using the adjective `Local` as prefix, followed by a descriptive noun that describes the value they hold. This makes it easier to know when a value comes from a `CompositionLocal`. Given that these are implicit dependencies, we should make them obvious.
@@ -328,11 +367,11 @@ To try to enforce common standard, and for consistency’s sake, we'll want to a
 ```kotlin
 // ❌
 @Composable
-fun Avatar(onShown: () -> Unit, onChanged: () -> Unit) { ... }
+fun Avatar(onShown: () -> Unit, onChanged: () -> Unit) { /* ... */ }
 
 // ✅
 @Composable
-fun Avatar(onShow: () -> Unit, onChange: () -> Unit) { ... }
+fun Avatar(onShow: () -> Unit, onChange: () -> Unit) { /* ... */ }
 ```
 
 !!! info ""

rules/common/src/main/kotlin/io/nlopez/compose/core/util/Lambdas.kt

@@ -13,19 +13,19 @@ import org.jetbrains.kotlin.psi.KtTypeElement
 import org.jetbrains.kotlin.psi.KtTypeReference
 import org.jetbrains.kotlin.psi.KtUserType
 
-fun KtTypeElement.isLambda(treatAsLambdaTypes: Set<String>): Boolean = when (this) {
+fun KtTypeElement.isLambda(treatAsLambdaTypes: Set<String> = emptySet()): Boolean = when (this) {
     is KtFunctionType -> true
     is KtNullableType -> innerType?.isLambda(treatAsLambdaTypes) == true
     is KtUserType -> referencedName in treatAsLambdaTypes
     else -> false
 }
 
-fun KtTypeReference.isLambda(treatAsLambdaTypes: Set<String>): Boolean =
+fun KtTypeReference.isLambda(treatAsLambdaTypes: Set<String> = emptySet()): Boolean =
     typeElement?.isLambda(treatAsLambdaTypes) == true
 
 fun KtTypeReference.isComposableLambda(
-    treatAsLambdaTypes: Set<String>,
-    treatAsComposableLambdaTypes: Set<String>,
+    treatAsLambdaTypes: Set<String> = emptySet(),
+    treatAsComposableLambdaTypes: Set<String> = emptySet(),
 ): Boolean = when (val element = typeElement) {
     null -> false
     is KtFunctionType -> isComposable

rules/common/src/main/kotlin/io/nlopez/compose/rules/LambdaParameterEventTrailing.kt

@@ -0,0 +1,51 @@
+// Copyright 2024 Nacho Lopez
+// SPDX-License-Identifier: Apache-2.0
+package io.nlopez.compose.rules
+
+import io.nlopez.compose.core.ComposeKtConfig
+import io.nlopez.compose.core.ComposeKtVisitor
+import io.nlopez.compose.core.Emitter
+import io.nlopez.compose.core.report
+import io.nlopez.compose.core.util.emitsContent
+import io.nlopez.compose.core.util.isComposable
+import io.nlopez.compose.core.util.isLambda
+import io.nlopez.compose.core.util.modifierParameter
+import org.jetbrains.kotlin.psi.KtFunction
+
+class LambdaParameterEventTrailing : ComposeKtVisitor {
+    override fun visitComposable(function: KtFunction, emitter: Emitter, config: ComposeKtConfig) {
+        // We want this rule to only run for functions that emit content
+        if (!function.emitsContent(config)) return
+
+        // We'd also want for it to have a modifier to apply the rule, which serves two purposes:
+        // - making sure that it's the separation between required and optional parameters
+        // - the lambda would be able to be moved before the modifier and not be the trailing one
+        if (function.modifierParameter(config) == null) return
+
+        val trailingParam = function.valueParameters.lastOrNull() ?: return
+
+        // Check if the trailing element...
+        // - is a lambda
+        // - is not @Composable
+        // - doesn't have a default value
+        // - starts with "on", meaning it's an event
+        val typeReference = trailingParam.typeReference ?: return
+        if (!typeReference.isLambda()) return
+        if (typeReference.isComposable) return
+        if (trailingParam.hasDefaultValue()) return
+        val name = trailingParam.name ?: return
+        if (!name.startsWith("on")) return
+
+        emitter.report(trailingParam, EventLambdaIsTrailingLambda)
+    }
+
+    companion object {
+        val EventLambdaIsTrailingLambda = """
+            Lambda parameters in a @Composable that are for events (e.g. onClick, onChange, etc) and are required (they don't have a default value) should not be used as the trailing parameter.
+
+            Composable functions that emit content usually reserve the trailing lambda syntax for the content slot, and that can lead to an assumption that other composables can be used in that lambda.
+
+            See https://mrmans0n.github.io/compose-rules/rules/#TODO for more information.
+        """.trimIndent()
+    }
+}

rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/ComposeRuleSetProvider.kt

@@ -18,6 +18,7 @@ class ComposeRuleSetProvider : RuleSetProvider {
             ContentEmitterReturningValuesCheck(config),
             ContentTrailingLambdaCheck(config),
             DefaultsVisibilityCheck(config),
+            LambdaParameterEventTrailingCheck(config),
             LambdaParameterInRestartableEffectCheck(config),
             Material2Check(config),
             ModifierClickableOrderCheck(config),

rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/LambdaParameterEventTrailingCheck.kt

@@ -0,0 +1,23 @@
+// Copyright 2024 Nacho Lopez
+// SPDX-License-Identifier: Apache-2.0
+package io.nlopez.compose.rules.detekt
+
+import io.gitlab.arturbosch.detekt.api.Config
+import io.gitlab.arturbosch.detekt.api.Debt
+import io.gitlab.arturbosch.detekt.api.Issue
+import io.gitlab.arturbosch.detekt.api.Severity
+import io.nlopez.compose.core.ComposeKtVisitor
+import io.nlopez.compose.rules.DetektRule
+import io.nlopez.compose.rules.LambdaParameterEventTrailing
+
+class LambdaParameterEventTrailingCheck(config: Config) :
+    DetektRule(config),
+    ComposeKtVisitor by LambdaParameterEventTrailing() {
+
+    override val issue: Issue = Issue(
+        id = "LambdaParameterEventTrailing",
+        severity = Severity.Style,
+        description = LambdaParameterEventTrailing.EventLambdaIsTrailingLambda,
+        debt = Debt.FIVE_MINS,
+    )
+}

rules/detekt/src/test/kotlin/io/nlopez/compose/rules/detekt/LambdaParameterEventTrailingCheckTest.kt

@@ -0,0 +1,76 @@
+// Copyright 2024 Nacho Lopez
+// SPDX-License-Identifier: Apache-2.0
+package io.nlopez.compose.rules.detekt
+
+import io.gitlab.arturbosch.detekt.api.Config
+import io.gitlab.arturbosch.detekt.api.SourceLocation
+import io.gitlab.arturbosch.detekt.test.assertThat
+import io.gitlab.arturbosch.detekt.test.lint
+import io.nlopez.compose.rules.LambdaParameterEventTrailing
+import org.intellij.lang.annotations.Language
+import org.junit.jupiter.api.Test
+
+class LambdaParameterEventTrailingCheckTest {
+
+    private val rule = LambdaParameterEventTrailingCheck(Config.empty)
+
+    @Test
+    fun `error out when detecting a lambda being as trailing`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Something(modifier: Modifier = Modifier, onClick: () -> Unit) {
+                    Text("Hello")
+                }
+            """.trimIndent()
+        val errors = rule.lint(code)
+        assertThat(errors)
+            .hasStartSourceLocations(
+                SourceLocation(2, 46),
+            )
+        for (error in errors) {
+            assertThat(error).hasMessage(LambdaParameterEventTrailing.EventLambdaIsTrailingLambda)
+        }
+    }
+
+    @Test
+    fun `passes when a lambda is required`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Something(onClick: () -> Unit, modifier: Modifier = Modifier) {
+                    Text("Hello")
+                }
+            """.trimIndent()
+        val errors = rule.lint(code)
+        assertThat(errors).isEmpty()
+    }
+
+    @Test
+    fun `passes when a lambda is composable`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Something(modifier: Modifier = Modifier, on: @Composable () -> Unit) {
+                    Text("Hello")
+                }
+            """.trimIndent()
+        val errors = rule.lint(code)
+        assertThat(errors).isEmpty()
+    }
+
+    @Test
+    fun `passes when the function doesnt emit content`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun something(onClick: () -> Unit) {}
+            """.trimIndent()
+        val errors = rule.lint(code)
+        assertThat(errors).isEmpty()
+    }
+}

rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/ComposeRuleSetProvider.kt

@@ -18,6 +18,7 @@ class ComposeRuleSetProvider :
         RuleProvider { ContentEmitterReturningValuesCheck() },
         RuleProvider { ContentTrailingLambdaCheck() },
         RuleProvider { DefaultsVisibilityCheck() },
+        RuleProvider { LambdaParameterEventTrailingCheck() },
         RuleProvider { LambdaParameterInRestartableEffectCheck() },
         RuleProvider { Material2Check() },
         RuleProvider { ModifierClickableOrderCheck() },

rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/LambdaParameterEventTrailingCheck.kt

@@ -0,0 +1,14 @@
+// Copyright 2024 Nacho Lopez
+// SPDX-License-Identifier: Apache-2.0
+package io.nlopez.compose.rules.ktlint
+
+import io.nlopez.compose.core.ComposeKtVisitor
+import io.nlopez.compose.rules.KtlintRule
+import io.nlopez.compose.rules.LambdaParameterEventTrailing
+
+class LambdaParameterEventTrailingCheck :
+    KtlintRule(
+        id = "compose:lambda-param-event-trailing",
+        editorConfigProperties = setOf(contentEmittersProperty, contentEmittersDenylist)
+    ),
+    ComposeKtVisitor by LambdaParameterEventTrailing()