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:

try {}
catch (Exception e) {
  logger.error("blah");
}

With the assumption that the following is better:

try {}
catch (Exception e) {
  logger.error("blah", e);
}

Here is the code for the custom check:

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;
	}
}

Comments are closed.