Added an error if @Delegate is used recursively.

2 files changed, 137 insertions, 52 deletions

diff --git a/src/core/lombok/javac/handlers/HandleDelegate.java b/src/core/lombok/javac/handlers/HandleDelegate.java
index 9cd8844e..6065242e 100644
--- a/src/core/lombok/javac/handlers/HandleDelegate.java
+++ b/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;
-			}
-		}
-		
-		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;
+		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 : 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;
diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java b/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java
index 6a0c25e7..b6e75476 100644
--- a/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java
+++ b/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;