The somewhat rare conflict in typevar names problem has now been solved in javac.

2 files changed, 160 insertions, 86 deletions

diff --git a/src/core/lombok/javac/FindTypeVarScanner.java b/src/core/lombok/javac/FindTypeVarScanner.java
new file mode 100644
index 00000000..4f921289
--- /dev/null
+++ b/src/core/lombok/javac/FindTypeVarScanner.java
@@ -0,0 +1,112 @@
+/*
+ * Copyright © 2010 Reinier Zwitserloot, Roel Spilker and Robbert Jan Grootjans.
+ * 
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ * 
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ * 
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package lombok.javac;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.lang.model.element.Name;
+import javax.lang.model.type.ArrayType;
+import javax.lang.model.type.DeclaredType;
+import javax.lang.model.type.ErrorType;
+import javax.lang.model.type.ExecutableType;
+import javax.lang.model.type.NoType;
+import javax.lang.model.type.NullType;
+import javax.lang.model.type.PrimitiveType;
+import javax.lang.model.type.TypeMirror;
+import javax.lang.model.type.TypeVariable;
+import javax.lang.model.type.WildcardType;
+import javax.lang.model.util.AbstractTypeVisitor6;
+
+import com.sun.tools.javac.code.Type;
+
+/**
+ * scanner (i.e. visits child nodes all the way to the leaves) that accumulates type variables. Call {@code visit} on any {@code TypeMirror} object with an instance
+ * to add all used type variable names such as {@code T} or {@code E} to the set that is returned by the {@link #getTypeVariables} method.
+ */
+public class FindTypeVarScanner extends AbstractTypeVisitor6<Void, Void> {
+	private Set<String> typeVariables = new HashSet<String>();
+	
+	public Set<String> getTypeVariables() {
+		return typeVariables;
+	}
+	
+	private Void subVisit(TypeMirror mirror) {
+		if (mirror == null) return null;
+		return mirror.accept(this, null);
+	}
+	
+	@Override public Void visitPrimitive(PrimitiveType t, Void p) {
+		return null;
+	}
+	
+	@Override public Void visitNull(NullType t, Void p) {
+		return null;
+	}
+	
+	
+	@Override public Void visitNoType(NoType t, Void p) {
+		return null;
+	}
+	
+	@Override public Void visitUnknown(TypeMirror t, Void p) {
+		return null;
+	}
+	
+	@Override public Void visitError(ErrorType t, Void p) {
+		return null;
+	}
+	
+	@Override public Void visitArray(ArrayType t, Void p) {
+		return subVisit(t.getComponentType());
+	}
+	
+	@Override public Void visitDeclared(DeclaredType t, Void p) {
+		for (TypeMirror subT : t.getTypeArguments()) subVisit(subT);
+		return null;
+	}
+	
+	@Override public Void visitTypeVariable(TypeVariable t, Void p) {
+		Name name = null;
+		try {
+			name = ((Type)t).tsym.name;
+		} catch (NullPointerException e) {}
+		if (name != null) typeVariables.add(name.toString());
+		subVisit(t.getLowerBound());
+		subVisit(t.getUpperBound());
+		return null;
+	}
+	
+	@Override public Void visitWildcard(WildcardType t, Void p) {
+		subVisit(t.getSuperBound());
+		subVisit(t.getExtendsBound());
+		return null;
+	}
+	
+	@Override public Void visitExecutable(ExecutableType t, Void p) {
+		subVisit(t.getReturnType());
+		for (TypeMirror subT : t.getParameterTypes()) subVisit(subT);
+		for (TypeMirror subT : t.getThrownTypes()) subVisit(subT);
+		for (TypeVariable subT : t.getTypeVariables()) subVisit(subT);
+		return null;
+	}
+}
diff --git a/src/core/lombok/javac/handlers/HandleDelegate.java b/src/core/lombok/javac/handlers/HandleDelegate.java
index 1acbbf8d..8338736e 100644
--- a/src/core/lombok/javac/handlers/HandleDelegate.java
+++ b/src/core/lombok/javac/handlers/HandleDelegate.java
@@ -33,22 +33,14 @@ import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.Modifier;
 import javax.lang.model.element.TypeParameterElement;
 import javax.lang.model.element.VariableElement;
