allow for coincidental versions

src/main/java/com/solubris/enforcer/Artifact.java

@@ -76,4 +76,28 @@ public boolean hasExplicitVersion() {
     public boolean hasImplicitVersion() {
         return !Objects.equals(getVersion(), getEffectiveVersion());
     }
+
+    /**
+     * Uniqueness is used to avoid violations on coincidental artifact versions.
+     *
+     * <p>What about groupId's like this
+     * - org.junit.jupiter
+     * - org.junit.platform
+     * Could remove one level of the package.
+     *
+     * <p>The coincidence problem puts the whole rule into jeopardy.
+     * Without a good solution, can only report warnings.
+     *
+     * <p>How is each violation affected:
+     * - redundantPropertyViolation
+     * grouping by groupId ruins this - property could be used for multiple artifacts with the different groupIds.
+     * - unusedPropertyViolation
+     * grouping by groupId is ok - some locations should not use the property
+     * - missingPropertyViolation
+     * grouping by groupId is ok - some locations should not use the property
+     *
+     */
+    public String uniqueness() {
+        return String.join(":", getGroupId(), getEffectiveVersion());
+    }
 }

src/main/java/com/solubris/enforcer/VersionPropertyRule.java

@@ -67,14 +67,14 @@ protected Stream<String> scan() {
                 .map(a -> a.withEffectiveVersion(fetchVersion(a, byKey)))
                 .filter(a -> a.getEffectiveVersion() != null)
                 .filter(artifact -> !isExcluded(artifact.getVersion()))
-                .collect(groupingBy(Artifact::getEffectiveVersion, toList()));
+                .collect(groupingBy(Artifact::uniqueness, toList()));
 
         // byVersion maps from property to list of artifacts
         // now can go through property keys and check usage
 
         return usages.entrySet().stream()
                 .map(e -> {
-                    String effectiveVersion = e.getKey();
+                    String effectiveVersion = e.getKey().split(":")[1];
                     List<Artifact> artifacts = e.getValue();
                     long propertyCount = artifacts.stream()
                             .filter(Artifact::hasImplicitVersion)
@@ -108,7 +108,8 @@ private String redundantPropertyViolation(Artifact artifact) {
      * The artifacts could either have the explicit version or a reference to the property.
      * Where the property represents the same version.
      * If the property uses a different version than the explicit version,
-     * then that would be detected at the moment - could be a different violation (property value doesn't match usage).
+     * then that would not be detected at the moment
+     * - could be a different violation (property value doesn't match usage).
      *
      * <p>Could the artifacts refer to different properties that have the same value?
      * That's possible due to coincidental properties - another edge case to consider.
@@ -128,17 +129,17 @@ private static String unusedPropertyViolation(String effectiveVersion, List<Arti
                 propertyName, effectiveVersion, unused);
     }
 
-    private String missingPropertyViolation(String version, List<Artifact> artifacts) {
+    private String missingPropertyViolation(String effectiveVersion, List<Artifact> artifacts) {
         if (!requirePropertiesForDuplicates) return null;
 
         String unused = artifacts.stream()
-                .filter(a -> Objects.equals(a.getVersion(), a.getEffectiveVersion()))
+                .filter(Artifact::hasExplicitVersion)   // TODO is this required since all artifacts at this point should be explicitly versioned
                 .map(Artifact::key)
                 .collect(joining(", "));
         return String.format(
                 "Version '%s' exists in multiple locations, please extract a version property. " +
                         "Unused locations: %s",
-                version, unused);
+                effectiveVersion, unused);
     }
 
     /**

src/test/java/com/solubris/enforcer/VersionPropertyRuleTest.java

@@ -44,7 +44,7 @@ void detectsSingleUseOfProperty(boolean allowed) {
 
     @Test
     void multipleUseOfPropertyAllowed() {
-        modelStubber.withDependency("junit", "junit", "junit.version", "4.13.2");
+        modelStubber.withDependency("org.junit.jupiter", "junit-engine", "junit.version", "4.13.2");
         modelStubber.withDependency("org.junit.jupiter", "junit-jupiter-api", "junit.version", "4.13.2");
 
         Stream<String> violations = rule.scan();
@@ -63,7 +63,7 @@ void multipleUseOfPropertyAllowed() {
     class RequirePropertiesForDuplicates {
         @Test
         void multipleExplicitVersionsNotAllowed() {
-            modelStubber.withDependency("junit", "junit", "4.13.2");
+            modelStubber.withDependency("org.junit.jupiter", "junit-engine", "4.13.2");
             modelStubber.withDependency("org.junit.jupiter", "junit-jupiter-api", "4.13.2");
 
 //            DependencyManagement depMgmt = new DependencyManagement();
@@ -75,14 +75,28 @@ void multipleExplicitVersionsNotAllowed() {
             assertThat(violations).hasSize(1);
         }
 
+        /**
+         * Entirely legitimate to use the same version for disparate dependencies.
+         * Assume they must have a different groupId.
+         */
+        @Test
+        void coincidentalVersionsAllowed() {
+            modelStubber.withDependency("com.pmd", "pmd.core", "4.13.2");
+            modelStubber.withDependency("org.junit.jupiter", "junit-jupiter-api", "4.13.2");
+
+            Stream<String> violations = rule.scan();
+
+            assertThat(violations).isEmpty();
+        }
+
         @Test
         void multipleExplicitVersionsNotAllowedCoveringAllTypes() {
             modelStubber.withAllTypes(() -> "4.13.2");
 
             Stream<String> violations = rule.scan();
 
             // TODO how to assert the multiple locations for this violation?
-            assertThat(violations).hasSize(1);
+            assertThat(violations).isNotEmpty();
         }
 
         @Test