[java] Fix UnusedPrivateMethod for @lombok.Builder.ObtainVia (#5111)

Merge pull request #5111 from kdebski85:issue-5110-lombok-obtain-via

docs/pages/release_notes.md

@@ -39,6 +39,7 @@ This is a {{ site.pmd.release_type }} release.
 * apex-performance
   * [#635](https://github.com/pmd/pmd/issues/635): \[apex] New Rule: Avoid soql/sosl queries without a where clause or limit statement
 * java-bestpractices
+  * [#5110](https://github.com/pmd/pmd/issues/5110): \[java] UnusedPrivateMethod for method referenced by lombok.Builder.ObtainVia
   * [#5117](https://github.com/pmd/pmd/issues/5117): \[java] UnusedPrivateMethod for methods annotated with jakarta.annotation.PostConstruct or PreDestroy
 * java-errorprone
   * [#1488](https://github.com/pmd/pmd/issues/1488): \[java] MissingStaticMethodInNonInstantiatableClass: False positive with Lombok Builder on Constructor
@@ -65,6 +66,7 @@ This is a {{ site.pmd.release_type }} release.
 * [#5088](https://github.com/pmd/pmd/pull/5088): \[plsql] Add support for 'DEFAULT' clause on the arguments of some oracle functions - [Arjen Duursma](https://github.com/duursma) (@duursma)
 * [#5107](https://github.com/pmd/pmd/pull/5107): \[doc] Update maven.md - Typo fixed for maven target - [karthikaiyasamy](https://github.com/karthikaiyasamy) (@karthikaiyasamy)
 * [#5109](https://github.com/pmd/pmd/pull/5109): \[java] Exclude constructor with lombok.Builder for MissingStaticMethodInNonInstantiatableClass - [Krzysztof Debski](https://github.com/kdebski85) (@kdebski85)
+* [#5111](https://github.com/pmd/pmd/pull/5111): \[java] Fix UnusedPrivateMethod for @<!-- -->lombok.Builder.ObtainVia - [Krzysztof Debski](https://github.com/kdebski85) (@kdebski85)
 * [#5118](https://github.com/pmd/pmd/pull/5118): \[java] FP for UnusedPrivateMethod with Jakarta @<!-- -->PostConstruct/PreDestroy annotations - [Krzysztof Debski](https://github.com/kdebski85) (@kdebski85)
 * [#5121](https://github.com/pmd/pmd/pull/5121): \[plsql] Fixed issue with missing optional table alias in MERGE usage - [Arjen Duursma](https://github.com/duursma) (@duursma)
 

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotation.java

@@ -81,8 +81,29 @@ public final class ASTAnnotation extends AbstractJavaTypeNode implements ASTMemb
      */
     public NodeStream<ASTMemberValue> getFlatValue(String attrName) {
         return NodeStream.of(getAttribute(attrName))
-                         .flatMap(v -> v instanceof ASTMemberValueArrayInitializer ? v.children(ASTMemberValue.class)
+                         .flatMap(ASTAnnotation::flatValue);
-                                                                                   : NodeStream.of(v));
+    }
+
+    /**
+     * Return expression values for all attributes.
+     * This may flatten an array initializer. For example, for the attribute
+     * named "value":
+     * <pre>{@code
+     * - @SuppressWarnings -> returns empty node stream
+     * - @SuppressWarning("fallthrough") -> returns ["fallthrough"]
+     * - @SuppressWarning(value={"fallthrough"}) -> returns ["fallthrough"]
+     * - @SuppressWarning({"fallthrough", "rawtypes"}) -> returns ["fallthrough", "rawtypes"]
+     * }</pre>
+     */
+    public NodeStream<ASTMemberValue> getFlatValues() {
+        return getMembers().map(ASTMemberValuePair::getValue)
+                           .flatMap(ASTAnnotation::flatValue);
+    }
+
+    private static NodeStream<ASTMemberValue> flatValue(ASTMemberValue value) {
+        return value instanceof ASTMemberValueArrayInitializer
+            ? value.children(ASTMemberValue.class)
+            : NodeStream.of(value);
     }
 
     /**

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java

@@ -14,13 +14,15 @@ import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.apache.commons.lang3.StringUtils;
+
 import net.sourceforge.pmd.lang.ast.NodeStream;
 import net.sourceforge.pmd.lang.java.ast.ASTAnnotation;
 import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
+import net.sourceforge.pmd.lang.java.ast.ASTMemberValue;
 import net.sourceforge.pmd.lang.java.ast.ASTMethodCall;
 import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTMethodReference;
-import net.sourceforge.pmd.lang.java.ast.ASTModifierList;
 import net.sourceforge.pmd.lang.java.ast.JavaNode;
 import net.sourceforge.pmd.lang.java.ast.MethodUsage;
 import net.sourceforge.pmd.lang.java.ast.ModifierOwner.Visibility;
@@ -51,23 +53,34 @@ public class UnusedPrivateMethodRule extends AbstractIgnoredAnnotationRule {
     @Override
     public Object visit(ASTCompilationUnit file, Object param) {
         // We do three traversals:
-        // - one to find methods referenced by Junit5 MethodSource annotations
+        // - one to find methods:
+        // --- referenced by any attribute of any annotation
+        // --- with name same as a method annotated with Junit5 MethodSource if the annotation value is empty
         // - one to find the "interesting methods", ie those that may be violations
         // - another to find the possible usages. We only try to resolve
         //   method calls/method refs that may refer to a method in the
         //   first set, ie, not every call in the file.
-
+        Set<String> methodsUsedByAnnotations =
-        Set<String> methodsUsedByAnnotations = file.descendants(ASTMethodDeclaration.class)
+                file.descendants(ASTAnnotation.class)
-            .crossFindBoundaries()
+                        .crossFindBoundaries()
-            .children(ASTModifierList.class)
+                        .toStream()
-            .children(ASTAnnotation.class)
+                        .flatMap(a -> Stream.concat(
-            .filter(t -> TypeTestUtil.isA("org.junit.jupiter.params.provider.MethodSource", t))
+                                        a.getFlatValues().toStream()
-            .toStream()
+                                                .map(ASTMemberValue::getConstValue)
-            // Get the referenced method names… if none, use the test method name instead
+                                                .filter(String.class::isInstance)
-            .flatMap(a -> a.getFlatValue("value").isEmpty()
+                                                .map(String.class::cast)
-                    ? Stream.of(a.ancestors(ASTMethodDeclaration.class).first().getName())
+                                                .filter(StringUtils::isNotEmpty),
-                    : a.getFlatValue("value").toStream().map(mv -> (String) mv.getConstValue()))
+                                        NodeStream.of(a)
-            .collect(Collectors.toSet());
+                                                .filter(it -> TypeTestUtil.isA("org.junit.jupiter.params.provider.MethodSource", it)
+                                                        && it.getFlatValue("value").isEmpty())
+                                                .ancestors(ASTMethodDeclaration.class)
+                                                .firstOpt()
+                                                .map(ASTMethodDeclaration::getName)
+                                                .map(Stream::of)
+                                                .orElse(Stream.empty())
+                                )
+                        )
+                        .collect(Collectors.toSet());
 
         Map<String, Set<ASTMethodDeclaration>> consideredNames =
             file.descendants(ASTMethodDeclaration.class)

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateMethod.xml

@@ -27,6 +27,18 @@ public class Foo {
         ]]></code>
     </test-code>
 
+    <test-code>
+        <description>simple unused annotated private method</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+import net.sourceforge.pmd.lang.java.symbols.testdata.MethodAnnotation;
+public class Foo {
+    @MethodAnnotation
+    private void foo() {}
+}
+        ]]></code>
+    </test-code>
+
     <test-code>
         <description>anonymous inner class calls private method</description>
         <expected-problems>0</expected-problems>
@@ -2100,4 +2112,26 @@ class FooTest{
             }
             ]]></code>
     </test-code>
+    <test-code>
+        <description>UnusedPrivateMethod for Lombok ObtainVia #5110</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import lombok.Builder;
+import lombok.Builder.ObtainVia;
+
+import java.util.Collections;
+import java.util.List;
+
+@Builder(toBuilder = true)
+public class ObtainViaTest {
+
+    @ObtainVia(method = "fooProvider")
+    private List<String> foo;
+
+    private List<String> fooProvider() {
+        return Collections.emptyList();
+    }
+}
+            ]]></code>
+    </test-code>
 </test-data>