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.

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

UselessCallOnNotNull: fix false positive for unresolved types #4880

Merged
merged 1 commit into from Jun 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -18,6 +18,7 @@ import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall
import org.jetbrains.kotlin.resolve.calls.util.getType
import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameOrNull
import org.jetbrains.kotlin.types.ErrorType
import org.jetbrains.kotlin.types.isNullable

/*
Expand Down Expand Up @@ -92,22 +93,29 @@ class UselessCallOnNotNull(config: Config = Config.empty) : Rule(config) {
val fqName = resolvedCall.resultingDescriptor.fqNameOrNull()
if (fqName == listOfNotNull) {
val varargs = resolvedCall.valueArguments.entries.single().value.arguments
if (varargs.none { it.isNullable() }) {
if (varargs.all { it.isNullable() == false }) {
report(CodeSmell(issue, Entity.from(expression), "Replace listOfNotNull with listOf"))
}
}
}

private fun ValueArgument.isNullable(): Boolean {
val wrapperType = getArgumentExpression()?.getType(bindingContext) ?: return false
/**
* Determines whether this [ValueArgument] is nullable, returning null if its type cannot be
* determined.
*/
private fun ValueArgument.isNullable(): Boolean? {
val wrapperType = getArgumentExpression()?.getType(bindingContext) ?: return null
val type = if (getSpreadElement() != null) {
// in case of a spread operator (`*list`),
// we actually want to get the generic parameter from the collection
wrapperType.arguments.first().type
} else {
wrapperType
}
return type.isNullable()

return type
.takeUnless { it is ErrorType }
?.isNullable()
}

private data class Conversion(val replacementName: String? = null)
Expand Down
Expand Up @@ -2,6 +2,7 @@ package io.gitlab.arturbosch.detekt.rules.style

import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
import io.gitlab.arturbosch.detekt.test.lintWithContext
import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.junit.jupiter.api.Test
Expand Down Expand Up @@ -233,6 +234,32 @@ class UselessCallOnNotNullSpec(val env: KotlinCoreEnvironment) {
assertThat(findings).isEmpty()
}

@Test
fun `does not report when calling listOfNotNull with values whose type is unknown`() {
val code = """
fun test() {
listOfNotNull(unknown)
}
""".trimIndent()

val findings = subject.lintWithContext(env, code)
assertThat(findings).isEmpty()
}

@Test
fun `does not report when calling listOfNotNull with values whose type is derived and unknown`() {
val code = """
import kotlin.random.Random

fun test() {
listOfNotNull(unknown.takeIf { Random.nextBoolean() })
}
""".trimIndent()

val findings = subject.lintWithContext(env, code)
assertThat(findings).isEmpty()
}

@Test
fun `reports when calling isNullOrEmpty on a list`() {
val code = """
Expand Down