fixed issue 391: Using 'staticConstructor' on @Data whilst an @XxxArgsConstructor is present means it gets ignored, but until now lombok didn't warn you about this.

7 files changed, 84 insertions, 6 deletions

diff --git a/src/core/lombok/eclipse/handlers/HandleConstructor.java b/src/core/lombok/eclipse/handlers/HandleConstructor.java
index eec41577..25d47870 100644
--- a/src/core/lombok/eclipse/handlers/HandleConstructor.java
+++ b/src/core/lombok/eclipse/handlers/HandleConstructor.java
@@ -156,20 +156,29 @@ public class HandleConstructor {
 	}
 	
 	public void generateConstructor(EclipseNode typeNode, AccessLevel level, List<EclipseNode> fields, String staticName, boolean skipIfConstructorExists, boolean suppressConstructorProperties, ASTNode source) {
+		boolean staticConstrRequired = staticName != null && !staticName.equals("");
+		
 		if (skipIfConstructorExists && constructorExists(typeNode) != MemberExistsResult.NOT_EXISTS) return;
 		if (skipIfConstructorExists) {
 			for (EclipseNode child : typeNode.down()) {
 				if (child.getKind() == Kind.ANNOTATION) {
 					if (annotationTypeMatches(NoArgsConstructor.class, child) ||
 							annotationTypeMatches(AllArgsConstructor.class, child) ||
-							annotationTypeMatches(RequiredArgsConstructor.class, child))
+							annotationTypeMatches(RequiredArgsConstructor.class, child)) {
+						
+						if (staticConstrRequired) {
+							// @Data has asked us to generate a constructor, but we're going to skip this instruction, as an explicit 'make a constructor' annotation
+							// will take care of it. However, @Data also wants a specific static name; this will be ignored; the appropriate way to do this is to use
+							// the 'staticName' parameter of the @XArgsConstructor you've stuck on your type.
+							// We should warn that we're ignoring @Data's 'staticConstructor' param.
+							typeNode.addWarning("Ignoring static constructor name: explicit @XxxArgsConstructor annotation present; its `staticName` parameter will be used.", source.sourceStart, source.sourceEnd);
+						}
 						return;
+					}
 				}
 			}
 		}
 		
