Should detekt defaults include framework specific values

[marschwar]

There are some built in rules that have default values that are specific to framework classes (mostly Android).

• FunctionNaming
• FunctionOnlyReturningConstant
• UnnecessaryAbstractClass
• IgnoredReturnValue (to some extend)

I do acknowledge that Android is a big part of the kotlin community, but in my opinion detekt should stay away from tailoring towards frameworks such as Android, Dagger or Spring.

Any thoughts?

[schalkms]

I agree that we shouldn’t have such opinionated defaults. Sometimes it slipped through the reviews. In general, we handled it in the past like you said.

[schalkms]

I just copied my response from the PR.

[cortinico]

I do acknowledge that Android is a big part of the kotlin community, but in my opinion detekt should stay away from tailoring towards frameworks such as Android, Dagger or Spring.

I believe we had a discussion about an Android specific ruleset (I can't find it right now) once.

I'm in favor of having framework-specific rules that are self contained.
Specifically for Android, there are specific patterns that idiomatic to the platform and it makes sense to have rules for those.

The Dagger/Spring discussion instead relies more on annotations and how we handle them. Specifically we discussed this in the past here: #3041 #2747 #2231 (comment)

I believe we should offer some sort of annotationExclusion pattern that allows us to treat all those cases that arise from framework-specific use cases.

[marschwar]

I believe we should offer some sort of annotationExclusion pattern that allows us to treat all those cases that arise from framework-specific use cases.

I totally agree, but I don't think we should have Dagger or Spring annotations as defaults exlusions

[cortinico]

Yup that would be ideal once we have such a solution in place

[marschwar]

I am not sure I understand.

I was talking about the FunctionOnlyReturningConstant for example where we have an option to ignore functions with certain annotations. So users can tailor this to their specific framework needs. That is super. The issue I have is, that by default we exclude functions annotated with @dagger.Provides. I think detekt should be framework agnostic and have an empty list as the default.

[BraisGabin]

I think that all of us are in the same page here. Dagger specific should not be in the default settings and we should have a more powerful way to exclude code (annotations, naming, regex...) It's a pain to add this kind of excludes for each rule each time that a users complain about it. Just to clarify, the I think that it's normal that the users complain about it. But we should find a better solution so we don't need to do custom excludes for each rule.

[G00fY2]

In #3944 I asked for reverting a removed Compose specific default configuration.

Now that I found this GitHub discussion, is see the point in keeping detekt defaults clean of framework specific settings (I would differentiate this from "opinionated" defaults).

I think for the users it is important to achieve this consistently and transparently. So I would suggest one PR which removes all framework specific rules, which is then listed in the "Notable Changes".

[Goooler]

For those who want to use Detekt's default rules without any detekt.yml generated, they need to suppress all FunctionNaming rules in Composable kt files, it's not convenient.

[BraisGabin]

Sure, but people using Spring could say the same. And we can add a long "etc" here. I think that there having a detekt.yml is the only way here.

[Goooler]

Not sure if we can add the ability to override rules in DetektExtension.

[BraisGabin]

That's an iteresting idea! It should be doable, for sure.

But I'm not sure if it's worth the effort and how much maintenance work will give to us.