Mark OptionalUnit rule to require type resolution. (#6359)

detekt · Aug 6, 2023 · cb77385 · cb77385
1 parent 02c152f
commit cb77385
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 20 deletions.
diff --git a/...es-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnit.kt b/...es-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnit.kt
@@ -7,6 +7,7 @@ import io.gitlab.arturbosch.detekt.api.Entity
 import io.gitlab.arturbosch.detekt.api.Issue
 import io.gitlab.arturbosch.detekt.api.Rule
 import io.gitlab.arturbosch.detekt.api.Severity
+import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution
 import io.gitlab.arturbosch.detekt.rules.isOverride
 import org.jetbrains.kotlin.cfg.WhenChecker
 import org.jetbrains.kotlin.js.translate.callTranslator.getReturnType
@@ -18,7 +19,6 @@ import org.jetbrains.kotlin.psi.KtNamedFunction
 import org.jetbrains.kotlin.psi.KtTypeReference
 import org.jetbrains.kotlin.psi.KtWhenExpression
 import org.jetbrains.kotlin.psi.psiUtil.siblings
-import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression
 import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall
 import org.jetbrains.kotlin.resolve.calls.util.getType
@@ -50,7 +50,7 @@ import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull
  * override fun foo() = Unit
  * </compliant>
  */
-@Suppress("ViolatesTypeResolutionRequirements")
+@RequiresTypeResolution
 class OptionalUnit(config: Config = Config.empty) : Rule(config) {
 
     override val issue = Issue(
@@ -77,7 +77,7 @@ class OptionalUnit(config: Config = Config.empty) : Rule(config) {
             .filter {
                 when {
                     it !is KtNameReferenceExpression || it.text != UNIT -> false
-                    it != lastStatement || bindingContext == BindingContext.EMPTY -> true
+                    it != lastStatement -> true
                     !it.isUsedAsExpression(bindingContext) -> true
                     else -> {
                         val prev =
@@ -104,8 +104,10 @@ class OptionalUnit(config: Config = Config.empty) : Rule(config) {
                 val elseExpression = `else`
                 if (elseExpression is KtIfExpression) elseExpression.canBeUsedAsValue() else elseExpression != null
             }
+
             is KtWhenExpression ->
                 entries.lastOrNull()?.elseKeyword != null || WhenChecker.getMissingCases(this, bindingContext).isEmpty()
+
             else ->
                 true
         }
@@ -131,7 +133,6 @@ class OptionalUnit(config: Config = Config.empty) : Rule(config) {
         "defines a return type of Unit. This is unnecessary and can safely be removed."
 
     private fun KtExpression.isGenericOrNothingType(): Boolean {
-        if (bindingContext == BindingContext.EMPTY) return false
         val isGenericType = getResolvedCall(bindingContext)?.getReturnType()?.isTypeParameter() == true
         val isNothingType = getType(bindingContext)?.isNothing() == true
         // Either the function initializer returns Nothing or it is a generic function

diff --git a/...tyle/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnitSpec.kt b/...tyle/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnitSpec.kt
@@ -3,7 +3,6 @@ package io.gitlab.arturbosch.detekt.rules.style.optional
 import io.gitlab.arturbosch.detekt.api.Config
 import io.gitlab.arturbosch.detekt.api.Finding
 import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
-import io.gitlab.arturbosch.detekt.test.compileAndLint
 import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
 import org.assertj.core.api.Assertions.assertThat
 import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
@@ -17,7 +16,7 @@ class OptionalUnitSpec(val env: KotlinCoreEnvironment) {
     val subject = OptionalUnit(Config.empty)
 
     @Test
-    fun `should report when a function has an explicit Unit return type with context`() {
+    fun `should report when a function has an explicit Unit return type`() {
         val code = """
             fun foo(): Unit { }
         """.trimIndent()
@@ -51,7 +50,7 @@ class OptionalUnitSpec(val env: KotlinCoreEnvironment) {
 
         @BeforeEach
         fun beforeEachTest() {
-            findings = subject.compileAndLint(code)
+            findings = subject.compileAndLintWithContext(env, code)
         }
 
         @Test
@@ -82,7 +81,7 @@ class OptionalUnitSpec(val env: KotlinCoreEnvironment) {
                     override fun returnsUnit() = Unit
                 }
             """.trimIndent()
-            val findings = subject.compileAndLint(code)
+            val findings = subject.compileAndLintWithContext(env, code)
             assertThat(findings).isEmpty()
         }
     }
@@ -109,7 +108,7 @@ class OptionalUnitSpec(val env: KotlinCoreEnvironment) {
 
         @BeforeEach
         fun beforeEachTest() {
-            findings = subject.compileAndLint(code)
+            findings = subject.compileAndLintWithContext(env, code)
         }
 
         @Test
@@ -139,7 +138,7 @@ class OptionalUnitSpec(val env: KotlinCoreEnvironment) {
                     val i: (Int) -> Unit = { _ -> }
                 }
             """.trimIndent()
-            val findings = subject.compileAndLint(code)
+            val findings = subject.compileAndLintWithContext(env, code)
             assertThat(findings).isEmpty()
         }
     }
@@ -153,7 +152,7 @@ class OptionalUnitSpec(val env: KotlinCoreEnvironment) {
                     fun method(i: Int) = Unit
                 }
             """.trimIndent()
-            val findings = subject.compileAndLint(code)
+            val findings = subject.compileAndLintWithContext(env, code)
             assertThat(findings).hasSize(1)
         }
     }
@@ -371,15 +370,6 @@ class OptionalUnitSpec(val env: KotlinCoreEnvironment) {
             assertThat(findings).isEmpty()
         }
 
-        @Test
-        fun `should report on function initializers when there is no context`() {
-            val code = """
-                fun test(): Unit = throw UnsupportedOperationException()
-            """.trimIndent()
-            val findings = subject.compileAndLint(code)
-            assertThat(findings).hasSize(1)
-        }
-
         @Test
         fun `should report when the function initializer takes in the type Nothing`() {
             val code = """