Add denylist for content emitters (#167)

Adds the ability to configure composables that probably emit content that we don't want to count towards the count for emissions. This is useful, for example, for composables that wrap stuff that will render in another window (like a Dialog or a Modal), which they do indeed emit content but we might not want to take it into account because it's not a mistake that is used at the same level as other stuff (e.g. a Scaffold composable).

Closes #166.

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

@@ -47,9 +47,11 @@ context(ComposeKtConfig)
 val KtCallExpression.emitsContent: Boolean
     get() {
         val methodName = calleeExpression?.text ?: return false
-        return methodName in ComposableEmittersList ||
-            ComposableEmittersListRegex.matches(methodName) ||
-            methodName in getSet("contentEmitters", emptySet()) ||
+
+        // If in denylist, we will assume it doesn't emit content (regardless of anything else).
+        if (methodName in getSet("contentEmittersDenylist", emptySet())) return false
+
+        return methodName in ComposableEmittersList + getSet("contentEmitters", emptySet()) ||
             containsComposablesWithModifiers
     }
 
@@ -70,7 +72,8 @@ private val KtCallExpression.containsComposablesWithModifiers: Boolean
 
 /**
  * This is a denylist with common composables that emit content in their own window. Feel free to add more elements
- * if you stumble upon them in code reviews that should have triggered an error from this rule.
+ * if you stumble upon false positives that should not have triggered an error from this rule, and are in foundational
+ * libraries.
  */
 private val ComposableNonEmittersList = setOf(
     "AlertDialog",
@@ -96,6 +99,7 @@ private val ComposableEmittersList by lazy {
         "LazyRow",
         "LazyVerticalGrid",
         "Row",
+        "Spacer",
         "Text",
         // android.compose.material
         "BottomDrawer",
@@ -150,18 +154,6 @@ private val ComposableEmittersList by lazy {
     )
 }
 
-private val ComposableEmittersListRegex by lazy {
-    Regex(
-        listOf(
-            "Spacer\\d*", // Spacer() + SpacerNUM()
-        ).joinToString(
-            separator = "|",
-            prefix = "(",
-            postfix = ")",
-        ),
-    )
-}
-
 val KtProperty.declaresCompositionLocal: Boolean
     get() = !isVar &&
         hasInitializer() &&

docs/detekt.md

@@ -73,8 +73,10 @@ Compose:
     active: true
   MultipleEmitters:
     active: true
-    # -- You can optionally add your own composables here
+    # -- You can optionally add your own composables here that will count as content emitters
     # 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
   MutableParams:
     active: true
   MutableStateParam:

docs/ktlint.md

@@ -41,13 +41,22 @@ You can use this same [uber jar from the releases page](https://github.com/mrman
 
 ### Providing custom content emitters
 
-There are some rules (`compose:content-emitter-returning-values-check`, `compose:modifier-not-used-at-root` and `compose:multiple-emitters-check`) that use predefined list of known composables that emit content. But you can add your own too! In your `.editorconfig` file, you'll need to add a `content_emitters` property followed by a list of composable names separated by commas. You would typically want the composables that are part of your custom design system to be in this list.
+There are some rules (`compose:content-emitter-returning-values-check`, `compose:modifier-not-used-at-root` and `compose:multiple-emitters-check`) that use predefined list of known composables that emit content. But you can add your own too! In your `.editorconfig` file, you'll need to add a `compose_content_emitters` property followed by a list of composable names separated by commas. You would typically want the composables that are part of your custom design system to be in this list.
 
 ```editorconfig
 [*.{kt,kts}]
 compose_content_emitters = MyComposable,MyOtherComposable
 ```
 
+### Providing exceptions to content emitters
+
+Sometimes we'll want to not count a Composable towards the multiple content emitters (`compose:multiple-emitters-check`) rule. This is useful, for example, if the composable function actually emits content but that content is painted in a different window (like a dialog or a modal). For those cases, we can use a denylist `compose_content_emitters_denyylist` to add those composable names separated by commas.
+
+```editorconfig
+[*.{kt,kts}]
+compose_content_emitters_denylist = MyModalComposable,MyDialogComposable
+```
+
 ### Providing custom ViewModel factories
 
 The `vm-injection-check` rule will check against common ViewModel factories (eg `viewModel` from AAC, `weaverViewModel` from Weaver, `hiltViewModel` from Hilt + Compose, etc), but you can configure your `.editorconfig` file to add your own, as a list of comma-separated strings:

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

@@ -13,7 +13,8 @@ import org.junit.jupiter.api.Test
 class MultipleContentEmittersCheckTest {
 
     private val testConfig = TestConfig(
-        "contentEmitters" to listOf("Potato", "Banana"),
+        "contentEmitters" to listOf("Potato", "Banana", "Apple"),
+        "contentEmittersDenylist" to listOf("Apple"),
     )
     private val rule = MultipleContentEmittersCheck(testConfig)
 
@@ -49,7 +50,7 @@ class MultipleContentEmittersCheckTest {
                 }
                 @Composable
                 fun RowScope.Something() {
-                    Spacer16()
+                    Spacer()
                     Text("Hola")
                 }
             """.trimIndent()
@@ -71,7 +72,7 @@ class MultipleContentEmittersCheckTest {
                 context(ColumnScope)
                 @Composable
                 fun Something() {
-                    Spacer16()
+                    Spacer()
                     Text("Hola")
                 }
             """.trimIndent()
@@ -91,12 +92,12 @@ class MultipleContentEmittersCheckTest {
                 }
                 @Composable
                 fun Something() {
-                    Spacer16()
+                    Spacer()
                     Text("Hola")
                 }
             """.trimIndent()
         val errors = rule.lint(code)
-        assertThat(errors).hasSize(2)
+        assertThat(errors)
             .hasStartSourceLocations(
                 SourceLocation(2, 5),
                 SourceLocation(7, 5),
@@ -135,7 +136,7 @@ class MultipleContentEmittersCheckTest {
                 }
             """.trimIndent()
         val errors = rule.lint(code)
-        assertThat(errors).hasSize(2)
+        assertThat(errors)
             .hasStartSourceLocations(
                 SourceLocation(6, 5),
                 SourceLocation(19, 5),
@@ -162,7 +163,7 @@ class MultipleContentEmittersCheckTest {
                 }
             """.trimIndent()
         val errors = rule.lint(code)
-        assertThat(errors).hasSize(1)
+        assertThat(errors)
             .hasStartSourceLocation(2, 5)
         assertThat(errors.first()).hasMessage(MultipleContentEmitters.MultipleContentEmittersDetected)
     }
@@ -195,4 +196,19 @@ class MultipleContentEmittersCheckTest {
             assertThat(error).hasMessage(MultipleContentEmitters.MultipleContentEmittersDetected)
         }
     }
+
+    @Test
+    fun `passes when the composable is in the denylist`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Something() {
+                    Text("Hi")
+                    Apple()
+                }
+            """.trimIndent()
+        val errors = rule.lint(code)
+        assertThat(errors).isEmpty()
+    }
 }

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

@@ -11,7 +11,26 @@ val contentEmittersProperty: EditorConfigProperty<String> =
         type = PropertyType.LowerCasingPropertyType(
             "compose_content_emitters",
             "A comma separated list of composable functions that emit content (e.g. UI)",
-            PropertyType.PropertyValueParser.IDENTITY_VALUE_PARSER,
+            PropertyValueParser.IDENTITY_VALUE_PARSER,
+            emptySet(),
+        ),
+        defaultValue = "",
+        propertyMapper = { property, _ ->
+            when {
+                property?.isUnset == true -> ""
+                property?.getValueAs<String>() != null -> property.getValueAs<String>()
+                else -> property?.getValueAs()
+            }
+        },
+    )
+
+val contentEmittersDenylist: EditorConfigProperty<String> =
+    EditorConfigProperty(
+        type = PropertyType.LowerCasingPropertyType(
+            "compose_content_emitters_denylist",
+            "A comma separated list of composable functions that we don't want to take into acccount " +
+                "when assessing if something is a content emitter",
+            PropertyValueParser.IDENTITY_VALUE_PARSER,
             emptySet(),
         ),
         defaultValue = "",

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

@@ -9,6 +9,6 @@ import io.nlopez.rules.core.ktlint.KtlintRule
 class MultipleContentEmittersCheck :
     KtlintRule(
         id = "compose:multiple-emitters-check",
-        editorConfigProperties = setOf(contentEmittersProperty),
+        editorConfigProperties = setOf(contentEmittersProperty, contentEmittersDenylist),
     ),
     ComposeKtVisitor by MultipleContentEmitters()

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

@@ -43,7 +43,7 @@ class MultipleContentEmittersCheckTest {
                 }
                 @Composable
                 fun RowScope.Something() {
-                    Spacer16()
+                    Spacer()
                     Text("Hola")
                 }
             """.trimIndent()
@@ -64,7 +64,7 @@ class MultipleContentEmittersCheckTest {
                 context(RowScope)
                 @Composable
                 fun Something() {
-                    Spacer16()
+                    Spacer()
                     Text("Hola")
                 }
             """.trimIndent()
@@ -83,7 +83,7 @@ class MultipleContentEmittersCheckTest {
                 }
                 @Composable
                 fun Something() {
-                    Spacer16()
+                    Spacer()
                     Text("Hola")
                 }
             """.trimIndent()
@@ -203,4 +203,20 @@ class MultipleContentEmittersCheckTest {
             ),
         )
     }
+
+    @Test
+    fun `passes when the composable is in the denylist`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Something() {
+                    Text("Hi")
+                    Spacer()
+                }
+            """.trimIndent()
+        emittersRuleAssertThat(code)
+            .withEditorConfigOverride(contentEmittersDenylist to "Spacer")
+            .hasNoLintViolations()
+    }
 }