-		boolean staticConstrRequired = staticName != null && !staticName.equals("");
-		
 		ConstructorDeclaration constr = createConstructor(staticConstrRequired ? AccessLevel.PRIVATE : level, typeNode, fields, suppressConstructorProperties, source);
 		injectMethod(typeNode, constr);
 		if (staticConstrRequired) {
diff --git a/src/core/lombok/javac/handlers/HandleConstructor.java b/src/core/lombok/javac/handlers/HandleConstructor.java
index a2463728..65ad2547 100644
--- a/src/core/lombok/javac/handlers/HandleConstructor.java
+++ b/src/core/lombok/javac/handlers/HandleConstructor.java
@@ -154,20 +154,29 @@ public class HandleConstructor {
 	}
 	
 	public void generateConstructor(JavacNode typeNode, AccessLevel level, List<JavacNode> fields, String staticName, boolean skipIfConstructorExists, boolean suppressConstructorProperties, JavacNode source) {
+		boolean staticConstrRequired = staticName != null && !staticName.equals("");
+		
 		if (skipIfConstructorExists && constructorExists(typeNode) != MemberExistsResult.NOT_EXISTS) return;
 		if (skipIfConstructorExists) {
 			for (JavacNode child : typeNode.down()) {
 				if (child.getKind() == Kind.ANNOTATION) {
 					if (annotationTypeMatches(NoArgsConstructor.class, child) ||
 							annotationTypeMatches(AllArgsConstructor.class, child) ||
-							annotationTypeMatches(RequiredArgsConstructor.class, child))
+							annotationTypeMatches(RequiredArgsConstructor.class, child)) {
+						
+						if (staticConstrRequired) {
+							// @Data has asked us to generate a constructor, but we're going to skip this instruction, as an explicit 'make a constructor' annotation
+							// will take care of it. However, @Data also wants a specific static name; this will be ignored; the appropriate way to do this is to use
+							// the 'staticName' parameter of the @XArgsConstructor you've stuck on your type.
+							// We should warn that we're ignoring @Data's 'staticConstructor' param.
+							source.addWarning("Ignoring static constructor name: explicit @XxxArgsConstructor annotation present; its `staticName` parameter will be used.");
+						}
 						return;
+					}
 				}
 			}
 		}
 		
-		boolean staticConstrRequired = staticName != null && !staticName.equals("");
-		
 		JCMethodDecl constr = createConstructor(staticConstrRequired ? AccessLevel.PRIVATE : level, typeNode, fields, suppressConstructorProperties, source.get());
 		injectMethod(typeNode, constr);
 		if (staticConstrRequired) {
diff --git a/test/transform/resource/after-delombok/ConflictingStaticConstructorNames.java b/test/transform/resource/after-delombok/ConflictingStaticConstructorNames.java
new file mode 100644
index 00000000..172ed298
--- /dev/null
+++ b/test/transform/resource/after-delombok/ConflictingStaticConstructorNames.java
@@ -0,0 +1,29 @@
+class ConflictingStaticConstructorNames {
+	@java.lang.Override
+	@java.lang.SuppressWarnings("all")
+	public boolean equals(final java.lang.Object o) {
+		if (o == this) return true;
+		if (!(o instanceof ConflictingStaticConstructorNames)) return false;
+		final ConflictingStaticConstructorNames other = (ConflictingStaticConstructorNames)o;
+		if (!other.canEqual((java.lang.Object)this)) return false;
+		return true;
+	}
+	@java.lang.SuppressWarnings("all")
+	public boolean canEqual(final java.lang.Object other) {
+		return other instanceof ConflictingStaticConstructorNames;
+	}
+	@java.lang.Override
+	@java.lang.SuppressWarnings("all")
+	public int hashCode() {
+		int result = 1;
+		return result;
+	}
+	@java.lang.Override
+	@java.lang.SuppressWarnings("all")
+	public java.lang.String toString() {
+		return "ConflictingStaticConstructorNames()";
+	}
+	@java.lang.SuppressWarnings("all")
+	public ConflictingStaticConstructorNames() {
+	}
+}
\ No newline at end of file
diff --git a/test/transform/resource/after-ecj/ConflictingStaticConstructorNames.java b/test/transform/resource/after-ecj/ConflictingStaticConstructorNames.java
new file mode 100644
index 00000000..8da11258
--- /dev/null
+++ b/test/transform/resource/after-ecj/ConflictingStaticConstructorNames.java
@@ -0,0 +1,25 @@
+@lombok.Data(staticConstructor = "of") @lombok.NoArgsConstructor class ConflictingStaticConstructorNames {
+  public @java.lang.Override @java.lang.SuppressWarnings("all") boolean equals(final java.lang.Object o) {
+    if ((o == this))
+        return true;
+    if ((! (o instanceof ConflictingStaticConstructorNames)))
+        return false;
+    final @java.lang.SuppressWarnings("all") ConflictingStaticConstructorNames other = (ConflictingStaticConstructorNames) o;
+    if ((! other.canEqual((java.lang.Object) this)))
+        return false;
+    return true;
+  }
+  public @java.lang.SuppressWarnings("all") boolean canEqual(final java.lang.Object other) {
+    return (other instanceof ConflictingStaticConstructorNames);
+  }
+  public @java.lang.Override @java.lang.SuppressWarnings("all") int hashCode() {
+    int result = 1;
+    return result;
+  }
+  public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() {
+    return "ConflictingStaticConstructorNames()";
+  }
+  public @java.lang.SuppressWarnings("all") ConflictingStaticConstructorNames() {
+    super();
+  }
+}
\ No newline at end of file
diff --git a/test/transform/resource/before/ConflictingStaticConstructorNames.java b/test/transform/resource/before/ConflictingStaticConstructorNames.java
new file mode 100644
index 00000000..494e3cd6
--- /dev/null
+++ b/test/transform/resource/before/ConflictingStaticConstructorNames.java
@@ -0,0 +1,4 @@
+@lombok.Data(staticConstructor="of")
+@lombok.NoArgsConstructor
+class ConflictingStaticConstructorNames {
+}
\ No newline at end of file
diff --git a/test/transform/resource/messages-delombok/ConflictingStaticConstructorNames.java.messages b/test/transform/resource/messages-delombok/ConflictingStaticConstructorNames.java.messages
new file mode 100644
index 00000000..40366cd7
--- /dev/null
+++ b/test/transform/resource/messages-delombok/ConflictingStaticConstructorNames.java.messages
@@ -0,0 +1 @@
+1:1 WARNING Ignoring static constructor name: explicit @XxxArgsConstructor annotation present; its `staticName` parameter will be used.
diff --git a/test/transform/resource/messages-ecj/ConflictingStaticConstructorNames.java.messages b/test/transform/resource/messages-ecj/ConflictingStaticConstructorNames.java.messages
new file mode 100644
index 00000000..ecbf4a61
--- /dev/null
+++ b/test/transform/resource/messages-ecj/ConflictingStaticConstructorNames.java.messages
@@ -0,0 +1 @@
+1 warning Ignoring static constructor name: explicit @XxxArgsConstructor annotation present; its `staticName` parameter will be used.