Simplify rule syntax and follow pattern of `Artifact.toString()`

2 files changed, 68 insertions, 21 deletions

diff --git a/vespa-enforcer-extensions/src/main/java/com/yahoo/vespa/maven/plugin/enforcer/EnforceDependencies.java b/vespa-enforcer-extensions/src/main/java/com/yahoo/vespa/maven/plugin/enforcer/EnforceDependencies.java
index 54ccac9d22a..4ff363a3e4e 100644
--- a/vespa-enforcer-extensions/src/main/java/com/yahoo/vespa/maven/plugin/enforcer/EnforceDependencies.java
+++ b/vespa-enforcer-extensions/src/main/java/com/yahoo/vespa/maven/plugin/enforcer/EnforceDependencies.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.maven.plugin.enforcer;
 
 import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.DefaultArtifact;
 import org.apache.maven.enforcer.rule.api.EnforcerRule;
 import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
 import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
@@ -100,20 +101,32 @@ public class EnforceDependencies implements EnforcerRule {
         return children;
     }
 
-    // Similar rule matching to bannedDependencies but without support for version ranges
+    /**
+     * Rules follows same structure as {@link DefaultArtifact#toString} output.
+     * Allowed variants:
+     * - {@code <groupId>:<artifactId>:<version>:<scope>}
+     * - {@code <groupId>:<artifactId>:<type>:<version>:<scope>}
+     * - {@code <groupId>:<artifactId>:<type>:<classifier>:<version>:<scope>}
+     */
     private static boolean matches(Artifact dependency, String rule) throws EnforcerRuleException {
         String[] segments = rule.split(":");
-        if (segments.length < 1 || segments.length > 6) throw new EnforcerRuleException("Invalid rule: " + rule);
-        if (!segmentMatches(dependency.getGroupId(), segments[0])) return false;
-        if (segments.length > 1 && !segmentMatches(dependency.getArtifactId(), segments[1])) return false;
-        if (segments.length > 2 && !versionMatches(dependency.getVersion(), segments[2])) return false;
-        if (segments.length > 3 && !segmentMatches(dependency.getType(), segments[3])) return false;
-        if (segments.length > 4 && !segmentMatches(dependency.getScope(), segments[4])) return false;
-        if (segments.length > 5 && dependency.hasClassifier() && !segmentMatches(dependency.getClassifier(), segments[5]))
-            return false;
-        return true;
+        if (segments.length < 4 || segments.length > 6) throw new EnforcerRuleException("Invalid rule: " + rule);
+        int index = 0;
+        if (!segmentMatches(dependency.getGroupId(), segments[index++])) return false;
+        if (!segmentMatches(dependency.getArtifactId(), segments[index++])) return false;
+        if (segments.length >= 5) {
+            if (!segmentMatches(dependency.getType(), segments[index++])) return false;
+        }
+        if (segments.length == 6) {
+            if (!dependency.hasClassifier() || !segmentMatches(dependency.getClassifier(), segments[index++])) return false;
+        } else {
+            if (dependency.hasClassifier()) return false;
+        }
+        if (!versionMatches(dependency.getVersion(), segments[index++])) return false;
+        return segmentMatches(dependency.getScope(), segments[index]);
     }
 
+    /** Matches simple glob like patterns using '?' and '*' */
     private static boolean segmentMatches(String value, String segmentPattern) {
         String regex = segmentPattern
                 .replace(".", "\\.").replace("*", ".*").replace(":", "\\:").replace('?', '.').replace("(", "\\(")
diff --git a/vespa-enforcer-extensions/src/test/java/com/yahoo/vespa/maven/plugin/enforcer/EnforceDependenciesTest.java b/vespa-enforcer-extensions/src/test/java/com/yahoo/vespa/maven/plugin/enforcer/EnforceDependenciesTest.java
index bd7989bd65b..e8eec1dbf89 100644
--- a/vespa-enforcer-extensions/src/test/java/com/yahoo/vespa/maven/plugin/enforcer/EnforceDependenciesTest.java
+++ b/vespa-enforcer-extensions/src/test/java/com/yahoo/vespa/maven/plugin/enforcer/EnforceDependenciesTest.java
@@ -23,8 +23,8 @@ class EnforceDependenciesTest {
                 artifact("com.yahoo.vespa", "container-core", "8.0.0", "provided"),
                 artifact("com.yahoo.vespa", "testutils", "8.0.0", "test"));
         Set<String> rules = Set.of(
-                "com.yahoo.vespa:container-core:*:jar:provided",
-                "com.yahoo.vespa:*:*:jar:test");
+                "com.yahoo.vespa:container-core:jar:*:provided",
+                "com.yahoo.vespa:*:jar:*:test");
         assertDoesNotThrow(() -> EnforceDependencies.validateDependencies(dependencies, rules, true));
     }
 
