Make UnstableCollections rule opt-in (#256)

Author: mrmans0n

docs/detekt.md

@@ -110,7 +110,7 @@ Compose:
   RememberContentMissing:
     active: true
   UnstableCollections:
-    active: true
+    active: false # Opt-in, disabled by default. Turn on if you want to enforce this (e.g. you have strong skipping disabled)
   ViewModelForwarding:
     active: true
     # -- You can optionally use this rule on things other than types ending in "ViewModel" or "Presenter" (which are the defaults). You can add your own via a regex here:

docs/ktlint.md

@@ -171,6 +171,15 @@ compose_disallow_material2 = true
 compose_allowed_from_m2 = icons.filled,Button
 ```
 
+### Enabling the unstable collections detector
+
+The `unstable-collections` rule will flag any usage of any unstable collection (e.g. List/Set/Map). This rule is disabled by default, so you'll need to explicitly enable it in your `.editorconfig` file:
+
+```editorconfig
+[*.{kt,kts}]
+compose_disallow_unstable_collections = true
+```
+
 ## Disabling a specific rule
 
 To disable a rule you have to follow the [instructions from the ktlint documentation](https://pinterest.github.io/ktlint/0.49.1/faq/#how-do-i-suppress-errors-for-a-lineblockfile), and use the id of the rule you want to disable with the `compose` tag.
@@ -179,5 +188,5 @@ For example, to disable the `naming-check` rule, the tag you'll need to disable
 
 ```kotlin
     @Suppress("ktlint:compose:naming-check")
-    ... your code here
+    fun YourComposableHere() { ... }
 ```

docs/rules.md

@@ -30,41 +30,6 @@ More info: [Immutable docs](https://developer.android.com/reference/kotlin/andro
 
 Related rule: TBD
 
-### Avoid using unstable collections
-
-Collections are defined as interfaces (e.g. `List<T>`, `Map<T>`, `Set<T>`) in Kotlin, which can't guarantee that they are actually immutable. For example, you could write:
-
-```kotlin
-// ❌ The compiler won't be able to infer that the list is immutable
-val list: List<String> = mutableListOf<String>()
-```
-
-The variable is constant, its declared type is not mutable but its implementation is still mutable. The Compose compiler cannot be sure of the immutability of this class as it just sees the declared type and as such declares it as unstable.
-
-To force the compiler to see a collection as truly 'immutable' you have a couple of options.
-
-You can use [Kotlinx Immutable Collections](https://github.com/Kotlin/kotlinx.collections.immutable):
-
-```kotlin
-// ✅ The compiler knows that this list is immutable
-val list: ImmutableList<String> = persistentListOf<String>()
-```
-
-Alternatively, you can wrap your collection in an annotated stable class to mark it as immutable for the Compose compiler.
-
-```kotlin
-// ✅ The compiler knows that this class is immutable
-@Immutable
-data class StringList(val items: List<String>)
-// ...
-val list: StringList = StringList(yourList)
-```
-> **Note**: It is preferred to use Kotlinx Immutable Collections for this. As you can see, the wrapped case only includes the immutability promise with the annotation, but the underlying List is still mutable.
-
-More info: [Jetpack Compose Stability Explained](https://medium.com/androiddevelopers/jetpack-compose-stability-explained-79c10db270c8), [Kotlinx Immutable Collections](https://github.com/Kotlin/kotlinx.collections.immutable)
-
-Related rule: [compose:unstable-collections](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/UnstableCollections.kt)
-
 ### Use mutableStateOf type-specific variants when possible
 
 `mutableIntStateOf`, `mutableLongStateOf`, `mutableDoubleStateOf`, `mutableFloatStateOf` are essentially counterparts to `mutableStateOf`, but with the added advantage of circumventing autoboxing on JVM platforms. This distinction renders them more memory efficient, making them the preferable choice when dealing with primitive types such as double, float, int, and long.
@@ -518,3 +483,40 @@ Enabling: [ktlint](https://mrmans0n.github.io/compose-rules/ktlint/#enabling-the
 More info: [Migration to Material 3](https://developer.android.com/develop/ui/compose/designsystems/material2-material3)
 
 Related rule: [compose:material-two](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/Material2.kt)
+
+### Avoid using unstable collections
+
+> **Note**: This rule will become unnecessary from the Compose version where strong skipping is enabled by default.
+
+Collections are defined as interfaces (e.g. `List<T>`, `Map<T>`, `Set<T>`) in Kotlin, which can't guarantee that they are actually immutable. For example, you could write:
+
+```kotlin
+// ❌ The compiler won't be able to infer that the list is immutable
+val list: List<String> = mutableListOf<String>()
+```
+
+The variable is constant, its declared type is not mutable but its implementation is still mutable. The Compose compiler cannot be sure of the immutability of this class as it just sees the declared type and as such declares it as unstable.
+
+To force the compiler to see a collection as truly 'immutable' you have a couple of options.
+
+You can use [Kotlinx Immutable Collections](https://github.com/Kotlin/kotlinx.collections.immutable):
+
+```kotlin
+// ✅ The compiler knows that this list is immutable
+val list: ImmutableList<String> = persistentListOf<String>()
+```
+
+Alternatively, you can wrap your collection in an annotated stable class to mark it as immutable for the Compose compiler.
+
+```kotlin
+// ✅ The compiler knows that this class is immutable
+@Immutable
+data class StringList(val items: List<String>)
+// ...
+val list: StringList = StringList(yourList)
+```
+> **Note**: It is preferred to use Kotlinx Immutable Collections for this. As you can see, the wrapped case only includes the immutability promise with the annotation, but the underlying List is still mutable.
+
+More info: [Jetpack Compose Stability Explained](https://medium.com/androiddevelopers/jetpack-compose-stability-explained-79c10db270c8), [Kotlinx Immutable Collections](https://github.com/Kotlin/kotlinx.collections.immutable)
+
+Related rule: [compose:unstable-collections](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/UnstableCollections.kt)

rules/detekt/src/main/resources/config/config.yml

@@ -52,7 +52,7 @@ Compose:
   RememberContentMissing:
     active: true
   UnstableCollections:
-    active: true
+    active: false
   ViewModelForwarding:
     active: true
   ViewModelInjection:

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

@@ -255,3 +255,14 @@ val disallowMaterial2: EditorConfigProperty<Boolean> =
         ),
         defaultValue = false,
     )
+
+val disallowUnstableCollections: EditorConfigProperty<Boolean> =
+    EditorConfigProperty(
+        type = PropertyType.LowerCasingPropertyType(
+            "compose_disallow_unstable_collections",
+            "When enabled, unstable collections (e.g. List/Set/Map) will be disallowed.",
+            PropertyValueParser.BOOLEAN_VALUE_PARSER,
+            setOf(true.toString(), false.toString()),
+        ),
+        defaultValue = false,
+    )

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

@@ -3,9 +3,30 @@
 package io.nlopez.compose.rules.ktlint
 
 import io.nlopez.compose.rules.UnstableCollections
+import io.nlopez.rules.core.ComposeKtConfig
 import io.nlopez.rules.core.ComposeKtVisitor
+import io.nlopez.rules.core.Emitter
 import io.nlopez.rules.core.ktlint.KtlintRule
+import org.jetbrains.kotlin.psi.KtFunction
 
 class UnstableCollectionsCheck :
-    KtlintRule("compose:unstable-collections"),
-    ComposeKtVisitor by UnstableCollections()
+    KtlintRule(
+        id = "compose:unstable-collections",
+        editorConfigProperties = setOf(disallowUnstableCollections),
+    ),
+    ComposeKtVisitor {
+
+    private val visitor = UnstableCollections()
+
+    override fun visitComposable(
+        function: KtFunction,
+        autoCorrect: Boolean,
+        emitter: Emitter,
+        config: ComposeKtConfig,
+    ) {
+        // ktlint allows all rules by default, so we'll add an extra param to make sure it's disabled by default
+        if (config.getBoolean("disallowUnstableCollections", false)) {
+            visitor.visitComposable(function, autoCorrect, emitter, config)
+        }
+    }
+}

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

@@ -24,23 +24,40 @@ class UnstableCollectionsCheckTest {
                 @Composable
                 fun Something(a: Map<String, Int>) {}
             """.trimIndent()
-        ruleAssertThat(code).hasLintViolationsWithoutAutoCorrect(
-            LintViolation(
-                line = 2,
-                col = 18,
-                detail = createErrorMessage("List<String>", "List", "a"),
-            ),
-            LintViolation(
-                line = 4,
-                col = 18,
-                detail = createErrorMessage("Set<String>", "Set", "a"),
-            ),
-            LintViolation(
-                line = 6,
-                col = 18,
-                detail = createErrorMessage("Map<String, Int>", "Map", "a"),
-            ),
-        )
+        ruleAssertThat(code)
+            .withEditorConfigOverride(disallowUnstableCollections to true)
+            .hasLintViolationsWithoutAutoCorrect(
+                LintViolation(
+                    line = 2,
+                    col = 18,
+                    detail = createErrorMessage("List<String>", "List", "a"),
+                ),
+                LintViolation(
+                    line = 4,
+                    col = 18,
+                    detail = createErrorMessage("Set<String>", "Set", "a"),
+                ),
+                LintViolation(
+                    line = 6,
+                    col = 18,
+                    detail = createErrorMessage("Map<String, Int>", "Map", "a"),
+                ),
+            )
+    }
+
+    @Test
+    fun `passes even if there are errors if disallowUnstableCollections is false`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Something(a: List<String>) {}
+                @Composable
+                fun Something(a: Set<String>) {}
+                @Composable
+                fun Something(a: Map<String, Int>) {}
+            """.trimIndent()
+        ruleAssertThat(code).hasNoLintViolations()
     }
 
     @Test
@@ -53,6 +70,8 @@ class UnstableCollectionsCheckTest {
                 @Composable
                 fun Something(a: StringList, b: StringSet, c: StringToIntMap) {}
             """.trimIndent()
-        ruleAssertThat(code).hasNoLintViolations()
+        ruleAssertThat(code)
+            .withEditorConfigOverride(disallowUnstableCollections to true)
+            .hasNoLintViolations()
     }
 }