-import javax.lang.model.type.ArrayType;
-import javax.lang.model.type.DeclaredType;
-import javax.lang.model.type.ErrorType;
 import javax.lang.model.type.ExecutableType;
-import javax.lang.model.type.NoType;
-import javax.lang.model.type.NullType;
-import javax.lang.model.type.PrimitiveType;
 import javax.lang.model.type.TypeKind;
 import javax.lang.model.type.TypeMirror;
-import javax.lang.model.type.TypeVariable;
-import javax.lang.model.type.TypeVisitor;
-import javax.lang.model.type.WildcardType;
 
 import lombok.Delegate;
 import lombok.core.AST.Kind;
 import lombok.core.AnnotationValues;
+import lombok.javac.FindTypeVarScanner;
 import lombok.javac.JavacAnnotationHandler;
 import lombok.javac.JavacNode;
 import lombok.javac.JavacResolution;
@@ -153,101 +145,71 @@ public class HandleDelegate implements JavacAnnotationHandler<Delegate> {
 	}
 	
 	private void generateAndAdd(MethodSig sig, JavacNode annotation, Name delegateFieldName) {
+		List<JCMethodDecl> toAdd = new ArrayList<JCMethodDecl>();
 		try {
-			JCMethodDecl method = createDelegateMethod(sig, annotation, delegateFieldName);
-			JavacHandlerUtil.injectMethod(annotation.up().up(), method);
+			toAdd.add(createDelegateMethod(sig, annotation, delegateFieldName));
 		} catch (TypeNotConvertibleException e) {
 			annotation.addError("Can't create delegate method for " + sig.name + ": " + e.getMessage());
+			return;
+		} catch (CantMakeDelegates e) {
+			annotation.addError("There's a conflict in the names of type parameters. Fix it by renaming the following type parameters of your class: " + e.conflicted);
+			return;
+		}
+		
+		for (JCMethodDecl method : toAdd) {
+			JavacHandlerUtil.injectMethod(annotation.up().up(), method);
 		}
 	}
 	
