skuzzle / restrict-imports-enforcer-rule

Gradle plugin & Maven Enforcer rule that restricts usage of unwanted imports in Java, Kotlin and Groovy source files.
MIT License
75 stars 12 forks source link

Allow to ignore certain banned imports in certain explicit classes #89

Closed skuzzle closed 1 year ago

skuzzle commented 1 year ago

This PR introduces the concept of not-fixables classes/compilation units. Those are identified by a single package pattern. Per not-fixable class one can specify one or more package patterns that are allowed here even if they are otherwise banned.

Configuration examples:

<RestrictImports>
    <bannedImport>java.util.*</bannedImport>
    <notFixables>
        <notFixable>
            <in>de.skuzzle.enforcer.restrictimports.SampleClass</in>
            <allowedImports>
                <allowedImport>java.util.ArrayList</allowedImport>
                <allowedImport>java.util.Map</allowedImport>
            </allowedImports>
            <because>Required by third-party API</because>
        </notFixable>
        <notFixable>
            <in>de.skuzzle.enforcer.restrictimports.SampleClass</in>
            <allowedImport>java.util.LinkedList</allowedImport>
        </notFixable>
    </notFixables>
</RestrictImports>
<RestrictImports>
    <bannedImport>java.util.*</bannedImport>
    <notFixable>
        <in>de.skuzzle.enforcer.restrictimports.SampleClass</in>
        <allowedImports>
            <allowedImport>java.util.ArrayList</allowedImport>
            <allowedImport>java.util.Map</allowedImport>
            <allowedImport>java.util.LinkedList</allowedImport>
        </allowedImports>
        <because>Required by third-party API</because>
    </notFixable>
</RestrictImports>

What I don't quite like is the considerate functional overlap between <notFixable> and <allowedImports>. In fact the latter can be represented as <notFixable><in>**</in><allowImports>...</allowImports></notFixable>. One difference is that allowedImports are applied per group while notFixables are valid globally independent of any group.

So maybe this will result in deprecating <allowedImports> but I don't like the implications this has to our users as they'd have to eventually restructure their configurations.

Closes #63

hazendaz commented 1 year ago

Is the more of a comment? If so, that could be unnecessary as it could just be an attribute instead. Maybe not as clean that way but thinking the tags are more for items actually used. Or would there be possible intent to print that out so its visible to user that some item violates but is ok due to the said issue? That might be useful especially if issue resolved and its forgotten to remove / fix.

hazendaz commented 1 year ago

overall this idea works.

skuzzle commented 1 year ago

Ok, for a first version I will leave it as outlined in the top comment. The feature will be marked as experimental. Consolidation of not fixable and allowed imports can happen with a next major version. Also I'll add some validation of whether a not fixable definition is actually required in a follow-up version. That will help to keep the build clean in case a not fixable entry isn't actually needed anymore.