From fda504fd2a704faeb581ad0ddf4de86a2428c45e Mon Sep 17 00:00:00 2001 From: Alex Vanyo Date: Mon, 21 Oct 2024 10:11:08 -0700 Subject: [PATCH 1/2] Improve SlotReusedDetector By resolving the call references to check against the parameter, this allows removing the custom shadowing logic, and also fixes cases where a different reference with the same name as the slot was causing false positives. Fixes #424 and fixes #434 --- .../slack/lint/compose/SlotReusedDetector.kt | 38 ++++--------- .../lint/compose/SlotReusedDetectorTest.kt | 56 +++++++++++++++++++ 2 files changed, 67 insertions(+), 27 deletions(-) diff --git a/compose-lint-checks/src/main/java/slack/lint/compose/SlotReusedDetector.kt b/compose-lint-checks/src/main/java/slack/lint/compose/SlotReusedDetector.kt index f75212d2..550d1fbc 100644 --- a/compose-lint-checks/src/main/java/slack/lint/compose/SlotReusedDetector.kt +++ b/compose-lint-checks/src/main/java/slack/lint/compose/SlotReusedDetector.kt @@ -8,13 +8,13 @@ import com.android.tools.lint.detector.api.JavaContext import com.android.tools.lint.detector.api.Severity import com.android.tools.lint.detector.api.SourceCodeScanner import com.android.tools.lint.detector.api.TextFormat +import com.intellij.psi.PsiManager import org.jetbrains.kotlin.psi.KtCallExpression import org.jetbrains.kotlin.psi.KtFunction -import org.jetbrains.kotlin.psi.KtLambdaExpression -import org.jetbrains.kotlin.psi.KtNameReferenceExpression -import org.jetbrains.kotlin.psi.psiUtil.isAncestor import org.jetbrains.uast.UElement import org.jetbrains.uast.UMethod +import org.jetbrains.uast.toUElement +import org.jetbrains.uast.tryResolve import slack.lint.compose.util.Priorities import slack.lint.compose.util.findChildrenByClass import slack.lint.compose.util.slotParameters @@ -46,37 +46,21 @@ class SlotReusedDetector : ComposableFunctionDetector(), SourceCodeScanner { val slotParameters = method.slotParameters(context.evaluator) val callExpressions = composableBlockExpression.findChildrenByClass().toList() - val innerLambdas = composableBlockExpression.findChildrenByClass().toList() - slotParameters.forEach { slotParameter -> - // Find lambdas that shadow this parameter name, to make sure that they aren't shadowing - // the references we are looking through - val lambdasWithMatchingParameterName = - innerLambdas.filter { innerLambda -> - // Otherwise look to see if any of the parameters on the inner lambda have the - // same name - innerLambda.valueParameters - // Ignore parameters with a destructuring declaration instead of a named - // parameter - .filter { it.destructuringDeclaration == null } - .any { it.name == slotParameter.name } - } + val psiManager = PsiManager.getInstance(context.project.ideaProject) + slotParameters.forEach { slotParameter -> // Count all direct calls of the slot parameter. // NOTE: this misses cases where the slot parameter is passed as an argument to another // method, which may or may not invoke the slot parameter, but there are cases where that is // valid, like using the slot parameter as the key for a remember val slotParameterCallCount = - callExpressions - .filter { reference -> - // The parameter is referenced if there is at least one reference that isn't shadowed by - // an inner lambda - lambdasWithMatchingParameterName.none { it.isAncestor(reference) } - } - .count { reference -> - (reference.calleeExpression as? KtNameReferenceExpression)?.getReferencedName() == - slotParameter.name - } + callExpressions.count { callExpression -> + psiManager.areElementsEquivalent( + callExpression.calleeExpression?.toUElement()?.tryResolve(), + slotParameter, + ) + } // Report an issue if the slot parameter was invoked in multiple places if (slotParameterCallCount > 1) { diff --git a/compose-lint-checks/src/test/java/slack/lint/compose/SlotReusedDetectorTest.kt b/compose-lint-checks/src/test/java/slack/lint/compose/SlotReusedDetectorTest.kt index 245dc7d1..624d51aa 100644 --- a/compose-lint-checks/src/test/java/slack/lint/compose/SlotReusedDetectorTest.kt +++ b/compose-lint-checks/src/test/java/slack/lint/compose/SlotReusedDetectorTest.kt @@ -290,4 +290,60 @@ class SlotReusedDetectorTest : BaseComposeLintTest() { lint().files(*commonStubs, kotlin(code)).run().expectClean() } + + @Test + fun `passes when using same name is used as a different function`() { + @Language("kotlin") + val code = + """ + import androidx.compose.runtime.Composable + import androidx.compose.ui.Modifier + + @Composable + fun Something( + modifier: Modifier = Modifier, + first: @Composable () -> Unit, + ) { + Box(modifier) { + first() + listOf("1").first { it == "2" } + } + } + + """ + .trimIndent() + + lint().files(*commonStubs, kotlin(code)).run().expectClean() + } + + @Test + fun `passes when using same slot name is used with a different receiver`() { + @Language("kotlin") + val code = + """ + import androidx.compose.runtime.Composable + import androidx.compose.ui.Modifier + + class Section( + // other stuff + val content: @Composable () -> Unit, + ) + + @Composable + fun Something( + modifier: Modifier = Modifier, + section: Section? = null, + content: @Composable () -> Unit, + ) { + Box(modifier) { + content() + section?.content() + } + } + + """ + .trimIndent() + + lint().files(*commonStubs, kotlin(code)).run().expectClean() + } } From c9bf70240b3c27076d0fdf9831f113821e526163 Mon Sep 17 00:00:00 2001 From: Alex Vanyo Date: Mon, 21 Oct 2024 14:38:36 -0700 Subject: [PATCH 2/2] =?UTF-8?q?Use=20better=20PsiEquivalentUtil=20spell=20?= =?UTF-8?q?=F0=9F=AA=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../slack/lint/compose/SlotReusedDetector.kt | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/compose-lint-checks/src/main/java/slack/lint/compose/SlotReusedDetector.kt b/compose-lint-checks/src/main/java/slack/lint/compose/SlotReusedDetector.kt index 550d1fbc..5e45e539 100644 --- a/compose-lint-checks/src/main/java/slack/lint/compose/SlotReusedDetector.kt +++ b/compose-lint-checks/src/main/java/slack/lint/compose/SlotReusedDetector.kt @@ -8,7 +8,8 @@ import com.android.tools.lint.detector.api.JavaContext import com.android.tools.lint.detector.api.Severity import com.android.tools.lint.detector.api.SourceCodeScanner import com.android.tools.lint.detector.api.TextFormat -import com.intellij.psi.PsiManager +import com.intellij.codeInsight.PsiEquivalenceUtil +import com.intellij.psi.PsiElement import org.jetbrains.kotlin.psi.KtCallExpression import org.jetbrains.kotlin.psi.KtFunction import org.jetbrains.uast.UElement @@ -47,8 +48,6 @@ class SlotReusedDetector : ComposableFunctionDetector(), SourceCodeScanner { val callExpressions = composableBlockExpression.findChildrenByClass().toList() - val psiManager = PsiManager.getInstance(context.project.ideaProject) - slotParameters.forEach { slotParameter -> // Count all direct calls of the slot parameter. // NOTE: this misses cases where the slot parameter is passed as an argument to another @@ -56,10 +55,13 @@ class SlotReusedDetector : ComposableFunctionDetector(), SourceCodeScanner { // valid, like using the slot parameter as the key for a remember val slotParameterCallCount = callExpressions.count { callExpression -> - psiManager.areElementsEquivalent( - callExpression.calleeExpression?.toUElement()?.tryResolve(), - slotParameter, - ) + val calleeElement: PsiElement? = + callExpression.calleeExpression?.toUElement()?.tryResolve() + val slotElement: PsiElement? = slotParameter.sourceElement + + calleeElement != null && + slotElement != null && + PsiEquivalenceUtil.areElementsEquivalent(calleeElement, slotElement) } // Report an issue if the slot parameter was invoked in multiple places