Added an error if @Delegate is used recursively.

src/core/lombok/javac/handlers/HandleDelegate.java

@@ -57,6 +57,7 @@ import lombok.javac.JavacResolution.TypeNotConvertibleException;
 
 import org.mangosdk.spi.ProviderFor;
 
+import com.sun.tools.javac.code.Attribute.Compound;
 import com.sun.tools.javac.code.Flags;
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.code.Symbol.TypeSymbol;
@@ -98,6 +99,8 @@ public class HandleDelegate extends JavacAnnotationHandler<Delegate> {
 			"finalize()"));
 	
 	private static final String LEGALITY_OF_DELEGATE = "@Delegate is legal only on instance fields or no-argument instance methods.";
+	private static final String RECURSION_NOT_ALLOWED = "@Delegate does not support recursion (delegating to a type that itself has @Delegate members). Member \"%s\" is @Delegate in type \"%s\"";
+
 	
 	@Override public void handle(AnnotationValues<Delegate> annotation, JCAnnotation ast, JavacNode annotationNode) {
 		handleFlagUsage(annotationNode, ConfigurationKeys.DELEGATE_FLAG_USAGE, "@Delegate");
@@ -177,31 +180,36 @@ public class HandleDelegate extends JavacAnnotationHandler<Delegate> {
 			}
 		}
 		 */
