Repository: kiwicom/orbit-compose (kotlin)

Issue:
Alert usage lint - put buttons to actions slot.
It would be useful to lint that buttons are put into action slot and not in content.

Pull request:
closes #469

Patch:
diff --git a/lint/src/main/kotlin/kiwi/orbit/compose/lint/OrbitComposeIssueRegistry.kt b/lint/src/main/kotlin/kiwi/orbit/compose/lint/OrbitComposeIssueRegistry.kt
index de206d3ee..2a94465d0 100644
--- a/lint/src/main/kotlin/kiwi/orbit/compose/lint/OrbitComposeIssueRegistry.kt
+++ b/lint/src/main/kotlin/kiwi/orbit/compose/lint/OrbitComposeIssueRegistry.kt
@@ -3,6 +3,7 @@ package kiwi.orbit.compose.lint
 import com.android.tools.lint.client.api.IssueRegistry
 import com.android.tools.lint.client.api.Vendor
 import com.android.tools.lint.detector.api.CURRENT_API
+import kiwi.orbit.compose.lint.detectors.DialogButtonsInContentSlotDetector
 import kiwi.orbit.compose.lint.detectors.MaterialDesignInsteadOrbitDesignDetector
 import kiwi.orbit.compose.lint.detectors.MaterialDialogWithOrbitButtonsDetector
 import kiwi.orbit.compose.lint.detectors.ScaffoldContentPaddingDetector
@@ -10,6 +11,7 @@ import kiwi.orbit.compose.lint.detectors.ScaffoldContentPaddingDetector
 @Suppress("UnstableApiUsage")
 class OrbitComposeIssueRegistry : IssueRegistry() {
     override val issues = listOf(
+        DialogButtonsInContentSlotDetector.ISSUE,
         MaterialDesignInsteadOrbitDesignDetector.ISSUE,
         MaterialDialogWithOrbitButtonsDetector.ISSUE,
         ScaffoldContentPaddingDetector.ISSUE,
diff --git a/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/DialogButtonsInContentSlotDetector.kt b/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/DialogButtonsInContentSlotDetector.kt
new file mode 100644
index 000000000..3da9d3709
--- /dev/null
+++ b/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/DialogButtonsInContentSlotDetector.kt
@@ -0,0 +1,85 @@
+package kiwi.orbit.compose.lint.detectors
+
+import com.android.tools.lint.detector.api.Category
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Implementation
+import com.android.tools.lint.detector.api.Issue
+import com.android.tools.lint.detector.api.JavaContext
+import com.android.tools.lint.detector.api.Scope
+import com.android.tools.lint.detector.api.Severity
+import com.android.tools.lint.detector.api.SourceCodeScanner
+import com.intellij.psi.PsiElement
+import com.intellij.psi.PsiMethod
+import org.jetbrains.uast.UBlockExpression
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.skipParenthesizedExprDown
+import org.jetbrains.uast.toUElement
+import org.jetbrains.uast.tryResolveNamed
+
+class DialogButtonsInContentSlotDetector : Detector(), SourceCodeScanner {
+
+    companion object {
+        internal val ISSUE: Issue = Issue.create(
+            id = "DialogButtonsInContentSlotDetector",
+            briefDescription = "It is a mistake to place CTAs into content slot of AlertDialogs.",
+            explanation = "Buttons in dialogs should be placed into action slot, where proper sizes and styling is provided.",
+            category = Category.CUSTOM_LINT_CHECKS,
+            priority = 7,
+            severity = Severity.ERROR,
+            implementation = Implementation(
+                DialogButtonsInContentSlotDetector::class.java,
+                Scope.JAVA_FILE_SCOPE,
+            ),
+        )
+
+        private const val ALERT_CONTENT_PARAM = "content"
+        private const val PACKAGE_ORBIT_ALERT = "kiwi.orbit.compose.ui.controls"
+
+        private const val PACKAGE_ORBIT_BUTTON = "kiwi.orbit.compose.ui.controls"
+        private const val NAME_ORBIT_BUTTON = "Button"
+    }
+
+    override fun getApplicableMethodNames(): List<String> = listOf(
+        "AlertInfo", "AlertSuccess", "AlertWarning", "AlertCritical",
+    )
+
+    override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
+        if (method.isInPackageName(PACKAGE_ORBIT_ALERT)) {
+            val contentArgument = method.getArgument(node, ALERT_CONTENT_PARAM)
+            val topBlockExpression = contentArgument?.body as? UBlockExpression ?: return
+            topBlockExpression.sourcePsi?.findAllBlockExpressionsInHierarchy()?.forEach {
+                it.checkBlockExpression(context)
+            }
+        }
+    }
+
+    private fun UBlockExpression.checkBlockExpression(context: JavaContext) {
+        expressions.forEach {
+            val resolvedBodyExpression = it.skipParenthesizedExprDown().tryResolveNamed() ?: return@forEach
+            val packageName = resolvedBodyExpression.getPackageName() ?: return@forEach
+
+            val expressionName = resolvedBodyExpression.name
+            val isButton = expressionName?.contains(NAME_ORBIT_BUTTON) == true
+            val isOrbit = packageName == PACKAGE_ORBIT_BUTTON
+
+            if (isButton && isOrbit) {
+                context.report(
+                    issue = ISSUE,
+                    scope = it,
+                    location = context.getLocation(it),
+                    message = "$expressionName in Dialog's content slot. Use actions slot for CTAs instead.",
+                )
+            }
+        }
+    }
+
+    private fun PsiElement.findAllBlockExpressionsInHierarchy(): List<UBlockExpression> {
+        val expressions = mutableListOf<UBlockExpression>()
+
+        val uElement = this.toUElement()
+        if (uElement is UBlockExpression) expressions.add(uElement)
+        expressions.addAll(children.flatMap { it.findAllBlockExpressionsInHierarchy() })
+
+        return expressions
+    }
+}
diff --git a/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/LintUtils.kt b/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/LintUtils.kt
index cdd0d08f7..e3e49a730 100644
--- a/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/LintUtils.kt
+++ b/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/LintUtils.kt
@@ -16,14 +16,45 @@
 
 package kiwi.orbit.compose.lint.detectors
 
+import com.android.tools.lint.detector.api.computeKotlinArgumentMapping
+import com.intellij.psi.PsiElement
+import com.intellij.psi.PsiJavaFile
+import com.intellij.psi.PsiMember
+import com.intellij.psi.PsiMethod
 import org.jetbrains.kotlin.psi.KtLambdaExpression
 import org.jetbrains.kotlin.psi.KtParameter
 import org.jetbrains.kotlin.psi.KtSimpleNameExpression
 import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
 import org.jetbrains.kotlin.psi.psiUtil.isAncestor
+import org.jetbrains.uast.UCallExpression
 import org.jetbrains.uast.ULambdaExpression
 import org.jetbrains.uast.toUElement
 
+internal fun PsiMethod.isInPackageName(packageName: String): Boolean {
+    val actual = (containingFile as? PsiJavaFile)?.packageName
+    return packageName == actual
+}
+
+internal fun PsiElement.getPackageName(): String? = when (this) {
+    is PsiMember -> this.containingClass?.qualifiedName?.let { it.substring(0, it.lastIndexOf(".")) }
+    else -> null
+}
+
+/**
+ * Get arguments of a method. Useful for retrieving Compose Lambda expression used as arguments.
+ */
+internal fun PsiMethod.getArgument(
+    node: UCallExpression,
+    argumentName: String,
+): ULambdaExpression? = computeKotlinArgumentMapping(node, this)
+    .orEmpty()
+    .filter { (_, parameter) ->
+        parameter.name == argumentName
+    }
+    .keys
+    .filterIsInstance<ULambdaExpression>()
+    .firstOrNull()
+
 /**
  * Returns a list of unreferenced parameters in [this]. If no parameters have been specified, but
  * there is an implicit `it` parameter, this will return a list containing an
diff --git a/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/MaterialDesignInsteadOrbitDesignDetector.kt b/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/MaterialDesignInsteadOrbitDesignDetector.kt
index bbe6e1aa3..4fa5bfb24 100644
--- a/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/MaterialDesignInsteadOrbitDesignDetector.kt
+++ b/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/MaterialDesignInsteadOrbitDesignDetector.kt
@@ -16,7 +16,6 @@ import org.jetbrains.uast.UCallExpression
 import org.jetbrains.uast.UElement
 import org.jetbrains.uast.UQualifiedReferenceExpression
 import org.jetbrains.uast.USimpleNameReferenceExpression
-import org.jetbrains.uast.getContainingUClass
 import org.jetbrains.uast.toUElement
 import org.jetbrains.uast.tryResolveNamed
 
@@ -33,8 +32,7 @@ class MaterialDesignInsteadOrbitDesignDetector : Detector(), Detector.UastScanne
         return object : UElementHandler() {
             override fun visitCallExpression(node: UCallExpression) {
                 val name = node.methodName ?: return
-                val wrapperName = node.resolve()?.containingClass?.qualifiedName ?: return
-                val packageName = getPackageName(wrapperName)
+                val packageName = node.resolve()?.getPackageName()?: return
                 val fqn = "$packageName.$name"
                 val (preferredName) = METHOD_NAMES.entries.firstOrNull { it.value.contains(fqn) } ?: return
 
@@ -44,8 +42,7 @@ class MaterialDesignInsteadOrbitDesignDetector : Detector(), Detector.UastScanne
                     val parentExpression = node.sourcePsi?.parents?.find { it is KtCallExpression }
                     val resolved = parentExpression.toUElement()?.tryResolveNamed()
                     val parentName = resolved?.name
-                    val parentWrapper = resolved.toUElement()?.getContainingUClass()?.qualifiedName ?: ""
-                    val parentPackage = getPackageName(parentWrapper)
+                    val parentPackage = resolved?.getPackageName() ?: ""
                     val parentFqn = "$parentPackage.$parentName"
                     if (allowedEntry.value.find { parentFqn.contains(it) } != null) {
                         return
@@ -222,9 +219,5 @@ class MaterialDesignInsteadOrbitDesignDetector : Detector(), Detector.UastScanne
                 "Using $name instead of $preferredName",
             )
         }
-
-        private fun getPackageName(fullyQualifiedName: String): String {
-            return fullyQualifiedName.substring(0, fullyQualifiedName.lastIndexOf("."))
-        }
     }
 }
diff --git a/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/MaterialDialogWithOrbitButtonsDetector.kt b/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/MaterialDialogWithOrbitButtonsDetector.kt
index 4f9ca5779..bb204955f 100644
--- a/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/MaterialDialogWithOrbitButtonsDetector.kt
+++ b/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/MaterialDialogWithOrbitButtonsDetector.kt
@@ -8,15 +8,10 @@ import com.android.tools.lint.detector.api.JavaContext
 import com.android.tools.lint.detector.api.Scope
 import com.android.tools.lint.detector.api.Severity
 import com.android.tools.lint.detector.api.SourceCodeScanner
-import com.android.tools.lint.detector.api.computeKotlinArgumentMapping
-import com.intellij.psi.PsiElement
-import com.intellij.psi.PsiJavaFile
-import com.intellij.psi.PsiMember
 import com.intellij.psi.PsiMethod
 import com.intellij.psi.PsiNamedElement
 import org.jetbrains.uast.UBlockExpression
 import org.jetbrains.uast.UCallExpression
-import org.jetbrains.uast.ULambdaExpression
 import org.jetbrains.uast.skipParenthesizedExprDown
 import org.jetbrains.uast.tryResolveNamed
 
@@ -51,7 +46,7 @@ class MaterialDialogWithOrbitButtonsDetector : Detector(), SourceCodeScanner {
 
     override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
         if (method.isInPackageName(PACKAGE_MATERIAL_ALERT)) {
-            val confirmButtonArgument = getArgument(node, method, MATERIAL_ALERT_CONFIRM_BUTTON_PARAM)
+            val confirmButtonArgument = method.getArgument(node, MATERIAL_ALERT_CONFIRM_BUTTON_PARAM)
 
             (confirmButtonArgument?.body as? UBlockExpression)?.let { body ->
                 val resolvedBodyExpression = body.resolveFirstBodyExpression() ?: return@let
@@ -77,7 +72,7 @@ class MaterialDialogWithOrbitButtonsDetector : Detector(), SourceCodeScanner {
                 }
             }
 
-            val dismissButtonArgument = getArgument(node, method, MATERIAL_ALERT_DISMISS_BUTTON_PARAM)
+            val dismissButtonArgument = method.getArgument(node, MATERIAL_ALERT_DISMISS_BUTTON_PARAM)
 
             (dismissButtonArgument?.body as? UBlockExpression)?.let { body ->
                 val resolvedBodyExpression = body.resolveFirstBodyExpression() ?: return@let
@@ -105,29 +100,6 @@ class MaterialDialogWithOrbitButtonsDetector : Detector(), SourceCodeScanner {
         }
     }
 
-    private fun getArgument(
-        node: UCallExpression,
-        method: PsiMethod,
-        argumentName: String,
-    ): ULambdaExpression? = computeKotlinArgumentMapping(node, method)
-        .orEmpty()
-        .filter { (_, parameter) ->
-            parameter.name == argumentName
-        }
-        .keys
-        .filterIsInstance<ULambdaExpression>()
-        .firstOrNull()
-
-    private fun PsiMethod.isInPackageName(packageName: String): Boolean {
-        val actual = (containingFile as? PsiJavaFile)?.packageName
-        return packageName == actual
-    }
-
-    private fun PsiElement.getPackageName(): String? = when (this) {
-        is PsiMember -> this.containingClass?.qualifiedName?.let { it.substring(0, it.lastIndexOf(".")) }
-        else -> null
-    }
-
     private fun UBlockExpression.resolveFirstBodyExpression(): PsiNamedElement? {
         return expressions.firstOrNull()?.skipParenthesizedExprDown()?.tryResolveNamed()
     }
diff --git a/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/ScaffoldContentPaddingDetector.kt b/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/ScaffoldContentPaddingDetector.kt
index 84f997cd8..58bbf9d5f 100644
--- a/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/ScaffoldContentPaddingDetector.kt
+++ b/lint/src/main/kotlin/kiwi/orbit/compose/lint/detectors/ScaffoldContentPaddingDetector.kt
@@ -25,7 +25,6 @@ import com.android.tools.lint.detector.api.Scope
 import com.android.tools.lint.detector.api.Severity
 import com.android.tools.lint.detector.api.SourceCodeScanner
 import com.android.tools.lint.detector.api.computeKotlinArgumentMapping
-import com.intellij.psi.PsiJavaFile
 import com.intellij.psi.PsiMethod
 import java.util.EnumSet
 import org.jetbrains.uast.UCallExpression
@@ -86,9 +85,4 @@ class ScaffoldContentPaddingDetector :
             }
         }
     }
-
-    private fun PsiMethod.isInPackageName(packageName: String): Boolean {
-        val actual = (containingFile as? PsiJavaFile)?.packageName
-        return packageName == actual
-    }
 }


Test patch:
diff --git a/lint/src/test/kotlin/kiwi/orbit/compose/lint/detectors/DialogButtonsInContentSlotDetectorTest.kt b/lint/src/test/kotlin/kiwi/orbit/compose/lint/detectors/DialogButtonsInContentSlotDetectorTest.kt
new file mode 100644
index 000000000..46432cf87
--- /dev/null
+++ b/lint/src/test/kotlin/kiwi/orbit/compose/lint/detectors/DialogButtonsInContentSlotDetectorTest.kt
@@ -0,0 +1,101 @@
+package kiwi.orbit.compose.lint.detectors
+
+import com.android.tools.lint.checks.infrastructure.TestFiles
+import com.android.tools.lint.checks.infrastructure.TestLintTask
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.SourceCodeScanner
+import org.junit.Test
+
+
+class DialogButtonsInContentSlotDetectorTest: Detector(), SourceCodeScanner {
+
+    companion object {
+        private val ALERT_INFO_STUB = TestFiles.kotlin(
+            """
+            package kiwi.orbit.compose.ui.controls
+
+            fun AlertInfo(
+                actions: () -> Unit = {},
+                content: () -> Unit,
+            ) 
+            """.trimIndent(),
+        )
+
+        private val ORBIT_BUTTON_PRIMARY_STUB = TestFiles.kotlin(
+            """
+            package kiwi.orbit.compose.ui.controls
+            
+            fun ButtonPrimary(
+                onClick: () -> Unit,
+            ) 
+            
+            fun Column(
+                content: () -> Unit,
+            ) 
+        """.trimIndent(),
+        )
+
+        private val ORBIT_BUTTON_CRITICAL_STUB = TestFiles.kotlin(
+            """
+            package kiwi.orbit.compose.ui.controls
+            
+            fun ButtonCritical(
+                onClick: () -> Unit,
+            ) 
+        """.trimIndent(),
+        )
+    }
+
+
+    @Test
+    fun testDetector() {
+        TestLintTask.lint()
+            .files(
+                TestFiles.kotlin(
+                    """
+                    package foo
+
+                    import kiwi.orbit.compose.ui.controls.AlertInfo
+                    import kiwi.orbit.compose.ui.controls.ButtonPrimary
+                    import kiwi.orbit.compose.ui.controls.ButtonCritical
+                    import kiwi.orbit.compose.ui.controls.Column
+
+                    fun test() {
+                        AlertInfo { ButtonPrimary( { } ) }
+                        AlertInfo { ButtonCritical( { } ) }
+                        AlertInfo { Column { ButtonPrimary( { } ) } }
+                        AlertInfo { 
+                            Column { 
+                            }
+                            ButtonPrimary( { } ) 
+                        }
+                    }
+                    """,
+                ),
+                ALERT_INFO_STUB,
+                ORBIT_BUTTON_PRIMARY_STUB,
+                ORBIT_BUTTON_CRITICAL_STUB,
+            )
+            .issues(DialogButtonsInContentSlotDetector.ISSUE)
+            .allowMissingSdk()
+            .run()
+            .expectErrorCount(4)
+            .expect(
+                """
+                    src/foo/test.kt:10: Error: ButtonPrimary in Dialog's content slot. Use actions slot for CTAs instead. [DialogButtonsInContentSlotDetector]
+                                            AlertInfo { ButtonPrimary( { } ) }
+                                                        ~~~~~~~~~~~~~~~~~~~~
+                    src/foo/test.kt:11: Error: ButtonCritical in Dialog's content slot. Use actions slot for CTAs instead. [DialogButtonsInContentSlotDetector]
+                                            AlertInfo { ButtonCritical( { } ) }
+                                                        ~~~~~~~~~~~~~~~~~~~~~
+                    src/foo/test.kt:12: Error: ButtonPrimary in Dialog's content slot. Use actions slot for CTAs instead. [DialogButtonsInContentSlotDetector]
+                                            AlertInfo { Column { ButtonPrimary( { } ) } }
+                                                                 ~~~~~~~~~~~~~~~~~~~~
+                    src/foo/test.kt:16: Error: ButtonPrimary in Dialog's content slot. Use actions slot for CTAs instead. [DialogButtonsInContentSlotDetector]
+                                                ButtonPrimary( { } ) 
+                                                ~~~~~~~~~~~~~~~~~~~~
+                    4 errors, 0 warnings
+                """.trimIndent(),
+            )
+    }
+}