From 8f86832322c86674d1395f555bc6895cd58ae795 Mon Sep 17 00:00:00 2001
From: dykov <dykovoleksii@gmail.com>
Date: Wed, 21 Aug 2024 15:38:25 +0300
Subject: [PATCH] Fix UseConstantAsMetricName rule - from XPath to Java

---
 .../arch4u/pmd/UseConstantAsMetricName.java   | 42 +++++++++++++++++++
 .../dgroup/arch4u/pmd/arch4u-ruleset.xml      | 27 ++++--------
 .../pmd/UseConstantAsMetricNameTest.java      |  6 +--
 .../pmd/xml/UseConstantAsMetricName.xml       | 34 +++++++++++++++
 4 files changed, 86 insertions(+), 23 deletions(-)
 create mode 100644 src/main/java/io/github/dgroup/arch4u/pmd/UseConstantAsMetricName.java

diff --git a/src/main/java/io/github/dgroup/arch4u/pmd/UseConstantAsMetricName.java b/src/main/java/io/github/dgroup/arch4u/pmd/UseConstantAsMetricName.java
new file mode 100644
index 0000000..77256a4
--- /dev/null
+++ b/src/main/java/io/github/dgroup/arch4u/pmd/UseConstantAsMetricName.java
@@ -0,0 +1,42 @@
+package io.github.dgroup.arch4u.pmd;
+
+import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
+import net.sourceforge.pmd.lang.java.ast.ASTMemberValuePair;
+import net.sourceforge.pmd.lang.java.ast.ASTStringLiteral;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.java.types.TypeTestUtil;
+import net.sourceforge.pmd.properties.PropertyDescriptor;
+import net.sourceforge.pmd.properties.PropertyFactory;
+
+import java.util.List;
+
+public class UseConstantAsMetricName extends AbstractJavaRule {
+
+    private static final PropertyDescriptor<List<String>> METRIC_ANNOTATIONS_DESCRIPTOR =
+            PropertyFactory.stringListProperty("metricAnnotations")
+                    .desc("List of the metric annotations.")
+                    .defaultValues("io.micrometer.core.annotation.Timed")
+                    .build();
+
+    private final List<String> metricAnnotations;
+
+    public UseConstantAsMetricName() {
+        definePropertyDescriptor(METRIC_ANNOTATIONS_DESCRIPTOR);
+        metricAnnotations = getProperty(METRIC_ANNOTATIONS_DESCRIPTOR);
+    }
+
+    @Override
+    public Object visit(ASTAnnotation annotation, Object data) {
+        if (isMetricAnnotation(annotation)) {
+            annotation.getMembers()
+                    .map(ASTMemberValuePair::getValue)
+                    .filter(val -> val instanceof ASTStringLiteral)
+                    .forEach(val -> asCtx(data).addViolation(val));
+        }
+        return super.visit(annotation, data);
+    }
+
+    private boolean isMetricAnnotation(ASTAnnotation node) {
+        return metricAnnotations.stream().anyMatch(annot -> TypeTestUtil.isExactlyA(annot, node));
+    }
+}
diff --git a/src/main/resources/io/github/dgroup/arch4u/pmd/arch4u-ruleset.xml b/src/main/resources/io/github/dgroup/arch4u/pmd/arch4u-ruleset.xml
index d609f66..40e08a1 100644
--- a/src/main/resources/io/github/dgroup/arch4u/pmd/arch4u-ruleset.xml
+++ b/src/main/resources/io/github/dgroup/arch4u/pmd/arch4u-ruleset.xml
@@ -158,7 +158,7 @@
     <priority>3</priority>
     <properties>
       <property name="restAnnotations"
-                description="List of the REST endpoint method annotations"
+                description="List of the REST endpoint method annotations. Use a comma (,) as a delimiter."
                 value="org.springframework.web.bind.annotation.GetMapping,
                 org.springframework.web.bind.annotation.DeleteMapping,
                 org.springframework.web.bind.annotation.PatchMapping,
@@ -166,7 +166,7 @@
                 org.springframework.web.bind.annotation.PutMapping,
                 org.springframework.web.bind.annotation.RequestMapping"/>
       <property name="mandatoryAnnotations"
-                description="List of the mandatory metric method annotations"
+                description="List of the mandatory metric method annotations. Use a comma (,) as a delimiter."
                 value="io.micrometer.core.annotation.Timed"/>
     </properties>
     <example>
@@ -191,35 +191,22 @@
       }
       ]]>
     </example>
