Fixed issue 513: If equals is present but hashCode isn't, @Data now generates a warning to explain this strange situation.

doc/changelog.markdown

@@ -1,6 +1,9 @@
 Lombok Changelog
 ----------------
 
+### v0.11.9 (Edgy Guinea Pig)
+* BUGFIX: When using `@Data`, warnings are not generated if certain aspects are not generated because you wrote explicit versions of them. However, this gets confusing with `equals` / `hashCode` / `canEqual`, as nothing is generated if any one of those methods is present. Now, if one of `equals` or `hashCode` is present but not the other one (or `canEqual` is present but `equals` and/or `hashCode` is missing), a warning is emitted to explain that lombok will not generate any of the equals / hashCode methods, and that you should either write them all yourself or remove them all. [Issue #513](https://code.google.com/p/projectlombok/issues/detail?id=513)
+
 ### v0.11.8 (April 23rd, 2013)
 * FEATURE: Major performance improvements in eclipse by profiling the project clean process.
 * CHANGE: {Experimental} The experimental `@Value` feature no longer implies the also experimental `@Wither`. If you like your `@Value` classes to make withers, add `@Wither` to the class right next to `@Value`.

src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2012 The Project Lombok Authors.
+ * Copyright (C) 2009-2013 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
@@ -204,18 +204,27 @@ public class HandleEqualsAndHashCode extends EclipseAnnotationHandler<EqualsAndH
 		}
 		
 		boolean isFinal = (typeDecl.modifiers & ClassFileConstants.AccFinal) != 0;
-		boolean needsCanEqual = !isDirectDescendantOfObject || !isFinal;
-		java.util.List<MemberExistsResult> existsResults = new ArrayList<MemberExistsResult>();
-		existsResults.add(methodExists("equals", typeNode, 1));
-		existsResults.add(methodExists("hashCode", typeNode, 0));
-		existsResults.add(methodExists("canEqual", typeNode, 1));
-		switch (Collections.max(existsResults)) {
+		boolean needsCanEqual = !isFinal || !isDirectDescendantOfObject;
+		MemberExistsResult equalsExists = methodExists("equals", typeNode, 1);
+		MemberExistsResult hashCodeExists = methodExists("hashCode", typeNode, 0);
+		MemberExistsResult canEqualExists = methodExists("canEqual", typeNode, 1);
+		switch (Collections.max(Arrays.asList(equalsExists, hashCodeExists, canEqualExists))) {
 		case EXISTS_BY_LOMBOK:
 			return;
 		case EXISTS_BY_USER:
 			if (whineIfExists) {
 				String msg = String.format("Not generating equals%s: A method with one of those names already exists. (Either all or none of these methods will be generated).", needsCanEqual ? ", hashCode and canEquals" : " and hashCode");
 				errorNode.addWarning(msg);
+			} else if (equalsExists == MemberExistsResult.NOT_EXISTS || hashCodeExists == MemberExistsResult.NOT_EXISTS) {
+				// This means equals OR hashCode exists and not both (or neither, but canEqual is there).
+				// Even though we should suppress the message about not generating these, this is such a weird and surprising situation we should ALWAYS generate a warning.
+				// The user code couldn't possibly (barring really weird subclassing shenanigans) be in a shippable state anyway; the implementations of these 3 methods are
+				// all inter-related and should be written by the same entity.
+				String msg = String.format("Not generating %s: One of equals, hashCode, and canEqual exists. " +
+						"You should either write all of these are none of these (in the latter case, lombok generates them).",
+						equalsExists == MemberExistsResult.NOT_EXISTS && hashCodeExists == MemberExistsResult.NOT_EXISTS ? "equals and hashCode" :
+						equalsExists == MemberExistsResult.NOT_EXISTS ? "equals" : "hashCode");
+				errorNode.addWarning(msg);
 			}
 			return;
 		case NOT_EXISTS:

src/core/lombok/javac/handlers/HandleEqualsAndHashCode.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009-2012 The Project Lombok Authors.
+ * Copyright (C) 2009-2013 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
@@ -25,6 +25,7 @@ import static lombok.javac.Javac.*;
 import static lombok.javac.handlers.JavacHandlerUtil.*;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 
 import lombok.EqualsAndHashCode;
@@ -178,17 +179,26 @@ public class HandleEqualsAndHashCode extends JavacAnnotationHandler<EqualsAndHas
 		
 		boolean isFinal = (((JCClassDecl)typeNode.get()).mods.flags & Flags.FINAL) != 0;
 		boolean needsCanEqual = !isFinal || !isDirectDescendantOfObject;
-		java.util.List<MemberExistsResult> existsResults = new ArrayList<MemberExistsResult>();
-		existsResults.add(methodExists("equals", typeNode, 1));
-		existsResults.add(methodExists("hashCode", typeNode, 0));
-		existsResults.add(methodExists("canEqual", typeNode, 1));
-		switch (Collections.max(existsResults)) {
+		MemberExistsResult equalsExists = methodExists("equals", typeNode, 1);
+		MemberExistsResult hashCodeExists = methodExists("hashCode", typeNode, 0);
+		MemberExistsResult canEqualExists = methodExists("canEqual", typeNode, 1);
+		switch (Collections.max(Arrays.asList(equalsExists, hashCodeExists, canEqualExists))) {
 		case EXISTS_BY_LOMBOK:
 			return;
 		case EXISTS_BY_USER:
 			if (whineIfExists) {
 				String msg = String.format("Not generating equals%s: A method with one of those names already exists. (Either all or none of these methods will be generated).", needsCanEqual ? ", hashCode and canEquals" : " and hashCode");
 				source.addWarning(msg);
+			} else if (equalsExists == MemberExistsResult.NOT_EXISTS || hashCodeExists == MemberExistsResult.NOT_EXISTS) {
+				// This means equals OR hashCode exists and not both (or neither, but canEqual is there).
+				// Even though we should suppress the message about not generating these, this is such a weird and surprising situation we should ALWAYS generate a warning.
+				// The user code couldn't possibly (barring really weird subclassing shenanigans) be in a shippable state anyway; the implementations of these 3 methods are
+				// all inter-related and should be written by the same entity.
+				String msg = String.format("Not generating %s: One of equals, hashCode, and canEqual exists. " +
+						"You should either write all of these are none of these (in the latter case, lombok generates them).",
+						equalsExists == MemberExistsResult.NOT_EXISTS && hashCodeExists == MemberExistsResult.NOT_EXISTS ? "equals and hashCode" :
+						equalsExists == MemberExistsResult.NOT_EXISTS ? "equals" : "hashCode");
+				source.addWarning(msg);
 			}
 			return;
 		case NOT_EXISTS:

test/transform/resource/after-delombok/EqualsAndHashCodeWithSomeExistingMethods.java

@@ -0,0 +1,82 @@
+import lombok.*;
+import static lombok.AccessLevel.NONE;
+class EqualsAndHashCodeWithSomeExistingMethods {
+	int x;
+	public int hashCode() {
+		return 42;
+	}
+	@java.lang.SuppressWarnings("all")
+	public EqualsAndHashCodeWithSomeExistingMethods() {
+		
+	}
+	@java.lang.Override
+	@java.lang.SuppressWarnings("all")
+	public java.lang.String toString() {
+		return "EqualsAndHashCodeWithSomeExistingMethods(x=" + this.x + ")";
+	}
+}
+class EqualsAndHashCodeWithSomeExistingMethods2 {
+	int x;
+	public boolean canEqual(Object other) {
+		return false;
+	}
+	@java.lang.SuppressWarnings("all")
+	public EqualsAndHashCodeWithSomeExistingMethods2() {
+	}
+	@java.lang.Override
+	@java.lang.SuppressWarnings("all")
+	public java.lang.String toString() {
+		return "EqualsAndHashCodeWithSomeExistingMethods2(x=" + this.x + ")";
+	}
+}
+class EqualsAndHashCodeWithAllExistingMethods {
+	int x;
+	public int hashCode() {
+		return 42;
+	}
+	public boolean equals(Object other) {
+		return false;
+	}
+	@java.lang.SuppressWarnings("all")
+	public EqualsAndHashCodeWithAllExistingMethods() {
+	}
+	@java.lang.Override
+	@java.lang.SuppressWarnings("all")
+	public java.lang.String toString() {
+		return "EqualsAndHashCodeWithAllExistingMethods(x=" + this.x + ")";
+	}
+}
+class EqualsAndHashCodeWithNoExistingMethods {
+	int x;
+	@java.lang.SuppressWarnings("all")
+	public EqualsAndHashCodeWithNoExistingMethods() {
+		
+	}
+	@java.lang.Override
+	@java.lang.SuppressWarnings("all")
+	public boolean equals(final java.lang.Object o) {
+		if (o == this) return true;
+		if (!(o instanceof EqualsAndHashCodeWithNoExistingMethods)) return false;
+		final EqualsAndHashCodeWithNoExistingMethods other = (EqualsAndHashCodeWithNoExistingMethods)o;
+		if (!other.canEqual((java.lang.Object)this)) return false;
+		if (this.x != other.x) return false;
+		return true;
+	}
+	@java.lang.SuppressWarnings("all")
+	public boolean canEqual(final java.lang.Object other) {
+		return other instanceof EqualsAndHashCodeWithNoExistingMethods;
+	}
+	@java.lang.Override
+	@java.lang.SuppressWarnings("all")
+	public int hashCode() {
+		final int PRIME = 31;
+		int result = 1;
+		result = result * PRIME + this.x;
+		return result;
+	}
+	@java.lang.Override
+	@java.lang.SuppressWarnings("all")
+	public java.lang.String toString() {
+		return "EqualsAndHashCodeWithNoExistingMethods(x=" + this.x + ")";
+	}
+}

test/transform/resource/after-ecj/EqualsAndHashCodeWithSomeExistingMethods.java

@@ -0,0 +1,71 @@
+import lombok.*;
+import static lombok.AccessLevel.NONE;
+@Data @Getter(NONE) @Setter(NONE) class EqualsAndHashCodeWithSomeExistingMethods {
+  int x;
+  public int hashCode() {
+    return 42;
+  }
+  public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() {
+    return (("EqualsAndHashCodeWithSomeExistingMethods(x=" + this.x) + ")");
+  }
+  public @java.lang.SuppressWarnings("all") EqualsAndHashCodeWithSomeExistingMethods() {
+    super();
+  }
+}
+@Data @Getter(NONE) @Setter(NONE) class EqualsAndHashCodeWithSomeExistingMethods2 {
+  int x;
+  public boolean canEqual(Object other) {
+    return false;
+  }
+  public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() {
+    return (("EqualsAndHashCodeWithSomeExistingMethods2(x=" + this.x) + ")");
+  }
+  public @java.lang.SuppressWarnings("all") EqualsAndHashCodeWithSomeExistingMethods2() {
+    super();
+  }
+}
+@Data @Getter(NONE) @Setter(NONE) class EqualsAndHashCodeWithAllExistingMethods {
+  int x;
+  public int hashCode() {
+    return 42;
+  }
+  public boolean equals(Object other) {
+    return false;
+  }
+  public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() {
+    return (("EqualsAndHashCodeWithAllExistingMethods(x=" + this.x) + ")");
+  }
+  public @java.lang.SuppressWarnings("all") EqualsAndHashCodeWithAllExistingMethods() {
+    super();
+  }
+}
+@Data @Getter(AccessLevel.NONE) @Setter(lombok.AccessLevel.NONE) class EqualsAndHashCodeWithNoExistingMethods {
+  int x;
+  public @java.lang.Override @java.lang.SuppressWarnings("all") boolean equals(final java.lang.Object o) {
+    if ((o == this))
+        return true;
+    if ((! (o instanceof EqualsAndHashCodeWithNoExistingMethods)))
+        return false;
+    final @java.lang.SuppressWarnings("all") EqualsAndHashCodeWithNoExistingMethods other = (EqualsAndHashCodeWithNoExistingMethods) o;
+    if ((! other.canEqual((java.lang.Object) this)))
+        return false;
+    if ((this.x != other.x))
+        return false;
+    return true;
+  }
+  public @java.lang.SuppressWarnings("all") boolean canEqual(final java.lang.Object other) {
+    return (other instanceof EqualsAndHashCodeWithNoExistingMethods);
+  }
+  public @java.lang.Override @java.lang.SuppressWarnings("all") int hashCode() {
+    final int PRIME = 31;
+    int result = 1;
+    result = ((result * PRIME) + this.x);
+    return result;
+  }
+  public @java.lang.Override @java.lang.SuppressWarnings("all") java.lang.String toString() {
+    return (("EqualsAndHashCodeWithNoExistingMethods(x=" + this.x) + ")");
+  }
+  public @java.lang.SuppressWarnings("all") EqualsAndHashCodeWithNoExistingMethods() {
+    super();
+  }
+}

test/transform/resource/before/EqualsAndHashCodeWithSomeExistingMethods.java

@@ -0,0 +1,47 @@
+import lombok.*;
+import static lombok.AccessLevel.NONE;
+
+@Data
+@Getter(NONE)
+@Setter(NONE)
+class EqualsAndHashCodeWithSomeExistingMethods {
+	int x;
+	
+	public int hashCode() {
+		return 42;
+	}
+}
+
+@Data
+@Getter(NONE)
+@Setter(NONE)
+class EqualsAndHashCodeWithSomeExistingMethods2 {
+	int x;
+	
+	public boolean canEqual(Object other) {
+		return false;
+	}
+}
+
+@Data
+@Getter(NONE)
+@Setter(NONE)
+class EqualsAndHashCodeWithAllExistingMethods {
+	int x;
+	
+	public int hashCode() {
+		return 42;
+	}
+	
+	public boolean equals(Object other) {
+		return false;
+	}
+}
+
+@Data
+@Getter(AccessLevel.NONE)
+@Setter(lombok.AccessLevel.NONE)
+class EqualsAndHashCodeWithNoExistingMethods {
+	int x;
+}
+

test/transform/resource/messages-delombok/EqualsAndHashCodeWithSomeExistingMethods.java.messages

@@ -0,0 +1,2 @@
+4:1 Not generating equals: One of equals, hashCode, and canEqual exists. You should either write all of these are none of these (in the latter case, lombok generates them).
+15:1 Not generating equals and hashCode: One of equals, hashCode, and canEqual exists. You should either write all of these are none of these (in the latter case, lombok generates them).

test/transform/resource/messages-ecj/EqualsAndHashCodeWithSomeExistingMethods.java.messages

@@ -0,0 +1,2 @@
+4:57 Not generating equals: One of equals, hashCode, and canEqual exists. You should either write all of these are none of these (in the latter case, lombok generates them).
+15:194 Not generating equals and hashCode: One of equals, hashCode, and canEqual exists. You should either write all of these are none of these (in the latter case, lombok generates them).