From 7626f6952864ffa7aff87c10cc3cbe16caea3ec1 Mon Sep 17 00:00:00 2001
From: Dominic Zirbel <dominiczirbel@gmail.com>
Date: Tue, 31 May 2022 14:25:13 -0700
Subject: [PATCH] UselessCallOnNotNull: fix false positive for unresolved types

Fix an issue where UselessCallOnNotNull would report issues when some of the argument types resolved as ErrorType. While it correctly handles cases where the BindingContext cannot resolve a type (the first test case), in cases where a call is made on an unknown type (the second test case, e.g. via takeIf{}) the BindingContext will report it as an ErrorType. Since ErrorType.isNullable() apparently returns false we need to make an exception to ensure the rule behaves as expected.
---
 .../rules/style/UselessCallOnNotNull.kt       | 16 ++++++++---
 .../rules/style/UselessCallOnNotNullSpec.kt   | 27 +++++++++++++++++++
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNull.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNull.kt
index 6ba56af64c0..0c12e3d61e8 100644
--- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNull.kt
+++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNull.kt
@@ -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
 
 /*
@@ -92,14 +93,18 @@ 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
@@ -107,7 +112,10 @@ class UselessCallOnNotNull(config: Config = Config.empty) : Rule(config) {
         } else {
             wrapperType
         }
-        return type.isNullable()
+
+        return type
+            .takeUnless { it is ErrorType }
+            ?.isNullable()
     }
 
     private data class Conversion(val replacementName: String? = null)
diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNullSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNullSpec.kt
index ff6050099f3..e9f950749ae 100644
--- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNullSpec.kt
+++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNullSpec.kt
@@ -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
@@ -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 = """