From 3a7f3cd9d2f7b19ebfba7ce004a8ae0f3b92100f Mon Sep 17 00:00:00 2001 From: Nacho Lopez Date: Tue, 10 Dec 2024 12:51:07 +0100 Subject: [PATCH] Remove ModifierComposable rule (#395) --- docs/detekt.md | 4 -- docs/rules.md | 6 +-- .../compose/rules/ModifierComposable.kt | 27 ---------- .../rules/detekt/ComposeRuleSetProvider.kt | 1 - .../rules/detekt/ModifierComposableCheck.kt | 22 -------- .../src/main/resources/config/config.yml | 2 - .../detekt/ModifierComposableCheckTest.kt | 43 ---------------- .../rules/ktlint/ComposeRuleSetProvider.kt | 1 - .../rules/ktlint/ModifierComposableCheck.kt | 14 ------ .../ktlint/ModifierComposableCheckTest.kt | 50 ------------------- 10 files changed, 2 insertions(+), 168 deletions(-) delete mode 100644 rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierComposable.kt delete mode 100644 rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/ModifierComposableCheck.kt delete mode 100644 rules/detekt/src/test/kotlin/io/nlopez/compose/rules/detekt/ModifierComposableCheckTest.kt delete mode 100644 rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/ModifierComposableCheck.kt delete mode 100644 rules/ktlint/src/test/kotlin/io/nlopez/compose/rules/ktlint/ModifierComposableCheckTest.kt diff --git a/docs/detekt.md b/docs/detekt.md index 5af7141e..0b82c10e 100644 --- a/docs/detekt.md +++ b/docs/detekt.md @@ -86,10 +86,6 @@ Compose: active: true # -- You can optionally add your own Modifier types # customModifiers: BananaModifier,PotatoModifier - ModifierComposable: - active: true - # -- You can optionally add your own Modifier types - # customModifiers: BananaModifier,PotatoModifier ModifierComposed: active: true # -- You can optionally add your own Modifier types diff --git a/docs/rules.md b/docs/rules.md index 0020e263..eb1caad3 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -621,15 +621,13 @@ More info: [Modifier documentation](https://developer.android.com/reference/kotl ### Avoid Modifier extension factory functions -Using `@Composable` builder functions for modifiers is not recommended, as they cause unnecessary recompositions. To avoid this, you should use [Modifier.Node](https://developer.android.com/reference/kotlin/androidx/compose/ui/Modifier.Node) instead. It will allow you to accomplish the same things while being very performant. - -There is another API for creating custom modifiers, `composed {}`. This API is no longer recommended due to the performance issues it created, and like with the extension factory functions case, Modifier.Node is recommended instead. +For `@Composable` extension factory functions, there is an API for creating custom modifiers, `composed {}`. This API is no longer recommended due to the performance issues it created, and like with the extension factory functions case, Modifier.Node is recommended instead. More info: [Modifier.Node](https://developer.android.com/reference/kotlin/androidx/compose/ui/Modifier.Node), [Compose Modifier.Node and where to find it, by Merab Tato Kutalia](https://proandroiddev.com/compose-modifier-node-and-where-to-find-it-merab-tato-kutalia-66f891c0e8), [Compose modifiers deep dive, with Leland Richardson](https://www.youtube.com/watch?v=BjGX2RftXsU) and [Composed modifier docs](https://developer.android.com/reference/kotlin/androidx/compose/ui/package-summary#(androidx.compose.ui.Modifier).composed(kotlin.Function1,kotlin.Function1)). !!! info "" - :material-chevron-right-box: [compose:modifier-composable-check](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierComposable.kt) and [compose:modifier-composed-check](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierComposed.kt) ktlint :material-chevron-right-box: [ModifierComposable](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierComposable.kt) and [ModifierComposed](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierComposed.kt) detekt + :material-chevron-right-box: [compose:modifier-composed-check](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierComposed.kt) ktlint :material-chevron-right-box: [ModifierComposed](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierComposed.kt) detekt ## ComponentDefaults diff --git a/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierComposable.kt b/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierComposable.kt deleted file mode 100644 index 21ce1bc4..00000000 --- a/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierComposable.kt +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2023 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.isModifierReceiver -import org.jetbrains.kotlin.psi.KtFunction - -class ModifierComposable : ComposeKtVisitor { - - override fun visitComposable(function: KtFunction, emitter: Emitter, config: ComposeKtConfig) { - if (!function.isModifierReceiver(config)) return - - emitter.report(function, ComposableModifier) - } - - companion object { - val ComposableModifier = """ - Using @Composable builder functions for modifiers is not recommended, as they cause unnecessary recompositions. - You should consider migrating this modifier to be based on Modifier.Node instead. - See https://mrmans0n.github.io/compose-rules/rules/#avoid-modifier-extension-factory-functions for more information. - """.trimIndent() - } -} diff --git a/rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/ComposeRuleSetProvider.kt b/rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/ComposeRuleSetProvider.kt index 8d135a18..4fc6bcc6 100644 --- a/rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/ComposeRuleSetProvider.kt +++ b/rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/ComposeRuleSetProvider.kt @@ -23,7 +23,6 @@ class ComposeRuleSetProvider : RuleSetProvider { LambdaParameterInRestartableEffectCheck(config), Material2Check(config), ModifierClickableOrderCheck(config), - ModifierComposableCheck(config), ModifierComposedCheck(config), ModifierMissingCheck(config), ModifierNamingCheck(config), diff --git a/rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/ModifierComposableCheck.kt b/rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/ModifierComposableCheck.kt deleted file mode 100644 index bbf5fa39..00000000 --- a/rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/ModifierComposableCheck.kt +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2023 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.ModifierComposable - -class ModifierComposableCheck(config: Config) : - DetektRule(config), - ComposeKtVisitor by ModifierComposable() { - override val issue: Issue = Issue( - id = "ModifierComposable", - severity = Severity.Performance, - description = ModifierComposable.ComposableModifier, - debt = Debt.TEN_MINS, - ) -} diff --git a/rules/detekt/src/main/resources/config/config.yml b/rules/detekt/src/main/resources/config/config.yml index 2e95a742..47505406 100644 --- a/rules/detekt/src/main/resources/config/config.yml +++ b/rules/detekt/src/main/resources/config/config.yml @@ -25,8 +25,6 @@ Compose: active: false ModifierClickableOrder: active: true - ModifierComposable: - active: true ModifierComposed: active: true ModifierMissing: diff --git a/rules/detekt/src/test/kotlin/io/nlopez/compose/rules/detekt/ModifierComposableCheckTest.kt b/rules/detekt/src/test/kotlin/io/nlopez/compose/rules/detekt/ModifierComposableCheckTest.kt deleted file mode 100644 index cf3834e8..00000000 --- a/rules/detekt/src/test/kotlin/io/nlopez/compose/rules/detekt/ModifierComposableCheckTest.kt +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright 2023 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.test.assertThat -import io.gitlab.arturbosch.detekt.test.lint -import io.nlopez.compose.rules.ModifierComposable -import org.intellij.lang.annotations.Language -import org.junit.jupiter.api.Test - -class ModifierComposableCheckTest { - - private val rule = ModifierComposableCheck(Config.empty) - - @Test - fun `errors when a composable Modifier extension is detected`() { - @Language("kotlin") - val code = - """ - @Composable - fun Modifier.something1(): Modifier { } - @Composable - fun Modifier.something2() = somethingElse() - """.trimIndent() - val errors = rule.lint(code) - assertThat(errors).hasTextLocations("something1", "something2") - assertThat(errors[0]).hasMessage(ModifierComposable.ComposableModifier) - assertThat(errors[1]).hasMessage(ModifierComposable.ComposableModifier) - } - - @Test - fun `do not error on a regular composable`() { - @Language("kotlin") - val code = """ - @Composable - fun TextHolder(text: String) {} - """.trimIndent() - - val errors = rule.lint(code) - assertThat(errors).isEmpty() - } -} diff --git a/rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/ComposeRuleSetProvider.kt b/rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/ComposeRuleSetProvider.kt index 2a5d65c5..3549db05 100644 --- a/rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/ComposeRuleSetProvider.kt +++ b/rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/ComposeRuleSetProvider.kt @@ -23,7 +23,6 @@ class ComposeRuleSetProvider : RuleProvider { LambdaParameterInRestartableEffectCheck() }, RuleProvider { Material2Check() }, RuleProvider { ModifierClickableOrderCheck() }, - RuleProvider { ModifierComposableCheck() }, RuleProvider { ModifierComposedCheck() }, RuleProvider { ModifierMissingCheck() }, RuleProvider { ModifierNamingCheck() }, diff --git a/rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/ModifierComposableCheck.kt b/rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/ModifierComposableCheck.kt deleted file mode 100644 index e8835b3d..00000000 --- a/rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/ModifierComposableCheck.kt +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2023 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.ModifierComposable - -class ModifierComposableCheck : - KtlintRule( - id = "compose:modifier-composable-check", - editorConfigProperties = setOf(customModifiers), - ), - ComposeKtVisitor by ModifierComposable() diff --git a/rules/ktlint/src/test/kotlin/io/nlopez/compose/rules/ktlint/ModifierComposableCheckTest.kt b/rules/ktlint/src/test/kotlin/io/nlopez/compose/rules/ktlint/ModifierComposableCheckTest.kt deleted file mode 100644 index d825876c..00000000 --- a/rules/ktlint/src/test/kotlin/io/nlopez/compose/rules/ktlint/ModifierComposableCheckTest.kt +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2023 Nacho Lopez -// SPDX-License-Identifier: Apache-2.0 -package io.nlopez.compose.rules.ktlint - -import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThatRule -import com.pinterest.ktlint.test.LintViolation -import io.nlopez.compose.rules.ModifierComposable -import org.intellij.lang.annotations.Language -import org.junit.jupiter.api.Test - -class ModifierComposableCheckTest { - - private val modifierRuleAssertThat = assertThatRule { ModifierComposableCheck() } - - @Test - fun `errors when a composable Modifier extension is detected`() { - @Language("kotlin") - val code = - """ - @Composable - fun Modifier.something(): Modifier { } - @Composable - fun Modifier.something() = somethingElse() - """.trimIndent() - - modifierRuleAssertThat(code).hasLintViolationsWithoutAutoCorrect( - LintViolation( - line = 2, - col = 14, - detail = ModifierComposable.ComposableModifier, - ), - LintViolation( - line = 4, - col = 14, - detail = ModifierComposable.ComposableModifier, - ), - ) - } - - @Test - fun `do not error on a regular composable`() { - @Language("kotlin") - val code = """ - @Composable - fun TextHolder(text: String) {} - """.trimIndent() - - modifierRuleAssertThat(code).hasNoLintViolations() - } -}