[fixes #2120] ecj was not generating explicit nullchecks for builder-setters.

9 files changed, 112 insertions, 4 deletions

diff --git a/doc/changelog.markdown b/doc/changelog.markdown
index acaa74de..0ed8afe1 100644
--- a/doc/changelog.markdown
+++ b/doc/changelog.markdown
@@ -13,6 +13,7 @@ Lombok Changelog
 * BUGFIX: If you use `@Builder` and manually write the `build()` method in your builder class, javac would error out instead of deferring to your implementation. [Issue #2050](https://github.com/rzwitserloot/lombok/issues/2050) [Issue #2061](https://github.com/rzwitserloot/lombok/issues/2061)
 * BUGFIX: `@SuperBuilder` together with `@Singular` on non-lists would produce an erroneous `emptyList` call. [Issue #2104](https://github.com/rzwitserloot/lombok/issues/2104).
 * IMPROBABLE BREAKING CHANGE: For fields and parameters marked non-null, if the method body starts with an assert statement to ensure the value isn't null, no code to throw an exception will be generated.
+* IMPROBABLE BREAKING CHANGE: When using `ecj` to compile java code with `@Builder` or `@SuperBuilder` in it, and a builder setter method was generated for a `@NonNull`-marked method, no explicit null check would be present. However, running `javac` on the exact same file _would_ produce the null check. Now ecj also produces this null check. [Issue #2120](https://github.com/rzwitserloot/lombok/issues/2120).
 
 ### v1.18.6 (February 12th, 2019)
 * FEATURE: Javadoc on fields will now also be copied to the Builders' setters. Thanks for the contribution, Emil Lundberg. [Issue #2008](https://github.com/rzwitserloot/lombok/issues/2008)
diff --git a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
index 010dc9d8..ddb2f198 100644
--- a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
+++ b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
@@ -725,6 +725,17 @@ public class EclipseHandlerUtil {
 		return false;
 	}
 	
+	public static boolean hasNonNullAnnotations(EclipseNode node, List<Annotation> anns) {
+		if (anns == null) return false;
+		for (Annotation annotation : anns) {
+			TypeReference typeRef = annotation.type;
+			if (typeRef != null && typeRef.getTypeName() != null) {
+				for (String bn : NONNULL_ANNOTATIONS) if (typeMatches(bn, node, typeRef)) return true;
+			}
+		}
+		return false;
+	}
+	
 	private static final Annotation[] EMPTY_ANNOTATIONS_ARRAY = new Annotation[0];
 	
 	/**
diff --git a/src/core/lombok/eclipse/handlers/HandleSetter.java b/src/core/lombok/eclipse/handlers/HandleSetter.java
index 529a7d19..d70d4acd 100644
--- a/src/core/lombok/eclipse/handlers/HandleSetter.java
+++ b/src/core/lombok/eclipse/handlers/HandleSetter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2017 The Project Lombok Authors.
+ * Copyright (C) 2009-2019 The Project Lombok Authors.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -238,7 +238,7 @@ public class HandleSetter extends EclipseAnnotationHandler<Setter> {
 		
 		Annotation[] copyableAnnotations = findCopyableAnnotations(fieldNode);
 		List<Statement> statements = new ArrayList<Statement>(5);
-		if (!hasNonNullAnnotations(fieldNode)) {
+		if (!hasNonNullAnnotations(fieldNode) && !hasNonNullAnnotations(fieldNode, onParam)) {
 			statements.add(assignment);
 		} else {
 			Statement nullCheck = generateNullCheck(field, sourceNode);
diff --git a/src/core/lombok/javac/handlers/HandleSetter.java b/src/core/lombok/javac/handlers/HandleSetter.java
index 28f5318d..c661ab89 100644
--- a/src/core/lombok/javac/handlers/HandleSetter.java
+++ b/src/core/lombok/javac/handlers/HandleSetter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2017 The Project Lombok Authors.
+ * Copyright (C) 2009-2019 The Project Lombok Authors.
  * 
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -234,7 +234,7 @@ public class HandleSetter extends JavacAnnotationHandler<Setter> {
 		long flags = JavacHandlerUtil.addFinalIfNeeded(Flags.PARAMETER, field.getContext());
 		JCVariableDecl param = treeMaker.VarDef(treeMaker.Modifiers(flags, annsOnParam), fieldDecl.name, fieldDecl.vartype, null);
 		
-		if (!hasNonNullAnnotations(field)) {
+		if (!hasNonNullAnnotations(field) && !hasNonNullAnnotations(field, onParam)) {
 			statements.append(treeMaker.Exec(assign));
 		} else {
 			JCStatement nullCheck = generateNullCheck(treeMaker, field, source);
diff --git a/src/core/lombok/javac/handlers/JavacHandlerUtil.java b/src/core/lombok/javac/handlers/JavacHandlerUtil.java
index 509a7397..e923c5c0 100644
--- a/src/core/lombok/javac/handlers/JavacHandlerUtil.java
+++ b/src/core/lombok/javac/handlers/JavacHandlerUtil.java
@@ -1418,6 +1418,15 @@ public class JavacHandlerUtil {
 		return false;
 	}
 	
+	public static boolean hasNonNullAnnotations(JavacNode node, List<JCAnnotation> anns) {
+		if (anns == null) return false;
+		for (JCAnnotation ann : anns) {
+			for (String nn : NONNULL_ANNOTATIONS) if (typeMatches(nn, node, ann)) return true;
+		}
+		
+		return false;
+	}
+	
 	/**
 	 * Searches the given field node for annotations and returns each one that is 'copyable' (either via configuration or from the base list).
 	 */
diff --git a/test/transform/resource/after-delombok/BuilderWithNonNull.java b/test/transform/resource/after-delombok/BuilderWithNonNull.java
new file mode 100644
index 00000000..bee7d415
--- /dev/null
+++ b/test/transform/resource/after-delombok/BuilderWithNonNull.java
@@ -0,0 +1,40 @@
+class BuilderWithNonNull {
+	@lombok.NonNull
+	private final String id;
+	@java.lang.SuppressWarnings("all")
+	BuilderWithNonNull(@lombok.NonNull final String id) {
+		if (id == null) {
+			throw new java.lang.NullPointerException("id is marked non-null but is null");
+		}
+		this.id = id;
+	}
+	@java.lang.SuppressWarnings("all")
+	public static class BuilderWithNonNullBuilder {
+		@java.lang.SuppressWarnings("all")
+		private String id;
+		@java.lang.SuppressWarnings("all")
+		BuilderWithNonNullBuilder() {
+		}
+		@java.lang.SuppressWarnings("all")
+		public BuilderWithNonNullBuilder id(@lombok.NonNull final String id) {
+			if (id == null) {
+				throw new java.lang.NullPointerException("id is marked non-null but is null");
+			}
+			this.id = id;
+			return this;
+		}
+		@java.lang.SuppressWarnings("all")
+		public BuilderWithNonNull build() {
+			return new BuilderWithNonNull(id);
+		}
+		@java.lang.Override
+		@java.lang.SuppressWarnings("all")
+		public java.lang.String toString() {
+			return "BuilderWithNonNull.BuilderWithNonNullBuilder(id=" + this.id + ")";
+		}
+	}
+	@java.lang.SuppressWarnings("all")
+	public static BuilderWithNonNullBuilder builder() {
+		return new BuilderWithNonNullBuilder();
+	}
+}
\ No newline at end of file
diff --git a/test/transform/resource/after-ecj/BuilderWithNonNull.java b/test/transform/resource/after-ecj/BuilderWithNonNull.java
new file mode 100644
index 00000000..a8ef93f0
--- /dev/null
+++ b/test/transform/resource/after-ecj/BuilderWithNonNull.java
@@ -0,0 +1,34 @@
+@lombok.Builder class BuilderWithNonNull {
+  public static @java.lang.SuppressWarnings("all") class BuilderWithNonNullBuilder {
+    private @java.lang.SuppressWarnings("all") String id;
+    @java.lang.SuppressWarnings("all") BuilderWithNonNullBuilder() {
+      super();
+    }
+    public @java.lang.SuppressWarnings("all") BuilderWithNonNullBuilder id(final @lombok.NonNull String id) {
+      if ((id == null))
+          {
+            throw new java.lang.NullPointerException("id is marked non-null but is null");
+          }
+      this.id = id;
+      return this;
+    }
+    public @java.lang.SuppressWarnings("all") BuilderWithNonNull build() {
+      return new BuilderWithNonNull(id);
+    }
+    public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() {
+      return (("BuilderWithNonNull.BuilderWithNonNullBuilder(id=" + this.id) + ")");
+    }
+  }
+  private final @lombok.NonNull String id;
+  @java.lang.SuppressWarnings("all") BuilderWithNonNull(final @lombok.NonNull String id) {
+    super();
+    if ((id == null))
+        {
+          throw new java.lang.NullPointerException("id is marked non-null but is null");
+        }
+    this.id = id;
+  }
+  public static @java.lang.SuppressWarnings("all") BuilderWithNonNullBuilder builder() {
+    return new BuilderWithNonNullBuilder();
+  }
+}
\ No newline at end of file
diff --git a/test/transform/resource/after-ecj/SuperBuilderWithNonNull.java b/test/transform/resource/after-ecj/SuperBuilderWithNonNull.java
index 4b5cb188..616d7083 100644
--- a/test/transform/resource/after-ecj/SuperBuilderWithNonNull.java
+++ b/test/transform/resource/after-ecj/SuperBuilderWithNonNull.java
@@ -10,6 +10,10 @@ public class SuperBuilderWithNonNull {
       protected abstract @java.lang.SuppressWarnings("all") B self();

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

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

+        if ((nonNullParentField == null))

+            {

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

+            }

         this.nonNullParentField = nonNullParentField;

         nonNullParentField$set = true;

         return self();

@@ -57,6 +61,10 @@ public class SuperBuilderWithNonNull {
       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 @lombok.NonNull String nonNullChildField) {

+        if ((nonNullChildField == null))

+            {

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

+            }

         this.nonNullChildField = nonNullChildField;

         return self();

       }

diff --git a/test/transform/resource/before/BuilderWithNonNull.java b/test/transform/resource/before/BuilderWithNonNull.java
new file mode 100644
index 00000000..03a54326
--- /dev/null
+++ b/test/transform/resource/before/BuilderWithNonNull.java
@@ -0,0 +1,5 @@
+@lombok.Builder
+class BuilderWithNonNull {
+	@lombok.NonNull
+	private final String id;
+}