UnnecessaryApply: fix false negative for assignment (#4948)

Fix a missed case of UnnecessaryApply where the apply contains a single field assignment but the result of the apply{} is unused. This pattern is actually used in 2/3 of the <noncompliant> examples and while it occurred in tests for false positives, no test actually checked that warnings were generated in the simple case.
This commit is contained in:
Dominic Zirbel
2022-06-27 08:09:25 -07:00
committed by GitHub
parent 0c879a7f37
commit c8eb24c04b
2 changed files with 60 additions and 5 deletions

View File

@@ -10,7 +10,9 @@ import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault
import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
import io.gitlab.arturbosch.detekt.rules.receiverIsUsed
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.psi.KtBinaryExpression
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
import org.jetbrains.kotlin.psi.KtNameReferenceExpression
@@ -75,11 +77,20 @@ class UnnecessaryApply(config: Config) : Rule(config) {
@Suppress("ReturnCount")
private fun KtCallExpression.hasOnlyOneMemberAccessStatement(): Boolean {
val lambda = lambdaArguments.firstOrNull()?.getLambdaExpression() ?: return false
val singleStatement = lambda.bodyExpression?.statements?.singleOrNull() ?: return false
if (singleStatement !is KtThisExpression &&
var singleStatement = lambda.bodyExpression?.statements?.singleOrNull() ?: return false
if (singleStatement is KtBinaryExpression) {
if (singleStatement.operationToken !in KtTokens.ALL_ASSIGNMENTS) return false
// for an assignment expression only consider whether members on the LHS use the apply{} context
singleStatement = singleStatement.left ?: return false
} else if (singleStatement !is KtThisExpression &&
singleStatement !is KtReferenceExpression &&
singleStatement !is KtDotQualifiedExpression
) return false
) {
return false
}
val lambdaDescriptor = bindingContext[BindingContext.FUNCTION, lambda.functionLiteral] ?: return false
return singleStatement.collectDescendantsOfType<KtNameReferenceExpression> {
val resolvedCall = it.getResolvedCall(bindingContext)

View File

@@ -117,12 +117,12 @@ class UnnecessaryApplySpec(val env: KotlinCoreEnvironment) {
class C {
var prop = 0
}
fun main() {
val list = ArrayList<C>()
list.add(
if (true) {
C().apply {
C().apply {
prop = 1
}
} else {
@@ -235,6 +235,50 @@ class UnnecessaryApplySpec(val env: KotlinCoreEnvironment) {
}
}
@Nested
inner class `unnecessary apply expressions that can be changed to assignment` {
@Test
fun `reports apply with a single assignment whose result is unused`() {
assertThat(
subject.compileAndLintWithContext(
env,
"""
class C {
var prop = 0
}
fun main() {
val c = C()
c.apply {
prop = 1
}
}
"""
)
).hasSize(1)
}
@Test
fun `does not report apply with a single assignment whose result is used`() {
assertThat(
subject.compileAndLintWithContext(
env,
"""
class C {
var prop = 0
}
fun main() {
val c = C().apply {
prop = 1
}
}
"""
)
).isEmpty()
}
}
@Nested
inner class `reported false positives - #1305` {