From a382c21d464c22658e8c2eea7d7d75c2a6747527 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Mon, 16 Jul 2012 23:18:58 +0200 Subject: 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. --- .../lombok/eclipse/handlers/HandleConstructor.java | 15 ++++++++--- .../lombok/javac/handlers/HandleConstructor.java | 15 ++++++++--- .../ConflictingStaticConstructorNames.java | 29 ++++++++++++++++++++++ .../ConflictingStaticConstructorNames.java | 25 +++++++++++++++++++ .../before/ConflictingStaticConstructorNames.java | 4 +++ ...ConflictingStaticConstructorNames.java.messages | 1 + ...ConflictingStaticConstructorNames.java.messages | 1 + 7 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 test/transform/resource/after-delombok/ConflictingStaticConstructorNames.java create mode 100644 test/transform/resource/after-ecj/ConflictingStaticConstructorNames.java create mode 100644 test/transform/resource/before/ConflictingStaticConstructorNames.java create mode 100644 test/transform/resource/messages-delombok/ConflictingStaticConstructorNames.java.messages create mode 100644 test/transform/resource/messages-ecj/ConflictingStaticConstructorNames.java.messages 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 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 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. -- cgit