Ignore modifier factory functions in ModifierMissing/ModifierWithoutD…

…efault (#314)
mrmans0n · Aug 7, 2024 · 4f99319 · 4f99319
1 parent f9d68d5
commit 4f99319
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 18 deletions.
diff --git a/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierMissing.kt b/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierMissing.kt
@@ -9,6 +9,7 @@ import io.nlopez.compose.core.report
 import io.nlopez.compose.core.util.definedInInterface
 import io.nlopez.compose.core.util.emitsContent
 import io.nlopez.compose.core.util.isInternal
+import io.nlopez.compose.core.util.isModifierReceiver
 import io.nlopez.compose.core.util.isOverride
 import io.nlopez.compose.core.util.isPreview
 import io.nlopez.compose.core.util.modifierParameter
@@ -18,16 +19,18 @@ import org.jetbrains.kotlin.psi.psiUtil.isPublic
 
 class ModifierMissing : ComposeKtVisitor {
 
-    override fun visitComposable(function: KtFunction, emitter: Emitter, config: ComposeKtConfig) {
+    override fun visitComposable(function: KtFunction, emitter: Emitter, config: ComposeKtConfig) = with(config) {
         // We want to find all composable functions that:
         //  - emit content
         //  - are not overridden or part of an interface
         //  - are not a @Preview composable
+        //  - are not Modifier factory functions
         if (
             function.returnsValue ||
             function.isOverride ||
             function.definedInInterface ||
-            function.isPreview
+            function.isPreview ||
+            function.isModifierReceiver
         ) {
             return
         }
@@ -48,10 +51,10 @@ class ModifierMissing : ComposeKtVisitor {
         if (!shouldCheck) return
 
         // If there is a modifier param, we bail
-        if (with(config) { function.modifierParameter } != null) return
+        if (function.modifierParameter != null) return
 
         // In case we didn't find any `modifier` parameters, we check if it emits content and report the error if so.
-        if (with(config) { function.emitsContent }) {
+        if (function.emitsContent) {
             emitter.report(function, MissingModifierContentComposable)
         }
     }

diff --git a/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierWithoutDefault.kt b/rules/common/src/main/kotlin/io/nlopez/compose/rules/ModifierWithoutDefault.kt
@@ -10,6 +10,7 @@ import io.nlopez.compose.core.util.definedInInterface
 import io.nlopez.compose.core.util.isAbstract
 import io.nlopez.compose.core.util.isActual
 import io.nlopez.compose.core.util.isModifier
+import io.nlopez.compose.core.util.isModifierReceiver
 import io.nlopez.compose.core.util.isOpen
 import io.nlopez.compose.core.util.isOverride
 import io.nlopez.compose.core.util.lastChildLeafOrSelf
@@ -18,30 +19,29 @@ import org.jetbrains.kotlin.psi.KtFunction
 
 class ModifierWithoutDefault : ComposeKtVisitor {
 
-    override fun visitComposable(function: KtFunction, emitter: Emitter, config: ComposeKtConfig) {
+    override fun visitComposable(function: KtFunction, emitter: Emitter, config: ComposeKtConfig) = with(config) {
         if (
             function.definedInInterface ||
             function.isActual ||
             function.isOverride ||
             function.isAbstract ||
-            function.isOpen
+            function.isOpen ||
+            function.isModifierReceiver
         ) {
             return
         }
 
-        with(config) {
-            // Look for modifier params in the composable signature, and if any without a default value is found, error out.
-            function.valueParameters.filter { it.isModifier }
-                .filterNot { it.hasDefaultValue() }
-                .forEach { modifierParameter ->
-                    emitter.report(modifierParameter, MissingModifierDefaultParam, true).ifFix {
-                        // This error is easily auto fixable, we just inject ` = Modifier` to the param
-                        val lastToken = modifierParameter.node.lastChildLeafOrSelf() as LeafPsiElement
-                        val currentText = lastToken.text
-                        lastToken.rawReplaceWithText("$currentText = Modifier")
-                    }
+        // Look for modifier params in the composable signature, and if any without a default value is found, error out.
+        function.valueParameters.filter { it.isModifier }
+            .filterNot { it.hasDefaultValue() }
+            .forEach { modifierParameter ->
+                emitter.report(modifierParameter, MissingModifierDefaultParam, true).ifFix {
+                    // This error is easily auto fixable, we just inject ` = Modifier` to the param
+                    val lastToken = modifierParameter.node.lastChildLeafOrSelf() as LeafPsiElement
+                    val currentText = lastToken.text
+                    lastToken.rawReplaceWithText("$currentText = Modifier")
                 }
-        }
+            }
     }
 
     companion object {

diff --git a/rules/detekt/src/test/kotlin/io/nlopez/compose/rules/detekt/ModifierMissingCheckTest.kt b/rules/detekt/src/test/kotlin/io/nlopez/compose/rules/detekt/ModifierMissingCheckTest.kt
@@ -356,4 +356,20 @@ class ModifierMissingCheckTest {
         assertThat(errors).hasTextLocations("MyDialog")
         assertThat(errors[0]).hasMessage(ModifierMissing.MissingModifierContentComposable)
     }
+
+    @Test
+    fun `Modifier factory functions are ignored`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Modifier.Something() {
+                    Row {
+                    }
+                }
+            """.trimIndent()
+
+        val errors = rule.lint(code)
+        assertThat(errors).isEmpty()
+    }
 }
diff --git a/.../detekt/src/test/kotlin/io/nlopez/compose/rules/detekt/ModifierWithoutDefaultCheckTest.kt b/.../detekt/src/test/kotlin/io/nlopez/compose/rules/detekt/ModifierWithoutDefaultCheckTest.kt
@@ -94,4 +94,19 @@ class ModifierWithoutDefaultCheckTest {
         val errors = rule.lint(code)
         assertThat(errors).isEmpty()
     }
+
+    @Test
+    fun `passes for Modifier factory functions`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Modifier.something(modifier: Modifier) {
+                    Row(modifier = modifier) {
+                    }
+                }
+            """.trimIndent()
+        val errors = rule.lint(code)
+        assertThat(errors).isEmpty()
+    }
 }
diff --git a/rules/ktlint/src/test/kotlin/io/nlopez/compose/rules/ktlint/ModifierMissingCheckTest.kt b/rules/ktlint/src/test/kotlin/io/nlopez/compose/rules/ktlint/ModifierMissingCheckTest.kt
@@ -395,4 +395,19 @@ class ModifierMissingCheckTest {
             ),
         )
     }
+
+    @Test
+    fun `Modifier factory functions are ignored`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Modifier.Something() {
+                    Row {
+                    }
+                }
+            """.trimIndent()
+
+        modifierRuleAssertThat(code).hasNoLintViolations()
+    }
 }
diff --git a/.../ktlint/src/test/kotlin/io/nlopez/compose/rules/ktlint/ModifierWithoutDefaultCheckTest.kt b/.../ktlint/src/test/kotlin/io/nlopez/compose/rules/ktlint/ModifierWithoutDefaultCheckTest.kt
@@ -103,4 +103,18 @@ class ModifierWithoutDefaultCheckTest {
             """.trimIndent()
         modifierRuleAssertThat(code).hasNoLintViolations()
     }
+
+    @Test
+    fun `passes for Modifier factory functions`() {
+        @Language("kotlin")
+        val code =
+            """
+                @Composable
+                fun Modifier.something(modifier: Modifier) {
+                    Row(modifier = modifier) {
+                    }
+                }
+            """.trimIndent()
+        modifierRuleAssertThat(code).hasNoLintViolations()
+    }
 }