checkProperty and checkExplicitVersion

README.md

@@ -2,10 +2,74 @@
 # TODO create syntactic sugar library
 
 # TODO's
+# ${project.version} can be excluded from the enforcer rule as it is a special property that is commonly used
+# in fact, ${project.*} can be excluded
+
+Split the enforcer into two rules:
+- one for checking the existing properties
+  - is the property used more than once?
+    - if not, it must be removed
+  - extract all artifacts from model
+    - versionUsages = group by version, ignoring explicit properties
+  - for each property:
+    - versionUsages.get(property.name).size() -> usages of the property
+      - if usages==1, then the property should be inlined and removed (ALLOW_SINGLE_USE_PROPERTIES=false)
+        - please replace usgae of 'property.name' with 'property.value'
+      - if usages==0, then the property should be removed (ALLOW_UNUSED_PROPERTIES=false)
+        - please remove 'property.name'
+    - what about if there is a version that is the same as the property value but not using the property?
+      - This should be replaced with the property as well
+      - and same rules apply to this version as well
+      - so if usage==1, but would be usage==2 if we replace it with the property, then we should replace it with the property and keep the property
+      - this means that versionUsages must capture both explicit and implicit usages of the version
+      - perhaps the artifact model should cover both these cases
+        - getVersion() returns the resolved version
+          - how to get the resolved version?
+            - perhaps this can come from the effective model
+            - how populate the resolved version?
+            - at time of building artifact model or as an enrichment step after building the artifact model?
+        - getVersionProperty() returns the property if it exists, otherwise null
+
+- one for checking repeated versions
+  - if a version is repeated, it must be replaced with a property
+  - extract all artifacts from model
+  - resolve all versions to their effective versions
+  - group by effectiveVersion
+    - this ensures that property usages are grouped together with non-property usages
+    - but only for dependencies in the original model
+    - for each version:
+      - if usages>1
+        - if all usages are through a property - nothing to do
+        - if some usages are through a property and some are not - replace all non-property usages
+        - if no usages are through a property - create a new property and replace all usages with the property
+      - if usages==1
+        - if usage is through a property - remove the property (ALLOW_SINGLE_USE_PROPERTIES=false)
+        - if usage is not through a property - nothing to do
+
+Should these be two separate rules or one rule that handles both cases?
+what would they be called?
+- VersionPropertyEnforcerRule
+  - check the properties and their usages
+  - versionPropertySuffix = "version" (configurable)
+- VersionUsageEnforcerRule
+  - check the versions and their usages
+
+ALLOW_SINGLE_USE_PROPERTIES - applicable to both rules
+
+Feels like these rules are intrinsically linked and should be part of the same rule,
+as they both deal with the same concept of properties and their usages.
+
+Even so, can ensure the implementation is cleanly separated within the same rule
+
+## PMD - collect to set should normally be replace with toUnmodifiableSet(), toUnmodifiableMap(), toUnmodifiableList()
+
+especially when the result is assigned to a final field or returned from a method
 
-## collect to set should normally be replace with toUnmodifiableSet()
 eg:
                 .collect(Collectors.toSet());
 replace with:
                 .collect(toUnmodifiableSet());
 
+What about this case:
+                .collect(groupingBy(Artifact::fullKey, toList()));
+

pom.xml

@@ -72,7 +72,7 @@
         <dependency>
             <groupId>org.junit.platform</groupId>
             <artifactId>junit-platform-commons</artifactId>
-            <version>${junit.version}</version>
+            <version>6.0.3</version>
             <scope>test</scope>
         </dependency>
         <dependency>
@@ -87,18 +87,21 @@
             <version>3.27.7</version>
             <scope>test</scope>
         </dependency>
-
-        <!-- Lombok for code generation (used by ArtifactV2) -->
+        <dependency>
+            <groupId>io.projectreactor</groupId>
+            <artifactId>reactor-core</artifactId>
+            <version>3.6.0</version>
+        </dependency>
         <dependency>
             <groupId>org.projectlombok</groupId>
             <artifactId>lombok</artifactId>
-            <version>1.18.36</version>
+            <version>1.18.42</version>
             <scope>provided</scope>
         </dependency>
         <dependency>
-            <groupId>com.google.code.findbugs</groupId>
-            <artifactId>findbugs-annotations</artifactId>
-            <version>3.0.1</version>
+            <groupId>com.github.spotbugs</groupId>
+            <artifactId>spotbugs-annotations</artifactId>
+            <version>4.8.6</version>
             <scope>provided</scope>
         </dependency>
         <dependency>
@@ -134,7 +137,7 @@
                         <path>
                             <groupId>org.projectlombok</groupId>
                             <artifactId>lombok</artifactId>
-                            <version>1.18.36</version>
+                            <version>1.18.42</version>
                         </path>
                     </annotationProcessorPaths>
                 </configuration>

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

@@ -9,6 +9,8 @@
 import org.apache.maven.model.Plugin;
 import org.apache.maven.model.ReportPlugin;
 
+import java.util.Objects;
+
 @Builder(toBuilder = true, setterPrefix = "with")
 @RequiredArgsConstructor
 @Getter
@@ -66,4 +68,12 @@ public String key() {
         // TODO can use versionlessKey()
         return groupId + ":" + artifactId;
     }
+
+    public boolean hasExplicitVersion() {
+        return Objects.equals(getVersion(), getEffectiveVersion());
+    }
+
+    public boolean hasImplicitVersion() {
+        return !Objects.equals(getVersion(), getEffectiveVersion());
+    }
 }

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

@@ -97,7 +97,10 @@ private static Stream<Dependency> pluginDependencies(Plugin plugin) {
      */
     private static Stream<Artifact> scanPlugins(Stream<Plugin> items, boolean managed, String profile) {
         return items.flatMap(p ->
-                prepend(new Artifact(p, managed, profile), pluginDependencies(p).map(d -> new Artifact(d, managed, profile)))
+                prepend(
+                        new Artifact(p, managed, profile),
+                        pluginDependencies(p).map(d -> new Artifact(d, managed, profile))
+                )
         );
     }
 

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

@@ -0,0 +1,11 @@
+package com.solubris.enforcer;
+
+public class PropertyUtil {
+    public static String asPlaceHolder(String version) {
+        return String.format("${%s}", version);
+    }
+
+    public static String fromPlaceHolder(String version) {
+        return version.substring(2, version.length() - 1);
+    }
+}