detekt / detekt

Static code analysis for Kotlin
https://detekt.dev
Apache License 2.0
6.07k stars 751 forks source link

`RedundantSuspendModifier` false-positives with extension functions #7223

Open sschuberth opened 3 weeks ago

sschuberth commented 3 weeks ago

Expected Behavior

RedundantSuspendModifier should not complain for suspend functions that call suspend functions

Observed Behavior

RedundantSuspendModifier is reported for this code:

suspend fun deleteForOrganization(organizationId: Long, name: String) {
    db.dbQuery {
        infrastructureServiceRepository.deleteForOrganizationAndName(organizationId, name)
    }
}

with

suspend fun <T> Database.dbQuery(
    transactionIsolation: Int = transactionManager.defaultIsolationLevel,
    readOnly: Boolean = transactionManager.defaultReadOnly,
    block: () -> T
): T =
    dbQueryCatching(transactionIsolation, readOnly, block).getOrThrow()

Steps to Reproduce

Run Detekt with type resolution (./gradlew detektMain) on https://github.com/eclipse-apoapsis/ort-server/blob/main/services/infrastructure/src/main/kotlin/InfrastructureServiceService.kt

Context

n/a

Your Environment

atulgpt commented 3 weeks ago

I have added TC for

@Test
fun `does not report when suspend function is called in extension method`() {
    val code = """
        import kotlinx.coroutines.delay
        suspend fun foo() { delay(1000) }
        suspend fun String.bar() {
            foo()
        }
        suspend fun  String.baz() = foo()
    """.trimIndent()
    assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
}

And extension method detection working fine. Are you running detect tasks with the correct type resolution?

sschuberth commented 3 weeks ago

Are you running detect tasks with the correct type resolution?

What do you mean by "correct"? But yes, I'm using type resolution in general, i.e. I run the detektMain task instead of the detekt task.

atulgpt commented 3 weeks ago

Hi @sschuberth it could detekt is unable to determine dbQueryCatching(transactionIsolation, readOnly, block) as suspend fun. Or can you give complete self-reproducible example where false +ve is coming