ViolatesTypeResolutionRequirements almost can't find itself

[TWiStErRob]

@BraisGabin ViolatesTypeResolutionRequirements looks for usages of bindingContext. This is apparent most of the time. Except, with receivers, it's possible to write extension functions that hide this.

There are two examples right in the file, which, if get refactored, will hide the fact that this rule is using bindingContext. (They're really nice reusable functions.)

Repro: Try to move the two funs out of the file and make them non-private, then run detektMain ->

ViolatesTypeResolutionRequirements.kt:31:7: ViolatesTypeResolutionRequirements is annotated with @RequiresTypeResolution but doesn't use bindingContext [ViolatesTypeResolutionRequirements]

Even without context receiver this would be possible, the functions would just look uglier at the callsite.

I wanted to raise this false negative for awareness (you might've thought about it before), but I didn't raise this as an issue, because I think the rule is still good as a best effort. Maybe this needs to be documented on the rule, or support added for extensions? Alternatively this can be prevented by making protected val bindingContext so that extension functions can't see it. This would force rule writers to explicitly expose it.

[BraisGabin]

Yes, the rule has potential false positives and negatives. I don't think we should change anything about it.

I think this kind of issues force the limits of detekt and we should find ways to make detekt able to catch this issues. We should not model ourselfs to the detekt limitations.

[TWiStErRob]

While I agree that tools shouldn't dictate design, they can definitely aid it. I think that the it's important to recognise the opportunity. This inability to detect all cases could be highlighting that the object model is too open. Why would anyone (rule devs) want to read, let alone write the bindingContext from outside of a rule class? Object oriented encapsulated design would dictate to close down the getter to protected and the setter to internal, or not?

[BraisGabin]

Sure! Rule is huge and has a lot of add-ons that should not be there. I even have a closed pr prototyping a new rule API. But that's something for 2.0. Making the binding context protected is a breaking change that doesn't provide us that much value, I think.