Monday, March 12, 2007

Taking care of abused Exceptions using JDT AST

How many times you went nuts while dare debugging a large application? and while debugging, faced desperate failure in getting any clue on what went wrong, no exception, no log?

Well, If you work on right smartly large bad code base, this experience is quite common. One of the main reasons, as I think, for this situation is swallowed and mishandled exceptions.

I ran in similar problems once, where it was becoming very hard to find the problems. I came to know mishandled exceptions in many places. And I decided to write a plug-in to manipulate AST and replace it with logging statements.

Here is the code for interesting parts of the plug-in. Created a catch clause visitor, which will visit all the catch clauses in AST. The AST will be parsed for each Compilation Unit in a loop.

private final class CatchClauseVisitor extends ASTVisitor {
@Override
public boolean visit(CatchClause node) {
allCatchClauses.add(node);
return super.visit(node);
}
}

This visitor can be used to collect clauses from AST:

ASTParser parser = ASTParser.newParser(AST.JLS3);
parser.setKind(ASTParser.K_COMPILATION_UNIT);
parser.setSource(lwUnit); // set source
parser.setResolveBindings(true); // we need bindings later on
CompilationUnit unit =(CompilationUnit) parser.createAST(null /* IProgressMonitor */);
unit.accept(
new CatchClauseVisitor());

Now that we have all catch clauses, we can iterate on them to execute transformations and rewrite the AST.

try {
for (CatchClause node : allCatchClauses) {
if (list.size() == 0) // SWALLOWED EXCEPTION
putLoggingStatement(node, null, rewrite, null);
else // code to replace SOP statements with logger call
for (Object object : list)
// ....
// computation of the new source code
TextEdit edits = rewrite.rewriteAST(doc,
lwUnit.getJavaProject().getOptions(
true));
edits.apply(doc);
String newSource = doc.get();
// update of the compilation unit
lwUnit.getBuffer().setContents(newSource);
lwUnit.save(null, false);

} catch (...){...}

And here is the putLoggingStatement method..

private void putLoggingStatement(CatchClause node, Object object,
ASTRewrite rewrite, ASTNode expStatement) {
MethodInvocation invocation = node.getAST().newMethodInvocation();
setLoggingMethodName(node, invocation);
SimpleName args = node.getAST().newSimpleName(
node.getException().getName().getIdentifier());
invocation.arguments().add(args);
if (expStatement == null) {
ExpressionStatement statement = node.getAST()
.newExpressionStatement(invocation);
ListRewrite listRewrite = rewrite.getListRewrite(node.getBody(),
Block.STATEMENTS_PROPERTY);
listRewrite.insertFirst(statement, null);
rewrite.replace(node, node, null); // replace the nodes
return;
}
ExpressionStatement statement = node.getAST().newExpressionStatement(
invocation);
setContextInstaceVariable(invocation);
rewrite.replace(expStatement, statement, null);
}
// ....

A number of details are removed for the sake of brevity and I don't claim the code to be professionally written. There are some improvements I can thing of, like, the SOP can be replaced with logging statement, a provision for rethrowing exception statement based on some config (no, no configs!)....

Finally let me tell you that writing the code involving AST can become complex and error prone, but it does a real good job. This small job of carefully writing a custom code manipulation saved me from doing it manually for hundreds of java source files as well as code maintainability got much better, what a relief!

4 comments:

KetanPadegaonkar said...

That's cool. Perhaps you could make a plugin with a view that could show these things.

Even better would be creating a category in the 'problems view' to highlight such problems, instead of just fixing such errors. Not all exceptions need to be logged!

Nirav said...

That's a good idea, but there can be sheer number of exception clauses and it will be frustrating to look at each of them (like thousands of warnings which people just ignore).

Selective logging is difficult to achieve without any config (or convention).. Any creative idea on how to handle it in better manner? like a multiple quick fix wizard as such.. hmm, will need to think about it.

Robert Konigsberg said...

This is a very good idea - I have two comments:

1. As a matter of practice, replace those found catch clauses with actual Java source. If you have a zillion of them, at least generate code for those where you have found them useful -- save the next guy some trouble.

How much more work would be involved to turn your solution into a builder and associated quick fix?

2. I think there is a much simpler solution using Spoon JDT, if you don't want to go crazy learning how to manipulate the JDT AST. I can't vouch for Spoon JDT, but from what I've seen, it's a much simpler framework.

Nirav said...

for your first comment:
I'm not sure what you mean by replacing catch clauses with java source.

Making a buider and applying quickfix will be quick and is nice idea. How about integrating it as a rule to PMD? :)

second comment:
Ack. will take a look to Spoon, never heard about it. AST isn't that bad though...