Add allowlist of composable names to VMForwarding rule (#169)

core-common/src/main/kotlin/io/nlopez/rules/core/util/KotlinUtils.kt

@@ -6,6 +6,8 @@ import java.util.Locale
 
 fun <T> T.runIf(value: Boolean, block: T.() -> T): T = if (value) block() else this
 
+fun <T, R> T.runIfNotNull(value: R?, block: T.(R) -> T): T = value?.let { block(it) } ?: this
+
 fun String?.matchesAnyOf(patterns: Sequence<Regex>): Boolean {
     if (isNullOrEmpty()) return false
     for (regex in patterns) {

docs/detekt.md

@@ -99,6 +99,8 @@ Compose:
     active: true
     # -- You can optionally add your own ViewModel factories here
     # viewModelFactories: hiltViewModel,potatoViewModel
+    # -- You can optionally add an allowlist for Composable names that won't be affected by this rule
+    # allowedForwarding: .*Content,.*FancyStuff
 ```
 
 ### Disabling a specific rule

docs/ktlint.md

@@ -93,6 +93,16 @@ The `vm-forwarding-check` rule will, by default, design as a state holder any cl
 compose_allowed_state_holder_names = .*ViewModel,.*Presenter,.*Component,.*SomethingElse
 ```
 
+
+### Allowlist for composable names that aren't affected by the ViewModelForwarding rule
+
+The `vm-forwarding-check` will catch VMs/state holder classes that are relayed to other composables. However, in some situations this can be a valid use-case. The rule can be configured so that all the names that matches a list of regexes are exempt to this rule. You can configure this in your `.editorconfig` file:
+
+```editorconfig
+[*.{kt,kts}]
+compose_allowed_forwarding = .*Content,.*SomethingElse
+```
+
 ### Configure the visibility of the composables where to check for missing modifiers
 
 The `modifier-missing-check` rule will, by default, only look for missing modifiers for public composables. If you want to lower the visibility threshold to check also internal compoosables, or all composables, you can configure it in your `.editorconfig` file:

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

@@ -31,16 +31,26 @@ class ViewModelForwarding : ComposeKtVisitor {
         if (parameters.isEmpty()) return
 
         val stateHolderValidNames = Regex(
-            config
-                .getList("allowedStateHolderNames", defaultStateHolderNames)
-                .ifEmpty { defaultStateHolderNames }
+            (config.getList("allowedStateHolderNames", emptyList()) + defaultStateHolderNames)
                 .joinToString(
                     separator = "|",
                     prefix = "(",
                     postfix = ")",
                 ),
         )
 
+        val allowedForwarding = config.getSet("allowedForwarding", emptySet())
+        val allowedForwardingRegex = when {
+            allowedForwarding.isNotEmpty() -> Regex(
+                allowedForwarding.joinToString(
+                    separator = "|",
+                    prefix = "(",
+                    postfix = ")",
+                ),
+            )
+            else -> null
+        }
+
         val viewModelParameterNames = parameters.filter { parameter ->
             // We can't do much better than looking at the types at face value
             parameter.typeReference?.text?.matches(stateHolderValidNames) == true
@@ -54,6 +64,10 @@ class ViewModelForwarding : ComposeKtVisitor {
             .filter { callExpression -> callExpression.calleeExpression?.text?.first()?.isUpperCase() ?: false }
             // Avoid LaunchedEffect/DisposableEffect/etc that can use the VM as a key
             .filterNot { callExpression -> callExpression.isRestartableEffect }
+            // Avoid explicitly allowlisted Composable names
+            .filterNot { callExpression ->
+                allowedForwardingRegex?.let { callExpression.calleeExpression?.text?.matches(it) } == true
+            }
             .flatMap { callExpression ->
                 // Get VALUE_ARGUMENT that has a REFERENCE_EXPRESSION. This would map to `viewModel` in this example:
                 // MyComposable(viewModel, ...)

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

@@ -2,7 +2,6 @@
 // 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.TestConfig
 import io.gitlab.arturbosch.detekt.test.assertThat
@@ -14,9 +13,10 @@ import org.junit.jupiter.api.Test
 class ViewModelForwardingCheckTest {
 
     private val testConfig = TestConfig(
-        "stateHolder" to listOf("bananaViewModel", "potatoViewModel"),
+        "allowedStateHolderNames" to listOf(".*Component", ".*StateHolder"),
+        "allowedForwarding" to listOf(".*Content"),
     )
-    private val rule = ViewModelForwardingCheck(Config.empty)
+    private val rule = ViewModelForwardingCheck(testConfig)
 
     @Test
     fun `allows the forwarding of ViewModels in overridden Composable functions`() {
@@ -112,4 +112,52 @@ class ViewModelForwardingCheckTest {
         val errors = rule.lint(code)
         assertThat(errors).isEmpty()
     }
+
+    @Test
+    fun `errors when a custom state holder is forwarded`() {
+        @Language("kotlin")
+        val code =
+            """
+            @Composable
+            fun MyComposable(viewModel: MyViewComponent) {
+                AnotherComposable(viewModel)
+            }
+            @Composable
+            fun MyComposable2(viewModel: MyStateHolder) {
+                AnotherComposable(viewModel)
+            }
+            """.trimIndent()
+        val errors = rule.lint(code)
+        assertThat(errors).hasStartSourceLocations(
+            SourceLocation(3, 5),
+            SourceLocation(7, 5),
+        )
+        for (error in errors) {
+            assertThat(error).hasMessage(ViewModelForwarding.AvoidViewModelForwarding)
+        }
+    }
+
+    @Test
+    fun `allows forwarding when a ViewModel is in the allowlist`() {
+        @Language("kotlin")
+        val code =
+            """
+            @Composable
+            fun MyComposable(viewModel: MyViewModel) {
+                AnotherComposableContent(viewModel)
+            }
+            @Composable
+            fun MyComposable2(viewModel: MyViewModel) {
+                Row {
+                    AnotherComposableContent(viewModel)
+                }
+            }
+            @Composable
+            fun MyComposable3(viewModel: MyViewModel) {
+                AnotherComposableContent(vm = viewModel)
+            }
+            """.trimIndent()
+        val errors = rule.lint(code)
+        assertThat(errors).isEmpty()
+    }
 }

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

@@ -133,6 +133,25 @@ val allowedStateHolderNames: EditorConfigProperty<String> =
         },
     )
 
+val allowedForwarding: EditorConfigProperty<String> =
+    EditorConfigProperty(
+        type = PropertyType.LowerCasingPropertyType(
+            "compose_allowed_forwarding",
+            "A comma separated list of regexes of composable names where forwarding a " +
+                "state holder / ViewModel / Presenter names is alright to do",
+            PropertyValueParser.IDENTITY_VALUE_PARSER,
+            emptySet(),
+        ),
+        defaultValue = "",
+        propertyMapper = { property, _ ->
+            when {
+                property?.isUnset == true -> ""
+                property?.getValueAs<String>() != null -> property.getValueAs<String>()
+                else -> property?.getValueAs()
+            }
+        },
+    )
+
 val customModifiers: EditorConfigProperty<String> =
     EditorConfigProperty(
         type = PropertyType.LowerCasingPropertyType(

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

@@ -9,6 +9,6 @@ import io.nlopez.rules.core.ktlint.KtlintRule
 class ViewModelForwardingCheck :
     KtlintRule(
         id = "compose:vm-forwarding-check",
-        editorConfigProperties = setOf(allowedStateHolderNames),
+        editorConfigProperties = setOf(allowedStateHolderNames, allowedForwarding),
     ),
     ComposeKtVisitor by ViewModelForwarding()

rules/ktlint/src/test/kotlin/io/nlopez/compose/rules/ktlint/ViewModelForwardingCheckTest.kt

@@ -112,4 +112,63 @@ class ViewModelForwardingCheckTest {
             """.trimIndent()
         forwardingRuleAssertThat(code).hasNoLintViolations()
     }
+
+    @Test
+    fun `errors when a custom state holder is forwarded`() {
+        @Language("kotlin")
+        val code =
+            """
+            @Composable
+            fun MyComposable(viewModel: MyViewComponent) {
+                AnotherComposable(viewModel)
+            }
+            @Composable
+            fun MyComposable2(viewModel: MyStateHolder) {
+                AnotherComposable(viewModel)
+            }
+            """.trimIndent()
+        forwardingRuleAssertThat(code)
+            .withEditorConfigOverride(
+                allowedStateHolderNames to ".*Component,.*StateHolder",
+            )
+            .hasLintViolationsWithoutAutoCorrect(
+                LintViolation(
+                    line = 3,
+                    col = 5,
+                    detail = ViewModelForwarding.AvoidViewModelForwarding,
+                ),
+                LintViolation(
+                    line = 7,
+                    col = 5,
+                    detail = ViewModelForwarding.AvoidViewModelForwarding,
+                ),
+            )
+    }
+
+    @Test
+    fun `allows forwarding when a ViewModel is in the allowlist`() {
+        @Language("kotlin")
+        val code =
+            """
+            @Composable
+            fun MyComposable(viewModel: MyViewModel) {
+                AnotherComposableContent(viewModel)
+            }
+            @Composable
+            fun MyComposable2(viewModel: MyViewModel) {
+                Row {
+                    AnotherComposableContent(viewModel)
+                }
+            }
+            @Composable
+            fun MyComposable3(viewModel: MyViewModel) {
+                AnotherComposableContent(vm = viewModel)
+            }
+            """.trimIndent()
+        forwardingRuleAssertThat(code)
+            .withEditorConfigOverride(
+                allowedForwarding to ".*Content",
+            )
+            .hasNoLintViolations()
+    }
 }