-		for (Type t : toExclude) {
-			if (t instanceof ClassType) {
-				ClassType ct = (ClassType) t;
-				addMethodBindings(signaturesToExclude, ct, annotationNode.getTypesUtil(), banList);
-			} else {
-				annotationNode.addError("@Delegate can only use concrete class types, not wildcards, arrays, type variables, or primitives.");
-				return;
+		
+		try {
+			for (Type t : toExclude) {
+				if (t instanceof ClassType) {
+					ClassType ct = (ClassType) t;
+					addMethodBindings(signaturesToExclude, ct, annotationNode.getTypesUtil(), banList);
+				} else {
+					annotationNode.addError("@Delegate can only use concrete class types, not wildcards, arrays, type variables, or primitives.");
+					return;
+				}
 			}
-		}
-		
-		for (MethodSig sig : signaturesToExclude) {
-			banList.add(printSig(sig.type, sig.name, annotationNode.getTypesUtil()));
-		}
-		
-		for (Type t : toDelegate) {
-			if (t instanceof ClassType) {
-				ClassType ct = (ClassType) t;
-				addMethodBindings(signaturesToDelegate, ct, annotationNode.getTypesUtil(), banList);
-			} else {
-				annotationNode.addError("@Delegate can only use concrete class types, not wildcards, arrays, type variables, or primitives.");
-				return;
+			
+			for (MethodSig sig : signaturesToExclude) {
+				banList.add(printSig(sig.type, sig.name, annotationNode.getTypesUtil()));
 			}
+			
+			for (Type t : toDelegate) {
+				if (t instanceof ClassType) {
+					ClassType ct = (ClassType) t;
+					addMethodBindings(signaturesToDelegate, ct, annotationNode.getTypesUtil(), banList);
+				} else {
+					annotationNode.addError("@Delegate can only use concrete class types, not wildcards, arrays, type variables, or primitives.");
+					return;
+				}
+			}
+			
+			for (MethodSig sig : signaturesToDelegate) generateAndAdd(sig, annotationNode, delegateName, delegateReceiver);
+		} catch (DelegateRecursion e) {
+			annotationNode.addError(String.format(RECURSION_NOT_ALLOWED, e.member, e.type));
 		}
-		
-		for (MethodSig sig : signaturesToDelegate) generateAndAdd(sig, annotationNode, delegateName, delegateReceiver);
 	}
 	
 	public void generateAndAdd(MethodSig sig, JavacNode annotation, Name delegateName, DelegateReceiver delegateReceiver) {
@@ -338,11 +346,30 @@ public class HandleDelegate extends JavacAnnotationHandler<Delegate> {
 		return collection == null ? com.sun.tools.javac.util.List.<T>nil() : collection.toList();
 	}
 	
-	public void addMethodBindings(List<MethodSig> signatures, ClassType ct, JavacTypes types, Set<String> banList) {
+	private static class DelegateRecursion extends Throwable {
+		final String type, member;
+		
+		public DelegateRecursion(String type, String member) {
+			this.type = type;
+			this.member = member;
+		}
+	}
+	
+	public void addMethodBindings(List<MethodSig> signatures, ClassType ct, JavacTypes types, Set<String> banList) throws DelegateRecursion {
 		TypeSymbol tsym = ct.asElement();
 		if (tsym == null) return;
 		
 		for (Symbol member : tsym.getEnclosedElements()) {
+			for (Compound am : member.getAnnotationMirrors()) {
+				String name = null;
+				try {
+					name = am.type.tsym.flatName().toString();
+				} catch (Exception ignore) {}
+				
+				if ("lombok.Delegate".equals(name) || "lombok.experimental.Delegate".equals(name)) {
+					throw new DelegateRecursion(ct.tsym.name.toString(), member.name.toString());
+				}
+			}
 			if (member.getKind() != ElementKind.METHOD) continue;
 			if (member.isStatic()) continue;
 			if (member.isConstructor()) continue;

src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java

@@ -62,10 +62,12 @@ import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.TypeParameter;
 import org.eclipse.jdt.internal.compiler.ast.TypeReference;
 import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
+import org.eclipse.jdt.internal.compiler.lookup.AnnotationBinding;
 import org.eclipse.jdt.internal.compiler.lookup.ArrayBinding;
 import org.eclipse.jdt.internal.compiler.lookup.BaseTypeBinding;
 import org.eclipse.jdt.internal.compiler.lookup.Binding;
 import org.eclipse.jdt.internal.compiler.lookup.ClassScope;
+import org.eclipse.jdt.internal.compiler.lookup.FieldBinding;
 import org.eclipse.jdt.internal.compiler.lookup.MethodBinding;
 import org.eclipse.jdt.internal.compiler.lookup.ParameterizedTypeBinding;
 import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding;
@@ -207,21 +209,27 @@ public class PatchDelegate {
 				List<ClassLiteralAccess> excludedRawTypes = rawTypes(ann, "excludes");
 				
 				List<BindingTuple> methodsToExclude = new ArrayList<BindingTuple>();
-				for (ClassLiteralAccess cla : excludedRawTypes) {
-					addAllMethodBindings(methodsToExclude, cla.type.resolveType(decl.initializerScope), new HashSet<String>(), field.name, ann);
-				}
-				
-				Set<String> banList = new HashSet<String>();
-				for (BindingTuple excluded : methodsToExclude) banList.add(printSig(excluded.parameterized));
-				
 				List<BindingTuple> methodsToDelegateForThisAnn = new ArrayList<BindingTuple>();
 				
-				if (rawTypes.isEmpty()) {
-					addAllMethodBindings(methodsToDelegateForThisAnn, field.type.resolveType(decl.initializerScope), banList, field.name, ann);
-				} else {
-					for (ClassLiteralAccess cla : rawTypes) {
-						addAllMethodBindings(methodsToDelegateForThisAnn, cla.type.resolveType(decl.initializerScope), banList, field.name, ann);
+				try {
+					for (ClassLiteralAccess cla : excludedRawTypes) {
+						addAllMethodBindings(methodsToExclude, cla.type.resolveType(decl.initializerScope), new HashSet<String>(), field.name, ann);
 					}
+					
+					Set<String> banList = new HashSet<String>();
+					for (BindingTuple excluded : methodsToExclude) banList.add(printSig(excluded.parameterized));
+					
+					if (rawTypes.isEmpty()) {
+						addAllMethodBindings(methodsToDelegateForThisAnn, field.type.resolveType(decl.initializerScope), banList, field.name, ann);
+					} else {
+						for (ClassLiteralAccess cla : rawTypes) {
+							addAllMethodBindings(methodsToDelegateForThisAnn, cla.type.resolveType(decl.initializerScope), banList, field.name, ann);
+						}
+					}
+				} catch (DelegateRecursion e) {
+					EclipseAST eclipseAst = TransformEclipseAST.getAST(cud, true);
+					eclipseAst.get(ann).addError(String.format(RECURSION_NOT_ALLOWED, new String(e.member), new String(e.type)));
+					break;
 				}
 				
 				// Not doing this right now because of problems - see commented-out-method for info.
@@ -239,6 +247,7 @@ public class PatchDelegate {
 	}
 	
 	private static final String LEGALITY_OF_DELEGATE = "@Delegate is legal only on instance fields or no-argument instance methods.";
+	private static final String RECURSION_NOT_ALLOWED = "@Delegate does not support recursion (delegating to a type that itself has @Delegate members). Member \"%s\" is @Delegate in type \"%s\"";
 	
 	private static void fillMethodBindingsForMethods(CompilationUnitDeclaration cud, ClassScope scope, List<BindingTuple> methodsToDelegate) {
 		TypeDeclaration decl = scope.referenceContext;
@@ -270,22 +279,28 @@ public class PatchDelegate {
 				List<ClassLiteralAccess> excludedRawTypes = rawTypes(ann, "excludes");
 				
 				List<BindingTuple> methodsToExclude = new ArrayList<BindingTuple>();
-				for (ClassLiteralAccess cla : excludedRawTypes) {
-					addAllMethodBindings(methodsToExclude, cla.type.resolveType(decl.initializerScope), new HashSet<String>(), method.selector, ann);
-				}
-				
-				Set<String> banList = new HashSet<String>();
-				for (BindingTuple excluded : methodsToExclude) banList.add(printSig(excluded.parameterized));
-				
 				List<BindingTuple> methodsToDelegateForThisAnn = new ArrayList<BindingTuple>();
 				
-				if (rawTypes.isEmpty()) {
-					if (method.returnType == null) continue;
-					addAllMethodBindings(methodsToDelegateForThisAnn, method.returnType.resolveType(decl.initializerScope), banList, method.selector, ann);
-				} else {
-					for (ClassLiteralAccess cla : rawTypes) {
-						addAllMethodBindings(methodsToDelegateForThisAnn, cla.type.resolveType(decl.initializerScope), banList, method.selector, ann);
+				try {
+					for (ClassLiteralAccess cla : excludedRawTypes) {
+						addAllMethodBindings(methodsToExclude, cla.type.resolveType(decl.initializerScope), new HashSet<String>(), method.selector, ann);
 					}
+					
+					Set<String> banList = new HashSet<String>();
+					for (BindingTuple excluded : methodsToExclude) banList.add(printSig(excluded.parameterized));
+					
+					if (rawTypes.isEmpty()) {
+						if (method.returnType == null) continue;
+						addAllMethodBindings(methodsToDelegateForThisAnn, method.returnType.resolveType(decl.initializerScope), banList, method.selector, ann);
+					} else {
+						for (ClassLiteralAccess cla : rawTypes) {
+							addAllMethodBindings(methodsToDelegateForThisAnn, cla.type.resolveType(decl.initializerScope), banList, method.selector, ann);
+						}
+					}
+				} catch (DelegateRecursion e) {
+					EclipseAST eclipseAst = TransformEclipseAST.getAST(cud, true);
+					eclipseAst.get(ann).addError(String.format(RECURSION_NOT_ALLOWED, new String(e.member), new String(e.type)));
+					break;
 				}
 				
 				// Not doing this right now because of problems - see commented-out-method for info.
@@ -306,7 +321,7 @@ public class PatchDelegate {
 		if (ann.type == null) return false;
 		TypeBinding tb = ann.type.resolveType(decl.initializerScope);
 		if (tb == null) return false;
-		if (!charArrayEquals("lombok", tb.qualifiedPackageName())) return false;
+		if (!charArrayEquals("lombok", tb.qualifiedPackageName()) && !charArrayEquals("lombok.experimental", tb.qualifiedPackageName())) return false;
 		if (!charArrayEquals("Delegate", tb.qualifiedSourceName())) return false;
 		return true;
 	}
@@ -671,12 +686,22 @@ public class PatchDelegate {
 		}
 	}
 	
-	private static void addAllMethodBindings(List<BindingTuple> list, TypeBinding binding, Set<String> banList, char[] fieldName, ASTNode responsible) {
+	private static void addAllMethodBindings(List<BindingTuple> list, TypeBinding binding, Set<String> banList, char[] fieldName, ASTNode responsible) throws DelegateRecursion {
 		banList.addAll(METHODS_IN_OBJECT);
 		addAllMethodBindings0(list, binding, banList, fieldName, responsible);
 	}
 	
-	private static void addAllMethodBindings0(List<BindingTuple> list, TypeBinding binding, Set<String> banList, char[] fieldName, ASTNode responsible) {
+	private static class DelegateRecursion extends Throwable {
+		final char[] type, member;
+		
+		public DelegateRecursion(char[] type, char[] member) {
+			this.type = type;
+			this.member = member;
+		}
+	}
+	
+	private static void addAllMethodBindings0(List<BindingTuple> list, TypeBinding binding, Set<String> banList, char[] fieldName, ASTNode responsible) throws DelegateRecursion {
+		if (binding instanceof SourceTypeBinding) ((SourceTypeBinding) binding).scope.environment().globalOptions.storeAnnotations = true;
 		if (binding == null) return;
 		
 		TypeBinding inner;
@@ -700,7 +725,12 @@ public class PatchDelegate {
 		
 		if (binding instanceof ReferenceBinding) {
 			ReferenceBinding rb = (ReferenceBinding) binding;
-			MethodBinding[] parameterizedSigs = rb.availableMethods();
+			MethodBinding[] availableMethods = rb.availableMethods();
+			FieldBinding[] availableFields = rb.availableFields();
+			failIfContainsAnnotation(binding, availableMethods); 
+			failIfContainsAnnotation(binding, availableFields); 
+			
+			MethodBinding[] parameterizedSigs = availableMethods;
 			MethodBinding[] baseSigs = parameterizedSigs;
 			if (binding instanceof ParameterizedTypeBinding) {
 				baseSigs = ((ParameterizedTypeBinding)binding).genericType().availableMethods();
@@ -731,6 +761,34 @@ public class PatchDelegate {
 		}
 	}
 	
+	private static final char[] STRING_LOMBOK = new char[] {'l', 'o', 'm', 'b', 'o', 'k'};
+	private static final char[] STRING_EXPERIMENTAL = new char[] {'e', 'x', 'p', 'e', 'r', 'i', 'm', 'e', 'n', 't', 'a', 'l'};
+	private static final char[] STRING_DELEGATE = new char[] {'D', 'e', 'l', 'e', 'g', 'a', 't', 'e'};
+	private static void failIfContainsAnnotation(TypeBinding parent, Binding[] bindings) throws DelegateRecursion {
+		if (bindings == null) return;
+		
+		for (Binding b : bindings) {
+			AnnotationBinding[] anns = null;
+			if (b instanceof MethodBinding) anns = ((MethodBinding) b).getAnnotations();
+			if (b instanceof FieldBinding) anns = ((FieldBinding) b).getAnnotations();
+			// anns = b.getAnnotations() would make a heck of a lot more sense, but that is a late addition to ecj, so would cause NoSuchMethodErrors! Don't use that!
+			if (anns == null) continue;
+			for (AnnotationBinding ann : anns) {
+				char[][] name = null;
+				try {
+					name = ann.getAnnotationType().compoundName;
+				} catch (Exception ignore) {}
+				
+				if (name == null || name.length < 2 || name.length > 3) continue;
+				if (!Arrays.equals(STRING_LOMBOK, name[0])) continue;
+				if (!Arrays.equals(STRING_DELEGATE, name[name.length - 1])) continue;
+				if (name.length == 3 && !Arrays.equals(STRING_EXPERIMENTAL, name[1])) continue;
+				
+				throw new DelegateRecursion(parent.readableName(), b.readableName());
+			}
+		}
+	}
+	
 	private static final class BindingTuple {
 		BindingTuple(MethodBinding parameterized, MethodBinding base, char[] fieldName, ASTNode responsible) {
 			this.parameterized = parameterized;

test/core/src/lombok/AbstractRunTests.java

@@ -215,8 +215,9 @@ public abstract class AbstractRunTests {
 			}
 			
 			while (expectedIterator.hasNext()) {
-				if (expectedIterator.next().isOptional()) continue;
-				fail(String.format("[%s] Expected message '%s' but ran out of actual messages", name, expectedIterator.next()));
+				CompilerMessageMatcher next = expectedIterator.next();
+				if (next.isOptional()) continue;
+				fail(String.format("[%s] Expected message '%s' but ran out of actual messages", name, next));
 			}
 			if (acHasNext) fail(String.format("[%s] Unexpected message: %s", name, actualIterator.next()));
 			break;

test/transform/resource/after-delombok/DelegateRecursion.java

@@ -1,20 +0,0 @@
-//ignore - This test fails, but, fixing it is going to take a long time and we don't have it in the planning. Having a failing test is very annoying for e.g. 'ant test'.
-
-class DelegateRecursionOuterMost {
-	private final DelegateRecursionCenter center = new DelegateRecursionCenter();
-	@java.lang.SuppressWarnings("all")
-	public void innerMostMethod() {
-		this.center.innerMostMethod();
-	}
-}
-class DelegateRecursionCenter {
-	private final DelegateRecursionInnerMost inner = new DelegateRecursionInnerMost();
-	@java.lang.SuppressWarnings("all")
-	public void innerMostMethod() {
-		this.inner.innerMostMethod();
-	}
-}
-class DelegateRecursionInnerMost {
-	public void innerMostMethod() {
-	}
-}

test/transform/resource/after-ecj/DelegateOnGetterNone.java

@@ -1,5 +1,5 @@
 import lombok.AccessLevel;
-import lombok.Delegate;
+import lombok.experimental.Delegate;
 import lombok.Getter;
 @Getter class DelegateOnGetterNone {
   private interface Bar {

test/transform/resource/after-ecj/DelegateOnMethods.java

@@ -1,4 +1,4 @@
-import lombok.Delegate;
+import lombok.experimental.Delegate;
 abstract class DelegateOnMethods {
   public static interface Bar {
     void bar(java.util.ArrayList<java.lang.String> list);

test/transform/resource/after-ecj/DelegateRecursion.java

@@ -1,26 +0,0 @@
-import lombok.Delegate;
-class DelegateRecursionOuterMost {
-  private final @Delegate DelegateRecursionCenter center = new DelegateRecursionCenter();
-  DelegateRecursionOuterMost() {
-    super();
-  }
-  public @java.lang.SuppressWarnings("all") void innerMostMethod() {
-    this.center.innerMostMethod();
-  }
-}
-class DelegateRecursionCenter {
-  private final @Delegate DelegateRecursionInnerMost inner = new DelegateRecursionInnerMost();
-  DelegateRecursionCenter() {
-    super();
-  }
-  public @java.lang.SuppressWarnings("all") void innerMostMethod() {
-    this.inner.innerMostMethod();
-  }
-}
-class DelegateRecursionInnerMost {
-  DelegateRecursionInnerMost() {
-    super();
-  }
-  public void innerMostMethod() {
-  }
-}

test/transform/resource/after-ecj/DelegateTypesAndExcludes.java

@@ -1,4 +1,4 @@
-import lombok.Delegate;
+import lombok.experimental.Delegate;
 class DelegatePlain {
   private static class FooImpl implements Foo {
     private FooImpl() {

test/transform/resource/after-ecj/DelegateWithDeprecated.java

@@ -1,4 +1,4 @@
-import lombok.Delegate;
+import lombok.experimental.Delegate;
 class DelegateWithDeprecated {
   private interface Bar {
     @Deprecated void deprecatedAnnotation();

test/transform/resource/before/DelegateOnGetterNone.java

@@ -1,5 +1,5 @@
 import lombok.AccessLevel;
-import lombok.Delegate;
+import lombok.experimental.Delegate;
 import lombok.Getter;
 
 @Getter

test/transform/resource/before/DelegateOnMethods.java

@@ -1,4 +1,4 @@
-import lombok.Delegate;
+import lombok.experimental.Delegate;
 
 abstract class DelegateOnMethods {
 

test/transform/resource/before/DelegateOnStatic.java

@@ -1,5 +1,5 @@
 //skip compare content
-import lombok.Delegate;
+import lombok.experimental.Delegate;
 import lombok.Getter;
 
 class DelegateOnStatic {

test/transform/resource/before/DelegateRecursion.java

@@ -1,4 +1,5 @@
-import lombok.Delegate;
+//skip compare content: This test is to see if the 'delegate recursion is not supported' error pops up.
+import lombok.experimental.Delegate;
 class DelegateRecursionOuterMost {
 	@Delegate
 	private final DelegateRecursionCenter center = new DelegateRecursionCenter();
@@ -12,4 +13,4 @@ class DelegateRecursionCenter {
 class DelegateRecursionInnerMost {
 	public void innerMostMethod() {
 	}
-}
+}

test/transform/resource/before/DelegateTypesAndExcludes.java

@@ -1,4 +1,4 @@
-import lombok.Delegate;
+import lombok.experimental.Delegate;
 class DelegatePlain {
 	@Delegate(types = Bar.class)
 	private final BarImpl bar = new BarImpl();

test/transform/resource/before/DelegateWithDeprecated.java

@@ -1,4 +1,4 @@
-import lombok.Delegate;
+import lombok.experimental.Delegate;
 
 class DelegateWithDeprecated {
 	@Delegate private Bar bar;

test/transform/resource/messages-delombok/DelegateRecursion.java.messages

@@ -0,0 +1 @@
+4 @Delegate does not support recursion (delegating to a type that itself has @Delegate members). Member "inner" is @Delegate in type "DelegateRecursionCenter"

test/transform/resource/messages-ecj/DelegateOnGetter.java.messages

@@ -0,0 +1,2 @@
+1 The type Delegate is deprecated
+6 The type Delegate is deprecated

test/transform/resource/messages-ecj/DelegateRecursion.java.messages

@@ -0,0 +1 @@
+4 @Delegate does not support recursion (delegating to a type that itself has @Delegate members). Member "inner" is @Delegate in type "DelegateRecursionCenter"

website/features/experimental/Delegate.html

@@ -74,7 +74,7 @@
 			</div>
 		</div>
 		<div class="footer">
-			<a href="index.html">Back to features</a> | <a href="FieldDefaults.html">Previous feature (@FieldDefaults)</a> | <a href="Wither.html">Next feature (@Wither)</a><br />
+			<a href="index.html">Back to experimental features</a> | <a href="FieldDefaults.html">Previous feature (@FieldDefaults)</a> | <a href="Wither.html">Next feature (@Wither)</a><br />
 			<a href="../../credits.html" class="creditsLink">credits</a> | <span class="copyright">Copyright &copy; 2010-2013 The Project Lombok Authors, licensed under the <a href="http://www.opensource.org/licenses/mit-license.php">MIT license</a>.</span>
 		</div>
 		<div style="clear: both;"></div>