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

Wildcard ban does not work? #76

Closed yeikel closed 1 year ago

yeikel commented 1 year ago

Hello there,

This does not seem to work


<bannedImports>
<!---
Matches multiple dependencies that shade/relocate Google annotations
Examples : com.couchbase.client.core.deps.com.google.common.annotations.VisibleForTesting
org.testcontainers.shaded.com.google.common.annotations
-->
<bannedImport>*.com.google.common.annotations.VisibleForTesting</bannedImport>
</bannedImports>

As described by the comment, the goal is to match multiple dependencies that shade/relocate Google annotations like com.couchbase.client.core.deps.com.google.common.annotations.VisibleForTesting

skuzzle commented 1 year ago

You probably need to prefix the pattern with **. A single * only matches a single package level.

yeikel commented 1 year ago

Thank you for your help, but that does not seem to work unless I am missing something:


<groups>
<group>
<reason>Use org.jetbrains.annotations.VisibleForTesting</reason>
<basePackage>**</basePackage>
<bannedImports>
<!---
 Matches multiple dependencies that shade/relocate Google annotations
Examples : 

    com.couchbase.client.core.deps.com.google.common.annotations.VisibleForTesting
    org.testcontainers.shaded.com.google.common.annotations

-->
<bannedImport>**.com.google.common.annotations.VisibleForTesting</bannedImport>
</bannedImport>
</bannedImports>
</group>
</groups>

+import com.couchbase.client.core.deps.com.google.common.annotations.VisibleForTesting;
 import com.github.jasync.sql.db.pool.PooledObject;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
+
 import pgnio.QueryBuildConnection.Prepared;
 import pgnio.QueryMessage.Row;
 import pgnio.QueryReadyConnection.AutoCommit;
@@ -42,6 +44,7 @@ public class AutoCommitPooledObject implements PooledObject {
     // return autoCommit.ctx.io.isReadWrite();
     // }

+    @VisibleForTesting
     pu

This works though :

  <bannedImport>com.couchbase.client.core.deps.com.google.common.annotations.VisibleForTesting</bannedImport>

[ERROR] Reason: Use org.jetbrains.annotations.VisibleForTesting
[ERROR]         in file: pgnio/AutoCommitPooledObject.java
[ERROR]                 com.couchbase.client.core.deps.com.google.common.annotations.VisibleForTesting  (Line: 3, Matched by: com.couchbase.client.core.deps.com.google.common.annotations.VisibleForTesting)
skuzzle commented 1 year ago

You are right, this is a bug. According to the documentation, **should match at least one package part. In this case however, it matches none because the package to match starts with com and com is also the first part in the pattern after the **.

There might be further issues with the way we currently implemented the matching algorithm. I created a reminder ticket (which needs some more specification) to improve this in the future: #78

yeikel commented 1 year ago

Thank you for the fix. That did it

yeikel commented 1 year ago

It seems that

<bannedImport>**.com.google.common.annotations.VisibleForTesting</bannedImport>

Does not match com.google.common.annotations.VisibleForTesting but I believe that this is expected, right?

skuzzle commented 1 year ago

Yes, for the time being that is expected because ** will consume at least one package part while matching. I guess the whole matching semantics should be reconsidered for the next major release. There are quite a few use cases that currently can not be expressed in a user friendly manor with the current implementation