diff --git a/core-common/src/main/kotlin/com/twitter/rules/core/util/Composables.kt b/core-common/src/main/kotlin/com/twitter/rules/core/util/Composables.kt index 4154a2d1..0e1ffe0f 100644 --- a/core-common/src/main/kotlin/com/twitter/rules/core/util/Composables.kt +++ b/core-common/src/main/kotlin/com/twitter/rules/core/util/Composables.kt @@ -3,6 +3,7 @@ package com.twitter.rules.core.util import com.twitter.rules.core.ComposeKtConfig.Companion.config +import org.jetbrains.kotlin.com.intellij.psi.PsiElement import org.jetbrains.kotlin.psi.KtCallExpression import org.jetbrains.kotlin.psi.KtCallableDeclaration import org.jetbrains.kotlin.psi.KtFunction @@ -11,7 +12,36 @@ import org.jetbrains.kotlin.psi.KtProperty import org.jetbrains.kotlin.psi.psiUtil.referenceExpression val KtFunction.emitsContent: Boolean - get() = if (isComposable) findChildrenByClass().any { it.emitsContent } else false + get() { + return if (isComposable) { + sequence { + tailrec suspend fun SequenceScope.scan(elements: List) { + if (elements.isEmpty()) return + val toProcess = elements + .mapNotNull { current -> + if (current is KtCallExpression) { + if (current.emitExplicitlyNoContent) { + null + } else { + yield(current) + current + } + } else { + current + } + } + .flatMap { it.children.toList() } + return scan(toProcess) + } + scan(listOf(this@emitsContent)) + }.any { it.emitsContent } + } else { + false + } + } + +private val KtCallExpression.emitExplicitlyNoContent: Boolean + get() = calleeExpression?.text in ComposableNonEmittersList val KtCallExpression.emitsContent: Boolean get() { @@ -28,6 +58,15 @@ private val KtCallExpression.containsComposablesWithModifiers: Boolean .filter { it.isNamed() } .any { it.getArgumentName()?.text == "modifier" } +/** + * This is a denylist with common composables that emit content in their own window. Feel free to add more elements + * if you stumble upon them in code reviews that should have triggered an error from this rule. + */ +private val ComposableNonEmittersList = setOf( + "AlertDialog", + "ModalBottomSheetLayout" +) + /** * This is an allowlist with common composables that emit content. Feel free to add more elements if you stumble * upon them in code reviews that should have triggered an error from this rule. diff --git a/core-common/src/main/kotlin/com/twitter/rules/core/util/PsiElements.kt b/core-common/src/main/kotlin/com/twitter/rules/core/util/PsiElements.kt index 03af82a2..68d9450d 100644 --- a/core-common/src/main/kotlin/com/twitter/rules/core/util/PsiElements.kt +++ b/core-common/src/main/kotlin/com/twitter/rules/core/util/PsiElements.kt @@ -5,28 +5,27 @@ package com.twitter.rules.core.util import org.jetbrains.kotlin.com.intellij.psi.PsiElement import org.jetbrains.kotlin.com.intellij.psi.PsiNameIdentifierOwner import org.jetbrains.kotlin.psi.psiUtil.startOffset -import java.util.* +import java.util.Deque +import java.util.LinkedList inline fun PsiElement.findChildrenByClass(): Sequence = sequence { - val klass = T::class val queue: Deque = LinkedList() queue.add(this@findChildrenByClass) while (queue.isNotEmpty()) { val current = queue.pop() - if (klass.isInstance(current)) { - yield(current as T) + if (current is T) { + yield(current) } queue.addAll(current.children) } } inline fun PsiElement.findDirectFirstChildByClass(): T? { - val klass = T::class var current = firstChild while (current != null) { - if (klass.isInstance(current)) { - return current as T + if (current is T) { + return current } current = current.nextSibling } @@ -35,11 +34,10 @@ inline fun PsiElement.findDirectFirstChildByClass(): T? inline fun PsiElement.findDirectChildrenByClass(): Sequence = sequence { - val klass = T::class var current = firstChild while (current != null) { - if (klass.isInstance(current)) { - yield(current as T) + if (current is T) { + yield(current) } current = current.nextSibling } diff --git a/rules/detekt/src/test/kotlin/com/twitter/compose/rules/detekt/ComposeModifierMissingCheckTest.kt b/rules/detekt/src/test/kotlin/com/twitter/compose/rules/detekt/ComposeModifierMissingCheckTest.kt index ef2c002c..a2d7744b 100644 --- a/rules/detekt/src/test/kotlin/com/twitter/compose/rules/detekt/ComposeModifierMissingCheckTest.kt +++ b/rules/detekt/src/test/kotlin/com/twitter/compose/rules/detekt/ComposeModifierMissingCheckTest.kt @@ -200,4 +200,45 @@ class ComposeModifierMissingCheckTest { val errors = rule.lint(code) assertThat(errors).isEmpty() } + + @Test + fun `non content emitting root composables are ignored`() { + @Language("kotlin") + val code = + """ + @Composable + fun MyDialog() { + AlertDialog( + onDismissRequest = { /*TODO*/ }, + buttons = { Text(text = "Button") }, + text = { Text(text = "Body") }, + ) + } + """.trimIndent() + + val errors = rule.lint(code) + assertThat(errors).isEmpty() + } + + @Test + fun `non content emitter with content emitter not ignored`() { + @Language("kotlin") + val code = + """ + @Composable + fun MyDialog() { + Text(text = "Unicorn") + + AlertDialog( + onDismissRequest = { /*TODO*/ }, + buttons = { Text(text = "Button") }, + text = { Text(text = "Body") }, + ) + } + """.trimIndent() + + val errors = rule.lint(code) + assertThat(errors).hasTextLocations("MyDialog") + assertThat(errors[0]).hasMessage(ComposeModifierMissing.MissingModifierContentComposable) + } } diff --git a/rules/ktlint/src/test/kotlin/com/twitter/compose/rules/ktlint/ComposeModifierMissingCheckTest.kt b/rules/ktlint/src/test/kotlin/com/twitter/compose/rules/ktlint/ComposeModifierMissingCheckTest.kt index 708ed180..65c38dfb 100644 --- a/rules/ktlint/src/test/kotlin/com/twitter/compose/rules/ktlint/ComposeModifierMissingCheckTest.kt +++ b/rules/ktlint/src/test/kotlin/com/twitter/compose/rules/ktlint/ComposeModifierMissingCheckTest.kt @@ -214,4 +214,48 @@ class ComposeModifierMissingCheckTest { modifierRuleAssertThat(code).hasNoLintViolations() } + + @Test + fun `non content emitting root composables are ignored`() { + @Language("kotlin") + val code = + """ + @Composable + fun MyDialog() { + AlertDialog( + onDismissRequest = { /*TODO*/ }, + buttons = { Text(text = "Button") }, + text = { Text(text = "Body") }, + ) + } + """.trimIndent() + + modifierRuleAssertThat(code).hasNoLintViolations() + } + + @Test + fun `non content emitter with content emitter not ignored`() { + @Language("kotlin") + val code = + """ + @Composable + fun MyDialog() { + Text(text = "Unicorn") + + AlertDialog( + onDismissRequest = { /*TODO*/ }, + buttons = { Text(text = "Button") }, + text = { Text(text = "Body") }, + ) + } + """.trimIndent() + + modifierRuleAssertThat(code).hasLintViolationsWithoutAutoCorrect( + LintViolation( + line = 2, + col = 5, + detail = ComposeModifierMissing.MissingModifierContentComposable + ) + ) + } }