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 ForQuickFixes.
	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 main method.
	


	

	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.
	


	

	Statements
	are basically executable lines of code, so we run into them pretty frequently.
	In fact, our code snippet has 
	two
	statements, an IfStatement and an ExpressionStatement corresponding to System.out.println("Test 1.0");.
	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[0].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

            
5m

            
Coding time

            
20m

            
Difficulty

            
Mildly spicy

            
Serves

            
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).
	
	

        

        

            This tells Eclipse we are extending the quickfix plugin.
            We only need to specify this once, and nest 1 or more quickfix declarations inside. 
            This is the class that will contain our code to resolve the bug
            Put the message here you want to display that explains what fix will be applied.
            Currently, it is a static message, but there are ways to have it be a dynamic message, covered in a later tutorial.
            By default, this will also be the description.
			What's the difference between the label and description?
            
            Finally, this is the bug pattern we want to contribute to.
            Each bug pattern can have as many resolutions as you want.
        

    


    Mouse over any of the lines with an asterisk for more information. 

	


	
The visitor


	

	Now, we'll make the class we promised to make in plugin.xml - LiteralStringComparisonResolution.
	BugResolution, the superclass has two methods that we are required to overwrite.
	Moreover, we will 

	

        

        

            
            The ASTUtil class has a few great methods to avoid lots of boilerplate.
            
            
            This method specifies whether we want access to the types (specifically, the ITypeBindings) of nodes.
            We do not need this for our visitor implementation (although the types could easily be included in the visitor logic). 
            In most cases, this should be true, but, if you find yourself not needing to verify types, return false, to improve performance.
            
            
            This method will be called when the user has opted to fix the bug.  
            So, given these three variables, we need to find the buggy code and create a fix.
            
            
            These three lines will appear in most every quickfix.
            What do they do?
            They find the node that contains the bug (line #12) and then traverse that node with the custom visitor (#13-14).
            
            
            This is our custom node visitor.
            ASTVisitor has a bunch of visit methods, but they are all basically empty by default.
            We'll override the ones we want next.
            
        

    


	


	

	Now, we'll code the visitor.  
	The purpose of the visitor will be to find the incorrect MethodInvocation and
	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:
	
	

        

        

            
            Remember, this is a class inside LiteralStringComparisonResolution.
            
            
            We make a collection and initialize it via the static initializer with the three method names we are looking for.
            
            
            These will be the three things we are looking for: the incorrect MethodInvocation,the String literal and the String variable.
            
            
            The collections that ASTNodes return (such as arguments()) are non-parameterized.  
            When we cast them (e.g. to a List<Expression>), this makes a compiler warning.
            This annotation silences said warning.
            
            
            The return value indicates whether or not you want to traverse the subtree (true is the default value for all visit() calls).  We return false when we have found the MethodInvocation we are looking for, true otherwise.
            
            
            To see if the MethodInvocation we are looking at is the one we want to fix, we check to see if the name matches something in our list and the first (and only) argument has a literal value.
            It is technically possible for this logic to find (and then fix) the wrong method.
            For the purposes of this tutorial, I've decided to omit it, but if you want to see the type-checking version, see this gist.
            
        

    

	


	
Correcting the code

	

	Now that we've identified the problemmatic code, we'll need to make a fixed version and then replace it. 
	

        

        

            
            Intuitiviely, this replaces one expression with another, namely our fixed version with our unfixed version.
            
            
            This creates a fresh MethodInvocation attached to the AST we are fixing.
            
            
            We set the name of the MethodInvocation to be to a copy of what the old one was.
            
            
            The expression of the MethodInvocation (i.e. what the method is being called on) will be the StringLiteral, 
            which we can simply reuse from the broken invocation.
            
            
            Again, we can move the old expression (our String variable) to the arguments of our fixed invocation.
            
        

    


    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.  
    Something like setExpression(visitor.stringLiteralExpression) would throw an IllegalArgumentException.
    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.
	


	
Reflecting Thoughts

    

    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:
 	
    
Advice

	
    
		
  • 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(someVar,otherVar).
		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.  
		
    • 
	

	


	
What's next?



	

	Stay tuned for the next iteration of this tutorial where we look to have a slightly more complicated example.  

	Here's a sneak preview: