I recently started a project called fb-contrib quickfixes that focuses on creating quickfixes for FindBugs-detected bugs.
At first, it was a bit difficult, because, to my knowledge, there isn't a nice friendly "how to make a quickfix" tutorial, so I thought I'd make one.
In this tutorial, we will make the following quickfix.
This is a good start if you want to contribute quickfixes yourself to the project or if you are interested in a hands-on exercise with abstract syntax trees (ASTs, for short - see below).
It fixes a bug called LSC_LITERAL_STRING_COMPARISON which, as you can see, is easily fixed by swapping the argument (the
String literal) and the
String variable (which could be null).
Prerequisite: Abstract Syntax Trees
You will be writing software for Eclipse to perform the quickfixes, so I'll start discussing how Eclipse deals with Java source code - Abstract Syntax Trees (ASTs).
The Wikipedia article covers it well, but basically, ASTs are a semantic representation of the source code - Eclipse knows what each line is, whether it's an if statement, a variable initialization statement, etc.
Let's look at an example of some source code and its representative AST.
The outer-most ASTNode is a TypeDeclaration for the class
TypeDeclarations have some bookkeeping things like modifiers, a name, whether it's an interface or not, but we mostly care about the stuff inside of it, lines 3-7, which are the BodyDeclarations.
Remember, in a class, you can define fields, methods and other types, so that is what these are - an ordered list of all fields, methods and types defined in ForQuickFixes. This happens to just be a single MethodDeclaration that corresponds to our
MethodDeclarations can also have bookkeeping things, like a name, modifiers, a list of arguments (of type SingleVariableDeclaration), but if we are looking to fix Literal String Comparisons, we only really care about what goes on inside the method - the body.
The body is a Block, which is basically a wrapper for a bunch of Statements.
are basically executable lines of code, so we run into them pretty frequently.
In fact, our code snippet has
statements, an IfStatement and an ExpressionStatement corresponding to
Statements generally have other Statements nested inside of them, as well as Expressions, the other key building block that you will encounter a lot. For example, the IfStatement is composed of other Statements and Expressions like this:
The Expression we are interested in is
args.equals("version"), which happens to be a MethodInvocation and it is currently the rightOperand of an InfixExpression.
This MethodInvocation has a name, an expression (aka the object upon which the method is invoked), and a
List<Type> of arguments.
The one argument is a StringLiteral.
Discovering, reasoning about, and navigating this AST is much easier (for me, at least) when I use a tool called ASTView, which is an Eclipse plugin.
Prerequisite: Visitor Pattern
An important tool for navigating the AST is the Visitor pattern. Basically what we will do is create an object which will be given to the AST. The AST will ping the object with notifications as it navigates the passed-in object through the entire the tree.
So, if we are interested in MethodInvocations, we will make the visiting object have a
visit(MethodInvocation node) method that will be called for every MethodInvocation in the AST, giving us a chance to poke around with all of the nodes.
An easy QuickFix Recipe
- Preparation time
- Coding time
- Mildly spicy
- a basic understanding of ASTs and Eclipses BFHN
For this tutorial, I'm providing an already setup Eclipse workspace, downloadable here.
As of writing, the 3.0.0 version of the FindBugs plugin does not support extension, but a dev version does.
Here is a compiled dev version (with fb-contrib bundled).
Uninstall FindBugs from Eclipse (if already installed) and then unzip the bundle and install it from Eclipse.
See http://stackoverflow.com/a/16074606/1447621 for more details.
If you want to do this in hard mode, follow the dev setup instructions for fb-contrib quickfixes - it will amount to basically the same thing.
Read the two prerequisite sections above if you haven't already.
You may also wish to install ASTView from above.
Registering a quickfix for a BugPattern
First, we need to start by registering our bug pattern to be fixed.
We do this by updating the plugin.xml file, which is responsible for declaring how we are extending other Eclipse plugins (among other things).
Mouse over any of the lines with an asterisk for more information.
Now, we'll make the class we promised to make in plugin.xml -
BugResolution, the superclass has two methods that we are required to overwrite.
Moreover, we will
Now, we'll code the visitor.
The purpose of the visitor will be to find the incorrect
also identify the
String literal and the
String variable so they can be swapped.
Since we are looking for a
MethodInvocation to fix, we will override
visit(MethodInvocation) as seen below:
Correcting the code
Now that we've identified the problemmatic code, we'll need to make a fixed version and then replace it.
You'll notice I don't directly copy the values from the incorrect MethodInvocation to the new one.
The reason is that if a node belongs to one AST, it can't be reassigned to another AST.
would throw an
The code example shows two ways to deal with that - making a brand new node (lines 17-18) or using rewrite.createMoveTarget (lines 20+21). The third way I know of is with rewrite.createCopyTarget, but I don't know how this differs from the MoveTarget - they seem to do the same thing.
While I'd like to say this was all easily created, it was a bit of a struggle.
ASTView helped some, but really what was useful was setting breakpoints, stepping through the various visitors and exploring the nodes with the Eclipse Debugging tools.
Hopefully, this guide has made it easier to get a feel for the Eclipse quickfix platform than I had.
Along the way, I did run into some issues that others should not repeat:
- Don't use toString() for important logic:
As specified in the JavaDoc, the toString() methods are for debugging only! Don't trust them to have real, accurate code
(although they frequently do).
There's almost certainly a way to get the actual, correct string you need.
E.g. use SimpleName's getIdentifier() instead of the toString() to get the name of a method.
Foo f = new Foo() is a ClassInstanceCreation, not a ConstructorInvocation:
ConstructorInvocations, as it turns out, refer to an invocation of constructor from a similar constructor like
This was annoying and confusing for me when I was trying to deal with fixing constructors.
- Detail Formatters for easier debugging: Eclipse has these things called Detail Formatters, which basically allow you to write your own toString() methods for classes you can't actually change.
I've found it useful to notate what methods return so when I'm debugging, to simplify coding:
The following gif shows an example detail formatter I wrote for ITypeBindings:
The great thing about Detail Formatters is that you write them once and they apply always, for all debugging sessions.
Stay tuned for the next iteration of this tutorial where we look to have a slightly more complicated example.
Here's a sneak preview: