diff options
8 files changed, 239 insertions, 13 deletions
diff --git a/doc/changelog.markdown b/doc/changelog.markdown index 15e3c1e0..32af72d3 100644 --- a/doc/changelog.markdown +++ b/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`. diff --git a/src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java b/src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java index 0c82b74c..6990e609 100644 --- a/src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java +++ b/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: diff --git a/src/core/lombok/javac/handlers/HandleEqualsAndHashCode.java b/src/core/lombok/javac/handlers/HandleEqualsAndHashCode.java index 5f69be9d..3b1e226f 100644 --- a/src/core/lombok/javac/handlers/HandleEqualsAndHashCode.java +++ b/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: diff --git a/test/transform/resource/after-delombok/EqualsAndHashCodeWithSomeExistingMethods.java b/test/transform/resource/after-delombok/EqualsAndHashCodeWithSomeExistingMethods.java new file mode 100644 index 00000000..0a6b1e7f --- /dev/null +++ b/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 + ")"; + } +} diff --git a/test/transform/resource/after-ecj/EqualsAndHashCodeWithSomeExistingMethods.java b/test/transform/resource/after-ecj/EqualsAndHashCodeWithSomeExistingMethods.java new file mode 100644 index 00000000..cdd771a4 --- /dev/null +++ b/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(); + } +} diff --git a/test/transform/resource/before/EqualsAndHashCodeWithSomeExistingMethods.java b/test/transform/resource/before/EqualsAndHashCodeWithSomeExistingMethods.java new file mode 100644 index 00000000..784e3b3f --- /dev/null +++ b/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; +} + diff --git a/test/transform/resource/messages-delombok/EqualsAndHashCodeWithSomeExistingMethods.java.messages b/test/transform/resource/messages-delombok/EqualsAndHashCodeWithSomeExistingMethods.java.messages new file mode 100644 index 00000000..9a0b29f3 --- /dev/null +++ b/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). diff --git a/test/transform/resource/messages-ecj/EqualsAndHashCodeWithSomeExistingMethods.java.messages b/test/transform/resource/messages-ecj/EqualsAndHashCodeWithSomeExistingMethods.java.messages new file mode 100644 index 00000000..cf6ebea2 --- /dev/null +++ b/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). |