Repository: detekt/detekt (kotlin)

Issue:
OptionalUnit conflicts with explicitApi=strict
## Expected Behavior
<!-- Tell us what should happen -->
It should be possible to enable [strict explicit API mode](https://kotlinlang.org/docs/whatsnew14.html#explicit-api-mode-for-library-authors) without getting conflicts with Detekt rules.

## Observed Behavior
<!-- Tell us what happens instead of the expected behavior -->
The OptionalUnit rule, if activated, considers an explicitly declared Unit return type as an error, and the compilation fails if the return type is implicit.

## Steps to Reproduce
<!-- Provide a link to a live example, or an unambiguous set of steps to -->
<!-- reproduce this bug. Include code to reproduce, if relevant -->
Enable both explicitApi = "strict" and OptionalUnit. 

## Context
<!-- How has this issue affected you? What are you trying to accomplish? -->
<!-- Providing context helps us come up with a solution that is most useful in the real world -->

## Your Environment
<!-- Include as many relevant details about the environment you experienced the bug in -->

Pull request:
Fixes https://github.com/detekt/detekt/issues/4647

Patch:
diff --git a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnit.kt b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnit.kt
index 3fdf373c4672..ab042ee7dee3 100644
--- a/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnit.kt
+++ b/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnit.kt
@@ -7,6 +7,8 @@ import io.gitlab.arturbosch.detekt.api.RequiresTypeResolution
 import io.gitlab.arturbosch.detekt.api.Rule
 import io.gitlab.arturbosch.detekt.rules.isOverride
 import org.jetbrains.kotlin.cfg.WhenChecker
+import org.jetbrains.kotlin.config.AnalysisFlags
+import org.jetbrains.kotlin.config.ExplicitApiMode
 import org.jetbrains.kotlin.psi.KtBlockExpression
 import org.jetbrains.kotlin.psi.KtExpression
 import org.jetbrains.kotlin.psi.KtIfExpression
@@ -14,6 +16,7 @@ import org.jetbrains.kotlin.psi.KtNameReferenceExpression
 import org.jetbrains.kotlin.psi.KtNamedFunction
 import org.jetbrains.kotlin.psi.KtTypeReference
 import org.jetbrains.kotlin.psi.KtWhenExpression
+import org.jetbrains.kotlin.psi.psiUtil.isPublic
 import org.jetbrains.kotlin.psi.psiUtil.siblings
 import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression
 import org.jetbrains.kotlin.resolve.calls.util.getResolvedCall
@@ -103,11 +106,19 @@ class OptionalUnit(config: Config) : Rule(
                 true
         }
 
+    private fun isExplicitApiModeActive(): Boolean {
+        val resources = compilerResources ?: return false
+        val flag = resources.languageVersionSettings.getFlag(AnalysisFlags.explicitApiMode)
+        return flag != ExplicitApiMode.DISABLED
+    }
+
     private fun checkFunctionWithExplicitReturnType(function: KtNamedFunction, typeReference: KtTypeReference) {
         val typeElementText = typeReference.typeElement?.text
         if (typeElementText == UNIT) {
             val initializer = function.initializer
             if (initializer?.isGenericOrNothingType() == true) return
+            // case when explicit api is on so in case of expression body we need Unit
+            if (initializer != null && isExplicitApiModeActive() && function.isPublic) return
             report(CodeSmell(Entity.from(typeReference), createMessage(function)))
         }
     }


Test patch:
diff --git a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnitSpec.kt b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnitSpec.kt
index ab4fd3856658..1696a474fb45 100644
--- a/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnitSpec.kt
+++ b/detekt-rules-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/optional/OptionalUnitSpec.kt
@@ -1,11 +1,21 @@
 package io.gitlab.arturbosch.detekt.rules.style.optional
 
+import io.github.detekt.test.utils.compileContentForTest
+import io.gitlab.arturbosch.detekt.api.CompilerResources
 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.compileAndLintWithContext
 import org.assertj.core.api.Assertions.assertThat
 import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
+import org.jetbrains.kotlin.config.AnalysisFlag
+import org.jetbrains.kotlin.config.AnalysisFlags
+import org.jetbrains.kotlin.config.ApiVersion
+import org.jetbrains.kotlin.config.ExplicitApiMode
+import org.jetbrains.kotlin.config.LanguageFeature
+import org.jetbrains.kotlin.config.LanguageVersion
+import org.jetbrains.kotlin.config.LanguageVersionSettings
+import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactoryImpl
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
@@ -396,4 +406,118 @@ class OptionalUnitSpec(val env: KotlinCoreEnvironment) {
             assertThat(findings).hasSize(1)
         }
     }
