[fixes #1812] `@Singular` marked collections which nevertheless somehow ended up null would cause an NPE during `toBuilder()` invocations.

9 files changed, 206 insertions, 42 deletions

diff --git a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
index 2dce285c..87df6d1b 100644
--- a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
+++ b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
@@ -24,6 +24,7 @@ package lombok.eclipse.handlers;
 import static lombok.core.handlers.HandlerUtil.*;
 import static lombok.eclipse.Eclipse.*;
 import static lombok.eclipse.EclipseAugments.*;
+import static lombok.eclipse.handlers.EclipseHandlerUtil.setGeneratedBy;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
@@ -1862,4 +1863,29 @@ public class EclipseHandlerUtil {
 		String p = typeDecl.superclass.toString();
 		return p.equals("Object") || p.equals("java.lang.Object");
 	}
+	
+	public static NameReference generateQualifiedNameRef(ASTNode source, char[]... varNames) {
+		int pS = source.sourceStart, pE = source.sourceEnd;
+		long p = (long)pS << 32 | pE;
+		
+		NameReference ref;
+		
+		if (varNames.length > 1) ref = new QualifiedNameReference(varNames, new long[varNames.length], pS, pE);
+		else ref = new SingleNameReference(varNames[0], p);
+		setGeneratedBy(ref, source);
+		return ref;
+	}
+	
+	public static TypeReference generateQualifiedTypeRef(ASTNode source, char[]... varNames) {
+		int pS = source.sourceStart, pE = source.sourceEnd;
+		long p = (long)pS << 32 | pE;
+		
+		TypeReference ref;
+		
+		long[] poss = Eclipse.poss(source, varNames.length);
+		if (varNames.length > 1) ref = new QualifiedTypeReference(varNames, poss);
+		else ref = new SingleTypeReference(varNames[0], p);
+		setGeneratedBy(ref, source);
+		return ref;
+	}
 }
diff --git a/src/core/lombok/eclipse/handlers/HandleBuilder.java b/src/core/lombok/eclipse/handlers/HandleBuilder.java
index 4301e51a..9a069c58 100644
--- a/src/core/lombok/eclipse/handlers/HandleBuilder.java
+++ b/src/core/lombok/eclipse/handlers/HandleBuilder.java
@@ -40,6 +40,7 @@ import org.eclipse.jdt.internal.compiler.ast.Assignment;
 import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.ConditionalExpression;
 import org.eclipse.jdt.internal.compiler.ast.ConstructorDeclaration;
+import org.eclipse.jdt.internal.compiler.ast.EqualExpression;
 import org.eclipse.jdt.internal.compiler.ast.Expression;
 import org.eclipse.jdt.internal.compiler.ast.FalseLiteral;
 import org.eclipse.jdt.internal.compiler.ast.FieldDeclaration;
@@ -47,6 +48,7 @@ import org.eclipse.jdt.internal.compiler.ast.FieldReference;
 import org.eclipse.jdt.internal.compiler.ast.IfStatement;
 import org.eclipse.jdt.internal.compiler.ast.MessageSend;
 import org.eclipse.jdt.internal.compiler.ast.MethodDeclaration;
+import org.eclipse.jdt.internal.compiler.ast.NullLiteral;
 import org.eclipse.jdt.internal.compiler.ast.OperatorIds;
 import org.eclipse.jdt.internal.compiler.ast.ParameterizedQualifiedTypeReference;
 import org.eclipse.jdt.internal.compiler.ast.ParameterizedSingleTypeReference;
@@ -492,6 +494,7 @@ public class HandleBuilder extends EclipseAnnotationHandler<Builder> {
 		}
 	}
 	
