Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive in the SleepInsteadOfDelay rule #5659

Closed
atulgpt opened this issue Jan 2, 2023 · 3 comments · Fixed by #5699
Closed

False positive in the SleepInsteadOfDelay rule #5659

atulgpt opened this issue Jan 2, 2023 · 3 comments · Fixed by #5699

Comments

@atulgpt
Copy link
Contributor

atulgpt commented Jan 2, 2023

Expected Behavior

In the code example below

private fun bar(lambda: () -> Unit) {
    lambda()
}

@Suppress("unused", "MagicNumber", "InjectDispatcher")
private fun foo(): Deferred<Unit> {
    return CoroutineScope(Dispatchers.IO).async {
        bar {
            Thread.sleep(1000)
        }
    }
}

The above example should not report by the rule SleepInsteadOfDelay as the user can't call delay coz lambda is not suspending lambda

Observed Behavior

The above example is reported by the rule SleepInsteadOfDelay as a false positive

Steps to Reproduce

Failing TC

@Test
fun `should not report Thread sleep() called in non-suspending block`() {
    @Suppress("DeferredResultUnused")
    val code = """
        import kotlinx.coroutines.CoroutineScope
        import kotlinx.coroutines.Dispatchers
        import kotlinx.coroutines.async
        
        fun bar(lambda: () -> Unit) {
            lambda()
        }
        fun foo() {
            CoroutineScope(Dispatchers.IO).async {
                bar {
                    Thread.sleep(1000L)
                }
            }
        }
    """.trimIndent()
    assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()
}

Your Environment

  • Version of detekt used: 1.22.0
  • Version of Gradle used (if applicable): NA
  • Operating System and version: MacBook Pro (16-inch, 2019) Version: 11.5.2 (20G95)
@atulgpt
Copy link
Contributor Author

atulgpt commented Jan 3, 2023

Hi @BraisGabin do you know the optimal approach to solve this? I was working on the SuspendFunInsideRunCatching rule and found a similar issue there as well

I think, now I have a rough idea of what is going wrong here. I can pick this once I am able to close my open PRs 😅

@BraisGabin
Copy link
Member

Go for it :)

@atulgpt
Copy link
Contributor Author

atulgpt commented Jan 15, 2023

Hi, @BraisGabin created a PR for the fix. To fix this slightly changed the traversal logic which fortunately fixed more failing TCs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants