Merge branch 'janrieke-superBuilderNonNull'

7 files changed, 272 insertions, 36 deletions

diff --git a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
index 8badb04c..a0102220 100644
--- a/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
+++ b/src/core/lombok/eclipse/handlers/HandleSuperBuilder.java
@@ -456,7 +456,6 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 		constructor.arguments = new Argument[] {new Argument("b".toCharArray(), p, builderType, Modifier.FINAL)};
 		
 		List<Statement> statements = new ArrayList<Statement>();
-		List<Statement> nullChecks = new ArrayList<Statement>();
 		
 		for (BuilderFieldData fieldNode : builderFields) {
 			char[] fieldName = removePrefixFromField(fieldNode.originalFieldNode);
@@ -475,7 +474,6 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 				assignmentExpr = new QualifiedNameReference(variableInBuilder, positions, s, e);
 			}
 			Statement assignment = new Assignment(fieldInThis, assignmentExpr, (int) p);
-			statements.add(assignment);
 			
 			// In case of @Builder.Default, set the value to the default if it was NOT set in the builder.
 			if (fieldNode.nameOfSetFlag != null) {
@@ -483,29 +481,30 @@ public class HandleSuperBuilder extends EclipseAnnotationHandler<SuperBuilder> {
 				long[] positions = new long[] {p, p};
 				QualifiedNameReference setVariableInBuilderRef = new QualifiedNameReference(setVariableInBuilder, positions, s, e);
 
-				MessageSend inv = new MessageSend();
-				inv.sourceStart = source.sourceStart;
-				inv.sourceEnd = source.sourceEnd;
-				inv.receiver = new SingleNameReference(((TypeDeclaration) typeNode.get()).name, 0L);
-				inv.selector = fieldNode.nameOfDefaultProvider;
-				inv.typeArguments = typeParameterNames(((TypeDeclaration) typeNode.get()).typeParameters);
+				MessageSend defaultMethodCall = new MessageSend();
+				defaultMethodCall.sourceStart = source.sourceStart;
+				defaultMethodCall.sourceEnd = source.sourceEnd;
+				defaultMethodCall.receiver = new SingleNameReference(((TypeDeclaration) typeNode.get()).name, 0L);
+				defaultMethodCall.selector = fieldNode.nameOfDefaultProvider;
+				defaultMethodCall.typeArguments = typeParameterNames(((TypeDeclaration) typeNode.get()).typeParameters);
 				
-				assignment = new Assignment(fieldInThis, inv, (int) p);
-				IfStatement ifBlockForDefault = new IfStatement(new UnaryExpression(setVariableInBuilderRef, OperatorIds.NOT), assignment, s, e);
+				Statement defaultAssignment = new Assignment(fieldInThis, defaultMethodCall, (int) p);
+				IfStatement ifBlockForDefault = new IfStatement(setVariableInBuilderRef, assignment, defaultAssignment, s, e);
 				statements.add(ifBlockForDefault);
+			} else {
+				statements.add(assignment);
 			}
 			
 			Annotation[] nonNulls = findAnnotations((FieldDeclaration)fieldNode.originalFieldNode.get(), NON_NULL_PATTERN);
 			if (nonNulls.length != 0) {
 				Statement nullCheck = generateNullCheck((FieldDeclaration)fieldNode.originalFieldNode.get(), sourceNode);
 				if (nullCheck != null) {
-					nullChecks.add(nullCheck);
+					statements.add(nullCheck);
 				}
 			}
 		}
 		
-		nullChecks.addAll(statements);
-		constructor.statements = nullChecks.isEmpty() ? null : nullChecks.toArray(new Statement[nullChecks.size()]);
+		constructor.statements = statements.isEmpty() ? null : statements.toArray(new Statement[statements.size()]);
 		
 		constructor.traverse(new SetGeneratedByVisitor(source), typeDeclaration.scope);
 		
diff --git a/src/core/lombok/javac/handlers/HandleSuperBuilder.java b/src/core/lombok/javac/handlers/HandleSuperBuilder.java
index c00bd66d..aaf94dd8 100644
--- a/src/core/lombok/javac/handlers/HandleSuperBuilder.java
+++ b/src/core/lombok/javac/handlers/HandleSuperBuilder.java
@@ -406,17 +406,10 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 		
 		AccessLevel level = AccessLevel.PROTECTED;
 		
-		ListBuffer<JCStatement> nullChecks = new ListBuffer<JCStatement>();
 		ListBuffer<JCStatement> statements = new ListBuffer<JCStatement>();
 		
 		Name builderVariableName = typeNode.toName("b");
 		for (BuilderFieldData bfd : builderFields) {
-			List<JCAnnotation> nonNulls = findAnnotations(bfd.originalFieldNode, NON_NULL_PATTERN);
-			if (!nonNulls.isEmpty()) {
-				JCStatement nullCheck = generateNullCheck(maker, bfd.originalFieldNode, source);
-				if (nullCheck != null) nullChecks.append(nullCheck);
-			}
-			
 			JCExpression rhs;
 			if (bfd.singularData != null && bfd.singularData.getSingularizer() != null) {
 				bfd.singularData.getSingularizer().appendBuildCode(bfd.singularData, bfd.originalFieldNode, bfd.type, statements, bfd.name, "b");
@@ -427,14 +420,21 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 			JCFieldAccess fieldInThis = maker.Select(maker.Ident(typeNode.toName("this")), bfd.rawName);
 			
 			JCStatement assign = maker.Exec(maker.Assign(fieldInThis, rhs));
-			statements.append(assign);
 			
-			// In case of @Builder.Default, set the value to the default if it was NOT set in the builder.
+			// In case of @Builder.Default, set the value to the default if it was not set in the builder.
 			if (bfd.nameOfSetFlag != null) {
 				JCFieldAccess setField = maker.Select(maker.Ident(builderVariableName), bfd.nameOfSetFlag);
 				fieldInThis = maker.Select(maker.Ident(typeNode.toName("this")), bfd.rawName);
 				JCAssign assignDefault = maker.Assign(fieldInThis, maker.Apply(typeParameterNames(maker, ((JCClassDecl) typeNode.get()).typarams), maker.Select(maker.Ident(((JCClassDecl) typeNode.get()).name), bfd.nameOfDefaultProvider), List.<JCExpression>nil()));
-				statements.append(maker.If(maker.Unary(CTC_NOT, setField), maker.Exec(assignDefault), null));
+				statements.append(maker.If(setField, assign, maker.Exec(assignDefault)));
+			} else {
+				statements.append(assign);
+			}
+			
+			List<JCAnnotation> nonNulls = findAnnotations(bfd.originalFieldNode, NON_NULL_PATTERN);
+			if (!nonNulls.isEmpty()) {
+				JCStatement nullCheck = generateNullCheck(maker, bfd.originalFieldNode, source);
+				if (nullCheck != null) statements.append(nullCheck);
 			}
 		}
 		
@@ -465,7 +465,7 @@ public class HandleSuperBuilder extends JavacAnnotationHandler<SuperBuilder> {
 		
 		JCMethodDecl constr = recursiveSetGeneratedBy(maker.MethodDef(mods, typeNode.toName("<init>"),
 			null, List.<JCTypeParameter>nil(), params.toList(), List.<JCExpression>nil(),
-			maker.Block(0L, nullChecks.appendList(statements).toList()), null), source.get(), typeNode.getContext());
+			maker.Block(0L, statements.toList()), null), source.get(), typeNode.getContext());
 		
 		injectMethod(typeNode, constr, null, Javac.createVoidType(typeNode.getSymbolTable(), CTC_VOID));
 	}
