[fixes #2335] ObtainVia(method=) on more than one arg would crash in javac

2 files changed, 61 insertions, 32 deletions

diff --git a/src/core/lombok/eclipse/handlers/HandleBuilder.java b/src/core/lombok/eclipse/handlers/HandleBuilder.java
index 3f711a19..bb031ebc 100755
--- a/src/core/lombok/eclipse/handlers/HandleBuilder.java
+++ b/src/core/lombok/eclipse/handlers/HandleBuilder.java
@@ -565,7 +565,9 @@ public class HandleBuilder extends EclipseAnnotationHandler<Builder> {
 		invoke.type = namePlusTypeParamsToTypeReference(type, builderClassName.toCharArray(), !isStatic, typeParams, p);
 		
 		Expression receiver = invoke;
-		List<Statement> statements = null;
+		List<Statement> preStatements = null;
+		List<Statement> postStatements = null;
+		
 		for (BuilderFieldData bfd : builderFields) {
 			String setterName = new String(bfd.name);
 			String setterPrefix = !prefix.isEmpty() ? prefix : fluent ? "" : "set";
@@ -584,23 +586,31 @@ public class HandleBuilder extends EclipseAnnotationHandler<Builder> {
 			} else {
 				String obtainName = bfd.obtainVia.method();
 				boolean obtainIsStatic = bfd.obtainVia.isStatic();
-				for (int i = 0; i < tgt.length; i++) {
-					MessageSend obtainExpr = new MessageSend();
-					if (obtainIsStatic) {
-						if (typeParams != null && typeParams.length > 0) {
-							obtainExpr.typeArguments = new TypeReference[typeParams.length];
-							for (int j = 0; j<typeParams.length; j++) {
-								obtainExpr.typeArguments[j] = new SingleTypeReference(typeParams[j].name, 0);
-							}
+				MessageSend obtainExpr = new MessageSend();
+				if (obtainIsStatic) {
+					if (typeParams != null && typeParams.length > 0) {
+						obtainExpr.typeArguments = new TypeReference[typeParams.length];
+						for (int j = 0; j<typeParams.length; j++) {
+							obtainExpr.typeArguments[j] = new SingleTypeReference(typeParams[j].name, 0);
 						}
-						obtainExpr.receiver = generateNameReference(type, 0);
-					} else {
-						obtainExpr.receiver = new ThisReference(0, 0);
 					}
-					obtainExpr.selector = obtainName.toCharArray();
-					if (obtainIsStatic) obtainExpr.arguments = new Expression[] {new ThisReference(0, 0)};
-					tgt[i] = obtainExpr;
+					obtainExpr.receiver = generateNameReference(type, 0);
+				} else {
+					obtainExpr.receiver = new ThisReference(0, 0);
 				}
+				obtainExpr.selector = obtainName.toCharArray();
+				if (obtainIsStatic) obtainExpr.arguments = new Expression[] {new ThisReference(0, 0)};
+				for (int i = 0; i < tgt.length; i++) tgt[i] = new SingleNameReference(bfd.name, 0L);
+				
+				// javac appears to cache the type of JCMethodInvocation expressions based on position, meaning, if you have 2 ObtainVia-based method invokes on different types, you get bizarre type mismatch errors.
+				// going via a local variable declaration solves the problem. We copy this behaviour
+				// for ecj so we match what javac's handler does.
+				LocalDeclaration ld = new LocalDeclaration(bfd.name, 0, 0);
+				ld.modifiers = ClassFileConstants.AccFinal;
+				ld.type = EclipseHandlerUtil.copyType(bfd.type, source);
+				ld.initialization = obtainExpr;
+				if (preStatements == null) preStatements = new ArrayList<Statement>();
+				preStatements.add(ld);
 			}
 			
 			ms.selector = setterName.toCharArray();
@@ -612,23 +622,28 @@ public class HandleBuilder extends EclipseAnnotationHandler<Builder> {
 				ms.arguments = new Expression[] {tgt[1]};
 				ms.receiver = new SingleNameReference(BUILDER_TEMP_VAR, p);
 				EqualExpression isNotNull = new EqualExpression(tgt[0], new NullLiteral(pS, pE), OperatorIds.NOT_EQUAL);
-				if (statements == null) statements = new ArrayList<Statement>();
-				statements.add(new IfStatement(isNotNull, ms, pS, pE));
+				if (postStatements == null) postStatements = new ArrayList<Statement>();
+				postStatements.add(new IfStatement(isNotNull, ms, pS, pE));
 			}
 		}
 		
-		if (statements != null) {
-			out.statements = new Statement[statements.size() + 2];
-			for (int i = 0; i < statements.size(); i++) out.statements[i + 1] = statements.get(i);
+		int preSs = preStatements == null ? 0 : preStatements.size();
+		int postSs = postStatements == null ? 0 : postStatements.size();
+		if (postSs > 0) {
+			out.statements = new Statement[preSs + postSs + 2];
+			for (int i = 0; i < preSs; i++) out.statements[i] = preStatements.get(i);
+			for (int i = 0; i < postSs; i++) out.statements[preSs + 1 + i] = postStatements.get(i);
 			LocalDeclaration b = new LocalDeclaration(BUILDER_TEMP_VAR, pS, pE);
-			out.statements[0] = b;
-			b.modifiers |= Modifier.FINAL;
+			out.statements[preSs] = b;
+			b.modifiers |= ClassFileConstants.AccFinal;
 			b.type = namePlusTypeParamsToTypeReference(type, builderClassName.toCharArray(), !isStatic, typeParams, p);
 			b.type.sourceStart = pS; b.type.sourceEnd = pE;
 			b.initialization = receiver;
-			out.statements[out.statements.length - 1] = new ReturnStatement(new SingleNameReference(BUILDER_TEMP_VAR, p), pS, pE);
+			out.statements[preSs + postSs + 1] = new ReturnStatement(new SingleNameReference(BUILDER_TEMP_VAR, p), pS, pE);
 		} else {
-			out.statements = new Statement[] {new ReturnStatement(receiver, pS, pE)};
+			out.statements = new Statement[preSs + 1];
+			for (int i = 0; i < preSs; i++) out.statements[i] = preStatements.get(i);
+			out.statements[preSs] = new ReturnStatement(receiver, pS, pE);
 		}
 		
 		if (cfv.generateUnique()) {
diff --git a/src/core/lombok/javac/handlers/HandleBuilder.java b/src/core/lombok/javac/handlers/HandleBuilder.java
index 1e8b8a22..0bd43ba4 100644
--- a/src/core/lombok/javac/handlers/HandleBuilder.java
+++ b/src/core/lombok/javac/handlers/HandleBuilder.java
@@ -45,6 +45,7 @@ import com.sun.tools.javac.tree.JCTree.JCIdent;
 import com.sun.tools.javac.tree.JCTree.JCIf;
 import com.sun.tools.javac.tree.JCTree.JCLiteral;
 import com.sun.tools.javac.tree.JCTree.JCMethodDecl;
+import com.sun.tools.javac.tree.JCTree.JCMethodInvocation;
 import com.sun.tools.javac.tree.JCTree.JCModifiers;
 import com.sun.tools.javac.tree.JCTree.JCNewClass;
 import com.sun.tools.javac.tree.JCTree.JCPrimitiveTypeTree;
@@ -553,7 +554,9 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> {
 		
 		JCExpression call = maker.NewClass(null, List.<JCExpression>nil(), namePlusTypeParamsToTypeReference(maker, type, type.toName(builderClassName), !isStatic, typeParams), List.<JCExpression>nil(), null);
 		JCExpression invoke = call;
+		ListBuffer<JCStatement> preStatements = null;
 		ListBuffer<JCStatement> statements = new ListBuffer<JCStatement>();
+		
 		for (BuilderFieldData bfd : builderFields) {
 			String setterPrefix = !prefix.isEmpty() ? prefix : fluent ? "" : "set";
 			String prefixedSetterName = bfd.name.toString();
@@ -566,17 +569,22 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> {
 					tgt[i] = maker.Select(maker.Ident(type.toName("this")), bfd.obtainVia == null ? bfd.rawName : type.toName(bfd.obtainVia.field()));
 				}
 			} else {
+				String name = bfd.obtainVia.method();
+				JCMethodInvocation inv;
 				if (bfd.obtainVia.isStatic()) {
-					for (int i = 0; i < tgt.length; i++) {
-						JCExpression c = maker.Select(maker.Ident(type.toName(type.getName())), type.toName(bfd.obtainVia.method()));
-						tgt[i] = maker.Apply(typeParameterNames(maker, typeParams), c, List.<JCExpression>of(maker.Ident(type.toName("this"))));
-					}
+					JCExpression c = maker.Select(maker.Ident(type.toName(type.getName())), type.toName(name));
+					inv = maker.Apply(typeParameterNames(maker, typeParams), c, List.<JCExpression>of(maker.Ident(type.toName("this"))));
 				} else {
-					for (int i = 0; i < tgt.length; i++) {
-						JCExpression c = maker.Select(maker.Ident(type.toName("this")), type.toName(bfd.obtainVia.method()));
-						tgt[i] = maker.Apply(List.<JCExpression>nil(), c, List.<JCExpression>nil());
-					}
+					JCExpression c = maker.Select(maker.Ident(type.toName("this")), type.toName(name));
+					inv = maker.Apply(List.<JCExpression>nil(), c, List.<JCExpression>nil());
 				}
+				for (int i = 0; i < tgt.length; i++) tgt[i] = maker.Ident(bfd.name);
+				
+				// javac appears to cache the type of JCMethodInvocation expressions based on position, meaning, if you have 2 ObtainVia-based method invokes on different types, you get bizarre type mismatch errors.
+				// going via a local variable declaration solves the problem.
+				JCExpression varType = JavacHandlerUtil.cloneType(maker, bfd.type, ast, type.getContext());
+				if (preStatements == null) preStatements = new ListBuffer<JCStatement>();
+				preStatements.append(maker.VarDef(maker.Modifiers(Flags.FINAL), bfd.name, varType, inv));
 			}
 			
 			JCExpression arg;
@@ -589,6 +597,7 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> {
 				statements.append(maker.If(isNotNull, maker.Exec(invokeBuilder), null));
 			}
 		}
+		
 		if (!statements.isEmpty()) {
 			JCExpression tempVarType = namePlusTypeParamsToTypeReference(maker, type, type.toName(builderClassName), !isStatic, typeParams);
 			statements.prepend(maker.VarDef(maker.Modifiers(Flags.FINAL), type.toName(BUILDER_TEMP_VAR), tempVarType, invoke));
@@ -596,6 +605,11 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> {
 		} else {
 			statements.append(maker.Return(invoke));
 		}
+		
+		if (preStatements != null) {
+			preStatements.appendList(statements);
+			statements = preStatements;
+		}
 		JCBlock body = maker.Block(0, statements.toList());
 		List<JCAnnotation> annsOnMethod = cfv.generateUnique() ? List.of(maker.Annotation(genTypeRef(type, CheckerFrameworkVersion.NAME__UNIQUE), List.<JCExpression>nil())) : List.<JCAnnotation>nil();
 		return maker.MethodDef(maker.Modifiers(toJavacModifier(access), annsOnMethod), type.toName(toBuilderMethodName), namePlusTypeParamsToTypeReference(maker, type, type.toName(builderClassName), !isStatic, typeParams), List.<JCTypeParameter>nil(), List.<JCVariableDecl>nil(), List.<JCExpression>nil(), body, null);