#1261 False positive "Avoid unused private methods" with Generics

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/ClassScope.java

@@ -10,11 +10,15 @@
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression;
 import net.sourceforge.pmd.lang.java.ast.ASTArgumentList;
+import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration;
+import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
 import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
 import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
 import net.sourceforge.pmd.lang.java.ast.ASTName;
 import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix;
+import net.sourceforge.pmd.lang.java.ast.ASTTypeParameter;
+import net.sourceforge.pmd.lang.java.ast.ASTTypeParameters;
 import net.sourceforge.pmd.lang.symboltable.NameDeclaration;
 import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
 import net.sourceforge.pmd.lang.symboltable.Scope;
@@ -293,12 +297,57 @@ private List<TypedNameDeclaration> determineArgumentTypes(JavaNameOccurrence occ
                     // This might cause confusion, if method overloading is used.
                     type = parameterTypes.get(i);
                 }
+                if (type != null && type.getType() == null) {
+                    Class<?> typeBound = resolveGenericType(argument, type.getTypeImage());
+                    if (typeBound != null) {
+                        type = new SimpleTypedNameDeclaration(type.getTypeImage(), typeBound);
+                    }
+                }
                 argumentTypes.add(type);
             }
         }
         return argumentTypes;
     }
 
+    /**
+     * Tries to resolve a given typeImage as a generic Type. If the Generic Type is found,
+     * any defined ClassOrInterfaceType below this type declaration is used (this is typically
+     * a type bound, e.g. {@code <T extends List>}.
+     *
+     * @param argument the node, from where to start searching.
+     * @param typeImage the type as string
+     * @return the resolved class or <code>null</code> if nothing was found.
+     */
+    private Class<?> resolveGenericType(Node argument, String typeImage) {
+        List<ASTTypeParameter> types = new ArrayList<ASTTypeParameter>();
+        // first search only within the same method
+        types.addAll(argument.getFirstParentOfType(ASTClassOrInterfaceBodyDeclaration.class)
+                .findDescendantsOfType(ASTTypeParameter.class));
+
+        // then search class level types
+        ASTTypeParameters classLevelTypeParameters = argument.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class)
+                .getFirstChildOfType(ASTTypeParameters.class);
+        if (classLevelTypeParameters != null) {
+            types.addAll(classLevelTypeParameters.findDescendantsOfType(ASTTypeParameter.class));
+        }
+        return resolveGenericType(typeImage, types);
+    }
+
+    private Class<?> resolveGenericType(String typeImage, List<ASTTypeParameter> types) {
+        for (ASTTypeParameter type : types) {
+            if (typeImage.equals(type.getImage())) {
+                ASTClassOrInterfaceType bound = type.getFirstDescendantOfType(ASTClassOrInterfaceType.class);
+                if (bound != null && bound.getType() != null) {
+                    return bound.getType();
+                }
+                if (bound != null) {
+                    return this.getEnclosingScope(SourceFileScope.class).resolveType(bound.getImage());
+                }
+            }
+        }
+        return null;
+    }
+
     private Node getNextSibling(Node current) {
         Node nextSibling = null;
         for (int i = 0; i < current.jjtGetParent().jjtGetNumChildren() - 1; i++) {

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/SimpleTypedNameDeclaration.java

@@ -66,10 +66,12 @@ public boolean equals(Object obj) {
             return false;
         SimpleTypedNameDeclaration other = (SimpleTypedNameDeclaration) obj;
         if (type == null) {
+            if (other.type == Object.class)
+                return true;
             if (other.type != null)
                 return false;
         }
-        if (type != null && type.equals(other.type))
+        if (type != null && (type.equals(other.type) || type == Object.class))
             return true;
 
         // if the type is given, only compare the type and don't care about the type image

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symboltable/SimpleTypedNameDeclarationTest.java

@@ -58,6 +58,9 @@ public void testEquals() {
         Assert.assertEquals(by(Long.TYPE, "long"), by(null, "char"));
         Assert.assertEquals(by(Long.TYPE, "long"), by(Character.TYPE, "char"));
         Assert.assertEquals(by(Long.TYPE, "long"), by(Character.class, "Character"));
+
+        // should always equal to Object
+        Assert.assertEquals(by(Object.class, "Object"), by(null, "Something"));
     }
 
     private static SimpleTypedNameDeclaration byClass(Class<?> c) {

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

@@ -1171,6 +1171,52 @@ public class TestLinkUriResolver {
             return fromTestLinkId((TestLinkId.ExternalTestLinkId) testLinkId);
         }
     }
+}
+        ]]></code>
+    </test-code>
+    <test-code>
+        <description>#1261 False positive "Avoid unused private methods" with Generics</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class TestPrivate<T> {
+    protected Object getProtected(final T bean) {
+        return getPrivate(bean);
+    }
+    private Object getPrivate(final Object bean) {
+        return bean;
+    }
+}
+        ]]></code>
+    </test-code>
+    <test-code>
+        <description>#1261 False positive "Avoid unused private methods" with Generics 2</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.List;
+
+public class TestPrivate<T extends List> {
+    protected Object getProtected(final T bean) {
+        return getPrivate(bean);
+    }
+    private Object getPrivate(final List bean) {
+        return bean;
+    }
+}
+        ]]></code>
+    </test-code>
+    <test-code>
+        <description>#1261 False positive "Avoid unused private methods" with Generics 3</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.util.List;
+
+public class TestPrivate {
+    protected <T extends List> Object getProtected(final T bean) {
+        return getPrivate(bean);
+    }
+    private Object getPrivate(final List bean) {
+        return bean;
+    }
 }
         ]]></code>
     </test-code>

src/site/markdown/overview/changelog.md

@@ -12,6 +12,7 @@
 
 * [#550](https://sourceforge.net/p/pmd/bugs/550/): False +: MissingBreakInSwitch
 * [#1252](https://sourceforge.net/p/pmd/bugs/1252/): net.sourceforge.pmd.lang.ast.TokenMgrError: Lexical error in file xxx.cpp
+* [#1261](https://sourceforge.net/p/pmd/bugs/1261/): False positive "Avoid unused private methods" with Generics
 * [#1262](https://sourceforge.net/p/pmd/bugs/1262/): False positive for MissingBreakInSwitch
 * [#1263](https://sourceforge.net/p/pmd/bugs/1263/): PMD reports CheckResultSet violation in completely unrelated source files.
 * [#1272](https://sourceforge.net/p/pmd/bugs/1272/): varargs in methods are causing IndexOutOfBoundException when trying to process files