+	private static final char[] EMPTY_LIST = "emptyList".toCharArray();
 	private MethodDeclaration generateToBuilderMethod(String methodName, String builderClassName, EclipseNode type, TypeParameter[] typeParams, List<BuilderFieldData> builderFields, boolean fluent, ASTNode source) {
 		// return new ThingieBuilder<A, B>().setA(this.a).setB(this.b);
 		
@@ -511,19 +514,34 @@ public class HandleBuilder extends EclipseAnnotationHandler<Builder> {
 		for (BuilderFieldData bfd : builderFields) {
 			char[] setterName = fluent ? bfd.name : HandlerUtil.buildAccessorName("set", new String(bfd.name)).toCharArray();
 			MessageSend ms = new MessageSend();
+			Expression[] tgt = new Expression[bfd.singularData == null ? 1 : 2];
+			
 			if (bfd.obtainVia == null || !bfd.obtainVia.field().isEmpty()) {
 				char[] fieldName = bfd.obtainVia == null ? bfd.rawName : bfd.obtainVia.field().toCharArray();
-				FieldReference fr = new FieldReference(fieldName, 0);
-				fr.receiver = new ThisReference(0, 0);
-				ms.arguments = new Expression[] {fr};
+				for (int i = 0; i < tgt.length; i++) {
+					FieldReference fr = new FieldReference(fieldName, 0);
+					fr.receiver = new ThisReference(0, 0);
+					tgt[i] = fr;
+				}
 			} else {
 				String obtainName = bfd.obtainVia.method();
 				boolean obtainIsStatic = bfd.obtainVia.isStatic();
-				MessageSend obtainExpr = new MessageSend();
-				obtainExpr.receiver = obtainIsStatic ? new SingleNameReference(type.getName().toCharArray(), 0) : new ThisReference(0, 0);
-				obtainExpr.selector = obtainName.toCharArray();
-				if (obtainIsStatic) obtainExpr.arguments = new Expression[] {new ThisReference(0, 0)};
-				ms.arguments = new Expression[] {obtainExpr};
+				for (int i = 0; i < tgt.length; i++) {
+					MessageSend obtainExpr = new MessageSend();
+					obtainExpr.receiver = obtainIsStatic ? new SingleNameReference(type.getName().toCharArray(), 0) : new ThisReference(0, 0);
+					obtainExpr.selector = obtainName.toCharArray();
+					if (obtainIsStatic) obtainExpr.arguments = new Expression[] {new ThisReference(0, 0)};
+					tgt[i] = obtainExpr;
+				}
+			}
+			if (bfd.singularData == null) {
+				ms.arguments = tgt;
+			} else {
+				Expression ifNull = new EqualExpression(tgt[0], new NullLiteral(0, 0), OperatorIds.EQUAL_EQUAL);
+				MessageSend emptyList = new MessageSend();
+				emptyList.receiver = generateQualifiedNameRef(source, TypeConstants.JAVA, TypeConstants.UTIL, "Collections".toCharArray());
+				emptyList.selector = EMPTY_LIST;
+				ms.arguments = new Expression[] {new ConditionalExpression(ifNull, emptyList, tgt[1])};
 			}
 			ms.receiver = receiver;
 			ms.selector = setterName;
diff --git a/src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java b/src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java
index c99b9b5f..6945e5d9 100644
--- a/src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java
+++ b/src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java
@@ -69,7 +69,6 @@ import org.eclipse.jdt.internal.compiler.ast.NullLiteral;
 import org.eclipse.jdt.internal.compiler.ast.OperatorIds;
 import org.eclipse.jdt.internal.compiler.ast.ParameterizedQualifiedTypeReference;
 import org.eclipse.jdt.internal.compiler.ast.ParameterizedSingleTypeReference;
-import org.eclipse.jdt.internal.compiler.ast.QualifiedNameReference;
 import org.eclipse.jdt.internal.compiler.ast.QualifiedTypeReference;
 import org.eclipse.jdt.internal.compiler.ast.ReturnStatement;
 import org.eclipse.jdt.internal.compiler.ast.SingleNameReference;
@@ -819,29 +818,4 @@ public class HandleEqualsAndHashCode extends EclipseAnnotationHandler<EqualsAndH
 		expr.sourceStart = pS; expr.sourceEnd = pE;
 		return expr;
 	}
-	
-	public NameReference generateQualifiedNameRef(ASTNode source, char[]... varNames) {
-		int pS = source.sourceStart, pE = source.sourceEnd;
-		long p = (long)pS << 32 | pE;
-		
-		NameReference ref;
-		
-		if (varNames.length > 1) ref = new QualifiedNameReference(varNames, new long[varNames.length], pS, pE);
-		else ref = new SingleNameReference(varNames[0], p);
-		setGeneratedBy(ref, source);
-		return ref;
-	}
-	
-	public TypeReference generateQualifiedTypeRef(ASTNode source, char[]... varNames) {
-		int pS = source.sourceStart, pE = source.sourceEnd;
-		long p = (long)pS << 32 | pE;
-		
-		TypeReference ref;
-		
-		long[] poss = Eclipse.poss(source, varNames.length);
-		if (varNames.length > 1) ref = new QualifiedTypeReference(varNames, poss);
-		else ref = new SingleTypeReference(varNames[0], p);
-		setGeneratedBy(ref, source);
-		return ref;
-	}
 }
diff --git a/src/core/lombok/javac/handlers/HandleBuilder.java b/src/core/lombok/javac/handlers/HandleBuilder.java
index f04ea8b1..edf6f2ae 100644
--- a/src/core/lombok/javac/handlers/HandleBuilder.java
+++ b/src/core/lombok/javac/handlers/HandleBuilder.java
@@ -492,18 +492,34 @@ public class HandleBuilder extends JavacAnnotationHandler<Builder> {
 		JCExpression invoke = call;
 		for (BuilderFieldData bfd : builderFields) {
 			Name setterName = fluent ? bfd.name : type.toName(HandlerUtil.buildAccessorName("set", bfd.name.toString()));
-			JCExpression arg;
+			JCExpression[] tgt = new JCExpression[bfd.singularData == null ? 1 : 2];
 			if (bfd.obtainVia == null || !bfd.obtainVia.field().isEmpty()) {
-				arg = maker.Select(maker.Ident(type.toName("this")), bfd.obtainVia == null ? bfd.rawName : type.toName(bfd.obtainVia.field()));
+				for (int i = 0; i < tgt.length; i++) {
+					tgt[i] = maker.Select(maker.Ident(type.toName("this")), bfd.obtainVia == null ? bfd.rawName : type.toName(bfd.obtainVia.field()));
+				}
 			} else {
 				if (bfd.obtainVia.isStatic()) {
-					JCExpression c = maker.Select(maker.Ident(type.toName(type.getName())), type.toName(bfd.obtainVia.method()));
-					arg = maker.Apply(List.<JCExpression>nil(), c, List.<JCExpression>of(maker.Ident(type.toName("this"))));
+					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(List.<JCExpression>nil(), c, List.<JCExpression>of(maker.Ident(type.toName("this"))));
+					}
 				} else {
-					JCExpression c = maker.Select(maker.Ident(type.toName("this")), type.toName(bfd.obtainVia.method()));
-					arg = maker.Apply(List.<JCExpression>nil(), c, List.<JCExpression>nil());
+					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 arg;
+			if (bfd.singularData == null) {
+				arg = tgt[0];
+			} else {
+				JCExpression eqNull = maker.Binary(CTC_EQUAL, tgt[0], maker.Literal(CTC_BOT, null));
+				JCExpression emptyList = maker.Apply(List.<JCExpression>nil(), chainDots(type, "java", "util", "Collections", "emptyList"), List.<JCExpression>nil());
+				arg = maker.Conditional(eqNull, emptyList, tgt[1]);
+			}
+			
 			invoke = maker.Apply(List.<JCExpression>nil(), maker.Select(invoke, setterName), List.of(arg));
 		}
 		JCStatement statement = maker.Return(invoke);
diff --git a/test/transform/resource/after-delombok/BuilderSingularToBuilderWithNull.java b/test/transform/resource/after-delombok/BuilderSingularToBuilderWithNull.java
new file mode 100644
index 00000000..1f472438
--- /dev/null
+++ b/test/transform/resource/after-delombok/BuilderSingularToBuilderWithNull.java
@@ -0,0 +1,63 @@
+class BuilderSingularToBuilderWithNull {
+	private java.util.List<String> elems;
+	public static void test() {
+		new BuilderSingularToBuilderWithNull(null).toBuilder();
+	}
+	@java.lang.SuppressWarnings("all")
+	BuilderSingularToBuilderWithNull(final java.util.List<String> elems) {
+		this.elems = elems;
+	}
+	@java.lang.SuppressWarnings("all")
+	public static class BuilderSingularToBuilderWithNullBuilder {
+		@java.lang.SuppressWarnings("all")
+		private java.util.ArrayList<String> elems;
+		@java.lang.SuppressWarnings("all")
+		BuilderSingularToBuilderWithNullBuilder() {
+		}
+		@java.lang.SuppressWarnings("all")
+		public BuilderSingularToBuilderWithNullBuilder elem(final String elem) {
+			if (this.elems == null) this.elems = new java.util.ArrayList<String>();
+			this.elems.add(elem);
+			return this;
+		}
+		@java.lang.SuppressWarnings("all")
+		public BuilderSingularToBuilderWithNullBuilder elems(final java.util.Collection<? extends String> elems) {
+			if (this.elems == null) this.elems = new java.util.ArrayList<String>();
+			this.elems.addAll(elems);
+			return this;
+		}
+		@java.lang.SuppressWarnings("all")
+		public BuilderSingularToBuilderWithNullBuilder clearElems() {
+			if (this.elems != null) this.elems.clear();
+			return this;
+		}
+		@java.lang.SuppressWarnings("all")
+		public BuilderSingularToBuilderWithNull build() {
+			java.util.List<String> elems;
+			switch (this.elems == null ? 0 : this.elems.size()) {
+			case 0: 
+				elems = java.util.Collections.emptyList();
+				break;
+			case 1: 
+				elems = java.util.Collections.singletonList(this.elems.get(0));
+				break;
+			default: 
+				elems = java.util.Collections.unmodifiableList(new java.util.ArrayList<String>(this.elems));
+			}
+			return new BuilderSingularToBuilderWithNull(elems);
+		}
+		@java.lang.Override
+		@java.lang.SuppressWarnings("all")
+		public java.lang.String toString() {
+			return "BuilderSingularToBuilderWithNull.BuilderSingularToBuilderWithNullBuilder(elems=" + this.elems + ")";
+		}
+	}
+	@java.lang.SuppressWarnings("all")
+	public static BuilderSingularToBuilderWithNullBuilder builder() {
+		return new BuilderSingularToBuilderWithNullBuilder();
+	}
+	@java.lang.SuppressWarnings("all")
+	public BuilderSingularToBuilderWithNullBuilder toBuilder() {
+		return new BuilderSingularToBuilderWithNullBuilder().elems(this.elems == null ? java.util.Collections.emptyList() : this.elems);
+	}
+}
diff --git a/test/transform/resource/after-delombok/BuilderWithToBuilder.java b/test/transform/resource/after-delombok/BuilderWithToBuilder.java
index 46387f0f..b644a16f 100644
--- a/test/transform/resource/after-delombok/BuilderWithToBuilder.java
+++ b/test/transform/resource/after-delombok/BuilderWithToBuilder.java
@@ -86,7 +86,7 @@ class BuilderWithToBuilder<T> {
 	}
 	@java.lang.SuppressWarnings("all")
 	public BuilderWithToBuilderBuilder<T> toBuilder() {
-		return new BuilderWithToBuilderBuilder<T>().one(this.mOne).two(this.mTwo).foo(BuilderWithToBuilder.rrr(this)).bars(this.bars);
+		return new BuilderWithToBuilderBuilder<T>().one(this.mOne).two(this.mTwo).foo(BuilderWithToBuilder.rrr(this)).bars(this.bars == null ? java.util.Collections.emptyList() : this.bars);
 	}
 }
 class ConstructorWithToBuilder<T> {
diff --git a/test/transform/resource/after-ecj/BuilderSingularToBuilderWithNull.java b/test/transform/resource/after-ecj/BuilderSingularToBuilderWithNull.java
new file mode 100644
index 00000000..7265e17a
--- /dev/null
+++ b/test/transform/resource/after-ecj/BuilderSingularToBuilderWithNull.java
@@ -0,0 +1,57 @@
+import lombok.Singular;
+@lombok.Builder(toBuilder = true) class BuilderSingularToBuilderWithNull {
+  public static @java.lang.SuppressWarnings("all") class BuilderSingularToBuilderWithNullBuilder {
+    private @java.lang.SuppressWarnings("all") java.util.ArrayList<String> elems;
+    @java.lang.SuppressWarnings("all") BuilderSingularToBuilderWithNullBuilder() {
+      super();
+    }
+    public @java.lang.SuppressWarnings("all") BuilderSingularToBuilderWithNullBuilder elem(String elem) {
+      if ((this.elems == null))
+          this.elems = new java.util.ArrayList<String>();
+      this.elems.add(elem);
+      return this;
+    }
+    public @java.lang.SuppressWarnings("all") BuilderSingularToBuilderWithNullBuilder elems(java.util.Collection<? extends String> elems) {
+      if ((this.elems == null))
+          this.elems = new java.util.ArrayList<String>();
+      this.elems.addAll(elems);
+      return this;
+    }
+    public @java.lang.SuppressWarnings("all") BuilderSingularToBuilderWithNullBuilder clearElems() {
+      if ((this.elems != null))
+          this.elems.clear();
+      return this;
+    }
+    public @java.lang.SuppressWarnings("all") BuilderSingularToBuilderWithNull build() {
+      java.util.List<String> elems;
+      switch (((this.elems == null) ? 0 : this.elems.size())) {
+      case 0 :
+          elems = java.util.Collections.emptyList();
+          break;
+      case 1 :
+          elems = java.util.Collections.singletonList(this.elems.get(0));
+          break;
+      default :
+          elems = java.util.Collections.unmodifiableList(new java.util.ArrayList<String>(this.elems));
+      }
+      return new BuilderSingularToBuilderWithNull(elems);
+    }
+    public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() {
+      return (("BuilderSingularToBuilderWithNull.BuilderSingularToBuilderWithNullBuilder(elems=" + this.elems) + ")");
+    }
+  }
+  private @Singular java.util.List<String> elems;
+  public static void test() {
+    new BuilderSingularToBuilderWithNull(null).toBuilder();
+  }
+  @java.lang.SuppressWarnings("all") BuilderSingularToBuilderWithNull(final java.util.List<String> elems) {
+    super();
+    this.elems = elems;
+  }
+  public static @java.lang.SuppressWarnings("all") BuilderSingularToBuilderWithNullBuilder builder() {
+    return new BuilderSingularToBuilderWithNullBuilder();
+  }
+  public @java.lang.SuppressWarnings("all") BuilderSingularToBuilderWithNullBuilder toBuilder() {
+    return new BuilderSingularToBuilderWithNullBuilder().elems(((this.elems == null) ? java.util.Collections.emptyList() : this.elems));
+  }
+}
diff --git a/test/transform/resource/after-ecj/BuilderWithToBuilder.java b/test/transform/resource/after-ecj/BuilderWithToBuilder.java
index d304293c..b9cc27dd 100644
--- a/test/transform/resource/after-ecj/BuilderWithToBuilder.java
+++ b/test/transform/resource/after-ecj/BuilderWithToBuilder.java
@@ -74,7 +74,7 @@ import lombok.Builder;
     return new BuilderWithToBuilderBuilder<T>();
   }
   public @java.lang.SuppressWarnings("all") BuilderWithToBuilderBuilder<T> toBuilder() {
-    return new BuilderWithToBuilderBuilder<T>().one(this.mOne).two(this.mTwo).foo(BuilderWithToBuilder.rrr(this)).bars(this.bars);
+    return new BuilderWithToBuilderBuilder<T>().one(this.mOne).two(this.mTwo).foo(BuilderWithToBuilder.rrr(this)).bars(((this.bars == null) ? java.util.Collections.emptyList() : this.bars));
   }
 }
 @lombok.experimental.Accessors(prefix = "m") class ConstructorWithToBuilder<T> {
diff --git a/test/transform/resource/before/BuilderSingularToBuilderWithNull.java b/test/transform/resource/before/BuilderSingularToBuilderWithNull.java
new file mode 100644
index 00000000..92a102cb
--- /dev/null
+++ b/test/transform/resource/before/BuilderSingularToBuilderWithNull.java
@@ -0,0 +1,10 @@
+import lombok.Singular;
+
+@lombok.Builder(toBuilder = true)
+class BuilderSingularToBuilderWithNull {
+	@Singular private java.util.List<String> elems;
+	
+	public static void test() {
+		new BuilderSingularToBuilderWithNull(null).toBuilder();
+	}
+}