From e6421509987c01e06b7c79ef406cc01ff174ae81 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Mon, 16 Jul 2012 22:04:39 +0200 Subject: Updated tests to reflect changes to delombok (delombok now kills super(), because attrib adds them even in places where that's wrong). Also split up the SynchronizedName test into separate cases for each expected failure mode. --- test/transform/resource/messages-delombok/SynchronizedName.java.messages | 1 - .../resource/messages-delombok/SynchronizedNameNoSuchField.java.messages | 1 + .../messages-delombok/SynchronizedNameStaticToInstanceRef.java.messages | 1 + 3 files changed, 2 insertions(+), 1 deletion(-) delete mode 100644 test/transform/resource/messages-delombok/SynchronizedName.java.messages create mode 100644 test/transform/resource/messages-delombok/SynchronizedNameNoSuchField.java.messages create mode 100644 test/transform/resource/messages-delombok/SynchronizedNameStaticToInstanceRef.java.messages (limited to 'test/transform/resource/messages-delombok') diff --git a/test/transform/resource/messages-delombok/SynchronizedName.java.messages b/test/transform/resource/messages-delombok/SynchronizedName.java.messages deleted file mode 100644 index 2af3ca1d..00000000 --- a/test/transform/resource/messages-delombok/SynchronizedName.java.messages +++ /dev/null @@ -1 +0,0 @@ -8:9 ERROR The field write does not exist. \ No newline at end of file diff --git a/test/transform/resource/messages-delombok/SynchronizedNameNoSuchField.java.messages b/test/transform/resource/messages-delombok/SynchronizedNameNoSuchField.java.messages new file mode 100644 index 00000000..bffd29e6 --- /dev/null +++ b/test/transform/resource/messages-delombok/SynchronizedNameNoSuchField.java.messages @@ -0,0 +1 @@ +5:9 ERROR The field write does not exist. diff --git a/test/transform/resource/messages-delombok/SynchronizedNameStaticToInstanceRef.java.messages b/test/transform/resource/messages-delombok/SynchronizedNameStaticToInstanceRef.java.messages new file mode 100644 index 00000000..1a084653 --- /dev/null +++ b/test/transform/resource/messages-delombok/SynchronizedNameStaticToInstanceRef.java.messages @@ -0,0 +1 @@ +-1:-1 ERROR non-static variable read cannot be referenced from a static context -- cgit From fe4985e2127e5fc54a0a1ae3c2c4fef17789f8bd Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Mon, 16 Jul 2012 22:51:56 +0200 Subject: Added setting position of generated nodes in javac's @Synchronized as a 'pilot' to see if we cause any problems with this approach. It does generate nicer error messages! Example: Using @Synchronized with named lock on a static method, naming a non-existent or instance lock. That used to error on line -1. --- .../lombok/javac/handlers/HandleSynchronized.java | 8 +++---- .../lombok/javac/handlers/JavacHandlerUtil.java | 26 +++++++++++++++++++--- ...nchronizedNameStaticToInstanceRef.java.messages | 2 +- 3 files changed, 28 insertions(+), 8 deletions(-) (limited to 'test/transform/resource/messages-delombok') diff --git a/src/core/lombok/javac/handlers/HandleSynchronized.java b/src/core/lombok/javac/handlers/HandleSynchronized.java index 04668317..1ba8d131 100644 --- a/src/core/lombok/javac/handlers/HandleSynchronized.java +++ b/src/core/lombok/javac/handlers/HandleSynchronized.java @@ -78,16 +78,16 @@ public class HandleSynchronized extends JavacAnnotationHandler { lockName = isStatic ? STATIC_LOCK_NAME : INSTANCE_LOCK_NAME; } - TreeMaker maker = methodNode.getTreeMaker(); + TreeMaker maker = methodNode.getTreeMaker().at(ast.pos); if (fieldExists(lockName, methodNode) == MemberExistsResult.NOT_EXISTS) { if (!autoMake) { annotationNode.addError("The field " + lockName + " does not exist."); return; } - JCExpression objectType = chainDots(methodNode, "java", "lang", "Object"); + JCExpression objectType = chainDots(methodNode, ast.pos, "java", "lang", "Object"); //We use 'new Object[0];' because unlike 'new Object();', empty arrays *ARE* serializable! - JCNewArray newObjectArray = maker.NewArray(chainDots(methodNode, "java", "lang", "Object"), + JCNewArray newObjectArray = maker.NewArray(chainDots(methodNode, ast.pos, "java", "lang", "Object"), List.of(maker.Literal(Javac.getCtcInt(TypeTags.class, "INT"), 0)), null); JCVariableDecl fieldDecl = recursiveSetGeneratedBy(maker.VarDef( maker.Modifiers(Flags.PRIVATE | Flags.FINAL | (isStatic ? Flags.STATIC : 0)), @@ -99,7 +99,7 @@ public class HandleSynchronized extends JavacAnnotationHandler { JCExpression lockNode; if (isStatic) { - lockNode = chainDots(methodNode, methodNode.up().getName(), lockName); + lockNode = chainDots(methodNode, ast.pos, methodNode.up().getName(), lockName); } else { lockNode = maker.Select(maker.Ident(methodNode.toName("this")), methodNode.toName(lockName)); } diff --git a/src/core/lombok/javac/handlers/JavacHandlerUtil.java b/src/core/lombok/javac/handlers/JavacHandlerUtil.java index 437ba6cb..d2905101 100644 --- a/src/core/lombok/javac/handlers/JavacHandlerUtil.java +++ b/src/core/lombok/javac/handlers/JavacHandlerUtil.java @@ -746,6 +746,8 @@ public class JavacHandlerUtil { * In javac, dotted access of any kind, from {@code java.lang.String} to {@code var.methodName} * is represented by a fold-left of {@code Select} nodes with the leftmost string represented by * a {@code Ident} node. This method generates such an expression. + *

+ * The position of the generated node(s) will be unpositioned (-1). * * For example, maker.Select(maker.Select(maker.Ident(NAME[java]), NAME[lang]), NAME[String]). * @@ -753,12 +755,30 @@ public class JavacHandlerUtil { * @see com.sun.tools.javac.tree.JCTree.JCFieldAccess */ public static JCExpression chainDots(JavacNode node, String... elems) { + return chainDots(node, -1, elems); + } + + /** + * In javac, dotted access of any kind, from {@code java.lang.String} to {@code var.methodName} + * is represented by a fold-left of {@code Select} nodes with the leftmost string represented by + * a {@code Ident} node. This method generates such an expression. + *

+ * The position of the generated node(s) will be equal to the {@code pos} parameter. + * + * For example, maker.Select(maker.Select(maker.Ident(NAME[java]), NAME[lang]), NAME[String]). + * + * @see com.sun.tools.javac.tree.JCTree.JCIdent + * @see com.sun.tools.javac.tree.JCTree.JCFieldAccess + */ + public static JCExpression chainDots(JavacNode node, int pos, String... elems) { assert elems != null; assert elems.length > 0; - JCExpression e = node.getTreeMaker().Ident(node.toName(elems[0])); + TreeMaker maker = node.getTreeMaker(); + if (pos != -1) maker = maker.at(pos); + JCExpression e = maker.Ident(node.toName(elems[0])); for (int i = 1 ; i < elems.length ; i++) { - e = node.getTreeMaker().Select(e, node.toName(elems[i])); + e = maker.Select(e, node.toName(elems[i])); } return e; @@ -776,7 +796,7 @@ public class JavacHandlerUtil { * @see com.sun.tools.javac.tree.JCTree.JCFieldAccess */ public static JCExpression chainDotsString(JavacNode node, String elems) { - return chainDots(node, elems.split("\\.")); + return chainDots(node, elems.split("\\.")); } /** diff --git a/test/transform/resource/messages-delombok/SynchronizedNameStaticToInstanceRef.java.messages b/test/transform/resource/messages-delombok/SynchronizedNameStaticToInstanceRef.java.messages index 1a084653..84336ebb 100644 --- a/test/transform/resource/messages-delombok/SynchronizedNameStaticToInstanceRef.java.messages +++ b/test/transform/resource/messages-delombok/SynchronizedNameStaticToInstanceRef.java.messages @@ -1 +1 @@ --1:-1 ERROR non-static variable read cannot be referenced from a static context +5:9 ERROR non-static variable read cannot be referenced from a static context \ No newline at end of file -- cgit 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 (limited to 'test/transform/resource/messages-delombok') 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