Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ComposeModifierReused when using nested KtDotQualifiedExpressions #106

Merged
merged 1 commit into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<String> {
var lastSize = 0
val tempModifierNames = mutableSetOf(initialName)
Expand Down Expand Up @@ -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.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
)
)
}
Expand Down