From 02446794b09682fc5e07bbaa298c9908772cbf42 Mon Sep 17 00:00:00 2001 From: Philipp Eichhorn Date: Fri, 13 Jul 2012 12:48:55 +0200 Subject: Fixed issue 397: Rare java.lang.StackOverflowError in JavacResolution. --- src/core/lombok/javac/JavacResolution.java | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src') diff --git a/src/core/lombok/javac/JavacResolution.java b/src/core/lombok/javac/JavacResolution.java index 6da60907..73bb38b0 100644 --- a/src/core/lombok/javac/JavacResolution.java +++ b/src/core/lombok/javac/JavacResolution.java @@ -456,12 +456,18 @@ public class JavacResolution { if (upper == null || upper.toString().equals("java.lang.Object")) { return maker.Wildcard(maker.TypeBoundKind(BoundKind.UNBOUND), null); } + if (upper.getTypeArguments().contains(type)) { + return maker.Wildcard(maker.TypeBoundKind(BoundKind.UNBOUND), null); + } return maker.Wildcard(maker.TypeBoundKind(BoundKind.EXTENDS), typeToJCTree(upper, ast, false, false)); } else { return maker.Wildcard(maker.TypeBoundKind(BoundKind.SUPER), typeToJCTree(lower, ast, false, false)); } } if (upper != null) { + if (upper.getTypeArguments().contains(type)) { + return maker.Wildcard(maker.TypeBoundKind(BoundKind.UNBOUND), null); + } return typeToJCTree(upper, ast, allowCompound, allowVoid); } -- cgit From 06826342f3be58cfb422f291f89e29be2cfde7f0 Mon Sep 17 00:00:00 2001 From: Philipp Eichhorn Date: Fri, 13 Jul 2012 15:27:52 +0200 Subject: Fixed issue 399: @ExtensionMethod broken for javac in nearly all cases --- src/core/lombok/javac/handlers/HandleExtensionMethod.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'src') diff --git a/src/core/lombok/javac/handlers/HandleExtensionMethod.java b/src/core/lombok/javac/handlers/HandleExtensionMethod.java index 2df6be45..92d7c0e4 100644 --- a/src/core/lombok/javac/handlers/HandleExtensionMethod.java +++ b/src/core/lombok/javac/handlers/HandleExtensionMethod.java @@ -55,7 +55,6 @@ import com.sun.tools.javac.tree.JCTree.JCExpression; import com.sun.tools.javac.tree.JCTree.JCFieldAccess; import com.sun.tools.javac.tree.JCTree.JCIdent; import com.sun.tools.javac.tree.JCTree.JCMethodInvocation; -import com.sun.tools.javac.tree.TreeMaker; /** * Handles the {@link ExtensionMethod} annotation for javac. @@ -180,10 +179,7 @@ public class HandleExtensionMethod extends JavacAnnotationHandler Date: Mon, 16 Jul 2012 20:58:19 +0200 Subject: Delombok now also runs attrib on more files; this will reveal more errors which is particularly important for tests. This does mean delombok really does need your entire source/classpath to work right but there's unfortunately no real avoiding this anyway, given that we're doing more and more with resolution. --- src/delombok/lombok/delombok/Delombok.java | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/delombok/lombok/delombok/Delombok.java b/src/delombok/lombok/delombok/Delombok.java index a639a1f6..3f521274 100644 --- a/src/delombok/lombok/delombok/Delombok.java +++ b/src/delombok/lombok/delombok/Delombok.java @@ -381,6 +381,7 @@ public class Delombok { } JavaCompiler delegate = compiler.processAnnotations(compiler.enterTrees(toJavacList(roots))); + delegate.flow(delegate.attribute(delegate.todo)); for (JCCompilationUnit unit : roots) { DelombokResult result = new DelombokResult(catcher.getComments(unit), unit, force || options.isChanged(unit)); if (verbose) feedback.printf("File: %s [%s]\n", unit.sourcefile.getName(), result.isChanged() ? "delomboked" : "unchanged"); -- cgit 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. --- src/delombok/lombok/delombok/PrettyCommentsPrinter.java | 11 ++++++++++- test/transform/resource/after-delombok/DataPlain.java | 1 - test/transform/resource/after-delombok/SneakyThrowsPlain.java | 1 - test/transform/resource/after-delombok/SynchronizedName.java | 11 ++--------- .../resource/after-delombok/SynchronizedNameNoSuchField.java | 7 +++++++ .../after-delombok/SynchronizedNameStaticToInstanceRef.java | 9 +++++++++ test/transform/resource/before/SynchronizedName.java | 6 ------ .../resource/before/SynchronizedNameNoSuchField.java | 8 ++++++++ .../resource/before/SynchronizedNameStaticToInstanceRef.java | 8 ++++++++ .../resource/messages-delombok/SynchronizedName.java.messages | 1 - .../SynchronizedNameNoSuchField.java.messages | 1 + .../SynchronizedNameStaticToInstanceRef.java.messages | 1 + .../SynchronizedNameStaticToInstanceRef.java.messages | 1 + .../resource/messages-idempotent/ValErrors.java.messages | 7 +++++++ 14 files changed, 54 insertions(+), 19 deletions(-) create mode 100644 test/transform/resource/after-delombok/SynchronizedNameNoSuchField.java create mode 100644 test/transform/resource/after-delombok/SynchronizedNameStaticToInstanceRef.java create mode 100644 test/transform/resource/before/SynchronizedNameNoSuchField.java create mode 100644 test/transform/resource/before/SynchronizedNameStaticToInstanceRef.java 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 create mode 100644 test/transform/resource/messages-idempotent/SynchronizedNameStaticToInstanceRef.java.messages create mode 100644 test/transform/resource/messages-idempotent/ValErrors.java.messages (limited to 'src') diff --git a/src/delombok/lombok/delombok/PrettyCommentsPrinter.java b/src/delombok/lombok/delombok/PrettyCommentsPrinter.java index 095928ad..2f47ccf6 100644 --- a/src/delombok/lombok/delombok/PrettyCommentsPrinter.java +++ b/src/delombok/lombok/delombok/PrettyCommentsPrinter.java @@ -1037,8 +1037,17 @@ public class PrettyCommentsPrinter extends JCTree.Visitor { throw new UncheckedIOException(e); } } - + + private boolean isNoArgsSuperCall(JCExpression expr) { + if (!(expr instanceof JCMethodInvocation)) return false; + JCMethodInvocation tree = (JCMethodInvocation) expr; + if (!tree.typeargs.isEmpty() || !tree.args.isEmpty()) return false; + if (!(tree.meth instanceof JCIdent)) return false; + return ((JCIdent) tree.meth).name.toString().equals("super"); + } + public void visitExec(JCExpressionStatement tree) { + if (isNoArgsSuperCall(tree.expr)) return; try { printExpr(tree.expr); if (prec == TreeInfo.notExpression) print(";"); diff --git a/test/transform/resource/after-delombok/DataPlain.java b/test/transform/resource/after-delombok/DataPlain.java index cb002e07..a8cb37af 100644 --- a/test/transform/resource/after-delombok/DataPlain.java +++ b/test/transform/resource/after-delombok/DataPlain.java @@ -155,7 +155,6 @@ final class Data3 { final class Data4 extends java.util.Timer { int x; Data4() { - super(); } @java.lang.SuppressWarnings("all") public int getX() { diff --git a/test/transform/resource/after-delombok/SneakyThrowsPlain.java b/test/transform/resource/after-delombok/SneakyThrowsPlain.java index 5c0890b5..f712ab55 100644 --- a/test/transform/resource/after-delombok/SneakyThrowsPlain.java +++ b/test/transform/resource/after-delombok/SneakyThrowsPlain.java @@ -1,6 +1,5 @@ class SneakyThrowsPlain { SneakyThrowsPlain() { - super(); try { System.out.println("constructor"); } catch (final java.lang.Throwable $ex) { diff --git a/test/transform/resource/after-delombok/SynchronizedName.java b/test/transform/resource/after-delombok/SynchronizedName.java index e7dd23ff..ab3c0431 100644 --- a/test/transform/resource/after-delombok/SynchronizedName.java +++ b/test/transform/resource/after-delombok/SynchronizedName.java @@ -1,19 +1,12 @@ class SynchronizedName { private Object read = new Object(); private static Object READ = new Object(); + void test1() { synchronized (this.read) { System.out.println("one"); } } - void test2() { - System.out.println("two"); - } - static void test3() { - synchronized (SynchronizedName.read) { - System.out.println("three"); - } - } void test4() { synchronized (this.READ) { System.out.println("four"); @@ -24,4 +17,4 @@ class SynchronizedName { System.out.println("five"); } } -} +} \ No newline at end of file diff --git a/test/transform/resource/after-delombok/SynchronizedNameNoSuchField.java b/test/transform/resource/after-delombok/SynchronizedNameNoSuchField.java new file mode 100644 index 00000000..d252985f --- /dev/null +++ b/test/transform/resource/after-delombok/SynchronizedNameNoSuchField.java @@ -0,0 +1,7 @@ +class SynchronizedNameNoSuchField { + private Object read = new Object(); + private static Object READ = new Object(); + void test2() { + System.out.println("two"); + } +} \ No newline at end of file diff --git a/test/transform/resource/after-delombok/SynchronizedNameStaticToInstanceRef.java b/test/transform/resource/after-delombok/SynchronizedNameStaticToInstanceRef.java new file mode 100644 index 00000000..8441570b --- /dev/null +++ b/test/transform/resource/after-delombok/SynchronizedNameStaticToInstanceRef.java @@ -0,0 +1,9 @@ +class SynchronizedNameStaticToInstanceRef { + private Object read = new Object(); + private static Object READ = new Object(); + static void test3() { + synchronized (SynchronizedNameStaticToInstanceRef.read) { + System.out.println("three"); + } + } +} \ No newline at end of file diff --git a/test/transform/resource/before/SynchronizedName.java b/test/transform/resource/before/SynchronizedName.java index 5d074113..7553a8ce 100644 --- a/test/transform/resource/before/SynchronizedName.java +++ b/test/transform/resource/before/SynchronizedName.java @@ -5,12 +5,6 @@ class SynchronizedName { @lombok.Synchronized("read") void test1() { System.out.println("one"); } - @lombok.Synchronized("write") void test2() { - System.out.println("two"); - } - @lombok.Synchronized("read") static void test3() { - System.out.println("three"); - } @lombok.Synchronized("READ") void test4() { System.out.println("four"); } diff --git a/test/transform/resource/before/SynchronizedNameNoSuchField.java b/test/transform/resource/before/SynchronizedNameNoSuchField.java new file mode 100644 index 00000000..343ba6ae --- /dev/null +++ b/test/transform/resource/before/SynchronizedNameNoSuchField.java @@ -0,0 +1,8 @@ +class SynchronizedNameNoSuchField { + private Object read = new Object(); + private static Object READ = new Object(); + + @lombok.Synchronized("write") void test2() { + System.out.println("two"); + } +} diff --git a/test/transform/resource/before/SynchronizedNameStaticToInstanceRef.java b/test/transform/resource/before/SynchronizedNameStaticToInstanceRef.java new file mode 100644 index 00000000..08f9dbf1 --- /dev/null +++ b/test/transform/resource/before/SynchronizedNameStaticToInstanceRef.java @@ -0,0 +1,8 @@ +class SynchronizedNameStaticToInstanceRef { + private Object read = new Object(); + private static Object READ = new Object(); + + @lombok.Synchronized("read") static void test3() { + System.out.println("three"); + } +} 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 diff --git a/test/transform/resource/messages-idempotent/SynchronizedNameStaticToInstanceRef.java.messages b/test/transform/resource/messages-idempotent/SynchronizedNameStaticToInstanceRef.java.messages new file mode 100644 index 00000000..c34e29f6 --- /dev/null +++ b/test/transform/resource/messages-idempotent/SynchronizedNameStaticToInstanceRef.java.messages @@ -0,0 +1 @@ +5:66 ERROR non-static variable read cannot be referenced from a static context diff --git a/test/transform/resource/messages-idempotent/ValErrors.java.messages b/test/transform/resource/messages-idempotent/ValErrors.java.messages new file mode 100644 index 00000000..6f666511 --- /dev/null +++ b/test/transform/resource/messages-idempotent/ValErrors.java.messages @@ -0,0 +1,7 @@ +3:44 ERROR cannot find symbol +symbol : variable d +location: class ValErrors +6:17 ERROR cannot find symbol +symbol : class val +location: class ValErrors +6:25 ERROR illegal initializer for -- 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 'src') 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 'src') 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 From dd18f63ee539a362fac2525c48d83ccff7a06dc9 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Mon, 16 Jul 2012 23:59:43 +0200 Subject: Fix for issue 396: Static constructors generated for classes with type parameters did not work in javac. --- .../lombok/javac/handlers/HandleConstructor.java | 13 +--- .../lombok/javac/handlers/JavacHandlerUtil.java | 74 ++++++++++++++++++++++ .../resource/after-delombok/Constructors.java | 14 ++++ .../transform/resource/after-ecj/Constructors.java | 11 ++++ test/transform/resource/before/Constructors.java | 4 ++ 5 files changed, 104 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/core/lombok/javac/handlers/HandleConstructor.java b/src/core/lombok/javac/handlers/HandleConstructor.java index 65ad2547..d701b41e 100644 --- a/src/core/lombok/javac/handlers/HandleConstructor.java +++ b/src/core/lombok/javac/handlers/HandleConstructor.java @@ -43,12 +43,10 @@ import com.sun.tools.javac.tree.JCTree.JCBlock; import com.sun.tools.javac.tree.JCTree.JCClassDecl; import com.sun.tools.javac.tree.JCTree.JCExpression; import com.sun.tools.javac.tree.JCTree.JCFieldAccess; -import com.sun.tools.javac.tree.JCTree.JCIdent; import com.sun.tools.javac.tree.JCTree.JCMethodDecl; import com.sun.tools.javac.tree.JCTree.JCModifiers; import com.sun.tools.javac.tree.JCTree.JCReturn; import com.sun.tools.javac.tree.JCTree.JCStatement; -import com.sun.tools.javac.tree.JCTree.JCTypeApply; import com.sun.tools.javac.tree.JCTree.JCTypeParameter; import com.sun.tools.javac.tree.JCTree.JCVariableDecl; import com.sun.tools.javac.util.List; @@ -268,16 +266,7 @@ public class HandleConstructor { for (JavacNode fieldNode : fields) { JCVariableDecl field = (JCVariableDecl) fieldNode.get(); - JCExpression pType; - if (field.vartype instanceof JCIdent) pType = maker.Ident(((JCIdent)field.vartype).name); - else if (field.vartype instanceof JCTypeApply) { - JCTypeApply typeApply = (JCTypeApply) field.vartype; - ListBuffer tArgs = ListBuffer.lb(); - for (JCExpression arg : typeApply.arguments) tArgs.append(arg); - pType = maker.TypeApply(typeApply.clazz, tArgs.toList()); - } else { - pType = field.vartype; - } + JCExpression pType = cloneType(maker, field.vartype, source); List nonNulls = findAnnotations(fieldNode, TransformationsUtil.NON_NULL_PATTERN); List nullables = findAnnotations(fieldNode, TransformationsUtil.NULLABLE_PATTERN); JCVariableDecl param = maker.VarDef(maker.Modifiers(Flags.FINAL, nonNulls.appendList(nullables)), field.name, pType, null); diff --git a/src/core/lombok/javac/handlers/JavacHandlerUtil.java b/src/core/lombok/javac/handlers/JavacHandlerUtil.java index d2905101..ce7421b1 100644 --- a/src/core/lombok/javac/handlers/JavacHandlerUtil.java +++ b/src/core/lombok/javac/handlers/JavacHandlerUtil.java @@ -45,14 +45,17 @@ import lombok.experimental.Accessors; import lombok.javac.Javac; import lombok.javac.JavacNode; +import com.sun.tools.javac.code.BoundKind; import com.sun.tools.javac.code.Flags; import com.sun.tools.javac.code.TypeTags; import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.tree.JCTree.JCAnnotation; +import com.sun.tools.javac.tree.JCTree.JCArrayTypeTree; import com.sun.tools.javac.tree.JCTree.JCAssign; import com.sun.tools.javac.tree.JCTree.JCClassDecl; import com.sun.tools.javac.tree.JCTree.JCCompilationUnit; import com.sun.tools.javac.tree.JCTree.JCExpression; +import com.sun.tools.javac.tree.JCTree.JCFieldAccess; import com.sun.tools.javac.tree.JCTree.JCIdent; import com.sun.tools.javac.tree.JCTree.JCImport; import com.sun.tools.javac.tree.JCTree.JCLiteral; @@ -60,8 +63,12 @@ import com.sun.tools.javac.tree.JCTree.JCMethodDecl; import com.sun.tools.javac.tree.JCTree.JCMethodInvocation; import com.sun.tools.javac.tree.JCTree.JCModifiers; import com.sun.tools.javac.tree.JCTree.JCNewArray; +import com.sun.tools.javac.tree.JCTree.JCPrimitiveTypeTree; import com.sun.tools.javac.tree.JCTree.JCStatement; +import com.sun.tools.javac.tree.JCTree.JCTypeApply; import com.sun.tools.javac.tree.JCTree.JCVariableDecl; +import com.sun.tools.javac.tree.JCTree.JCWildcard; +import com.sun.tools.javac.tree.JCTree.TypeBoundKind; import com.sun.tools.javac.tree.TreeMaker; import com.sun.tools.javac.tree.TreeScanner; import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition; @@ -920,4 +927,71 @@ public class JavacHandlerUtil { return node; } + + /** + * Creates a full clone of a given javac AST type node. Every part is cloned (every identifier, every select, every wildcard, every type apply). + * + * If there's any node in the tree that we don't know how to clone, that part isn't cloned. However, we wouldn't know what could possibly show up that we + * can't currently clone; that's just a safeguard. + * + * This should be used if the type looks the same in the code, but resolves differently. For example, a static method that has some generics in it named after + * the class's own parameter, but as its a static method, the static method's notion of {@code T} is different from the class notion of {@code T}. If you're duplicating + * a type used in the class context, you need to use this method. + */ + public static JCExpression cloneType(TreeMaker maker, JCExpression in, JCTree source) { + JCExpression out = cloneType0(maker, in); + if (out != null) recursiveSetGeneratedBy(out, source); + return out; + } + + private static JCExpression cloneType0(TreeMaker maker, JCTree in) { + if (in == null) return null; + + if (in instanceof JCPrimitiveTypeTree) return (JCExpression) in; + + if (in instanceof JCIdent) { + return maker.Ident(((JCIdent) in).name); + } + + if (in instanceof JCFieldAccess) { + JCFieldAccess fa = (JCFieldAccess) in; + return maker.Select(cloneType0(maker, fa.selected), fa.name); + } + + if (in instanceof JCArrayTypeTree) { + JCArrayTypeTree att = (JCArrayTypeTree) in; + return maker.TypeArray(cloneType0(maker, att.elemtype)); + } + + if (in instanceof JCTypeApply) { + JCTypeApply ta = (JCTypeApply) in; + ListBuffer lb = ListBuffer.lb(); + for (JCExpression typeArg : ta.arguments) { + lb.append(cloneType0(maker, typeArg)); + } + return maker.TypeApply(cloneType0(maker, ta.clazz), lb.toList()); + } + + if (in instanceof JCWildcard) { + JCWildcard w = (JCWildcard) in; + JCExpression newInner = cloneType0(maker, w.inner); + TypeBoundKind newKind; + switch (w.getKind()) { + case SUPER_WILDCARD: + newKind = maker.TypeBoundKind(BoundKind.SUPER); + break; + case EXTENDS_WILDCARD: + newKind = maker.TypeBoundKind(BoundKind.EXTENDS); + break; + default: + case UNBOUNDED_WILDCARD: + newKind = maker.TypeBoundKind(BoundKind.UNBOUND); + break; + } + return maker.Wildcard(newKind, newInner); + } + + // This is somewhat unsafe, but it's better than outright throwing an exception here. Returning null will just cause an exception down the pipeline. + return (JCExpression) in; + } } diff --git a/test/transform/resource/after-delombok/Constructors.java b/test/transform/resource/after-delombok/Constructors.java index db48b6b8..d4633dbc 100644 --- a/test/transform/resource/after-delombok/Constructors.java +++ b/test/transform/resource/after-delombok/Constructors.java @@ -58,4 +58,18 @@ class RequiredArgsConstructorStaticNameGenerics { public static RequiredArgsConstructorStaticNameGenerics of(final T x) { return new RequiredArgsConstructorStaticNameGenerics(x); } +} +class RequiredArgsConstructorStaticNameGenerics2 { + final Class x; + String name; + + @java.lang.SuppressWarnings("all") + private RequiredArgsConstructorStaticNameGenerics2(final Class x) { + this.x = x; + } + + @java.lang.SuppressWarnings("all") + public static RequiredArgsConstructorStaticNameGenerics2 of(final Class x) { + return new RequiredArgsConstructorStaticNameGenerics2(x); + } } \ No newline at end of file diff --git a/test/transform/resource/after-ecj/Constructors.java b/test/transform/resource/after-ecj/Constructors.java index e6dd8a21..e994702f 100644 --- a/test/transform/resource/after-ecj/Constructors.java +++ b/test/transform/resource/after-ecj/Constructors.java @@ -51,4 +51,15 @@ public static @java.lang.SuppressWarnings("all") RequiredArgsConstructorStaticNameGenerics of(final T x) { return new RequiredArgsConstructorStaticNameGenerics(x); } +} +@lombok.RequiredArgsConstructor(staticName = "of") class RequiredArgsConstructorStaticNameGenerics2 { + final Class x; + String name; + private @java.lang.SuppressWarnings("all") RequiredArgsConstructorStaticNameGenerics2(final Class x) { + super(); + this.x = x; + } + public static @java.lang.SuppressWarnings("all") RequiredArgsConstructorStaticNameGenerics2 of(final Class x) { + return new RequiredArgsConstructorStaticNameGenerics2(x); + } } \ No newline at end of file diff --git a/test/transform/resource/before/Constructors.java b/test/transform/resource/before/Constructors.java index 272fa850..d3ed3504 100644 --- a/test/transform/resource/before/Constructors.java +++ b/test/transform/resource/before/Constructors.java @@ -22,3 +22,7 @@ final T x; String name; } +@lombok.RequiredArgsConstructor(staticName="of") class RequiredArgsConstructorStaticNameGenerics2 { + final Class x; + String name; +} -- cgit From 98c704c7cb4b21d0d6e11f55c52da89cb7c22502 Mon Sep 17 00:00:00 2001 From: Philipp Eichhorn Date: Wed, 18 Jul 2012 19:53:29 +0200 Subject: Lombok not longer removes the feature annotations and the import declarations of said annotations when running in Netbeans. This solves another batch of usability issues in Netbeans. --- src/core/lombok/javac/handlers/JavacHandlerUtil.java | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/core/lombok/javac/handlers/JavacHandlerUtil.java b/src/core/lombok/javac/handlers/JavacHandlerUtil.java index ce7421b1..fcafb88e 100644 --- a/src/core/lombok/javac/handlers/JavacHandlerUtil.java +++ b/src/core/lombok/javac/handlers/JavacHandlerUtil.java @@ -250,6 +250,7 @@ public class JavacHandlerUtil { * Only does this if the DeleteLombokAnnotations class is in the context. */ public static void deleteAnnotationIfNeccessary(JavacNode annotation, Class annotationType) { + if (inNetbeansEditor(annotation)) return; if (!annotation.shouldDeleteLombokAnnotations()) return; JavacNode parentNode = annotation.directUp(); switch (parentNode.getKind()) { @@ -280,6 +281,7 @@ public class JavacHandlerUtil { } public static void deleteImportFromCompilationUnit(JavacNode node, String name) { + if (inNetbeansEditor(node)) return; if (!node.shouldDeleteLombokAnnotations()) return; ListBuffer newDefs = ListBuffer.lb(); -- cgit From 34055fcdff786c9b809ce1a08c1c9218968ebc7d Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Sun, 22 Jul 2012 07:17:52 +0200 Subject: A potential fix for issue #394; Memory leaks in eclipse introduced in lombok 0.11.2 due to a fix involving WeakHashMaps for lazy getters of type boolean. --- src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java index 7120a602..ae488612 100644 --- a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java +++ b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java @@ -773,18 +773,19 @@ public class EclipseHandlerUtil { } } - private static final Map generatedLazyGetters = new WeakHashMap(); + private static final Map generatedLazyGettersWithPrimitiveBoolean = new WeakHashMap(); + private static final Object MARKER = new Object(); static void registerCreatedLazyGetter(FieldDeclaration field, char[] methodName, TypeReference returnType) { - generatedLazyGetters.put(field, new GetterMethod(methodName, returnType)); + if (!nameEquals(returnType.getTypeName(), "boolean") || returnType.dimensions() > 0) return; + generatedLazyGettersWithPrimitiveBoolean.put(field, MARKER); } private static GetterMethod findGetter(EclipseNode field) { FieldDeclaration fieldDeclaration = (FieldDeclaration) field.get(); - GetterMethod gm = generatedLazyGetters.get(fieldDeclaration); - if (gm != null) return gm; + boolean forceBool = generatedLazyGettersWithPrimitiveBoolean.containsKey(fieldDeclaration); TypeReference fieldType = fieldDeclaration.type; - boolean isBoolean = nameEquals(fieldType.getTypeName(), "boolean") && fieldType.dimensions() == 0; + boolean isBoolean = forceBool || (nameEquals(fieldType.getTypeName(), "boolean") && fieldType.dimensions() == 0); EclipseNode typeNode = field.up(); for (String potentialGetterName : toAllGetterNames(field, isBoolean)) { -- cgit