Skip to content

Commit

Permalink
Extract emission to count code to Composables.kt (#168)
Browse files Browse the repository at this point in the history 
Refactor to push up the layers the direct/indirect count emitters. It simplifies the MultipleEmitters and ContentEmitterReturningValues rules, as they had a lot of shared code.
  • Loading branch information
@mrmans0n
mrmans0n authored Dec 15, 2023
1 parent 7646aa2 commit 70912c3
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 78 deletions.
69 changes: 69 additions & 0 deletions core-common/src/main/kotlin/io/nlopez/rules/core/util/Composables.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ package io.nlopez.rules.core.util

import io.nlopez.rules.core.ComposeKtConfig
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.KtForExpression
import org.jetbrains.kotlin.psi.KtFunction
import org.jetbrains.kotlin.psi.KtProperty
import org.jetbrains.kotlin.psi.psiUtil.referenceExpression
Expand Down Expand Up @@ -70,6 +72,73 @@ private val KtCallExpression.containsComposablesWithModifiers: Boolean
.any { it.rootExpression.text == "Modifier" }
}

context(ComposeKtConfig)
private val KtFunction.directUiEmitterCount: Int
get() = bodyBlockExpression?.let { block ->
// If there's content emitted in a for loop, we assume there's at
// least two iterations and thus count any emitters in them as multiple
val forLoopCount = when {
block.forLoopHasUiEmitters -> 2
else -> 0
}
block.directUiEmitterCount + forLoopCount
} ?: 0

context(ComposeKtConfig)
private val KtBlockExpression.forLoopHasUiEmitters: Boolean
get() = statements.filterIsInstance<KtForExpression>().any {
when (val body = it.body) {
is KtBlockExpression -> body.directUiEmitterCount > 0
is KtCallExpression -> body.emitsContent
else -> false
}
}

context(ComposeKtConfig)
private val KtBlockExpression.directUiEmitterCount: Int
get() = statements.filterIsInstance<KtCallExpression>().count { it.emitsContent }

context(ComposeKtConfig)
private fun KtFunction.indirectUiEmitterCount(mapping: Map<KtFunction, Int>): Int {
val bodyBlock = bodyBlockExpression ?: return 0
return bodyBlock.statements
.filterIsInstance<KtCallExpression>()
.count { callExpression ->
// If it's a direct hit on our list, it should count directly
if (callExpression.emitsContent) return@count true

val name = callExpression.calleeExpression?.text ?: return@count false
// If the hit is in the provided mapping, it means it is using a composable that we know emits UI,
// that we inferred from previous passes
val value = mapping.mapKeys { entry -> entry.key.name }[name] ?: return@count false
value > 0
}
}

context(ComposeKtConfig)
fun Sequence<KtFunction>.createDirectComposableToEmissionCountMapping(): Map<KtFunction, Int> =
associateWith { it.directUiEmitterCount }

context(ComposeKtConfig)
fun refineComposableToEmissionCountMapping(
initialMapping: Map<KtFunction, Int>,
): Map<KtFunction, Int> {
var current = initialMapping

var shouldMakeAnotherPass = true
while (shouldMakeAnotherPass) {
val updatedMapping = current.mapValues { (functionNode, _) ->
functionNode.indirectUiEmitterCount(current)
}
when {
updatedMapping != current -> current = updatedMapping
else -> shouldMakeAnotherPass = false
}
}

return current
}