diff --git a/test/transform/resource/after-delombok/SuperBuilderWithDefaults.java b/test/transform/resource/after-delombok/SuperBuilderWithDefaults.java
index 06d88f4d..17707bf5 100644
--- a/test/transform/resource/after-delombok/SuperBuilderWithDefaults.java
+++ b/test/transform/resource/after-delombok/SuperBuilderWithDefaults.java
@@ -61,10 +61,10 @@ public class SuperBuilderWithDefaults {
 		}

 		@java.lang.SuppressWarnings("all")

 		protected Parent(final ParentBuilder<N, ?, ?> b) {

-			this.millis = b.millis;

-			if (!b.millis$set) this.millis = Parent.<N>$default$millis();

-			this.numberField = b.numberField;

-			if (!b.numberField$set) this.numberField = Parent.<N>$default$numberField();

+			if (b.millis$set) this.millis = b.millis;

+			 else this.millis = Parent.<N>$default$millis();

+			if (b.numberField$set) this.numberField = b.numberField; 

+			 else this.numberField = Parent.<N>$default$numberField();

 		}

 		@java.lang.SuppressWarnings("all")

 		public static <N extends Number> ParentBuilder<N, ?, ?> builder() {

@@ -120,8 +120,8 @@ public class SuperBuilderWithDefaults {
 		@java.lang.SuppressWarnings("all")

 		protected Child(final ChildBuilder<?, ?> b) {

 			super(b);

-			this.doubleField = b.doubleField;

-			if (!b.doubleField$set) this.doubleField = Child.$default$doubleField();

+			if (b.doubleField$set) this.doubleField = b.doubleField;

+			 else this.doubleField = Child.$default$doubleField();

 		}

 		@java.lang.SuppressWarnings("all")

 		public static ChildBuilder<?, ?> builder() {

diff --git a/test/transform/resource/after-delombok/SuperBuilderWithNonNull.java b/test/transform/resource/after-delombok/SuperBuilderWithNonNull.java
new file mode 100644
index 00000000..5eba938f
--- /dev/null
+++ b/test/transform/resource/after-delombok/SuperBuilderWithNonNull.java
@@ -0,0 +1,117 @@
+import java.util.List;

+public class SuperBuilderWithNonNull {

+	public static class Parent {

+		@lombok.NonNull

+		final String nonNullParentField;

+		@java.lang.SuppressWarnings("all")

+		private static String $default$nonNullParentField() {

+			return "default";

+		}

+		@java.lang.SuppressWarnings("all")

+		public static abstract class ParentBuilder<C extends Parent, B extends ParentBuilder<C, B>> {

+			@java.lang.SuppressWarnings("all")

+			private boolean nonNullParentField$set;

+			@java.lang.SuppressWarnings("all")

+			private String nonNullParentField;

+			@java.lang.SuppressWarnings("all")

+			protected abstract B self();

+			@java.lang.SuppressWarnings("all")

+			public abstract C build();

+			@java.lang.SuppressWarnings("all")

+			public B nonNullParentField(final String nonNullParentField) {

+				this.nonNullParentField = nonNullParentField;

+				nonNullParentField$set = true;

+				return self();

+			}

+			@java.lang.Override

+			@java.lang.SuppressWarnings("all")

+			public java.lang.String toString() {

+				return "SuperBuilderWithNonNull.Parent.ParentBuilder(nonNullParentField=" + this.nonNullParentField + ")";

+			}

+		}

+		@java.lang.SuppressWarnings("all")

+		private static final class ParentBuilderImpl extends ParentBuilder<Parent, ParentBuilderImpl> {

+			@java.lang.SuppressWarnings("all")

+			private ParentBuilderImpl() {

+			}

+			@java.lang.Override

+			@java.lang.SuppressWarnings("all")

+			protected ParentBuilderImpl self() {

+				return this;

+			}

+			@java.lang.Override

+			@java.lang.SuppressWarnings("all")

+			public Parent build() {

+				return new Parent(this);

+			}

+		}

+		@java.lang.SuppressWarnings("all")

+		protected Parent(final ParentBuilder<?, ?> b) {

+			if (b.nonNullParentField$set) this.nonNullParentField = b.nonNullParentField;

+			 else this.nonNullParentField = Parent.$default$nonNullParentField();

+			if (nonNullParentField == null) {

+				throw new java.lang.NullPointerException("nonNullParentField is marked @NonNull but is null");

+			}

+		}

+		@java.lang.SuppressWarnings("all")

+		public static ParentBuilder<?, ?> builder() {

+			return new ParentBuilderImpl();

+		}

+	}

+	public static class Child extends Parent {

+		@lombok.NonNull

+		String nonNullChildField;

+		@java.lang.SuppressWarnings("all")

+		public static abstract class ChildBuilder<C extends Child, B extends ChildBuilder<C, B>> extends Parent.ParentBuilder<C, B> {

+			@java.lang.SuppressWarnings("all")

+			private String nonNullChildField;

+			@java.lang.Override

+			@java.lang.SuppressWarnings("all")

+			protected abstract B self();

+			@java.lang.Override

+			@java.lang.SuppressWarnings("all")

+			public abstract C build();

+			@java.lang.SuppressWarnings("all")

+			public B nonNullChildField(final String nonNullChildField) {

+				this.nonNullChildField = nonNullChildField;

+				return self();

+			}

+			@java.lang.Override

+			@java.lang.SuppressWarnings("all")

+			public java.lang.String toString() {

+				return "SuperBuilderWithNonNull.Child.ChildBuilder(super=" + super.toString() + ", nonNullChildField=" + this.nonNullChildField + ")";

+			}

+		}

+		@java.lang.SuppressWarnings("all")

+		private static final class ChildBuilderImpl extends ChildBuilder<Child, ChildBuilderImpl> {

+			@java.lang.SuppressWarnings("all")

+			private ChildBuilderImpl() {

+			}

+			@java.lang.Override

+			@java.lang.SuppressWarnings("all")

+			protected ChildBuilderImpl self() {

+				return this;

+			}

+			@java.lang.Override

+			@java.lang.SuppressWarnings("all")

+			public Child build() {

+				return new Child(this);

+			}

+		}

+		@java.lang.SuppressWarnings("all")

+		protected Child(final ChildBuilder<?, ?> b) {

+			super(b);

+			this.nonNullChildField = b.nonNullChildField;

+			if (nonNullChildField == null) {

+				throw new java.lang.NullPointerException("nonNullChildField is marked @NonNull but is null");

+			}

+		}

+		@java.lang.SuppressWarnings("all")

+		public static ChildBuilder<?, ?> builder() {

+			return new ChildBuilderImpl();

+		}

+	}

+	public static void test() {

+		Child x = Child.builder().nonNullChildField("child").nonNullParentField("parent").build();

+	}

+}

diff --git a/test/transform/resource/after-ecj/SuperBuilderWithDefaults.java b/test/transform/resource/after-ecj/SuperBuilderWithDefaults.java
index 731bd9f2..ef0713c5 100644
--- a/test/transform/resource/after-ecj/SuperBuilderWithDefaults.java
+++ b/test/transform/resource/after-ecj/SuperBuilderWithDefaults.java
@@ -46,11 +46,13 @@ public class SuperBuilderWithDefaults {
     }

     protected @java.lang.SuppressWarnings("all") Parent(final ParentBuilder<N, ?, ?> b) {

       super();

-      this.millis = b.millis;

-      if ((! b.millis$set))

+      if (b.millis$set)

+          this.millis = b.millis;

+      else

           this.millis = Parent.<N>$default$millis();

-      this.numberField = b.numberField;

-      if ((! b.numberField$set)) 

+      if (b.numberField$set)

+          this.numberField = b.numberField;

+      else

           this.numberField = Parent.<N>$default$numberField();

     }

     public static @java.lang.SuppressWarnings("all") <N extends Number>ParentBuilder<N, ?, ?> builder() {

@@ -92,8 +94,9 @@ public class SuperBuilderWithDefaults {
     }

     protected @java.lang.SuppressWarnings("all") Child(final ChildBuilder<?, ?> b) {

       super(b);

-      this.doubleField = b.doubleField;

-      if ((! b.doubleField$set)) 

+      if (b.doubleField$set) 

+          this.doubleField = b.doubleField;

+      else

           this.doubleField = Child.$default$doubleField();

     }

     public static @java.lang.SuppressWarnings("all") ChildBuilder<?, ?> builder() {

diff --git a/test/transform/resource/after-ecj/SuperBuilderWithNonNull.java b/test/transform/resource/after-ecj/SuperBuilderWithNonNull.java
new file mode 100644
index 00000000..106b8326
--- /dev/null
+++ b/test/transform/resource/after-ecj/SuperBuilderWithNonNull.java
@@ -0,0 +1,97 @@
+import java.util.List;

+public class SuperBuilderWithNonNull {

+  public static @lombok.experimental.SuperBuilder class Parent {

+    public static abstract @java.lang.SuppressWarnings("all") class ParentBuilder<C extends Parent, B extends ParentBuilder<C, B>> {

+      private @java.lang.SuppressWarnings("all") String nonNullParentField;

+      private @java.lang.SuppressWarnings("all") boolean nonNullParentField$set;

+      public ParentBuilder() {

+        super();

+      }

+      protected abstract @java.lang.SuppressWarnings("all") B self();

+      public abstract @java.lang.SuppressWarnings("all") C build();

+      public @java.lang.SuppressWarnings("all") B nonNullParentField(final String nonNullParentField) {

+        this.nonNullParentField = nonNullParentField;

+        nonNullParentField$set = true;

+        return self();

+      }

+      public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() {

+        return (("SuperBuilderWithNonNull.Parent.ParentBuilder(nonNullParentField=" + this.nonNullParentField) + ")");

+      }

+    }

+    private static final @java.lang.SuppressWarnings("all") class ParentBuilderImpl extends ParentBuilder<Parent, ParentBuilderImpl> {

+      private ParentBuilderImpl() {

+        super();

+      }

+      protected @java.lang.Override @java.lang.SuppressWarnings("all") ParentBuilderImpl self() {

+        return this;

+      }

+      public @java.lang.Override @java.lang.SuppressWarnings("all") Parent build() {

+        return new Parent(this);

+      }

+    }

+    final @lombok.NonNull @lombok.Builder.Default String nonNullParentField;

+    private static @java.lang.SuppressWarnings("all") String $default$nonNullParentField() {

+      return "default";

+    }

+    protected @java.lang.SuppressWarnings("all") Parent(final ParentBuilder<?, ?> b) {

+      super();

+      if (b.nonNullParentField$set)

+          this.nonNullParentField = b.nonNullParentField;

+      else

+          this.nonNullParentField = Parent.$default$nonNullParentField();

+      if ((nonNullParentField == null))

+          {

+            throw new java.lang.NullPointerException("nonNullParentField is marked @NonNull but is null");

+          }

+    }

+    public static @java.lang.SuppressWarnings("all") ParentBuilder<?, ?> builder() {

+      return new ParentBuilderImpl();

+    }

+  }

+  public static @lombok.experimental.SuperBuilder class Child extends Parent {

+    public static abstract @java.lang.SuppressWarnings("all") class ChildBuilder<C extends Child, B extends ChildBuilder<C, B>> extends Parent.ParentBuilder<C, B> {

+      private @java.lang.SuppressWarnings("all") String nonNullChildField;

+      public ChildBuilder() {

+        super();

+      }

+      protected abstract @java.lang.Override @java.lang.SuppressWarnings("all") B self();

+      public abstract @java.lang.Override @java.lang.SuppressWarnings("all") C build();

+      public @java.lang.SuppressWarnings("all") B nonNullChildField(final String nonNullChildField) {

+        this.nonNullChildField = nonNullChildField;

+        return self();

+      }

+      public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() {

+        return (((("SuperBuilderWithNonNull.Child.ChildBuilder(super=" + super.toString()) + ", nonNullChildField=") + this.nonNullChildField) + ")");

+      }

+    }

+    private static final @java.lang.SuppressWarnings("all") class ChildBuilderImpl extends ChildBuilder<Child, ChildBuilderImpl> {

+      private ChildBuilderImpl() {

+        super();

+      }

+      protected @java.lang.Override @java.lang.SuppressWarnings("all") ChildBuilderImpl self() {

+        return this;

+      }

+      public @java.lang.Override @java.lang.SuppressWarnings("all") Child build() {

+        return new Child(this);

+      }

+    }

+    @lombok.NonNull String nonNullChildField;

+    protected @java.lang.SuppressWarnings("all") Child(final ChildBuilder<?, ?> b) {

+      super(b);

+      this.nonNullChildField = b.nonNullChildField;

+      if ((nonNullChildField == null))

+          {

+            throw new java.lang.NullPointerException("nonNullChildField is marked @NonNull but is null");

+          }

+    }

+    public static @java.lang.SuppressWarnings("all") ChildBuilder<?, ?> builder() {

+      return new ChildBuilderImpl();

+    }

+  }

+  public SuperBuilderWithNonNull() {

+    super();

+  }

+  public static void test() {

+    Child x = Child.builder().nonNullChildField("child").nonNullParentField("parent").build();

+  }

+}

diff --git a/test/transform/resource/before/SuperBuilderWithNonNull.java b/test/transform/resource/before/SuperBuilderWithNonNull.java
new file mode 100644
index 00000000..34668bbc
--- /dev/null
+++ b/test/transform/resource/before/SuperBuilderWithNonNull.java
@@ -0,0 +1,20 @@
+import java.util.List;
+
+public class SuperBuilderWithNonNull {
+	@lombok.experimental.SuperBuilder
+	public static class Parent {
+		@lombok.NonNull
+		@lombok.Builder.Default
+		final String nonNullParentField = "default";
+	}
+	
+	@lombok.experimental.SuperBuilder
+	public static class Child extends Parent {
+		@lombok.NonNull
+		String nonNullChildField;
+	}
+	
+	public static void test() {
+		Child x = Child.builder().nonNullChildField("child").nonNullParentField("parent").build();
+	}
+}