[java] AvoidReassigningCatchVariables

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

@@ -0,0 +1,51 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.rule.bestpractices;
+
+import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator;
+import net.sourceforge.pmd.lang.java.ast.ASTCatchStatement;
+import net.sourceforge.pmd.lang.java.ast.ASTName;
+import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
+import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
+import net.sourceforge.pmd.lang.java.ast.JavaNode;
+import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
+import net.sourceforge.pmd.lang.symboltable.NameOccurrence;
+
+public class AvoidReassigningCatchVariablesRule extends AbstractJavaRule {
+
+    public AvoidReassigningCatchVariablesRule() {
+        addRuleChainVisit(ASTCatchStatement.class);
+    }
+
+    @Override
+    public Object visit(ASTCatchStatement catchStatement, Object data) {
+        ASTVariableDeclaratorId caughtExceptionId = catchStatement.getExceptionId();
+        String caughtExceptionVar = caughtExceptionId.getName();
+        for (NameOccurrence usage : caughtExceptionId.getUsages()) {
+            JavaNode operation = getOperationOfUsage(usage);
+            if (isAssignment(operation)) {
+                String assignedVar = getAssignedVariableName(operation);
+                if (caughtExceptionVar.equals(assignedVar)) {
+                    addViolation(data, operation, caughtExceptionVar);
+                }
+            }
+        }
+        return data;
+    }
+
+    private JavaNode getOperationOfUsage(NameOccurrence usage) {
+        return usage.getLocation()
+                .getFirstParentOfType(ASTPrimaryExpression.class)
+                .getParent();
+    }
+
+    private boolean isAssignment(JavaNode operation) {
+        return operation.hasDescendantOfType(ASTAssignmentOperator.class);
+    }
+
+    private String getAssignedVariableName(JavaNode operation) {
+        return operation.getFirstDescendantOfType(ASTName.class).getImage();
+    }
+}

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

@@ -209,6 +209,36 @@ class Foo {
         </example>
     </rule>
 
+    <rule name="AvoidReassigningCatchVariables"
+          since="6.27.0"
+          message="Avoid reassigning caught exception ''{0}''"
+          class="net.sourceforge.pmd.lang.java.rule.bestpractices.AvoidReassigningCatchVariablesRule"
+          externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#avoidreassigningcatchvariables">
+        <description>
+Reassigning exception variables caught in a catch statement should be avoided because of:
+1) If it is needed, multi catch can be easily added and code will still compile
+2) Following the principle of least surprise we want to make sure that a variable caught in a catch statement is always the one thrown in a try block
+        </description>
+        <priority>2</priority>
+        <example><![CDATA[
+public class Foo {
+    public void foo() {
+        try {
+            // do something
+        } catch (Exception e) {
+            e = new NullPointerException(); // not recommended
+        }
+
+        try {
+            // do something
+        } catch (MyException | ServerException e) {
+            e = new RuntimeException(); // won't compile
+        }
+    }
+}
+        ]]></example>
+    </rule>
+
     <rule name="AvoidReassigningLoopVariables"
           since="6.11.0"
           message="Avoid reassigning the loop control variable ''{0}''"

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningCatchVariablesTest.java

@@ -0,0 +1,11 @@
+/**
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.lang.java.rule.bestpractices;
+
+import net.sourceforge.pmd.testframework.PmdRuleTst;
+
+public class AvoidReassigningCatchVariablesTest extends PmdRuleTst {
+    // no additional unit tests
+}

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

@@ -0,0 +1,44 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<test-data
+        xmlns="http://pmd.sourceforge.net/rule-tests"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
+
+    <test-code>
+        <description>simple catch variable reassignment</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>8</expected-linenumbers>
+        <code><![CDATA[
+import java.io.File;
+import java.io.IOException;
+public class Foo {
+    public void foo() {
+        try {
+            new File("/text.txt");
+        } catch (IOException e) {
+            e = new NullPointerException();
+        }
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>new exception variable allocation is OK</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+import java.io.File;
+import java.io.IOException;
+public class Foo {
+    public void foo() {
+        try {
+            new File("/text.txt");
+        } catch (IOException e) {
+            Exception t;
+            t = new NullPointerException();
+        }
+    }
+}
+        ]]></code>
+    </test-code>
+</test-data>