detekt: Merge (rather than replace) multiple configs for ForbiddenImport rule

Expected Behavior of the rule

Currently Detekt supports specifying multiple config files, which will be merged together at runtime with priority in reverse of the order they are specified. If a later file defines a given rule, that definition will supersede any previous definitions of that rule. In general, this is a great feature. However, it does not work well with ForbiddenImports - the reason being that if you wanted to build upon the previous forbidden imports, it will overwrite those previous rules and they will not longer be applied. It would be great if instead of overwriting, it merged the config for ForbiddenImports, so that you keep any previously defined disallowed imports along with ones in the newer config files. I realize this is a behavior change, so wanted to start a discussion around it!

For example, if in one config I have:

ForbiddenImport:
    active: true
    imports:
      - 'android.util.Log'

and in another I have:

ForbiddenImport:
    active: true
    imports:
      - 'android.util.ArrayMap'

and I apply both of these config files, only one of those imports would be disallowed, not both.

Context

We want to create compounding ForbiddenImport rules for different pieces of code - basically a standard set of rules that are best practices, and then more detailed rules applied to subsets of code (usually in separate gradle modules).

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 16 (12 by maintainers)

Most upvoted comments

Thanks for the feedback and the great ideas!

I think the inheritance mode could lead to surprising behavior. What happens if you declare conflicting configuration options? This inheritance model can only work for yaml lists.