diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryApply.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryApply.kt index 5ed09649101..dc9a4647843 100644 --- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryApply.kt +++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryApply.kt @@ -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 { val resolvedCall = it.getResolvedCall(bindingContext) diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryApplySpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryApplySpec.kt index cd051c8214c..b9f6ce081ea 100644 --- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryApplySpec.kt +++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryApplySpec.kt @@ -117,12 +117,12 @@ class UnnecessaryApplySpec(val env: KotlinCoreEnvironment) { class C { var prop = 0 } - + fun main() { val list = ArrayList() 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` {