Merge branch 'pr-2641'

[java] AvoidThrowingNullPointerException marks all NullPointerException… #2641

docs/pages/release_notes.md

@@ -33,6 +33,7 @@ This is a {{ site.pmd.release_type }} release.
 *   java-design
     *   [#2174](https://github.com/pmd/pmd/issues/2174): \[java] LawOfDemeter: False positive with 'this' pointer
     *   [#2189](https://github.com/pmd/pmd/issues/2189): \[java] LawOfDemeter: False positive when casting to derived class
+    *   [#2580](https://github.com/pmd/pmd/issues/2580): \[java] AvoidThrowingNullPointerException marks all NullPointerException objects as wrong, whether or not thrown
 *   java-errorprone
     *   [#2578](https://github.com/pmd/pmd/issues/2578): \[java] AvoidCallingFinalize detects some false positives
     *   [#2634](https://github.com/pmd/pmd/issues/2634): \[java] NullPointerException in rule ProperCloneImplementation
@@ -59,6 +60,7 @@ This is a {{ site.pmd.release_type }} release.
 *   [#2597](https://github.com/pmd/pmd/pull/2597): \[dependencies] Fix issue #2594, update exec-maven-plugin everywhere - [Artem Krosheninnikov](https://github.com/KroArtem)
 *   [#2621](https://github.com/pmd/pmd/pull/2621): \[visualforce] add new safe resource for VfUnescapeEl - [Peter Chittum](https://github.com/pchittum)
 *   [#2640](https://github.com/pmd/pmd/pull/2640): \[java] NullPointerException in rule ProperCloneImplementation - [Mykhailo Palahuta](https://github.com/Drofff)
+*   [#2641](https://github.com/pmd/pmd/pull/2641): \[java] AvoidThrowingNullPointerException marks all NullPointerException… - [Mykhailo Palahuta](https://github.com/Drofff)
 *   [#2643](https://github.com/pmd/pmd/pull/2643): \[java] AvoidCallingFinalize detects some false positives (2578) - [Mykhailo Palahuta](https://github.com/Drofff)
 
 {% endtocmaker %}

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/AvoidThrowingNullPointerExceptionRule.java

@@ -0,0 +1,103 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.rule.design;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator;
+import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType;
+import net.sourceforge.pmd.lang.java.ast.ASTName;
+import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableInitializer;
+import net.sourceforge.pmd.lang.java.ast.JavaNode;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+
+/**
+ * Finds <code>throw</code> statements containing <code>NullPointerException</code>
+ * instances as thrown values
+ *
+ * @author <a href="mailto:michaeller.2012@gmail.com">Mykhailo Palahuta</a>
+ */
+public class AvoidThrowingNullPointerExceptionRule extends AbstractJavaRule {
+
+    private final Set<String> npeInstances = new HashSet<>();
+
+    @Override
+    public Object visit(ASTVariableInitializer varInitializer, Object data) {
+        String initialedVarName = getInitializedVariableName(varInitializer);
+        processAssignmentToVariable(varInitializer, initialedVarName);
+        return super.visit(varInitializer, data);
+    }
+
+    private String getInitializedVariableName(ASTVariableInitializer initializer) {
+        ASTVariableDeclaratorId varDeclaratorId = initializer.getParent()
+                .getFirstDescendantOfType(ASTVariableDeclaratorId.class);
+        return varDeclaratorId != null ? varDeclaratorId.getName() : null;
+    }
+
+    @Override
+    public Object visit(ASTAssignmentOperator assignment, Object data) {
+        String assignedVarName = getAssignedVariableName(assignment);
+        processAssignmentToVariable(assignment, assignedVarName);
+        return super.visit(assignment, data);
+    }
+
+    private String getAssignedVariableName(ASTAssignmentOperator assignment) {
+        ASTName varName = assignment.getParent().getFirstDescendantOfType(ASTName.class);
+        return varName != null ? varName.getImage() : null;
+    }
+
+    private void processAssignmentToVariable(JavaNode assignment, String varName) {
+        Class<?> assignedValueType = getAssignedValueType(assignment);
+        if (isNullPointerException(assignedValueType)) {
+            npeInstances.add(varName);
+        } else {
+            npeInstances.remove(varName);
+        }
+    }
+
+    private Class<?> getAssignedValueType(JavaNode assignment) {
+        ASTClassOrInterfaceType assignedValueType = assignment.getParent()
+                .getFirstDescendantOfType(ASTClassOrInterfaceType.class);
+        return assignedValueType != null ? assignedValueType.getType() : null;
+    }
+
+    @Override
+    public Object visit(ASTThrowStatement throwStatement, Object data) {
+        if (throwsNullPointerException(throwStatement)) {
+            addViolation(data, throwStatement);
+        }
+        return super.visit(throwStatement, data);
+    }
+
+    private boolean throwsNullPointerException(ASTThrowStatement throwStatement) {
+        return throwsNullPointerExceptionType(throwStatement)
+                || throwsNullPointerExceptionVariable(throwStatement);
+    }
+
+    private boolean throwsNullPointerExceptionType(ASTThrowStatement throwStatement) {
+        ASTClassOrInterfaceType thrownType = throwStatement.getFirstDescendantOfType(ASTClassOrInterfaceType.class);
+        if (thrownType != null) {
+            Class<?> thrownException = thrownType.getType();
+            return isNullPointerException(thrownException);
+        }
+        return false;
+    }
+
+    private boolean isNullPointerException(Class<?> clazz) {
+        return NullPointerException.class.equals(clazz);
+    }
+
+    private boolean throwsNullPointerExceptionVariable(ASTThrowStatement throwStatement) {
+        ASTName thrownVar = throwStatement.getFirstDescendantOfType(ASTName.class);
+        if (thrownVar != null) {
+            String thrownVarName = thrownVar.getImage();
+            return npeInstances.contains(thrownVarName);
+        }
+        return false;
+    }
+}

pmd-java/src/main/resources/category/java/design.xml

@@ -200,7 +200,7 @@ public void bar() {
           language="java"
           since="1.8"
           message="Avoid throwing null pointer exceptions."
-          class="net.sourceforge.pmd.lang.rule.XPathRule"
+          class="net.sourceforge.pmd.lang.java.rule.design.AvoidThrowingNullPointerExceptionRule"
           externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#avoidthrowingnullpointerexception">
         <description>
 <![CDATA[
@@ -234,16 +234,6 @@ public class Foo {
 ]]>
         </description>
         <priority>1</priority>
-        <properties>
-            <property name="version" value="2.0"/>
-            <property name="xpath">
-                <value>
-<![CDATA[
-//AllocationExpression/ClassOrInterfaceType[@Image='NullPointerException']
-]]>
-                </value>
-            </property>
-        </properties>
         <example>
 <![CDATA[
 public class Foo {

pmd-java/src/main/resources/rulesets/java/quickstart.xml

@@ -122,7 +122,7 @@
     <!-- <rule ref="category/java/design.xml/AvoidDeeplyNestedIfStmts" /> -->
     <!-- <rule ref="category/java/design.xml/AvoidRethrowingException" /> -->
     <!-- <rule ref="category/java/design.xml/AvoidThrowingNewInstanceOfSameException" /> -->
-    <!-- <rule ref="category/java/design.xml/AvoidThrowingNullPointerException" /> -->
+    <!--<rule ref="category/java/design.xml/AvoidThrowingNullPointerException" />-->
     <!-- <rule ref="category/java/design.xml/AvoidThrowingRawExceptionTypes" /> -->
     <!-- <rule ref="category/java/design.xml/AvoidUncheckedExceptionsInSignatures" /> -->
     <rule ref="category/java/design.xml/ClassWithOnlyPrivateConstructorsShouldBeFinal"/>

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/AvoidThrowingNullPointerException.xml

@@ -12,6 +12,94 @@ public class Foo {
     void bar() {
         throw new NullPointerException();
     }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>no problems if NullPointerException is only instantiated but not thrown</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    void bar() {
+        Exception e = new NullPointerException("Test message");
+        String msg = e.getMessage();
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>problem should be detected even if NullPointerException is stored in some intermediate variable</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    void bar() {
+        Exception e = new NullPointerException();
+        throw e;
+    }
+}
+        ]]></code>
+    </test-code>
+    
+    <test-code>
+        <description>variables with same name false positive test</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    void foo() {
+        Exception e = new NullPointerException();
+        e.printStackTrace();
+    }
+
+    void bar() {
+        Exception e = new RuntimeException();
+        throw e;
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>ok, variable has been reassigned to RuntimeException before thrown</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    void bar() {
+        Exception e = new NullPointerException();
+        e = new RuntimeException();
+        throw e;
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>ok, variable is reassigned with NullPointerException after thrown</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    void bar(String s) {
+        Exception e = new RuntimeException();
+        if (s.equals("throw")) {
+            throw e;
+        }
+        e = new NullPointerException();
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>bad, variable had been reassigned with NullPointerException before thrown</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class Foo {
+    void bar() {
+        Exception e = new RuntimeException();
+        e = new NullPointerException();
+        throw e;
+    }
 }
         ]]></code>
     </test-code>