+
+    @Nested
+    inner class `with explicit api` {
+        private val blockFunWithUnitCode = compileContentForTest(
+            """
+                fun foo(): Unit { }
+            """.trimIndent()
+        )
+
+        private fun getExpressionFunWithUnitCode(modifier: String = "") = compileContentForTest(
+            """
+                $modifier fun foo(): Unit = println("")
+            """.trimIndent()
+        )
+
+        private fun fakeCompilerResources(mode: ExplicitApiMode): CompilerResources {
+            val languageVersionSettings = object : LanguageVersionSettings {
+                override val apiVersion: ApiVersion get() = error("This is a fake")
+
+                override val languageVersion: LanguageVersion get() = error("This is a fake")
+
+                override fun getFeatureSupport(feature: LanguageFeature) = error("This is a fake")
+
+                override fun <T> getFlag(flag: AnalysisFlag<T>): T {
+                    @Suppress("UNCHECKED_CAST")
+                    return when (flag) {
+                        AnalysisFlags.explicitApiMode -> mode as T
+                        else -> error("This is a fake")
+                    }
+                }
+
+                override fun isPreRelease() = error("This is a fake")
+            }
+            return CompilerResources(
+                languageVersionSettings,
+                DataFlowValueFactoryImpl(languageVersionSettings)
+            )
+        }
+
+        @Test
+        fun `should report when a function has an explicit Unit return type when config is strict`() {
+            val findings = subject.visitFile(
+                blockFunWithUnitCode,
+                compilerResources = fakeCompilerResources(ExplicitApiMode.STRICT)
+            )
+            assertThat(findings).hasSize(1)
+        }
+
+        @Test
+        fun `should report when a function has an explicit Unit return type when config is warning`() {
+            val findings = subject.visitFile(
+                blockFunWithUnitCode,
+                compilerResources = fakeCompilerResources(ExplicitApiMode.WARNING)
+            )
+            assertThat(findings).hasSize(1)
+        }
+
+        @Test
+        fun `should report when a function has an explicit Unit return type when config is disabled`() {
+            val findings = subject.visitFile(
+                blockFunWithUnitCode,
+                compilerResources = fakeCompilerResources(ExplicitApiMode.DISABLED)
+            )
+            assertThat(findings).hasSize(1)
+        }
+
+        @Test
+        fun `should not report when a function has expression with Unit return type when config is strict`() {
+            val findings = subject.visitFile(
+                getExpressionFunWithUnitCode(),
+                compilerResources = fakeCompilerResources(ExplicitApiMode.STRICT)
+            )
+            assertThat(findings).isEmpty()
+        }
+
+        @Test
+        fun `should not report when a function has expression with Unit return type when config is warning`() {
+            val findings = subject.visitFile(
+                getExpressionFunWithUnitCode(),
+                compilerResources = fakeCompilerResources(ExplicitApiMode.WARNING)
+            )
+            assertThat(findings).isEmpty()
+        }
+
+        @Test
+        fun `should report private when a function has expression with Unit return type when config is strict`() {
+            val findingsWithPrivate = subject.visitFile(
+                getExpressionFunWithUnitCode("private"),
+                compilerResources = fakeCompilerResources(ExplicitApiMode.STRICT)
+            )
+            assertThat(findingsWithPrivate).hasSize(1)
+
+            val findingsWithInternal = subject.visitFile(
+                getExpressionFunWithUnitCode("internal"),
+                compilerResources = fakeCompilerResources(ExplicitApiMode.STRICT)
+            )
+            assertThat(findingsWithInternal).hasSize(1)
+        }
+
+        @Test
+        fun `should report private and internal when a function has expression with Unit return type when config is warning`() {
+            val findingsWithPrivate = subject.visitFile(
+                getExpressionFunWithUnitCode("private"),
+                compilerResources = fakeCompilerResources(ExplicitApiMode.WARNING)
+            )
+            assertThat(findingsWithPrivate).hasSize(1)
+
+            val findingsWithInternal = subject.visitFile(
+                getExpressionFunWithUnitCode("internal"),
+                compilerResources = fakeCompilerResources(ExplicitApiMode.WARNING)
+            )
+            assertThat(findingsWithInternal).hasSize(1)
+        }
+    }
 }