-  </rule><!--
+  </rule>
 
   <rule name="UseConstantAsMetricName"
         since="0.1.0"
         language="java"
         externalInfoUrl="https://github.com/dgroup/arch4u-pmd/discussions/75"
         message="Use constant for metric name instead of hardcoded string literals: https://github.com/dgroup/arch4u-pmd/discussions/75"
-        class="net.sourceforge.pmd.lang.rule.xpath.XPathRule">
+        class="io.github.dgroup.arch4u.pmd.UseConstantAsMetricName">
     <description>
       The monitoring metric name should be provided into annotation @Timed by constant, not a hardcoded string.
     </description>
     <priority>3</priority>
     <properties>
-      <property name="metricAnnotations" type="List[String]" delimiter="|"
-                description="List of the metric annotations"
+      <property name="metricAnnotations"
+                description="List of the metric annotations. Use a comma (,) as a delimiter."
                 value="io.micrometer.core.annotation.Timed"/>
-      <property name="version" value="2.0"/>
-      <property name="xpath">
-        <value>
-          <![CDATA[
-          //Annotation[some $annotation in $metricAnnotations satisfies pmd-java:typeIsExactly($annotation)]
-          [
-          SingleMemberAnnotation/MemberValue/PrimaryExpression/PrimaryPrefix/Literal
-          or
-          NormalAnnotation/MemberValuePairs/MemberValuePair[@Image='value']/MemberValue/PrimaryExpression/PrimaryPrefix/Literal
-          ]
-          ]]>
-        </value>
-      </property>
     </properties>
     <example>
       <![CDATA[
@@ -240,7 +227,7 @@
       }
       ]]>
     </example>
-  </rule> -->
+  </rule>
 
 
   <!-- Rules related to error handling --> <!--
diff --git a/src/test/java/io/github/dgroup/arch4u/pmd/UseConstantAsMetricNameTest.java b/src/test/java/io/github/dgroup/arch4u/pmd/UseConstantAsMetricNameTest.java
index 9593e10..fd85b43 100644
--- a/src/test/java/io/github/dgroup/arch4u/pmd/UseConstantAsMetricNameTest.java
+++ b/src/test/java/io/github/dgroup/arch4u/pmd/UseConstantAsMetricNameTest.java
@@ -27,15 +27,15 @@
 import net.sourceforge.pmd.test.SimpleAggregatorTst;
 
 /**
- * Test case for {@code UseConstantForMetricAnnotation} rule.
+ * Test case for {@link UseConstantAsMetricName} rule.
  *
  * @since 0.1.0
  */
 @SuppressWarnings({"PMD.TestClassWithoutTestCases", "PMD.JUnit4TestShouldUseBeforeAnnotation"})
 public final class UseConstantAsMetricNameTest extends SimpleAggregatorTst {
 
-    /*@Override
+    @Override
     public void setUp() {
         addRule("io/github/dgroup/arch4u/pmd/arch4u-ruleset.xml", "UseConstantAsMetricName");
     }
-*/}
+}
diff --git a/src/test/resources/io/github/dgroup/arch4u/pmd/xml/UseConstantAsMetricName.xml b/src/test/resources/io/github/dgroup/arch4u/pmd/xml/UseConstantAsMetricName.xml
index 962452e..9efa9a4 100644
--- a/src/test/resources/io/github/dgroup/arch4u/pmd/xml/UseConstantAsMetricName.xml
+++ b/src/test/resources/io/github/dgroup/arch4u/pmd/xml/UseConstantAsMetricName.xml
@@ -69,4 +69,38 @@ public class Foo {
         ]]></code>
   </test-code>
 
+  <test-code>
+    <description>[OK]: default annotation value</description>
+    <expected-problems>0</expected-problems>
+    <code><![CDATA[
+import io.micrometer.core.annotation.Timed;
+
+public class Foo {
+   @Timed
+   public void bar() {
+   }
+
+   @Timed()
+   public void foo() {
+   }
+}
+        ]]></code>
+  </test-code>
+
+  <test-code>
+    <description>[OK]: the annotation has several params</description>
+    <expected-problems>0</expected-problems>
+    <code><![CDATA[
+import io.micrometer.core.annotation.Timed;
+
+public class Foo {
+   String METRIC = "value";
+
+   @Timed(histogram=false, longTask=true, value=METRIC)
+   public void foo() {
+   }
+}
+        ]]></code>
+  </test-code>
+
 </test-data>