Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

UnnecessaryApply: fix false negative for assignment #4948

Merged
merged 1 commit into from Jun 27, 2022
Merged

UnnecessaryApply: fix false negative for assignment #4948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 14 additions & 3 deletions ...t-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryApply.kt
View file
Open in desktop
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
48 changes: 46 additions & 2 deletions ...les-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnnecessaryApplySpec.kt
View file
Open in desktop
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