Merge pull request #1958 from adangel/issue-1952

[java] Compare ignored annotations by fully qualified names only
2019-09-04 00:58:55 -03:00
parent 1d6148f06b 282f29f781
commit 46ea77427c
6 changed files with 75 additions and 5 deletions
@@ -44,6 +44,7 @@ about the usage and features of the rule designer.
    *   [#1990](https://github.com/pmd/pmd/pull/1990): \[core] Incremental analysis mixes XPath rule violations
 *   java-bestpractices
    *   [#1862](https://github.com/pmd/pmd/issues/1862): \[java] New rule for MessageDigest.getInstance
+    *   [#1952](https://github.com/pmd/pmd/issues/1952): \[java] UnusedPrivateField not triggering if @Value annotation present
 *   java-codestyle
    *   [#1951](https://github.com/pmd/pmd/issues/1951): \[java] UnnecessaryFullyQualifiedName rule triggered when variable name clashes with package name
 *   java-errorprone
@@ -19,6 +19,10 @@ public final class TypeHelper {
     * Checks whether the resolved type of the given {@link TypeNode} n is of the type
     * given by the clazzName. If the clazzName is on the auxclasspath, then also subclasses
     * are considered.
+     * 
+     * <p>If clazzName is not on the auxclasspath (so it can't be resolved), then a string
+     * comparison of the class names are performed. This might result in comparing only
+     * the simple name of the classes.
     *
     * @param n the type node to check
     * @param clazzName the class name to compare to
@@ -27,13 +31,13 @@ public final class TypeHelper {
    public static boolean isA(final TypeNode n, final String clazzName) {
        final Class<?> clazz = loadClassWithNodeClassloader(n, clazzName);

-        if (clazz != null) {
+        if (clazz != null || n.getType() != null) {
            return isA(n, clazz);
        }

        return clazzName.equals(n.getImage()) || clazzName.endsWith("." + n.getImage());
    }
-    
+
    /**
     * Checks whether the resolved type of the given {@link TypeNode} n is exactly of the type
     * given by the clazzName.
@@ -133,8 +137,8 @@ public final class TypeHelper {

    public static boolean subclasses(TypeNode n, Class<?> clazz) {
        Class<?> type = n.getType();
-        if (type == null) {
-            return n.hasImageEqualTo(clazz.getSimpleName()) || n.hasImageEqualTo(clazz.getName());
+        if (type == null || clazz == null) {
+            return false; // If in auxclasspath, both should be resolvable, or are not the same
        }

        return clazz.isAssignableFrom(type);
@@ -4,8 +4,26 @@

 package net.sourceforge.pmd.lang.java.rule.bestpractices;

+import org.junit.Assert;
+import org.junit.Test;
+
 import net.sourceforge.pmd.testframework.PmdRuleTst;

 public class UnusedPrivateFieldTest extends PmdRuleTst {
-    // no additional unit tests
+
+    /**
+     * This test will fail, as soon Lombok classes are on the test classpath.
+     * The test classpath is used as auxclasspath during unit tests.
+     * If lombok is present, then the test case for #1952 will never fail
+     * and won't reproduce the false-negative case anymore.
+     */
+    @Test
+    public void makeSureLombokIsNotOnClasspath() {
+        try {
+            Class.forName("lombok.Value");
+            Assert.fail();
+        } catch (ClassNotFoundException e) {
+            // this is ok
+        }
+    }
 }
@@ -0,0 +1,13 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.rule.bestpractices.unusedprivatefield;
+
+/**
+ * Sample annotation used to test whether we correctly distinguish between {@code lombok.Value} and
+ * any other annotation called {@code Value}.
+ */
+public @interface Value {
+
+}
@@ -632,6 +632,21 @@ import lombok.experimental.Delegate;

 public class Foo {
    @Delegate private String bar;
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>#1952 [java] UnusedPrivateField not triggering if @Value annotation present</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>6</expected-linenumbers>
+        <code><![CDATA[
+import net.sourceforge.pmd.lang.java.rule.bestpractices.unusedprivatefield.Value;
+
+public class Foo {
+
+    @Value
+    private String bar;
 }
        ]]></code>
    </test-code>
@@ -9,6 +9,8 @@ Fail, BigInteger(1)
     ]]></description>
        <expected-problems>1</expected-problems>
        <code><![CDATA[
+import java.math.BigInteger;
+
 public class Foo {
 BigInteger b = new BigInteger("1");
 }
@@ -21,6 +23,8 @@ Pass, BigInteger(10)
     ]]></description>
        <expected-problems>0</expected-problems>
        <code><![CDATA[
+import java.math.BigInteger;
+
 public class Foo {
 BigInteger b = new BigInteger("10");
 }
@@ -33,6 +37,8 @@ Fail, BigInteger(0)
     ]]></description>
        <expected-problems>1</expected-problems>
        <code><![CDATA[
+import java.math.BigInteger;
+
 public class Foo {
 BigInteger b = new BigInteger("0");
 }
@@ -45,6 +51,8 @@ Pass, BigDecimal(i - 1)
     ]]></description>
        <expected-problems>0</expected-problems>
        <code><![CDATA[
+import java.math.BigDecimal;
+
 public class Foo {
 int i = 42;
 BigDecimal b = new BigDecimal(i - 1);
@@ -58,6 +66,9 @@ Pass, BigInteger("10") and BigDecimal in 1.4 mode
     ]]></description>
        <expected-problems>0</expected-problems>
        <code><![CDATA[
+import java.math.BigInteger;
+import java.math.BigDecimal;
+
 public class Foo {
 void test14() {
  BigInteger b = new BigInteger("10");
@@ -79,6 +90,8 @@ Fail, BigInteger(10) 1.5 mode
     ]]></description>
        <expected-problems>1</expected-problems>
        <code><![CDATA[
+import java.math.BigInteger;
+
 public class Foo {
 BigInteger b = new BigInteger("10");
 }
@@ -91,6 +104,8 @@ Fail, BigDecimal(1)
     ]]></description>
        <expected-problems>1</expected-problems>
        <code><![CDATA[
+import java.math.BigDecimal;
+
 public class Foo {
 BigDecimal b = new BigDecimal(1);
 }
@@ -103,6 +118,8 @@ Fail, BigDecimal(10)
     ]]></description>
        <expected-problems>1</expected-problems>
        <code><![CDATA[
+import java.math.BigDecimal;
+
 public class Foo {
 BigDecimal b = new BigDecimal(10);
 }
@@ -115,6 +132,8 @@ Fail, BigDecimal(0)
     ]]></description>
        <expected-problems>1</expected-problems>
        <code><![CDATA[
+import java.math.BigDecimal;
+
 public class Foo {
 BigDecimal b = new BigDecimal(0);
 }