Detekt: Support for BindingContext in FileProcessListener

[damianw]

Current Behavior

Implementations of Rule have access to the BindingContext, granting them the ability to perform complex analysis within the scope of the entire compilation unit. On the other hand, the FileProcessListener (and AbstractProcessor, etc) APIs do not provide any way to access the BindingContext, limiting their ability to resolve types, functions, and so forth.

Expected Behavior

In order to enable the same level of analysis for implementations of FileProcessListener, the API should provide them a BindingContext against which references can be resolved.

Context

I am in the process of implementing "occurrences counting" metrics for our codebase such as "number of invocations of function A" and "number of constructor calls to classes which implement interface X". Complex queries such as this require a BindingContext, but the provided mechanism for making such queries, AbstractProjectMetricProcessor, does not provide it.

[arturbosch]

Yep, noticed the same.
We could either introduce new functions with KtFile and BindingContext or use @JvmOverloads for not introduce a breaking change.

[cortinico]

We could either introduce new functions with KtFile and BindingContext or use @JvmOverloads for not introduce a breaking change.

I took a quick look at this.

I think that the breaking change here is unavoidable with @JvmOverloads as its not allowed on interface methods:

https://github.com/detekt/detekt/blob/2de31f1b7e1887238157275b0511240197089da5/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/FileProcessListener.kt#L18-L22

What we could do, is use @JvmDefault and provide something like:

@JvmDefault
fun onProcess(file: KtFile, bindingContext: BindingContext) {
    onProcess(file)
}

But we would need to add a compiler flag. Is this something we want to do?

[arturbosch]

What we could do, is use @JvmDefault and provide something like:

@JvmDefault
fun onProcess(file: KtFile, bindingContext: BindingContext) {
    onProcess(file)
}

But we would need to add a compiler flag. Is this something we want to do?

Is there a benefit in using @JvmDefault instead of a plain interface function calling onProcess(file)? I guess less bytecode because it is the 'official' java way on the JVM.

[cortinico]

Is there a benefit in using @JvmDefault instead of a plain interface function calling onProcess(file)? I guess less bytecode because it is the 'official' java way on the JVM.

Yes but I think I overlooked the API change here. According to eclipse.org adding a API method should always be considered a breaking change (as our clients might already have a method with the same signature).

In this specific case, If we don't use @JvmDefault this is going to break compatibility towards our Java users as they will need to implement the onProcess(KtFile, BindingContext) method. Adding the @JvmDefault will make sure that a default method is generated, hence users of this interface won't be forced to implement the onProcess(KtFile, BindingContext) method.

However, I expect the majority of our users to use Kotlin to code their custom rules/processors. Therefore, we should be fine with just:

fun onProcess(file: KtFile, bindingContext: BindingContext) {
    onProcess(file)
}

and we should probably not be worried about our Java users (as we probably don't have any).