Added HandleCleanup support for javac, and fixed a bug in AST.java which caused nested @Cleanup annotations to simply be ignored (rebuild was broken).

HandleCleanup seems to work swimmingly now on both targets. yay!

3 files changed, 101 insertions, 12 deletions

diff --git a/src/lombok/core/AST.java b/src/lombok/core/AST.java
index f4d4d628..febe53ab 100644
--- a/src/lombok/core/AST.java
+++ b/src/lombok/core/AST.java
@@ -75,20 +75,18 @@ public abstract class AST<N> {
 	@SuppressWarnings("unchecked")
 	private Node replaceNewWithExistingOld(Map<N, Node> oldNodes, Node newNode) {
 		Node oldNode = oldNodes.get(newNode.get());
-		if ( oldNode == null ) return newNode;
+		Node targetNode = oldNode == null ? newNode : oldNode;
 		
-		List<Object> oldChildren = new ArrayList<Object>();
+		List children = new ArrayList();
 		for ( Node child : newNode.children ) {
 			Node oldChild = replaceNewWithExistingOld(oldNodes, child);
-			if ( oldChild == null ) oldChildren.add(child);
-			else {
-				oldChildren.add(oldChild);
-				oldChild.parent = oldNode;
-			}
+			children.add(oldChild);
+			oldChild.parent = targetNode;
 		}
 		
-		oldNode.children.addAll((Collection) oldChildren);
-		return oldNode;
+		targetNode.children.clear();
+		((List)targetNode.children).addAll(children);
+		return targetNode;
 	}
 	
 	public abstract class Node {
@@ -210,7 +208,7 @@ public abstract class AST<N> {
 		 * Careful - the node you call this on must not itself have been removed or rehomed - it rebuilds <i>all children</i>.
 		 */
 		public void rebuild() {
-			Map<N, Node> oldNodes = new HashMap<N, Node>();
+			Map<N, Node> oldNodes = new IdentityHashMap<N, Node>();
 			gatherAndRemoveChildren(oldNodes);
 			
 			Node newNode = buildTree(get(), kind);
@@ -222,6 +220,7 @@ public abstract class AST<N> {
 			for ( Node child : children ) child.gatherAndRemoveChildren(map);
 			map.put(get(), this);
 			identityDetector.remove(get());
+			children.clear();
 			nodeMap.remove(get());
 		}
 		
diff --git a/src/lombok/eclipse/handlers/HandleCleanup.java b/src/lombok/eclipse/handlers/HandleCleanup.java
index 867bd0e5..a28c6c6a 100644
--- a/src/lombok/eclipse/handlers/HandleCleanup.java
+++ b/src/lombok/eclipse/handlers/HandleCleanup.java
@@ -36,7 +36,7 @@ public class HandleCleanup implements EclipseAnnotationHandler<Cleanup> {
 		LocalDeclaration decl = (LocalDeclaration)annotationNode.up().get();
 		
 		Node ancestor = annotationNode.up().directUp();
-		ASTNode blockNode = annotationNode.up().directUp().get();
+		ASTNode blockNode = ancestor.get();
 		
 		final boolean isSwitch;
 		final Statement[] statements;
@@ -69,7 +69,7 @@ public class HandleCleanup implements EclipseAnnotationHandler<Cleanup> {
 			return true;
 		}
 		
-		start++;
+		start++;  //We start with try{} *AFTER* the var declaration.
 		
 		int end;
 		if ( isSwitch ) {
diff --git a/src/lombok/javac/handlers/HandleCleanup.java b/src/lombok/javac/handlers/HandleCleanup.java
new file mode 100644
index 00000000..3035a1b0
--- /dev/null
+++ b/src/lombok/javac/handlers/HandleCleanup.java
@@ -0,0 +1,90 @@
+package lombok.javac.handlers;
+
+import lombok.Cleanup;
+import lombok.core.AnnotationValues;
+import lombok.core.AST.Kind;
+import lombok.javac.JavacAnnotationHandler;
+import lombok.javac.JavacAST.Node;
+
+import org.mangosdk.spi.ProviderFor;
+
+import com.sun.tools.javac.tree.JCTree;
+import com.sun.tools.javac.tree.TreeMaker;
+import com.sun.tools.javac.tree.JCTree.JCAnnotation;
+import com.sun.tools.javac.tree.JCTree.JCBlock;
+import com.sun.tools.javac.tree.JCTree.JCCase;
+import com.sun.tools.javac.tree.JCTree.JCCatch;
+import com.sun.tools.javac.tree.JCTree.JCExpression;
+import com.sun.tools.javac.tree.JCTree.JCFieldAccess;
+import com.sun.tools.javac.tree.JCTree.JCMethodDecl;
+import com.sun.tools.javac.tree.JCTree.JCStatement;
+import com.sun.tools.javac.tree.JCTree.JCVariableDecl;
+import com.sun.tools.javac.util.List;
+
+@ProviderFor(JavacAnnotationHandler.class)
+public class HandleCleanup implements JavacAnnotationHandler<Cleanup> {
+	@Override public boolean handle(AnnotationValues<Cleanup> annotation, JCAnnotation ast, Node annotationNode) {
+		String cleanupName = annotation.getInstance().cleanupMethod();
+		if ( cleanupName.length() == 0 ) {
+			annotationNode.addError("cleanupName cannot be the empty string.");
+			return true;
+		}
+		
+		if ( annotationNode.up().getKind() != Kind.LOCAL ) {
+			annotationNode.addError("@Cleanup is legal only on local variable declarations.");
+			return true;
+		}
+		
+		JCVariableDecl decl = (JCVariableDecl)annotationNode.up().get();
+		
+		Node ancestor = annotationNode.up().directUp();
+		JCTree blockNode = ancestor.get();
+		
+		final List<JCStatement> statements;
+		if ( blockNode instanceof JCBlock ) {
+			statements = ((JCBlock)blockNode).stats;
+		} else if ( blockNode instanceof JCCase ) {
+			statements = ((JCCase)blockNode).stats;
+		} else if ( blockNode instanceof JCMethodDecl ) {
+			statements = ((JCMethodDecl)blockNode).body.stats;
+		} else {
+			annotationNode.addError("@Cleanup is legal only on a local variable declaration inside a block.");
+			return true;
+		}
+		
+		boolean seenDeclaration = false;
+		List<JCStatement> tryBlock = List.nil();
+		List<JCStatement> newStatements = List.nil();
+		for ( JCStatement statement : statements ) {
+			if ( !seenDeclaration ) {
+				if ( statement == decl ) seenDeclaration = true;
+				newStatements = newStatements.append(statement);
+			} else tryBlock = tryBlock.append(statement);
+		}
+		
+		if ( !seenDeclaration ) {
+			annotationNode.addError("LOMBOK BUG: Can't find this local variable declaration inside its parent.");
+			return true;
+		}
+		
+		TreeMaker maker = annotationNode.getTreeMaker();
+		JCFieldAccess cleanupCall = maker.Select(maker.Ident(decl.name), annotationNode.toName(cleanupName));
+		List<JCStatement> finalizerBlock = List.<JCStatement>of(maker.Exec(
+				maker.Apply(List.<JCExpression>nil(), cleanupCall, List.<JCExpression>nil())));
+		
+		JCBlock finalizer = maker.Block(0, finalizerBlock);
+		newStatements = newStatements.append(maker.Try(maker.Block(0, tryBlock), List.<JCCatch>nil(), finalizer));
+		
+		if ( blockNode instanceof JCBlock ) {
+			((JCBlock)blockNode).stats = newStatements;
+		} else if ( blockNode instanceof JCCase ) {
+			((JCCase)blockNode).stats = newStatements;
+		} else if ( blockNode instanceof JCMethodDecl ) {
+			((JCMethodDecl)blockNode).body.stats = newStatements;
+		} else throw new AssertionError("Should not get here");
+		
+		ancestor.rebuild();
+		
+		return true;
+	}
+}