diff --git a/rules/common/src/main/kotlin/com/twitter/compose/rules/ComposeModifierReused.kt b/rules/common/src/main/kotlin/com/twitter/compose/rules/ComposeModifierReused.kt index 6fa890a2..393348bf 100644 --- a/rules/common/src/main/kotlin/com/twitter/compose/rules/ComposeModifierReused.kt +++ b/rules/common/src/main/kotlin/com/twitter/compose/rules/ComposeModifierReused.kt @@ -11,6 +11,7 @@ import org.jetbrains.kotlin.com.intellij.psi.PsiElement import org.jetbrains.kotlin.psi.KtBlockExpression import org.jetbrains.kotlin.psi.KtCallExpression import org.jetbrains.kotlin.psi.KtDotQualifiedExpression +import org.jetbrains.kotlin.psi.KtExpression import org.jetbrains.kotlin.psi.KtFunction import org.jetbrains.kotlin.psi.KtProperty import org.jetbrains.kotlin.psi.KtReferenceExpression @@ -75,13 +76,24 @@ class ComposeModifierReused : ComposeKtVisitor { } // if it's MyComposable(modifier.fillMaxWidth()) or similar is KtDotQualifiedExpression -> { - modifierNames.contains(expression.receiverExpression.text) + // On cases of multiple nested KtDotQualifiedExpressions (e.g. multiple chained methods) + // we need to iterate until we find the start of the chain + modifierNames.contains(expression.rootExpression.text) } else -> false } } + private val KtDotQualifiedExpression.rootExpression: KtExpression + get() { + var current: KtExpression = receiverExpression + while (current is KtDotQualifiedExpression) { + current = current.receiverExpression + } + return current + } + private fun KtBlockExpression.obtainAllModifierNames(initialName: String): List { var lastSize = 0 val tempModifierNames = mutableSetOf(initialName) @@ -117,8 +129,7 @@ class ComposeModifierReused : ComposeKtVisitor { companion object { val ModifierShouldBeUsedOnceOnly = """ - Modifiers should only be used once and by the root level layout of a Composable. This is true even if - appended to or with other modifiers e.g. 'modifier.fillMaxWidth()'. + Modifiers should only be used once and by the root level layout of a Composable. This is true even if appended to or with other modifiers e.g. 'modifier.fillMaxWidth()'. Use Modifier (with a capital 'M') to construct a new Modifier that you can pass to other composables. diff --git a/rules/detekt/src/test/kotlin/com/twitter/compose/rules/detekt/ComposeModifierReusedCheckTest.kt b/rules/detekt/src/test/kotlin/com/twitter/compose/rules/detekt/ComposeModifierReusedCheckTest.kt index 53c2eae0..7ab84a31 100644 --- a/rules/detekt/src/test/kotlin/com/twitter/compose/rules/detekt/ComposeModifierReusedCheckTest.kt +++ b/rules/detekt/src/test/kotlin/com/twitter/compose/rules/detekt/ComposeModifierReusedCheckTest.kt @@ -47,10 +47,23 @@ class ComposeModifierReusedCheckTest { SomethingElse(myMod) } } + @Composable + fun FoundThisOneInTheWild(modifier: Modifier = Modifier) { + Box( + modifier = modifier + .size(AvatarSize.Default.size) + .clip(CircleShape) + .then(colorModifier) + ) { + Box( + modifier = modifier.padding(spacesBorderWidth) + ) + } + } """.trimIndent() val errors = rule.lint(code) - assertThat(errors).hasSize(9) + assertThat(errors) .hasSourceLocations( SourceLocation(3, 5), SourceLocation(4, 9), @@ -60,7 +73,9 @@ class ComposeModifierReusedCheckTest { SourceLocation(19, 5), SourceLocation(20, 5), SourceLocation(25, 9), - SourceLocation(26, 9) + SourceLocation(26, 9), + SourceLocation(31, 5), + SourceLocation(37, 9) ) for (error in errors) { assertThat(error).hasMessage(ComposeModifierReused.ModifierShouldBeUsedOnceOnly) diff --git a/rules/ktlint/src/test/kotlin/com/twitter/compose/rules/ktlint/ComposeModifierReusedCheckTest.kt b/rules/ktlint/src/test/kotlin/com/twitter/compose/rules/ktlint/ComposeModifierReusedCheckTest.kt index fea65b3e..cbb96b9f 100644 --- a/rules/ktlint/src/test/kotlin/com/twitter/compose/rules/ktlint/ComposeModifierReusedCheckTest.kt +++ b/rules/ktlint/src/test/kotlin/com/twitter/compose/rules/ktlint/ComposeModifierReusedCheckTest.kt @@ -45,6 +45,19 @@ class ComposeModifierReusedCheckTest { SomethingElse(myMod) } } + @Composable + fun FoundThisOneInTheWild(modifier: Modifier = Modifier) { + Box( + modifier = modifier + .size(AvatarSize.Default.size) + .clip(CircleShape) + .then(colorModifier) + ) { + Box( + modifier = modifier.padding(spacesBorderWidth) + ) + } + } """.trimIndent() modifierRuleAssertThat(code).hasLintViolationsWithoutAutoCorrect( @@ -92,6 +105,16 @@ class ComposeModifierReusedCheckTest { line = 26, col = 9, detail = ComposeModifierReused.ModifierShouldBeUsedOnceOnly + ), + LintViolation( + line = 31, + col = 5, + detail = ComposeModifierReused.ModifierShouldBeUsedOnceOnly + ), + LintViolation( + line = 37, + col = 9, + detail = ComposeModifierReused.ModifierShouldBeUsedOnceOnly ) ) }