UnusedImports: Lots of false positives

[3flex]

Expected Behavior

java.util.HashMap is not reported as unused, since the type is used in the function value parameter:

import java.util.HashMap

fun doesNothing(thing: HashMap<String, String>) {
    // nothing
}

Observed Behavior

java.util.HashMap is reported as unused by UnusedImports rule.

Steps to Reproduce

Run detekt with type resolution enabled on the code provided above.

Context

Lots of false positives in detekt 1.13.x.

Your Environment

• Version of detekt used: 1.13.1
• Version of Gradle used (if applicable): 6.6.1
• Operating System and version: Windows 10
• Link to your project (if it's a public repository): N/A

[cortinico]

I've just done a quick run and I was able to reproduce but only with type resolution enabled. UnusedImports is one of those mixed rule #2994 that should probably be avoided.

[BraisGabin]

Well, actually you don't need to import HashMap to make your code work. Extracted from Kotlin:

package kotlin.collections

@SinceKotlin("1.1") public actual typealias HashMap<K, V> = java.util.HashMap<K, V>

I don't know if there's a good reason to keep that import.

[3flex]

Thanks @BraisGabin it was just one very simple example. Here's another:

import dagger.Module
import dagger.hilt.InstallIn
import dagger.hilt.components.SingletonComponent

@Module
@InstallIn(SingletonComponent::class)
object DbModule {
    // other code exists here
}

In this example all three imports are being reported, but they're being used. Actually every single import in the real file is being flagged by this rule now.

[cortinico]

I've done further investigations on this front. The offending commit is #3033

Currently the UnusedImports is substantially broken for users with Type Resolution enabled. The problem is that the UnusedImports rule was modified to run differently when Type Resolution is enabled in #3033. Tests were not updated instead (see #3077 has 6 failing tests on that rule).

The problem is actually here:

detekt/detekt-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedImports.kt

Lines 75 to 82 in 1c6a7ea

	return if (bindingContext == BindingContext.EMPTY) {
	identifier !in namedReferencesAsString
	} else {
	val fqNames = namedReferences.mapNotNull {
	it.getResolvedCall(bindingContext)?.resultingDescriptor?.fqNameOrNull()
	}
	importPath?.fqName?.let { it !in fqNames } == true
	}

The assumption in this code is that all the KtReferenceExpression will succeed the getResolvedCall() resolution. This is not the case for the two posted snippets as there KtReferenceExpression is actually referring a KtUserType (an annotation and a parameter type).
We would have to adapt the code to treat all the possible variants of KtReferenceExpression (I'm not aware of how many are there and how error prone this could be, might be eventually easy).

I advice we revert #3033 (to close this issue) and eventually reopen #3020.

Ideally we should consider having UnusedImports working only with TR enabled or have a separate rule (see #2994 for more context on this topic).