Add UnstableCollections rule (#99)

* Add UnstableCollections rule

The Compose compiler cannot infer stability for kotlin collection interfaces (List/Set/Map), so any composable with one of those (which is pretty likely) as a parameter will be deemed unstable by the compiler.
This rule detects those cases and prompt the user to use the Kotlinx Immutable Collections alternative instead, or an `@Immutable` wrapper.

* Remove space

core-common/src/main/kotlin/com/twitter/rules/core/util/KtParameters.kt → core-common/src/main/kotlin/com/twitter/rules/core/util/KtCallableDeclarations.kt

@@ -2,12 +2,12 @@
 // SPDX-License-Identifier: Apache-2.0
 package com.twitter.rules.core.util
 
-import org.jetbrains.kotlin.psi.KtParameter
+import org.jetbrains.kotlin.psi.KtCallableDeclaration
 
-val KtParameter.isTypeMutable: Boolean
-    get() = typeReference?.text?.matchesAnyOf(KnownMutableCommonTypes) == true
+val KtCallableDeclaration.isTypeMutable: Boolean
+    get() = typeReference?.text?.matchesAnyOf(KnownMutableCommonTypesRegex) == true
 
-private val KnownMutableCommonTypes = sequenceOf(
+val KnownMutableCommonTypesRegex = sequenceOf(
     // Set
     "MutableSet<.*>\\??",
     "ArraySet<.*>\\??",
@@ -39,3 +39,12 @@ private val KnownMutableCommonTypes = sequenceOf(
     "BehaviorRelay<.*>\\??",
     "ReplayRelay<.*>\\??"
 ).map { Regex(it) }
+
+val KtCallableDeclaration.isTypeUnstableCollection: Boolean
+    get() = typeReference?.text?.matchesAnyOf(KnownUnstableCollectionTypesRegex) == true
+
+val KnownUnstableCollectionTypesRegex = sequenceOf(
+    "Set<.*>\\??",
+    "List<.*>\\??",
+    "Map<.*>\\??"
+).map { Regex(it) }

docs/detekt.md

@@ -54,6 +54,8 @@ TwitterCompose:
     active: true
   RememberMissing:
     active: true
+  UnstableCollections:
+    active: true
   ViewModelForwarding:
     active: true
   ViewModelInjection:

docs/rules.md

@@ -29,6 +29,38 @@ 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
+    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
+    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
+    @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: [twitter-compose:unstable-collections](https://github.com/twitter/compose-rules/blob/main/rules/common/src/main/kotlin/com/twitter/compose/rules/ComposeUnstableCollections.kt)
+
 ## Composables
 
 ### Do not use inherently mutable types as parameters

rules/common/src/main/kotlin/com/twitter/compose/rules/ComposeUnstableCollections.kt

@@ -0,0 +1,39 @@
+// Copyright 2022 Twitter, Inc.
+// SPDX-License-Identifier: Apache-2.0
+package com.twitter.compose.rules
+
+import com.twitter.rules.core.ComposeKtVisitor
+import com.twitter.rules.core.Emitter
+import com.twitter.rules.core.report
+import com.twitter.rules.core.util.isTypeUnstableCollection
+import org.jetbrains.kotlin.psi.KtFunction
+import java.util.*
+
+class ComposeUnstableCollections : ComposeKtVisitor {
+
+    override fun visitComposable(function: KtFunction, autoCorrect: Boolean, emitter: Emitter) {
+        for (param in function.valueParameters.filter { it.isTypeUnstableCollection }) {
+            val variableName = param.nameAsSafeName.asString()
+            val type = param.typeReference?.text ?: "List/Set/Map"
+            val message = createErrorMessage(
+                type = type,
+                rawType = type.replace(DiamondRegex, ""),
+                variable = variableName
+            )
+            emitter.report(param.typeReference ?: param, message)
+        }
+    }
+
+    companion object {
+        private val DiamondRegex by lazy(LazyThreadSafetyMode.NONE) { Regex("<.*>\\??") }
+        private val String.capitalized: String
+            get() = replaceFirstChar { if (it.isLowerCase()) it.titlecase(Locale.getDefault()) else it.toString() }
+
+        fun createErrorMessage(type: String, rawType: String, variable: String) = """
+            The Compose Compiler cannot infer the stability of a parameter if a $type is used in it, even if the item type is stable.
+            You should use Kotlinx Immutable Collections instead: `$variable: Immutable$type` or create an `@Immutable` wrapper for this class: `@Immutable data class ${variable.capitalized}}$rawType(val items: $type)`
+
+            See https://twitter.github.io/compose-rules/rules/#avoid-using-unstable-collections for more information.
+        """.trimIndent()
+    }
+}

rules/detekt/src/main/kotlin/com/twitter/compose/rules/detekt/ComposeUnstableCollectionsCheck.kt

@@ -0,0 +1,27 @@
+// Copyright 2022 Twitter, Inc.
+// SPDX-License-Identifier: Apache-2.0
+package com.twitter.compose.rules.detekt
+
+import com.twitter.compose.rules.ComposeUnstableCollections
+import com.twitter.rules.core.ComposeKtVisitor
+import com.twitter.rules.core.detekt.TwitterDetektRule
+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
+
+class ComposeUnstableCollectionsCheck(config: Config) :
+    TwitterDetektRule(config),
+    ComposeKtVisitor by ComposeUnstableCollections() {
+    override val issue: Issue = Issue(
+        id = "UnstableCollections",
+        severity = Severity.Defect,
+        description = """
+            The Compose Compiler cannot infer the stability of a parameter if a List/Set/Map is used in it, even if the item type is stable.
+            You should use Kotlinx Immutable Collections instead, or create an `@Immutable` wrapper for this class.
+
+            See https://twitter.github.io/compose-rules/rules/#avoid-using-unstable-collections for more information.
+        """.trimIndent(),
+        debt = Debt.TWENTY_MINS
+    )
+}

rules/detekt/src/main/kotlin/com/twitter/compose/rules/detekt/TwitterComposeRuleSetProvider.kt

@@ -26,6 +26,7 @@ class TwitterComposeRuleSetProvider : RuleSetProvider {
             ComposePreviewNamingCheck(config),
             ComposePreviewPublicCheck(config),
             ComposeRememberMissingCheck(config),
+            ComposeUnstableCollectionsCheck(config),
             ComposeViewModelForwardingCheck(config),
             ComposeViewModelInjectionCheck(config)
         )

rules/detekt/src/test/kotlin/com/twitter/compose/rules/detekt/ComposeUnstableCollectionsCheckTest.kt

@@ -0,0 +1,54 @@
+// Copyright 2022 Twitter, Inc.
+// SPDX-License-Identifier: Apache-2.0
+package com.twitter.compose.rules.detekt
+
+import com.twitter.compose.rules.ComposeUnstableCollections.Companion.createErrorMessage
+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 org.intellij.lang.annotations.Language
+import org.junit.jupiter.api.Test
+
+class ComposeUnstableCollectionsCheckTest {
+
+    private val rule = ComposeUnstableCollectionsCheck(Config.empty)
+
+    @Test
+    fun `errors when a Composable has a List Set Map parameter`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Something(a: List<String>) {}
+                @Composable
+                fun Something(a: Set<String>) {}
+                @Composable
+                fun Something(a: Map<String, Int>) {}
+            """.trimIndent()
+        val errors = rule.lint(code)
+        assertThat(errors)
+            .hasSourceLocations(
+                SourceLocation(2, 18),
+                SourceLocation(4, 18),
+                SourceLocation(6, 18)
+            )
+        assertThat(errors[0]).hasMessage(createErrorMessage("List<String>", "List", "a"))
+        assertThat(errors[1]).hasMessage(createErrorMessage("Set<String>", "Set", "a"))
+        assertThat(errors[2]).hasMessage(createErrorMessage("Map<String, Int>", "Map", "a"))
+    }
+
+    @Test
+    fun `no errors when a Composable has valid parameters`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Something(a: ImmutableList<String>, b: ImmutableSet<String>, c: ImmutableMap<String, Int>) {}
+                @Composable
+                fun Something(a: StringList, b: StringSet, c: StringToIntMap) {}
+            """.trimIndent()
+        val errors = rule.lint(code)
+        assertThat(errors).isEmpty()
+    }
+}

rules/ktlint/src/main/kotlin/com/twitter/compose/rules/ktlint/ComposeUnstableCollectionsCheck.kt

@@ -0,0 +1,11 @@
+// Copyright 2022 Twitter, Inc.
+// SPDX-License-Identifier: Apache-2.0
+package com.twitter.compose.rules.ktlint
+
+import com.twitter.compose.rules.ComposeUnstableCollections
+import com.twitter.rules.core.ComposeKtVisitor
+import com.twitter.rules.core.ktlint.TwitterKtlintRule
+
+class ComposeUnstableCollectionsCheck :
+    TwitterKtlintRule("twitter-compose:unstable-collections"),
+    ComposeKtVisitor by ComposeUnstableCollections()

rules/ktlint/src/main/kotlin/com/twitter/compose/rules/ktlint/TwitterComposeRuleSetProvider.kt

@@ -34,6 +34,7 @@ class TwitterComposeRuleSetProvider :
         ComposePreviewNamingCheck(),
         ComposePreviewPublicCheck(),
         ComposeRememberMissingCheck(),
+        ComposeUnstableCollectionsCheck(),
         ComposeViewModelForwardingCheck(),
         ComposeViewModelInjectionCheck()
     )
@@ -54,6 +55,7 @@ class TwitterComposeRuleSetProvider :
         RuleProvider { ComposePreviewNamingCheck() },
         RuleProvider { ComposePreviewPublicCheck() },
         RuleProvider { ComposeRememberMissingCheck() },
+        RuleProvider { ComposeUnstableCollectionsCheck() },
         RuleProvider { ComposeViewModelForwardingCheck() },
         RuleProvider { ComposeViewModelInjectionCheck() }
     )

rules/ktlint/src/test/kotlin/com/twitter/compose/rules/ktlint/ComposeUnstableCollectionsCheckTest.kt

@@ -0,0 +1,58 @@
+// Copyright 2022 Twitter, Inc.
+// SPDX-License-Identifier: Apache-2.0
+package com.twitter.compose.rules.ktlint
+
+import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThatRule
+import com.pinterest.ktlint.test.LintViolation
+import com.twitter.compose.rules.ComposeUnstableCollections.Companion.createErrorMessage
+import org.intellij.lang.annotations.Language
+import org.junit.jupiter.api.Test
+
+class ComposeUnstableCollectionsCheckTest {
+
+    private val ruleAssertThat = assertThatRule { ComposeUnstableCollectionsCheck() }
+
+    @Test
+    fun `errors when a Composable has a List Set Map parameter`() {
+        @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).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 `no errors when a Composable has valid parameters`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Something(a: ImmutableList<String>, b: ImmutableSet<String>, c: ImmutableMap<String, Int>) {}
+                @Composable
+                fun Something(a: StringList, b: StringSet, c: StringToIntMap) {}
+            """.trimIndent()
+        ruleAssertThat(code).hasNoLintViolations()
+    }
+}