Skip to content

Commit

Permalink
UnnecessaryApply: fix false negative for assignment (#4948)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dzirbel committed Jun 27, 2022
1 parent 0c879a7 commit c8eb24c
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 5 deletions.
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Expand Up @@ -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 {
Expand Down Expand Up @@ -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` {

Expand Down

0 comments on commit c8eb24c

Please sign in to comment.