Categories
Coding

Custom Checkstyle Rule Example

Here is an example of a custom Checkstyle rule that catches the following situation: sometimes an Exception can be caught and an error message logged, but the underlying exception (and thus the stack trace) may not be logged at all. This is not a problem generally, unless the exception tends wrap an underlying cause (i.e. one or more nested exceptions). The rule is designed to catch instances like:

[sourcecode language=”java”]
try {}
catch (Exception e) {
logger.error("blah");
}
[/sourcecode]

With the assumption that the following is better:

[sourcecode language=”java”]
try {}
catch (Exception e) {
logger.error("blah", e);
}
[/sourcecode]

Here is the code for the custom check:

[sourcecode language=”java”]
package au.com.national.efx.build;
import java.util.ArrayList;
import java.util.List;

import com.puppycrawl.tools.checkstyle.api.Check;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;

/**
* Check that attempts to catch instances of the following:
* <code>
* catch (Exception e) { logger.error("foo"); }
* </code>
*
* with the assumption that the following is preferable:
*
* <code>
* catch (Exception e) { logger.error("foo", e); }
* </code>
* @author rwinston
*
*/
public class SwallowedExceptionInLoggerCheck extends Check {

@Override
public int[] getDefaultTokens() {
return new int[] { TokenTypes.LITERAL_CATCH };
}

/**
* Get ident of exception
* Try to find it in logger error/warn parameter list
*/
@Override
public void visitToken(DetailAST aAST) {
super.visitToken(aAST);

final DetailAST parameterDef = aAST.findFirstToken(TokenTypes.PARAMETER_DEF);
final DetailAST ident = parameterDef.findFirstToken(TokenTypes.IDENT);
final String exceptionIdent = ident.getText();

final DetailAST slist = aAST.findFirstToken(TokenTypes.SLIST); // Find ‘{‘
// Find all method calls within catch block
final List<DetailAST> variables = findChildASTsOfType(slist, TokenTypes.METHOD_CALL);

try {
for (DetailAST methodCall : variables) {
DetailAST dot = methodCall.findFirstToken(TokenTypes.DOT);

// I’m assuming the last child will be the method name called
DetailAST lastIdent = dot.getLastChild();

if (lastIdent.getText().equals("error")) {
// Ok, now check that the ELIST contains an IDENT matching
// the exception name
DetailAST elist = methodCall.findFirstToken(TokenTypes.ELIST);
boolean exceptionInParameterList = false;
for (DetailAST identAST : findChildASTsOfType(elist, TokenTypes.IDENT)) {
if (identAST.getText().equals(exceptionIdent))
exceptionInParameterList = true;
}

if (!exceptionInParameterList) {
log(methodCall, "error() method does not contain caught Exception as a parameter");
}
}
}
} catch (Exception e) { e.printStackTrace(); }

}

/**
* Recursively traverse an expression tree and return all
* ASTs matching a specific token type
* @param parent
* @param type
* @return
*/
private List<DetailAST> findChildASTsOfType(DetailAST parent, int type) {
List<DetailAST> children = new ArrayList<DetailAST>();

DetailAST child = parent.getFirstChild();
while (child != null) {
if (child.getType() == type)
children.add(child);
else {
children.addAll(findChildASTsOfType(child, type));
}
child = child.getNextSibling();
}
return children;
}

}

[/sourcecode]