Improve binding context management

[BraisGabin]

This is the first step to work on #3577

We had different ways/places to disable a rule to don't run when it doesn't have a proper bindingContext. But it wasn't a very efficient way because we were checking it while we where traversing the PSI tree. detekt has this visitCondition that let to any rule to decide if it should be executed or not. This should be way more performant because we disable the rule before we start traversing the PSI tree.

And the new rule ensures that we make this in a consistent way.

This PR uses the new detekt-authors module implemented on #5129

[BraisGabin]

🤔 ArrayPrimitive is an "interesting" case. It's annotated with @RequiresTypeSolving but actually is a rule with mixed behavior (#2994). Here we have two options: Or we remove the annotation and keep the rule with its mixed behaviour or we remove the mixed behaviour and pass it to a true @RequiresTypeSolving rule.

[marschwar]

I like to idea of removing this boilerplate check, but:

• Are you sure there is only one rule with mixed behavior. We had so many bug reports where this was the cause and it feels like we complained about this much more often than just for one rule.
• If we do this, I propose to change the retention of the RequiresTypeResolution annotation so that we can extract it from the class file and remove the rule in the Analyzer or Rule#visitCondition

[BraisGabin]

• Are you sure there is only one rule with mixed behavior. We had so many bug reports where this was the cause and it feels like we complained about this much more often than just for one rule.

No, there are more rules with mixed behavior. But those are not annotated with RequiresTypeResolution because those rules doesn't require it. The problem is this rule that doesn't require it but it claims so.

• If we do this, I propose to change the retention of the RequiresTypeResolution annotation so that we can extract it from the class file and remove the rule in the Analyzer or Rule#visitCondition

I was thinking about that too. But that would force us to use reflection apis. I'm not against. It would be a way cleaner approach if we are fine with the usage of reflection.

[marschwar]

Why would we not use reflection? Because of a runtime performance penalty?

[BraisGabin]

Why would we not use reflection? Because of a runtime performance penalty?

To be honest, because I'm an Android developer and reflection is the devil there. But you are completely right, I don't see a good reason to don't use reflection here. That would make this rule completely useless and that's good! The code that doesn't exist doesn't have bugs :) I'm keeping this open for now but I'll open other PR implementing the reflection idea. If we merge the other one I'll close this one.

[cortinico]

My 2 cents here: shouldn't we just instead have a Rule subclass or a parameter to the Rule constructor, or a property inside Rule that users can override?

While a generic visitCondition works, it does not prevent from rules accidentally using TR and forgetting to add the @RequiresTypeResolution annotation.

[marschwar]

While a generic visitCondition works, it does not prevent from rules accidentally using TR and forgetting to add the @RequiresTypeResolution annotation.

You are right. That is why #5148 is still needed

[BraisGabin]

I'm closing this one. #5176 is clearly a better solution.