-	private JCMethodDecl createDelegateMethod(MethodSig sig, JavacNode annotation, Name delegateFieldName) throws TypeNotConvertibleException {
+	private static class CantMakeDelegates extends Exception {
+		Set<String> conflicted;
+	}
+	
+	private JCMethodDecl createDelegateMethod(MethodSig sig, JavacNode annotation, Name delegateFieldName) throws TypeNotConvertibleException, CantMakeDelegates {
 		/** public <T, U, ...> ReturnType methodName(ParamType1 name1, ParamType2 name2, ...) throws T1, T2, ... {
 		 *      (return) delegate.<T, U>methodName(name1, name2);
 		 *  }
 		 */
 		
+		// There's a rare but problematic case if a delegate method has its own type variables, and the delegated type does too, and the method uses both.
+		// If for example the delegated type has <E>, and the method has <T>, but in our class we have a <T> at the class level, then we have two different
+		// type variables both named 'T'. We detect this situation and error out asking the programmer to rename their type variable.
+		// As first step, we check if there's a conflict between the delegate method's type vars and our own class.
+		
 		if (!sig.elem.getTypeParameters().isEmpty()) {
-			// There's a rare but problematic case if a delegate method has its own type variables, and the delegated type does too, and the method uses both.
-			// If for example the delegated type has <E>, and the method has <T>, but in our class we have a <T> at the class level, then we have two different
-			// type variables both named 'T'. We detect this situation and error out asking the programmer to rename their type variable.
-			final Set<String> usedTypeVars = new HashSet<String>();
-			class TypeScanner implements TypeVisitor<Void, Void> {
-				@Override public Void visit(TypeMirror t, Void p) {
-					return null;
-				}
-				
-				@Override public Void visit(TypeMirror t) {
-					return null;
-				}
-				
-				@Override public Void visitPrimitive(PrimitiveType t, Void p) {
-					return null;
-				}
-				
-				@Override public Void visitNull(NullType t, Void p) {
-					return null;
-				}
-				
-				@Override public Void visitArray(ArrayType t, Void p) {
-					t.getComponentType().accept(this, null);
-					return null;
-				}
-				
-				@Override public Void visitDeclared(DeclaredType t, Void p) {
-					for (TypeMirror arg : t.getTypeArguments()) {
-						arg.accept(this, null);
+			Set<String> usedInOurType = new HashSet<String>();
+			
+			JavacNode enclosingType = annotation;
+			while (enclosingType != null) {
+				if (enclosingType.getKind() == Kind.TYPE) {
+					List<JCTypeParameter> typarams = ((JCClassDecl)enclosingType.get()).typarams;
+					if (typarams != null) for (JCTypeParameter param : typarams) {
+						if (param.name != null) usedInOurType.add(param.name.toString());
 					}
-					
-					return null;
-				}
-				
-				@Override public Void visitError(ErrorType t, Void p) {
-					return null;
-				}
-				
-				@Override public Void visitTypeVariable(TypeVariable t, Void p) {
-					String name = t.asElement().getSimpleName().toString();
-					usedTypeVars.add(name);
-					return null;
-				}
-				
-				@Override public Void visitWildcard(WildcardType t, Void p) {
-					if (t.getExtendsBound() != null) t.getExtendsBound().accept(this, null);
-					if (t.getSuperBound() != null) t.getSuperBound().accept(this, null);
-					return null;
-				}
-				
-				@Override public Void visitExecutable(ExecutableType t, Void p) {
-					return null;
-				}
-				
-				@Override public Void visitNoType(NoType t, Void p) {
-					return null;
 				}
-				
-				@Override public Void visitUnknown(TypeMirror t, Void p) {
-					return null;
-				}
-			}
-			
-			TypeScanner scanner = new TypeScanner();
-			
-			sig.elem.getReturnType().accept(scanner, null);
-			for (VariableElement param : sig.elem.getParameters()) {
-				param.asType().accept(scanner, null);
-			}
-			for (TypeMirror ex : sig.elem.getThrownTypes()) {
-				ex.accept(scanner, null);
+				enclosingType = enclosingType.up();
 			}
 			
-			for (TypeParameterElement ownVar : sig.elem.getTypeParameters()) {
-				usedTypeVars.remove(ownVar.toString());
+			Set<String> usedInMethodSig = new HashSet<String>();
+			for (TypeParameterElement param : sig.elem.getTypeParameters()) {
+				usedInMethodSig.add(param.getSimpleName().toString());
 			}
 			
-			if (!usedTypeVars.isEmpty()) {
-				
-				
+			usedInMethodSig.retainAll(usedInOurType);
+			if (!usedInMethodSig.isEmpty()) {
+				// We might be delegating a List<T>, and we are making method <T> toArray(). A conflict is possible.
+				// But only if the toArray method also uses type vars from its class, otherwise we're only shadowing,
+				// which is okay as we'll add a @SuppressWarnings.
+				FindTypeVarScanner scanner = new FindTypeVarScanner();
+				sig.elem.asType().accept(scanner, null);
+				Set<String> names = new HashSet<String>(scanner.getTypeVariables());
+				names.removeAll(usedInMethodSig);
+				if (!names.isEmpty()) {
+					// We have a confirmed conflict. We could dig deeper as this may still be a false alarm, but its already an exceedingly rare case.
+					CantMakeDelegates cmd = new CantMakeDelegates();
+					cmd.conflicted = usedInMethodSig;
+					throw cmd;
+				}
 			}
 		}