/**
* This is a denylist with common composables that emit content in their own window. Feel free to add more elements
* if you stumble upon false positives that should not have triggered an error from this rule, and are in foundational
Expand Down
26 changes: 8 additions & 18 deletions rules/common/src/main/kotlin/io/nlopez/compose/rules/ContentEmitterReturningValues.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,54 +2,44 @@
// SPDX-License-Identifier: Apache-2.0
package io.nlopez.compose.rules

import io.nlopez.compose.rules.MultipleContentEmitters.Companion.directUiEmitterCount
import io.nlopez.compose.rules.MultipleContentEmitters.Companion.indirectUiEmitterCount
import io.nlopez.rules.core.ComposeKtConfig
import io.nlopez.rules.core.ComposeKtVisitor
import io.nlopez.rules.core.Emitter
import io.nlopez.rules.core.report
import io.nlopez.rules.core.util.createDirectComposableToEmissionCountMapping
import io.nlopez.rules.core.util.findChildrenByClass
import io.nlopez.rules.core.util.hasReceiverType
import io.nlopez.rules.core.util.isComposable
import io.nlopez.rules.core.util.refineComposableToEmissionCountMapping
import io.nlopez.rules.core.util.returnsValue
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.KtFunction

class ContentEmitterReturningValues : ComposeKtVisitor {

override fun visitFile(file: KtFile, autoCorrect: Boolean, emitter: Emitter, config: ComposeKtConfig) {
val composableToEmissionCount = file.findChildrenByClass<KtFunction>()
val composables = file.findChildrenByClass<KtFunction>()
.filter { it.isComposable }
// We don't want to analyze composables that are extension functions, as they might be things like
// BoxScope which are legit, and we want to avoid false positives.
.filter { it.hasBlockBody() }
// We want only methods with a body
.filterNot { it.hasReceiverType }
// Now we want to get the count of direct emitters in them: the composables we know for a fact that output UI
.associateWith { with(config) { it.directUiEmitterCount } }

// Now we want to get the count of direct emitters in them: the composables we know for a fact that output UI
val composableToEmissionCount = with(config) { composables.createDirectComposableToEmissionCountMapping() }

// Now we can give some extra passes through the list of composables, and try to get a more accurate count.
// We want to make sure that if these composables are using other composables in this file that emit UI,
// those are taken into account too. For example:
// @Composable fun Comp1() { Text("Hi") }
// @Composable fun Comp2() { Text("Hola") }
// @Composable fun Comp3() { Comp1() Comp2() } // This wouldn't be picked up at first, but should after 1 loop
var current = composableToEmissionCount

var shouldMakeAnotherPass = true
while (shouldMakeAnotherPass) {
val updatedMapping = current.mapValues { (functionNode, _) ->
with(config) { functionNode.indirectUiEmitterCount(current) }
}
when {
updatedMapping != current -> current = updatedMapping
else -> shouldMakeAnotherPass = false
}
}
val mapping = with(config) { refineComposableToEmissionCountMapping(composableToEmissionCount) }

// Data in currentMapping should have all the # of emissions inferred for each composable in this file,
// so we want to iterate through all of them
current.filterValues { it > 0 }.keys
mapping.filterValues { it > 0 }.keys
// If the function doesn't have a return type or returns Unit explicitly, it's valid. Otherwise, show error.
.filter { it.returnsValue }
// In here we will have functions that emit UI and return a type other than Unit, which is no bueno.
Expand Down
64 changes: 4 additions & 60 deletions rules/common/src/main/kotlin/io/nlopez/compose/rules/MultipleContentEmitters.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@ import io.nlopez.rules.core.ComposeKtConfig
import io.nlopez.rules.core.ComposeKtVisitor
import io.nlopez.rules.core.Emitter
import io.nlopez.rules.core.report
import io.nlopez.rules.core.util.emitsContent
import io.nlopez.rules.core.util.createDirectComposableToEmissionCountMapping
import io.nlopez.rules.core.util.findChildrenByClass
import io.nlopez.rules.core.util.hasReceiverType
import io.nlopez.rules.core.util.isComposable
import org.jetbrains.kotlin.psi.KtBlockExpression
import org.jetbrains.kotlin.psi.KtCallExpression
import io.nlopez.rules.core.util.refineComposableToEmissionCountMapping
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.KtForExpression
import org.jetbrains.kotlin.psi.KtFunction

class MultipleContentEmitters : ComposeKtVisitor {
Expand All @@ -32,7 +30,7 @@ class MultipleContentEmitters : ComposeKtVisitor {
.filterNot { it.hasReceiverType }

// Now we want to get the count of direct emitters in them: the composables we know for a fact that output UI
val composableToEmissionCount = composables.associateWith { with(config) { it.directUiEmitterCount } }
val composableToEmissionCount = with(config) { composables.createDirectComposableToEmissionCountMapping() }

// We can start showing errors, for composables that emit more than once (from the list of known composables)
val directEmissionsReported = composableToEmissionCount.filterValues { it > 1 }.keys
Expand All @@ -46,18 +44,7 @@ class MultipleContentEmitters : ComposeKtVisitor {
// @Composable fun Comp1() { Text("Hi") }
// @Composable fun Comp2() { Text("Hola") }
// @Composable fun Comp3() { Comp1() Comp2() } // This wouldn't be picked up at first, but should after 1 loop
var currentMapping = composableToEmissionCount

var shouldMakeAnotherPass = true
while (shouldMakeAnotherPass) {
val updatedMapping = currentMapping.mapValues { (functionNode, _) ->
with(config) { functionNode.indirectUiEmitterCount(currentMapping) }
}
when {
updatedMapping != currentMapping -> currentMapping = updatedMapping
else -> shouldMakeAnotherPass = false
}
}
val currentMapping = with(config) { refineComposableToEmissionCountMapping(composableToEmissionCount) }

// Here we have the settled data after all the needed passes, so we want to show errors for them,
// if they were not caught already by the 1st emission loop
Expand All @@ -71,49 +58,6 @@ class MultipleContentEmitters : ComposeKtVisitor {

companion object {

context(ComposeKtConfig)
internal val KtFunction.directUiEmitterCount: Int
get() = bodyBlockExpression?.let { block ->
// If there's content emitted in a for loop, we assume there's at
// least two iterations and thus count any emitters in them as multiple
val forLoopCount = when {
block.forLoopHasUiEmitters -> 2
else -> 0
}
block.directUiEmitterCount + forLoopCount
} ?: 0

context(ComposeKtConfig)
internal val KtBlockExpression.forLoopHasUiEmitters: Boolean
get() = statements.filterIsInstance<KtForExpression>().any {
when (val body = it.body) {
is KtBlockExpression -> body.directUiEmitterCount > 0
is KtCallExpression -> body.emitsContent
else -> false
}
}

context(ComposeKtConfig)
internal val KtBlockExpression.directUiEmitterCount: Int
get() = statements.filterIsInstance<KtCallExpression>().count { it.emitsContent }

context(ComposeKtConfig)
internal fun KtFunction.indirectUiEmitterCount(mapping: Map<KtFunction, Int>): Int {
val bodyBlock = bodyBlockExpression ?: return 0
return bodyBlock.statements
.filterIsInstance<KtCallExpression>()
.count { callExpression ->
// If it's a direct hit on our list, it should count directly
if (callExpression.emitsContent) return@count true

val name = callExpression.calleeExpression?.text ?: return@count false
// If the hit is in the provided mapping, it means it is using a composable that we know emits UI,
// that we inferred from previous passes
val value = mapping.mapKeys { entry -> entry.key.name }.getOrElse(name) { return@count false }
value > 0
}
}

val MultipleContentEmittersDetected = """
Composable functions should only be emitting content into the composition from one source at their top level.
Expand Down

0 comments on commit 70912c3

Please sign in to comment.