fix method

Author: Vincent Potucek

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

@@ -9,7 +9,6 @@
 import static net.sourceforge.pmd.util.CollectionUtil.setOf;
 
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -41,15 +40,15 @@
 public class UnusedPrivateMethodRule extends AbstractIgnoredAnnotationRule {
 
     private static final Set<String> SERIALIZATION_METHODS =
-            setOf("readObject", "readObjectNoData", "writeObject", "readResolve", "writeReplace");
+        setOf("readObject", "readObjectNoData", "writeObject", "readResolve", "writeReplace");
 
     @Override
     protected Collection<String> defaultSuppressionAnnotations() {
         return listOf(
-                "java.lang.Deprecated",
-                "jakarta.annotation.PostConstruct",
-                "jakarta.annotation.PreDestroy",
-                "lombok.EqualsAndHashCode.Include"
+            "java.lang.Deprecated",
+            "jakarta.annotation.PostConstruct",
+            "jakarta.annotation.PreDestroy",
+            "lombok.EqualsAndHashCode.Include"
         );
     }
 
@@ -62,73 +61,40 @@ public Object visit(ASTCompilationUnit file, Object param) {
         Set<String> methodsUsedByAnnotations = methodsUsedByAnnotations(file);
         Map<JExecutableSymbol, ASTMethodDeclaration> candidates = findCandidates(file, methodsUsedByAnnotations);
 
-        // First, remove methods that are called from within the same class
-        removeMethodsCalledFromSameClass(file, candidates);
-
         file.descendants()
-                .crossFindBoundaries()
-                .<MethodUsage>map(asInstanceOf(ASTMethodCall.class, ASTMethodReference.class))
-                .forEach(ref -> {
-                    OverloadSelectionResult selectionInfo = ref.getOverloadSelectionInfo();
-                    JExecutableSymbol calledMethod = selectionInfo.getMethodType().getSymbol();
-                    if (calledMethod.isUnresolved()) {
-                        handleUnresolvedCall(ref, selectionInfo, candidates);
-                        return;
+            .crossFindBoundaries()
+            .<MethodUsage>map(asInstanceOf(ASTMethodCall.class, ASTMethodReference.class))
+            .forEach(ref -> {
+                OverloadSelectionResult selectionInfo = ref.getOverloadSelectionInfo();
+                JExecutableSymbol calledMethod = selectionInfo.getMethodType().getSymbol();
+                if (calledMethod.isUnresolved()) {
+                    handleUnresolvedCall(ref, selectionInfo, candidates);
+                    return;
+                }
+                candidates.compute(calledMethod, (sym2, reffed) -> {
+                    if (reffed != null && ref.ancestors(ASTMethodDeclaration.class).first() != reffed) {
+                        // remove mapping, but only if it is called from outside itself
+                        return null;
                     }
-                    candidates.compute(calledMethod, (sym2, reffed) -> {
-                        if (reffed != null && ref.ancestors(ASTMethodDeclaration.class).first() != reffed) {
-                            // remove mapping, but only if it is called from outside itself
-                            return null;
-                        }
-                        return reffed;
-                    });
+                    return reffed;
                 });
+            });
 
         for (ASTMethodDeclaration unusedMethod : candidates.values()) {
             asCtx(param).addViolation(unusedMethod, PrettyPrintingUtil.displaySignature(unusedMethod));
         }
         return null;
     }
 
-    /**
-     * Remove methods that are called from within the same class from the candidates map.
-     * This handles the case where private methods call other private methods in the same class.
-     */
-    private void removeMethodsCalledFromSameClass(ASTCompilationUnit file, Map<JExecutableSymbol, ASTMethodDeclaration> candidates) {
-        if (candidates.isEmpty()) {
-            return;
-        }
-
-        // Create a map of method names to their symbols for faster lookup
-        Map<String, JExecutableSymbol> methodNameToSymbol = new HashMap<>();
-        for (JExecutableSymbol symbol : candidates.keySet()) {
-            methodNameToSymbol.put(symbol.getPackageName(), symbol);
-        }
-
-        // Check each candidate method for calls to other private methods in the same class
-        for (ASTMethodDeclaration method : candidates.values()) {
-            method.descendants(ASTMethodCall.class)
-                    .crossFindBoundaries()
-                    .forEach(call -> {
-                        String calledMethodName = call.getMethodName();
-                        JExecutableSymbol calledSymbol = methodNameToSymbol.get(calledMethodName);
-                        if (calledSymbol != null && candidates.containsKey(calledSymbol)) {
-                            // This method calls another private method in the same class, so remove the called method
-                            candidates.remove(calledSymbol);
-                        }
-                    });
-        }
-    }
-
     private static void handleUnresolvedCall(MethodUsage ref, OverloadSelectionResult selectionInfo, Map<JExecutableSymbol, ASTMethodDeclaration> candidates) {
         // If the type is may be an instance of this class, then the method may be
         // a call to a private method here. In that case we whitelist all methods
         // with that name.
         JTypeMirror receive = selectionInfo.getTypeToSearch();
         boolean receiverMayBeInstanceOfThisClass =
                 receive == null
-                        || TypeOps.isSpecialUnresolved(receive)
-                        || receive.equals(ref.getEnclosingType().getTypeMirror());
+                || TypeOps.isSpecialUnresolved(receive)
+                || receive.equals(ref.getEnclosingType().getTypeMirror());
 
         if (receiverMayBeInstanceOfThisClass) {
             candidates.values().removeIf(it -> it.getName().equals(ref.getMethodName()));
@@ -143,43 +109,43 @@ private static void handleUnresolvedCall(MethodUsage ref, OverloadSelectionResul
      */
     private Map<JExecutableSymbol, ASTMethodDeclaration> findCandidates(ASTCompilationUnit file, Set<String> methodsUsedByAnnotations) {
         return file.descendants(ASTMethodDeclaration.class)
-                .crossFindBoundaries()
-                .filter(
-                        it -> it.getVisibility() == Visibility.V_PRIVATE
-                                && !hasIgnoredAnnotation(it)
-                                && !hasExcludedName(it)
-                                && !(it.getArity() == 0 && methodsUsedByAnnotations.contains(it.getName())))
-                .collect(Collectors.toMap(
-                        ASTMethodDeclaration::getSymbol,
-                        m -> m
-                ));
+                   .crossFindBoundaries()
+                   .filter(
+                       it -> it.getVisibility() == Visibility.V_PRIVATE
+                           && !hasIgnoredAnnotation(it)
+                           && !hasExcludedName(it)
+                           && !(it.getArity() == 0 && methodsUsedByAnnotations.contains(it.getName())))
+                   .collect(Collectors.toMap(
+                       ASTMethodDeclaration::getSymbol,
+                       m -> m
+                   ));
     }
 
     private static Set<String> methodsUsedByAnnotations(ASTCompilationUnit file) {
         return file.descendants(ASTAnnotation.class)
-                .crossFindBoundaries()
-                .toStream()
-                .flatMap(UnusedPrivateMethodRule::extractMethodsFromAnnotation)
-                .collect(Collectors.toSet());
+            .crossFindBoundaries()
+            .toStream()
+            .flatMap(UnusedPrivateMethodRule::extractMethodsFromAnnotation)
+            .collect(Collectors.toSet());
     }
 
     private static Stream<String> extractMethodsFromAnnotation(ASTAnnotation a) {
         return Stream.concat(
-                a.getFlatValues().toStream()
-                        .map(ASTMemberValue::getConstValue)
-                        .map(asInstanceOf(String.class))
-                        .filter(StringUtils::isNotEmpty),
-                NodeStream.of(a)
-                        .filter(it -> TypeTestUtil.isA("org.junit.jupiter.params.provider.MethodSource", it)
-                                && it.getFlatValue("value").isEmpty())
-                        .ancestors(ASTMethodDeclaration.class)
-                        .take(1)
-                        .toStream()
-                        .map(ASTMethodDeclaration::getName)
+            a.getFlatValues().toStream()
+                .map(ASTMemberValue::getConstValue)
+                .map(asInstanceOf(String.class))
+                .filter(StringUtils::isNotEmpty),
+            NodeStream.of(a)
+                .filter(it -> TypeTestUtil.isA("org.junit.jupiter.params.provider.MethodSource", it)
+                    && it.getFlatValue("value").isEmpty())
+                .ancestors(ASTMethodDeclaration.class)
+                .take(1)
+                .toStream()
+                .map(ASTMethodDeclaration::getName)
         );
     }
 
     private boolean hasExcludedName(ASTMethodDeclaration node) {
         return SERIALIZATION_METHODS.contains(node.getName());
     }
-}
+}

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

@@ -2785,22 +2785,6 @@ class Foo {
         <description>addAllImpl</description>
         <expected-problems>0</expected-problems>
         <code><![CDATA[
-/*
- * Copyright (C) 2007 The Guava Authors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
 package com.google.common.collect;
 
 import static com.google.common.base.Preconditions.checkArgument;
@@ -2840,8 +2824,8 @@ import java.util.stream.Collector;
 import org.jspecify.annotations.Nullable;
 
 @GwtCompatible
-public final class Multisets {
-  private Multisets() {}
+public final class addAllImpl {
+  private addAllImpl() {}
 
   /** An implementation of {@link Multiset#addAll}. */
   static <E extends @Nullable Object> boolean addAllImpl(
@@ -2867,6 +2851,124 @@ public final class Multisets {
 }
             ]]></code>
     </test-code>
+    <test-code>
+        <description>addAllImpl2</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+package com.google.common.collect;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.CollectPreconditions.checkNonnegative;
+import static com.google.common.collect.CollectPreconditions.checkRemove;
+import static java.lang.Math.max;
+import static java.lang.Math.min;
+import static java.util.Arrays.asList;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.annotations.GwtCompatible;
+import com.google.common.annotations.GwtIncompatible;
+import com.google.common.annotations.J2ktIncompatible;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.collect.Multiset.Entry;
+import com.google.common.math.IntMath;
+import com.google.common.primitives.Ints;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import com.google.errorprone.annotations.InlineMe;
+import com.google.errorprone.annotations.concurrent.LazyInit;
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.Set;
+import java.util.Spliterator;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.function.ToIntFunction;
+import java.util.stream.Collector;
+import org.jspecify.annotations.Nullable;
+
+@GwtCompatible
+public final class addAllImpl {
+  private addAllImpl() {}
+
+  /** An implementation of {@link Multiset#addAll}. */
+  static <E extends @Nullable Object> boolean addAllImpl(
+      Multiset<E> self, Collection<? extends E> elements) {
+    return addAllImpl(self, (Multiset<? extends E>) elements);
+  }
+
+  /** A specialization of {@code addAllImpl} for when {@code elements} is itself a Multiset. */
+  private static <E extends @Nullable Object> boolean addAllImpl(
+      Multiset<E> self, Multiset<? extends E> elements) {
+  }
+}
+            ]]></code>
+    </test-code>
+    <test-code>
+        <description>addAllImplFoo</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+package com.google.common.collect;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.CollectPreconditions.checkNonnegative;
+import static com.google.common.collect.CollectPreconditions.checkRemove;
+import static java.lang.Math.max;
+import static java.lang.Math.min;
+import static java.util.Arrays.asList;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.annotations.GwtCompatible;
+import com.google.common.annotations.GwtIncompatible;
+import com.google.common.annotations.J2ktIncompatible;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
+import com.google.common.collect.Multiset.Entry;
+import com.google.common.math.IntMath;
+import com.google.common.primitives.Ints;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import com.google.errorprone.annotations.InlineMe;
+import com.google.errorprone.annotations.concurrent.LazyInit;
+import java.io.Serializable;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.Set;
+import java.util.Spliterator;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.function.ToIntFunction;
+import java.util.stream.Collector;
+import org.jspecify.annotations.Nullable;
+
+@GwtCompatible
+public final class addAllImpl {
+  private addAllImpl() {}
+
+  /** An implementation of {@link Multiset#addAll}. */
+  static <E extends @Nullable Object> boolean addAllImpl(
+      Multiset<E> self, Collection<? extends E> elements) {
+    return addAllImplFoo(self, (Multiset<? extends E>) elements);
+  }
+
+  /** A specialization of {@code addAllImpl} for when {@code elements} is itself a Multiset. */
+  private static <E extends @Nullable Object> boolean addAllImplFoo(
+      Multiset<E> self, Multiset<? extends E> elements) {
+  }
+}
+            ]]></code>
+    </test-code>