@@ -33,7 +33,7 @@ class EnforceDependenciesTest {
         Set<Artifact> dependencies = Set.of(
                 artifact("com.yahoo.vespa", "container-core", "8.0.0", "provided"),
                 artifact("com.yahoo.vespa", "testutils", "8.0.0", "test"));
-        Set<String> rules = Set.of("com.yahoo.vespa:*:*:jar:test");
+        Set<String> rules = Set.of("com.yahoo.vespa:*:jar:*:test");
         EnforcerRuleException exception = assertThrows(
                 EnforcerRuleException.class,
                 () -> EnforceDependencies.validateDependencies(dependencies, rules, true));
@@ -51,8 +51,8 @@ class EnforceDependenciesTest {
         Set<Artifact> dependencies = Set.of(
                 artifact("com.yahoo.vespa", "testutils", "8.0.0", "test"));
         Set<String> rules = Set.of(
-                "com.yahoo.vespa:container-core:*:jar:provided",
-                "com.yahoo.vespa:*:*:jar:test");
+                "com.yahoo.vespa:container-core:jar:*:provided",
+                "com.yahoo.vespa:*:jar:*:test");
         EnforcerRuleException exception = assertThrows(
                 EnforcerRuleException.class,
                 () -> EnforceDependencies.validateDependencies(dependencies, rules, true));
@@ -60,7 +60,7 @@ class EnforceDependenciesTest {
                 """
                 Vespa dependency enforcer failed:
                 Rules not matching any dependency:
-                 - com.yahoo.vespa:container-core:*:jar:provided
+                 - com.yahoo.vespa:container-core:jar:*:provided
                 """;
         assertEquals(expectedErrorMessage, exception.getMessage());
     }
@@ -70,7 +70,7 @@ class EnforceDependenciesTest {
         Set<Artifact> dependencies = Set.of(
                 artifact("com.yahoo.vespa", "testutils", "8.0.0", "test"));
         Set<String> rules = Set.of(
-                "com.yahoo.vespa:testutils:7.0.0:jar:test");
+                "com.yahoo.vespa:testutils:jar:7.0.0:test");
         EnforcerRuleException exception = assertThrows(
                 EnforcerRuleException.class,
                 () -> EnforceDependencies.validateDependencies(dependencies, rules, true));
@@ -80,7 +80,7 @@ class EnforceDependenciesTest {
                 Dependencies not matching any rule:
                  - com.yahoo.vespa:testutils:jar:8.0.0:test
                 Rules not matching any dependency:
-                 - com.yahoo.vespa:testutils:7.0.0:jar:test
+                 - com.yahoo.vespa:testutils:jar:7.0.0:test
                 """;
         assertEquals(expectedErrorMessage, exception.getMessage());
     }
@@ -90,7 +90,7 @@ class EnforceDependenciesTest {
         Set<Artifact> dependencies = Set.of(
                 artifact("com.yahoo.vespa", "testutils", "8.0.0", "test"));
         Set<String> rules = Set.of(
-                "com.yahoo.vespa:testutils:8.0.0:jar:provided");
+                "com.yahoo.vespa:testutils:jar:8.0.0:provided");
         EnforcerRuleException exception = assertThrows(
                 EnforcerRuleException.class,
                 () -> EnforceDependencies.validateDependencies(dependencies, rules, true));
@@ -100,14 +100,48 @@ class EnforceDependenciesTest {
                 Dependencies not matching any rule:
                  - com.yahoo.vespa:testutils:jar:8.0.0:test
                 Rules not matching any dependency:
-                 - com.yahoo.vespa:testutils:8.0.0:jar:provided
+                 - com.yahoo.vespa:testutils:jar:8.0.0:provided
+                """;
+        assertEquals(expectedErrorMessage, exception.getMessage());
+    }
+
+    @Test
+    void matches_shorter_rule_variant_without_type() {
+        Set<Artifact> dependencies = Set.of(
+                artifact("com.yahoo.vespa", "testutils", "8.0.0", "test"));
+        assertDoesNotThrow(() -> EnforceDependencies.validateDependencies(
+                dependencies, Set.of("com.yahoo.vespa:testutils:jar:8.0.0:test"), true));
+        assertDoesNotThrow(() -> EnforceDependencies.validateDependencies(
+                dependencies, Set.of("com.yahoo.vespa:testutils:8.0.0:test"), true));
+    }
+
+    @Test
+    void matches_artifact_with_classifier() {
+        Set<Artifact> dependencies = Set.of(
+                artifact("com.google.inject", "guice", "4.2.3", "provided", "no_aop"));
+        assertDoesNotThrow(() -> EnforceDependencies.validateDependencies(
+                dependencies, Set.of("com.google.inject:guice:jar:no_aop:4.2.3:provided"), true));
+        EnforcerRuleException exception = assertThrows(
+                EnforcerRuleException.class,
+                () -> EnforceDependencies.validateDependencies(
+                        dependencies, Set.of("com.google.inject:guice:4.2.3:provided"), true));
+        String expectedErrorMessage =
+                """
+                Vespa dependency enforcer failed:
+                Dependencies not matching any rule:
+                 - com.google.inject:guice:jar:no_aop:4.2.3:provided
+                Rules not matching any dependency:
+                 - com.google.inject:guice:4.2.3:provided
                 """;
         assertEquals(expectedErrorMessage, exception.getMessage());
     }
 
     private static Artifact artifact(String groupId, String artifactId, String version, String scope) {
+        return artifact(groupId, artifactId, version, scope, null);
+    }
+    private static Artifact artifact(String groupId, String artifactId, String version, String scope, String classifier) {
         return new DefaultArtifact(
-                groupId, artifactId, version, scope, "jar", /*classifier*/null, new DefaultArtifactHandler("jar"));
+                groupId, artifactId, version, scope, "jar", classifier, new DefaultArtifactHandler("jar"));
     }
 
 }
\ No newline at end of file