skuzzle/restrict-imports-enforcer-rule

PackagePattern should allow to match both static and non-static imports

Closed this issue ยท 9 comments

We should introduce a syntax that would optionally match a static modifier. I'm thinking about using ? as in regular expressions. Example: static? java.util.**
Another idea would be to introduce another flag that would make all configured patterns automatically optionally match the static modifier.
See also the discussion in #51

My two cents:

  • Are there really very many use cases where one would want to restrict an import but not also restrict a static import of the same class? I think your "introduce another flag" option seems preferable. Something like <includeStaticImports>true</includeStaticImports>could just automatically duplicate the existing rules into static ones.
  • Haven't looked at how you're parsing internally but the ** reminds me of Java glob patterns and it'd be great to use that entire syntax (with a dot "file separator"). Heck, with the package:pattern syntax that could even allow for regex matching of packages. :)

I agree and I also consider to make this the default behavior in version 2.
I've also considered using regex or "ant patterns" for matching. Only thing in the way is the 'specificity' calculation needed for the rule groups feature

I've decided against a new flag, because it just didn't feel right to me. Instead, I've changed the matching behavior to the following:

  • Every configured package pattern will automatically also match static imports
  • If a configured package pattern contains the static modifier, then it will only match static imports

I wanted to achieve the more natural default behavior without extra configuration, but still not break the use case of 'forbidding static imports' (See examples).

This will be released with version 2.0.0

@dbwiddis Would you mind giving the latest SNAPSHOT version a try? It contains both this feature and a bug fix for the whitespace thing with the explicit static package pattern (Check also the Release Notes in the development branch)
Just enable snapshot dependencies in the pom like this and change the dependency's version to 2.0.0-SNAPSHOT:

 <repositories>
    <repository>
      <id>oss.sonatype.org-snapshot</id>
      <url>https://oss.sonatype.org/content/repositories/snapshots</url>
      <releases>
        <enabled>false</enabled>
      </releases>
      <snapshots>
        <enabled>true</enabled>
      </snapshots>
    </repository>
  </repositories>

@dbwiddis Would you mind giving the latest SNAPSHOT version a try?

I've tried but can't manage to get snapshots pointing to ossrh. I've copied your text above into both my pom and .m2/settings, and confirmed the same format elsewhere... it's looking for the distribution in https://repository.apache.org/snapshots instead.

Interesting. I actually never tried using these SNAPSHOTS myself. Let me investigate that

Figured it out. I needed

<pluginRepositories>
   <pluginRepository>
      ...
   </pluginRepository>
</pluginRepositories>

So now I have at least a good error message with my previous config:

RestrictImports rule configuration error: There are ambiguous banned import definitions: **, static **

OK, looks good. Here's my final configuration that works.

                        <configuration>
                            <rules>
                                <RestrictImports>
                                    <includeTestCode>true</includeTestCode>
                                    <!-- Disallow all imports except explicitly 
                                        allowed -->
                                    <reason>Disallow transitive dependencies</reason>
                                    <bannedImports>
                                        <bannedImport>**</bannedImport>
                                    </bannedImports>
                                    <allowedImports>
                                        <!-- Allow oshi itself :-) -->
                                        <allowedImport>oshi.**</allowedImport>
                                        <!-- Allow core Java usages -->
                                        <allowedImport>java.**</allowedImport>
                                        <!-- Allow known dependencies -->
                                        <allowedImport>com.sun.jna.**</allowedImport>
                                        <allowedImport>org.slf4j.**</allowedImport>
                                        <allowedImport>org.junit.jupiter.api.**</allowedImport>
                                        <allowedImport>static org.hamcrest.**</allowedImport>
                                    </allowedImports>
                                    <!-- No restrictions on oshi-demo -->
                                    <exclusion>oshi.demo.**</exclusion>
                                </RestrictImports>
                            </rules>
                        </configuration>

Also works without the static on hamcrest, indicating that default part works.

Also fails if I add static to one of my imports other than hamcrest.

Also no problems with extra whitespace on the static.

Thank you very much, I really appreciate your efforts. I think I'll release 2.0.0 pretty soon now