diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningCatchVariablesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningCatchVariablesRule.java new file mode 100644 index 0000000000..e709c05e9a --- /dev/null +++ b/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(); + } +} diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 6bc9247e8e..0d9663b654 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -209,6 +209,36 @@ class Foo { + + +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 + + 2 + + + + + + + simple catch variable reassignment + 1 + 8 + + + + + new exception variable allocation is OK + 0 + + +