diff --git a/build.gradle.kts b/build.gradle.kts index 2420538..4449c1a 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -22,7 +22,7 @@ val ktlint: Configuration by configurations.creating dependencies { compileOnly("io.gitlab.arturbosch.detekt:detekt-api:1.22.0") - testImplementation(platform("io.arrow-kt:arrow-stack:1.1.5")) + testImplementation(platform("io.arrow-kt:arrow-stack:1.1.6-alpha.91")) testImplementation("io.arrow-kt:arrow-core") testImplementation("io.gitlab.arturbosch.detekt:detekt-test:1.22.0") diff --git a/src/main/kotlin/com/wolt/arrow/detekt/rules/NoEffectScopeBindableValueAsStatement.kt b/src/main/kotlin/com/wolt/arrow/detekt/rules/NoEffectScopeBindableValueAsStatement.kt index d15e1a0..a87c53b 100644 --- a/src/main/kotlin/com/wolt/arrow/detekt/rules/NoEffectScopeBindableValueAsStatement.kt +++ b/src/main/kotlin/com/wolt/arrow/detekt/rules/NoEffectScopeBindableValueAsStatement.kt @@ -13,6 +13,7 @@ import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.KtBlockExpression import org.jetbrains.kotlin.psi.KtExpression import org.jetbrains.kotlin.psi.KtLambdaExpression +import org.jetbrains.kotlin.psi.KtNamedFunction import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForReceiver import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForSelectorOrThis import org.jetbrains.kotlin.resolve.BindingContext @@ -31,6 +32,25 @@ class NoEffectScopeBindableValueAsStatement(config: Config) : Rule(config) { Debt.FIVE_MINS, ) + override fun visitNamedFunction(function: KtNamedFunction) { + super.visitNamedFunction(function) + + if (bindingContext == BindingContext.EMPTY) { + return + } + + val receiverType = bindingContext[BindingContext.FUNCTION, function] + ?.extensionReceiverParameter + ?.type + ?: return + + val isEffectScope = isBindableScope(receiverType) || receiverType.supertypes().any(::isBindableScope) + + if (isEffectScope) { + EffectScopeVisitor().visitNamedFunction(function) + } + } + override fun visitLambdaExpression(lambdaExpression: KtLambdaExpression) { super.visitLambdaExpression(lambdaExpression) @@ -45,20 +65,21 @@ class NoEffectScopeBindableValueAsStatement(config: Config) : Rule(config) { ?.type ?: return - val isEffectScope = isEffectScope(firstArgumentType) || firstArgumentType.supertypes().any(::isEffectScope) + val isEffectScope = isBindableScope(firstArgumentType) || firstArgumentType.supertypes().any(::isBindableScope) if (isEffectScope) { EffectScopeVisitor().visitLambdaExpression(lambdaExpression) } } - private fun isEffectScope(type: KotlinType): Boolean = type + private fun isBindableScope(type: KotlinType): Boolean = type .constructor .declarationDescriptor ?.fqNameSafe ?.let { it == FqName("arrow.core.continuations.EffectScope") || - it == FqName("arrow.core.continuations.EagerEffectScope") + it == FqName("arrow.core.continuations.EagerEffectScope") || + it == FqName("arrow.core.raise.Raise") } ?: false @@ -105,13 +126,25 @@ class NoEffectScopeBindableValueAsStatement(config: Config) : Rule(config) { .getQualifiedExpressionForReceiver() ?.selectorExpression - private fun isBindable(type: KotlinType): Boolean = - type - .constructor - .declarationDescriptor - ?.fqNameSafe - ?.let { it in BindableFqNames } - ?: false + private fun isBindable(type: KotlinType): Boolean { + val isBindableFqNames = + type + .constructor + .declarationDescriptor + ?.fqNameSafe + ?.let { it in BindableFqNames } ?: false + + val isRaiseExtensionFunction = type.annotations.hasAnnotation(FqName("kotlin.ExtensionFunctionType")) && + type + .arguments + .firstOrNull() + ?.type + ?.constructor + ?.declarationDescriptor + ?.fqNameSafe == FqName("arrow.core.raise.Raise") + + return isBindableFqNames || isRaiseExtensionFunction + } } companion object { diff --git a/src/test/kotlin/com/wolt/arrow/detekt/rules/NoRaiseBindableValueAsStatementTest.kt b/src/test/kotlin/com/wolt/arrow/detekt/rules/NoRaiseBindableValueAsStatementTest.kt new file mode 100644 index 0000000..4f3cb12 --- /dev/null +++ b/src/test/kotlin/com/wolt/arrow/detekt/rules/NoRaiseBindableValueAsStatementTest.kt @@ -0,0 +1,483 @@ +package com.wolt.arrow.detekt.rules + +import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest +import io.gitlab.arturbosch.detekt.test.lintWithContext +import io.kotest.matchers.collections.shouldHaveSize +import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment +import org.junit.jupiter.api.DisplayName +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test + +@KotlinCoreEnvironmentTest +internal class NoRaiseBindableValueAsStatementTest(private val env: KotlinCoreEnvironment) { + + @Nested + @DisplayName("bindable variations") + inner class BindableVariations { + @Test + fun `reports unbound Either`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.Effect + import arrow.core.raise.effect + + fun test(): Effect = effect { + Either.Right(1) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound Validated`() { + val code = """ + import arrow.core.Validated + import arrow.core.raise.Effect + import arrow.core.raise.effect + + fun test(): Effect = effect { + Validated.Valid(1) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound EagerEffect`() { + val code = """ + import arrow.core.raise.Effect + import arrow.core.raise.effect + import arrow.core.raise.eagerEffect + + fun test(): Effect = effect { + eagerEffect { 1 } + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound Effect`() { + val code = """ + import arrow.core.raise.Effect + import arrow.core.raise.effect + + fun test(): Effect = effect { + effect { 1 } + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound Option`() { + val code = """ + import arrow.core.Option + import arrow.core.raise.Effect + import arrow.core.raise.effect + + fun test(): Effect = effect { + Option(1) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound Result`() { + val code = """ + import arrow.core.raise.Effect + import arrow.core.raise.effect + + fun test(): Effect = effect { + Result.success(1) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound Ior`() { + val code = """ + import arrow.core.Ior + import arrow.core.raise.Effect + import arrow.core.raise.effect + + fun test(): Effect = effect { + Ior.Right(5) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports bindable value with more than one level of inheritance`() { + val code = """ + import arrow.core.raise.effect + import arrow.core.raise.Effect + + abstract class MyEffect : Effect + + class MyEffectChild : MyEffect { + override suspend fun invoke(p1: Raise): Int = TODO() + } + + fun test(): Effect = effect { + MyEffectChild() + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + } + + @Nested + @DisplayName("effect scope variations") + inner class EffectScopeVariations { + @Test + fun `reports unbound value inside effect {} scope`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.Effect + import arrow.core.raise.effect + + fun test(): Effect = effect { + Either.Right(1) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound value inside eagerEffect {} scope`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.EagerEffect + import arrow.core.raise.eagerEffect + + fun test(): EagerEffect = eagerEffect { + Either.Right(1) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound value inside either {} scope`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.either + + suspend fun test(): Either = either { + Either.Right(1) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound value inside option {} scope`() { + val code = """ + import arrow.core.Option + import arrow.core.raise.option + + suspend fun test(): Option = option { + Option(1) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound value inside nullable {} scope`() { + val code = """ + import arrow.core.Option + import arrow.core.raise.nullable + + suspend fun test(): Int? = nullable { + Option(1) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound value inside ior {} scope`() { + val code = """ + import arrow.core.Ior + import arrow.typeclasses.Semigroup + import arrow.core.raise.ior + + suspend fun test(): Ior, Int> = ior(Semigroup.list()) { + Ior.Right(1) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound value inside result {} scope`() { + val code = """ + import arrow.core.raise.result + + suspend fun test(): Result = result { + Result.success(1) + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound value inside scope with more than one level of inheritance`() { + val code = """ + import arrow.core.None + import arrow.core.raise.Raise + import arrow.core.raise.Effect + import arrow.core.raise.effect + + abstract class MyOptionScope : Raise + + class MyChildScope : MyOptionScope() { + override fun raise(r: None): Nothing = TODO() + } + + object myChildScope { + inline operator fun invoke(crossinline f: suspend MyChildScope.() -> A): Effect = TODO() + } + + fun test(): Effect = myChildScope { + effect { 1 } + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound value inside extension function with receiver extending Raise`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.Raise + import arrow.core.raise.either + + fun t(): Either = either { 1 } + + class MyChildRaise: Raise { + override fun raise(r: String): Nothing = TODO() + } + + fun MyChildRaise.test(): Int { + t() + return 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports Raise extension function with unbound value`() { + val code = """ + import arrow.core.raise.Raise + import arrow.core.raise.either + import arrow.core.Either + + fun Raise.test(): Int { + Either.Right(1) + return 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + } + + @Test + fun `reports unbound reference expression`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.either + + val e = Either.Right(1) + + fun test(): Either = either { + e + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound reference expression with dot qualified expression`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.either + + object T { + val e = Either.Right(1) + } + + fun test(): Either = either { + T.e + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound call expression`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.either + + fun a() = Either.Right(1) + + fun test(): Either = either { + a() + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports Raise extension function with unbound call expression`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.Raise + import arrow.core.raise.either + + fun a() = Either.Right(1) + + fun Raise.test(): Int { + a() + return 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound implicit return with Unit type`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.either + + fun test(): Either = either { + Either.Right(1) + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound value inside an if`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.either + + fun test(): Either = either { + if (true) { + Either.Right(1) + } + 1 + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `reports unbound value inside nested context`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.either + + fun test(): Either = either { + with (1) { Either.Right(this) } + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 1 + } + + @Test + fun `does not report if the value is handled`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.either + import arrow.core.getOrHandle + + fun test(): Either = either { + Either.Right(1).getOrHandle { 1 } + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 0 + } + + @Test + fun `does not report if the value is bound`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.either + + fun test(): Either = either { + Either.Right(1).bind() + } + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 0 + } + + @Test + fun `does not report Raise extension function with nothing to bind`() { + val code = """ + import arrow.core.Either + import arrow.core.raise.Raise + import arrow.core.raise.either + + fun Raise.test(): Int = raise("failure") + """ + val findings = NoEffectScopeBindableValueAsStatement(Config.empty).lintWithContext(env, code) + findings shouldHaveSize 0 + } +}