From 3adf835e48f064a4b8ef1d24d3d32d1827c03f28 Mon Sep 17 00:00:00 2001 From: Jappe van der Hel Date: Wed, 4 Jan 2012 00:18:35 +0100 Subject: "QualifiedName"'s are now also marked as generated during ASTConverter phase (just like "SimpleName"'s) This fixes "Rename" when inner types are used (for both the main type and the inner type) --- .../lombok/eclipse/agent/EclipsePatcher.java | 55 ++++++++++++++++++++-- .../lombok/eclipse/agent/PatchFixes.java | 2 +- 2 files changed, 51 insertions(+), 6 deletions(-) (limited to 'src/eclipseAgent/lombok/eclipse/agent') diff --git a/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java b/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java index a721b468..21bf23f3 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java +++ b/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java @@ -395,14 +395,26 @@ public class EclipsePatcher extends Agent { .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "setIsGeneratedFlag", "void", "org.eclipse.jdt.core.dom.ASTNode", "org.eclipse.jdt.internal.compiler.ast.ASTNode")) .transplant().build()); - + sm.addScript(ScriptBuilder.wrapReturnValue() .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "convertToFieldDeclaration", "org.eclipse.jdt.core.dom.FieldDeclaration", "org.eclipse.jdt.internal.compiler.ast.FieldDeclaration")) +/* Targets beneath are only patched because the resulting dom nodes should be marked if generated. + * However I couldn't find a usecase where these were actually used + */ + .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "convertToType", "org.eclipse.jdt.core.dom.Type", "org.eclipse.jdt.internal.compiler.ast.NameReference")) + .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "convertType", "org.eclipse.jdt.core.dom.Type", "org.eclipse.jdt.internal.compiler.ast.TypeReference")) + .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "convertToVariableDeclarationExpression", "org.eclipse.jdt.core.dom.VariableDeclarationExpression", "org.eclipse.jdt.internal.compiler.ast.LocalDeclaration")) + .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "convertToSingleVariableDeclaration", "org.eclipse.jdt.core.dom.SingleVariableDeclaration", "org.eclipse.jdt.internal.compiler.ast.LocalDeclaration")) + .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "convertToVariableDeclarationFragment", "org.eclipse.jdt.core.dom.VariableDeclarationFragment", "org.eclipse.jdt.internal.compiler.ast.FieldDeclaration")) + .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "convertToVariableDeclarationFragment", "org.eclipse.jdt.core.dom.VariableDeclarationFragment", "org.eclipse.jdt.internal.compiler.ast.LocalDeclaration")) + .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "convertToVariableDeclarationStatement", "org.eclipse.jdt.core.dom.VariableDeclarationStatement", "org.eclipse.jdt.internal.compiler.ast.LocalDeclaration")) +/* Targets above are only patched because the resulting dom nodes should be marked if generated. */ .request(StackRequest.PARAM1, StackRequest.RETURN_VALUE) .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "setIsGeneratedFlag", "void", "org.eclipse.jdt.core.dom.ASTNode", "org.eclipse.jdt.internal.compiler.ast.ASTNode")) .transplant().build()); + /* Set generated flag for SimpleNames */ sm.addScript(ScriptBuilder.wrapMethodCall() .target(new TargetMatcher() { @Override public boolean matches(String classSpec, String methodName, String descriptor) { @@ -420,16 +432,49 @@ public class EclipsePatcher extends Agent { } }).methodToWrap(new Hook("org.eclipse.jdt.core.dom.SimpleName", "", "void", "org.eclipse.jdt.core.dom.AST")) .requestExtra(StackRequest.PARAM1) - .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "setIsGeneratedFlagForSimpleName", "void", - "org.eclipse.jdt.core.dom.SimpleName", "java.lang.Object")) + .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "setIsGeneratedFlagForName", "void", + "org.eclipse.jdt.core.dom.Name", "java.lang.Object")) .transplant().build()); sm.addScript(ScriptBuilder.wrapMethodCall() .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "convert", "org.eclipse.jdt.core.dom.ASTNode", "boolean", "org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration")) .methodToWrap(new Hook("org.eclipse.jdt.core.dom.SimpleName", "", "void", "org.eclipse.jdt.core.dom.AST")) .requestExtra(StackRequest.PARAM2) - .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "setIsGeneratedFlagForSimpleName", "void", - "org.eclipse.jdt.core.dom.SimpleName", "java.lang.Object")) + .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "setIsGeneratedFlagForName", "void", + "org.eclipse.jdt.core.dom.Name", "java.lang.Object")) + .transplant().build()); + + /* Set generated flag for QualifiedNames */ + sm.addScript(ScriptBuilder.wrapMethodCall() + .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "setQualifiedNameNameAndSourceRanges", "org.eclipse.jdt.core.dom.QualifiedName", "char[][]", "long[]", "int", "org.eclipse.jdt.internal.compiler.ast.ASTNode")) + .methodToWrap(new Hook("org.eclipse.jdt.core.dom.SimpleName", "", "void", "org.eclipse.jdt.core.dom.AST")) + .requestExtra(StackRequest.PARAM4) + .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "setIsGeneratedFlagForName", "void", + "org.eclipse.jdt.core.dom.Name", "java.lang.Object")) + .transplant().build()); + + sm.addScript(ScriptBuilder.wrapMethodCall() + .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "setQualifiedNameNameAndSourceRanges", "org.eclipse.jdt.core.dom.QualifiedName", "char[][]", "long[]", "int", "org.eclipse.jdt.internal.compiler.ast.ASTNode")) + .methodToWrap(new Hook("org.eclipse.jdt.core.dom.QualifiedName", "", "void", "org.eclipse.jdt.core.dom.AST")) + .requestExtra(StackRequest.PARAM4) + .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "setIsGeneratedFlagForName", "void", + "org.eclipse.jdt.core.dom.Name", "java.lang.Object")) + .transplant().build()); + + sm.addScript(ScriptBuilder.wrapMethodCall() + .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "setQualifiedNameNameAndSourceRanges", "org.eclipse.jdt.core.dom.QualifiedName", "char[][]", "long[]", "org.eclipse.jdt.internal.compiler.ast.ASTNode")) + .methodToWrap(new Hook("org.eclipse.jdt.core.dom.SimpleName", "", "void", "org.eclipse.jdt.core.dom.AST")) + .requestExtra(StackRequest.PARAM3) + .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "setIsGeneratedFlagForName", "void", + "org.eclipse.jdt.core.dom.Name", "java.lang.Object")) + .transplant().build()); + + sm.addScript(ScriptBuilder.wrapMethodCall() + .target(new MethodTarget("org.eclipse.jdt.core.dom.ASTConverter", "setQualifiedNameNameAndSourceRanges", "org.eclipse.jdt.core.dom.QualifiedName", "char[][]", "long[]", "org.eclipse.jdt.internal.compiler.ast.ASTNode")) + .methodToWrap(new Hook("org.eclipse.jdt.core.dom.QualifiedName", "", "void", "org.eclipse.jdt.core.dom.AST")) + .requestExtra(StackRequest.PARAM3) + .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "setIsGeneratedFlagForName", "void", + "org.eclipse.jdt.core.dom.Name", "java.lang.Object")) .transplant().build()); } diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java index 53d60bde..9f7abbe3 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java +++ b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java @@ -212,7 +212,7 @@ public class PatchFixes { } } - public static void setIsGeneratedFlagForSimpleName(SimpleName name, Object internalNode) throws Exception { + public static void setIsGeneratedFlagForName(org.eclipse.jdt.core.dom.Name name, Object internalNode) throws Exception { if (internalNode instanceof org.eclipse.jdt.internal.compiler.ast.ASTNode) { if (internalNode.getClass().getField("$generatedBy").get(internalNode) != null) { name.getClass().getField("$isGenerated").set(name, true); -- cgit From 1a08aa5cbca685ffd8b056f4a4fc64ce9912027c Mon Sep 17 00:00:00 2001 From: Jappe van der Hel Date: Tue, 10 Jan 2012 13:22:38 +0100 Subject: removeGeneratedMethods was broken, causing the rename to fail --- src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/eclipseAgent/lombok/eclipse/agent') diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java index 9f7abbe3..c2ab45e0 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java +++ b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java @@ -258,9 +258,9 @@ public class PatchFixes { public static IMethod[] removeGeneratedMethods(IMethod[] methods) throws Exception { List result = new ArrayList(); for (IMethod m : methods) { - if (m.getNameRange().getLength() > 0) result.add(m); + if (m.getNameRange().getLength() > 0 && !m.getNameRange().equals(m.getSourceRange())) result.add(m); } - return result.size() == methods.length ? methods : result.toArray(new IMethod[0]); + return result.size() == methods.length ? methods : result.toArray(new IMethod[result.size()]); } public static SimpleName[] removeGeneratedSimpleNames(SimpleName[] in) throws Exception { -- cgit From d27b857bb092088ac2496058952026382f54c081 Mon Sep 17 00:00:00 2001 From: Jappe van der Hel Date: Tue, 10 Jan 2012 13:30:16 +0100 Subject: 1) We now honor the "public" and "abstract" modifier settings 2) Annotations are now handled, but @SuppressWarnings is skipped --- .../lombok/eclipse/agent/EclipsePatcher.java | 12 +-- .../lombok/eclipse/agent/PatchFixes.java | 87 +++++++++++++++++++--- 2 files changed, 84 insertions(+), 15 deletions(-) (limited to 'src/eclipseAgent/lombok/eclipse/agent') diff --git a/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java b/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java index a721b468..df7c71db 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java +++ b/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java @@ -123,9 +123,9 @@ public class EclipsePatcher extends Agent { "org.eclipse.jdt.core.dom.MethodDeclaration" )) .methodToWrap(new Hook("org.eclipse.jface.text.IDocument", "get", "java.lang.String", "int", "int")) - .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "getRealMethodDeclarationSource", "java.lang.String", "java.lang.String", "org.eclipse.jdt.core.dom.MethodDeclaration")) - .requestExtra(StackRequest.PARAM4) - .build()); + .wrapMethod(new Hook("lombok.eclipse.agent.PatchFixes", "getRealMethodDeclarationSource", "java.lang.String", "java.lang.String", "java.lang.Object", "org.eclipse.jdt.core.dom.MethodDeclaration")) + .requestExtra(StackRequest.THIS, StackRequest.PARAM4) + .transplant().build()); /* get real generated node in stead of a random one generated by the annotation */ sm.addScript(ScriptBuilder.replaceMethodCall() @@ -133,21 +133,21 @@ public class EclipsePatcher extends Agent { .target(new MethodTarget("org.eclipse.jdt.internal.corext.refactoring.structure.ExtractInterfaceProcessor", "createMethodComments")) .methodToReplace(new Hook("org.eclipse.jdt.internal.corext.refactoring.structure.ASTNodeSearchUtil", "getMethodDeclarationNode", "org.eclipse.jdt.core.dom.MethodDeclaration", "org.eclipse.jdt.core.IMethod", "org.eclipse.jdt.core.dom.CompilationUnit")) .replacementMethod(new Hook("lombok.eclipse.agent.PatchFixes", "getRealMethodDeclarationNode", "org.eclipse.jdt.core.dom.MethodDeclaration", "org.eclipse.jdt.core.IMethod", "org.eclipse.jdt.core.dom.CompilationUnit")) - .build()); + .transplant().build()); /* Do not add @Override's for generated methods */ sm.addScript(ScriptBuilder.exitEarly() .target(new MethodTarget("org.eclipse.jdt.core.dom.rewrite.ListRewrite", "insertFirst")) .decisionMethod(new Hook("lombok.eclipse.agent.PatchFixes", "isListRewriteOnGeneratedNode", "boolean", "org.eclipse.jdt.core.dom.rewrite.ListRewrite")) .request(StackRequest.THIS) - .build()); + .transplant().build()); /* Do not add comments for generated methods */ sm.addScript(ScriptBuilder.exitEarly() .target(new MethodTarget("org.eclipse.jdt.internal.corext.refactoring.structure.ExtractInterfaceProcessor", "createMethodComment")) .decisionMethod(new Hook("lombok.eclipse.agent.PatchFixes", "isGenerated", "boolean", "org.eclipse.jdt.core.dom.ASTNode")) .request(StackRequest.PARAM2) - .build()); + .transplant().build()); } private static void patchAboutDialog(ScriptManager sm) { diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java index 53d60bde..38928fec 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java +++ b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java @@ -27,6 +27,7 @@ import java.io.IOException; import java.io.OutputStream; import java.lang.reflect.Field; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Stack; @@ -91,15 +92,49 @@ public class PatchFixes { return list; } - /* Very practical implementation, but works for getter and setter even with type parameters */ - public static java.lang.String getRealMethodDeclarationSource(java.lang.String original, org.eclipse.jdt.core.dom.MethodDeclaration declaration) { + public static java.lang.String getRealMethodDeclarationSource(java.lang.String original, Object processor, org.eclipse.jdt.core.dom.MethodDeclaration declaration) throws Exception { if (!isGenerated(declaration)) return original; - StringBuilder signature = new StringBuilder(); + boolean fPublic = (Boolean)processor.getClass().getDeclaredField("fPublic").get(processor); + boolean fAbstract = (Boolean)processor.getClass().getDeclaredField("fAbstract").get(processor); + + final List skippedAnnotations = new ArrayList(); + skippedAnnotations.add("java.lang.Override"); // always skipped by extract interface + skippedAnnotations.add("java.lang.SuppressWarnings"); // always added by lombok + + // Near copy of ExtractInterFaceProcessor.createMethodDeclaration(...) + // couldn't find a way to pass these as parameters + boolean publicFound= false; + boolean abstractFound= false; - // We should get these from the refactor action - boolean needsPublic = true, needsAbstract = true; + List annotations = new ArrayList(); + org.eclipse.jdt.core.dom.Modifier modifier= null; + org.eclipse.jdt.core.dom.IExtendedModifier extended= null; + for (final Iterator iterator= declaration.modifiers().iterator(); iterator.hasNext();) { + extended= (org.eclipse.jdt.core.dom.IExtendedModifier) iterator.next(); + if (!extended.isAnnotation()) { + modifier= (org.eclipse.jdt.core.dom.Modifier) extended; + if (fPublic && modifier.getKeyword().equals(org.eclipse.jdt.core.dom.Modifier.ModifierKeyword.PUBLIC_KEYWORD)) { + publicFound= true; + continue; + } + if (fAbstract && modifier.getKeyword().equals(org.eclipse.jdt.core.dom.Modifier.ModifierKeyword.ABSTRACT_KEYWORD)) { + abstractFound= true; + continue; + } + } else { + org.eclipse.jdt.core.dom.Annotation annotation = (org.eclipse.jdt.core.dom.Annotation)extended; + if (!skippedAnnotations.contains(annotation.resolveTypeBinding().getQualifiedName())) + annotations.add(annotation); + } + } + + boolean needsPublic = fPublic && !publicFound; + boolean needsAbstract = fAbstract && !abstractFound; + // END Near copy of ExtractInterFaceProcessor.createMethodDeclaration(...) + StringBuilder signature = new StringBuilder(); + addAnnotations(annotations, signature); if (needsPublic) signature.append("public "); if (needsAbstract) signature.append("abstract "); @@ -112,15 +147,48 @@ public class PatchFixes { for (Object parameter : declaration.parameters()) { if (!first) signature.append(", "); first = false; - // The annotations are still missing - // Note: what happens to imports for the annotations? - // I assume they have been taken care of by the default extraction system signature.append(parameter); } signature.append(");"); return signature.toString(); } + + // part of getRealMethodDeclarationSource(...) + public static void addAnnotations(List annotations, StringBuilder signature) { + for (org.eclipse.jdt.core.dom.Annotation annotation : annotations) { + boolean lombokFound = false; + List values = new ArrayList(); + if (annotation.isSingleMemberAnnotation()) { + org.eclipse.jdt.core.dom.SingleMemberAnnotation smAnn = (org.eclipse.jdt.core.dom.SingleMemberAnnotation) annotation; + values.add(smAnn.getValue().toString()); + + } else if (annotation.isNormalAnnotation()) { + org.eclipse.jdt.core.dom.NormalAnnotation normalAnn = (org.eclipse.jdt.core.dom.NormalAnnotation) annotation; + for (Object value : normalAnn.values()) { + values.add(value.toString()); + } + } + + if (!lombokFound) { + signature.append("@").append(annotation.resolveTypeBinding().getQualifiedName()); + if (!values.isEmpty()) { + signature.append("("); + boolean first = true; + for (String string : values) { + if (first) { + first = false; + } else { + signature.append(","); + } + signature.append('"').append(string).append('"'); + } + signature.append(")"); + } + signature.append(" "); + } + } + } public static org.eclipse.jdt.core.dom.MethodDeclaration getRealMethodDeclarationNode(org.eclipse.jdt.core.IMethod sourceMethod, org.eclipse.jdt.core.dom.CompilationUnit cuUnit) throws JavaModelException { @@ -154,7 +222,8 @@ public class PatchFixes { return methodDeclarationNode; } - private static org.eclipse.jdt.core.dom.AbstractTypeDeclaration findTypeDeclaration(IType searchType, List nodes) { + // part of getRealMethodDeclarationNode + public static org.eclipse.jdt.core.dom.AbstractTypeDeclaration findTypeDeclaration(IType searchType, List nodes) { for (Object object : nodes) { if (object instanceof org.eclipse.jdt.core.dom.AbstractTypeDeclaration) { org.eclipse.jdt.core.dom.AbstractTypeDeclaration typeDeclaration = (org.eclipse.jdt.core.dom.AbstractTypeDeclaration) object; -- cgit From 1514316a5d9ad721625b27625f2716e74e2520c2 Mon Sep 17 00:00:00 2001 From: Sander Koning Date: Mon, 16 Jan 2012 21:26:39 +0100 Subject: Code review --- .../lombok/eclipse/agent/PatchFixes.java | 98 +++++++++------------- 1 file changed, 39 insertions(+), 59 deletions(-) (limited to 'src/eclipseAgent/lombok/eclipse/agent') diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java index 38928fec..ec071042 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java +++ b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java @@ -42,7 +42,9 @@ import org.eclipse.jdt.core.IMethod; import org.eclipse.jdt.core.IType; import org.eclipse.jdt.core.JavaModelException; import org.eclipse.jdt.core.dom.MethodDeclaration; +import org.eclipse.jdt.core.dom.Modifier; import org.eclipse.jdt.core.dom.SimpleName; +import org.eclipse.jdt.core.dom.SingleVariableDeclaration; import org.eclipse.jdt.internal.compiler.ast.Annotation; import org.eclipse.jdt.internal.core.dom.rewrite.NodeRewriteEvent; import org.eclipse.jdt.internal.core.dom.rewrite.RewriteEvent; @@ -95,48 +97,20 @@ public class PatchFixes { public static java.lang.String getRealMethodDeclarationSource(java.lang.String original, Object processor, org.eclipse.jdt.core.dom.MethodDeclaration declaration) throws Exception { if (!isGenerated(declaration)) return original; - boolean fPublic = (Boolean)processor.getClass().getDeclaredField("fPublic").get(processor); - boolean fAbstract = (Boolean)processor.getClass().getDeclaredField("fAbstract").get(processor); - - final List skippedAnnotations = new ArrayList(); - skippedAnnotations.add("java.lang.Override"); // always skipped by extract interface - skippedAnnotations.add("java.lang.SuppressWarnings"); // always added by lombok - - // Near copy of ExtractInterFaceProcessor.createMethodDeclaration(...) - // couldn't find a way to pass these as parameters - boolean publicFound= false; - boolean abstractFound= false; - List annotations = new ArrayList(); - org.eclipse.jdt.core.dom.Modifier modifier= null; - org.eclipse.jdt.core.dom.IExtendedModifier extended= null; - for (final Iterator iterator= declaration.modifiers().iterator(); iterator.hasNext();) { - extended= (org.eclipse.jdt.core.dom.IExtendedModifier) iterator.next(); - if (!extended.isAnnotation()) { - modifier= (org.eclipse.jdt.core.dom.Modifier) extended; - if (fPublic && modifier.getKeyword().equals(org.eclipse.jdt.core.dom.Modifier.ModifierKeyword.PUBLIC_KEYWORD)) { - publicFound= true; - continue; - } - if (fAbstract && modifier.getKeyword().equals(org.eclipse.jdt.core.dom.Modifier.ModifierKeyword.ABSTRACT_KEYWORD)) { - abstractFound= true; - continue; - } - } else { - org.eclipse.jdt.core.dom.Annotation annotation = (org.eclipse.jdt.core.dom.Annotation)extended; - if (!skippedAnnotations.contains(annotation.resolveTypeBinding().getQualifiedName())) - annotations.add(annotation); + for (Object modifier : declaration.modifiers()) { + if (modifier instanceof org.eclipse.jdt.core.dom.Annotation) { + org.eclipse.jdt.core.dom.Annotation annotation = (org.eclipse.jdt.core.dom.Annotation)modifier; + String qualifiedAnnotationName = annotation.resolveTypeBinding().getQualifiedName(); + if (!"java.lang.Override".equals(qualifiedAnnotationName) && !"java.lang.SuppressWarnings".equals(qualifiedAnnotationName)) annotations.add(annotation); } } - - boolean needsPublic = fPublic && !publicFound; - boolean needsAbstract = fAbstract && !abstractFound; - // END Near copy of ExtractInterFaceProcessor.createMethodDeclaration(...) StringBuilder signature = new StringBuilder(); addAnnotations(annotations, signature); - if (needsPublic) signature.append("public "); - if (needsAbstract) signature.append("abstract "); + + if ((Boolean)processor.getClass().getDeclaredField("fPublic").get(processor)) signature.append("public "); + if ((Boolean)processor.getClass().getDeclaredField("fAbstract").get(processor)) signature.append("abstract "); signature .append(declaration.getReturnType2().toString()) @@ -147,50 +121,56 @@ public class PatchFixes { for (Object parameter : declaration.parameters()) { if (!first) signature.append(", "); first = false; + // We should also add the annotations of the parameters signature.append(parameter); } signature.append(");"); return signature.toString(); } - + // part of getRealMethodDeclarationSource(...) public static void addAnnotations(List annotations, StringBuilder signature) { + /* + * We SHOULD be able to handle the following cases: + * @Override + * @Override() + * @SuppressWarnings("all") + * @SuppressWarnings({"all", "unused"}) + * @SuppressWarnings(value = "all") + * @SuppressWarnings(value = {"all", "unused"}) + * @EqualsAndHashCode(callSuper=true, of="id") + * + * Currently, we only seem to correctly support: + * @Override + * @Override() N.B. We lose the parentheses here, since there are no values. No big deal. + * @SuppressWarnings("all") + */ for (org.eclipse.jdt.core.dom.Annotation annotation : annotations) { - boolean lombokFound = false; List values = new ArrayList(); if (annotation.isSingleMemberAnnotation()) { org.eclipse.jdt.core.dom.SingleMemberAnnotation smAnn = (org.eclipse.jdt.core.dom.SingleMemberAnnotation) annotation; values.add(smAnn.getValue().toString()); - } else if (annotation.isNormalAnnotation()) { org.eclipse.jdt.core.dom.NormalAnnotation normalAnn = (org.eclipse.jdt.core.dom.NormalAnnotation) annotation; - for (Object value : normalAnn.values()) { - values.add(value.toString()); - } + for (Object value : normalAnn.values()) values.add(value.toString()); } - - if (!lombokFound) { - signature.append("@").append(annotation.resolveTypeBinding().getQualifiedName()); - if (!values.isEmpty()) { - signature.append("("); - boolean first = true; - for (String string : values) { - if (first) { - first = false; - } else { - signature.append(","); - } - signature.append('"').append(string).append('"'); - } - signature.append(")"); + + signature.append("@").append(annotation.resolveTypeBinding().getQualifiedName()); + if (!values.isEmpty()) { + signature.append("("); + boolean first = true; + for (String string : values) { + if (!first) signature.append(", "); + first = false; + signature.append('"').append(string).append('"'); } - signature.append(" "); + signature.append(")"); } + signature.append(" "); } } - public static org.eclipse.jdt.core.dom.MethodDeclaration getRealMethodDeclarationNode(org.eclipse.jdt.core.IMethod sourceMethod, org.eclipse.jdt.core.dom.CompilationUnit cuUnit) throws JavaModelException { MethodDeclaration methodDeclarationNode = ASTNodeSearchUtil.getMethodDeclarationNode(sourceMethod, cuUnit); if (isGenerated(methodDeclarationNode)) { -- cgit From 7627d03afd8d10cbdefc93f0317f4d5f98cc6526 Mon Sep 17 00:00:00 2001 From: Sander Koning Date: Mon, 16 Jan 2012 22:34:12 +0100 Subject: Issue 325: revert old behaviour of fixRetrieveRightBrace... --- src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/eclipseAgent/lombok/eclipse/agent') diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java index 65852687..86f2b9cf 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java +++ b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java @@ -237,7 +237,8 @@ public class PatchFixes { } public static int fixRetrieveRightBraceOrSemiColonPosition(int original, int end) { - return original == -1 ? end : original; + return original; + // return original == -1 ? end : original; // Need to fix: see issue 325. } public static final int ALREADY_PROCESSED_FLAG = 0x800000; //Bit 24 -- cgit From 5185f97cd6dd7697eeca2455897cbad9dea9ed96 Mon Sep 17 00:00:00 2001 From: Jappe van der Hel Date: Wed, 18 Jan 2012 11:53:17 +0100 Subject: 1) rollback of rollback fixRetrieveRightBraceOrSemiColonPosition 2) HandleEqualsAndHashCode now use SetGeneratedByVisitor to ensure "correct" sourcepositions 3) SetGeneratedByVisitor now sets QualifiedNameReference.sourcePositions --- src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java | 3 +++ src/core/lombok/eclipse/handlers/SetGeneratedByVisitor.java | 11 +++++++++-- src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java | 4 ++-- 3 files changed, 14 insertions(+), 4 deletions(-) (limited to 'src/eclipseAgent/lombok/eclipse/agent') diff --git a/src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java b/src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java index d4c63da3..ef01835c 100644 --- a/src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java +++ b/src/core/lombok/eclipse/handlers/HandleEqualsAndHashCode.java @@ -222,14 +222,17 @@ public class HandleEqualsAndHashCode extends EclipseAnnotationHandler Date: Mon, 5 Dec 2011 22:42:50 +0100 Subject: [Issue 275] Allow @Delegate on no-argument methods --- src/core/lombok/Delegate.java | 2 +- src/core/lombok/javac/handlers/HandleDelegate.java | 67 ++++++-- .../lombok/eclipse/agent/PatchDelegate.java | 184 +++++++++++++++------ .../resource/after-delombok/DelegateOnMethods.java | 12 ++ .../resource/after-ecj/DelegateOnMethods.java | 1 + .../resource/after-eclipse/DelegateOnMethods.java | 13 ++ .../resource/before/DelegateOnMethods.java | 11 ++ 7 files changed, 222 insertions(+), 68 deletions(-) create mode 100644 test/transform/resource/after-delombok/DelegateOnMethods.java create mode 100644 test/transform/resource/after-ecj/DelegateOnMethods.java create mode 100644 test/transform/resource/after-eclipse/DelegateOnMethods.java create mode 100644 test/transform/resource/before/DelegateOnMethods.java (limited to 'src/eclipseAgent/lombok/eclipse/agent') diff --git a/src/core/lombok/Delegate.java b/src/core/lombok/Delegate.java index 6d03d649..9ab9acae 100644 --- a/src/core/lombok/Delegate.java +++ b/src/core/lombok/Delegate.java @@ -40,7 +40,7 @@ import java.lang.annotation.Target; * that exist in {@link Object}, the {@code canEqual(Object)} method, and any methods that appear in types * that are listed in the {@code excludes} property. */ -@Target(ElementType.FIELD) +@Target({ElementType.FIELD, ElementType.METHOD}) @Retention(RetentionPolicy.SOURCE) public @interface Delegate { /** diff --git a/src/core/lombok/javac/handlers/HandleDelegate.java b/src/core/lombok/javac/handlers/HandleDelegate.java index f6a81474..3674ae5a 100644 --- a/src/core/lombok/javac/handlers/HandleDelegate.java +++ b/src/core/lombok/javac/handlers/HandleDelegate.java @@ -94,23 +94,42 @@ public class HandleDelegate extends JavacAnnotationHandler { @Override public void handle(AnnotationValues annotation, JCAnnotation ast, JavacNode annotationNode) { deleteAnnotationIfNeccessary(annotationNode, Delegate.class); - if (annotationNode.up().getKind() != Kind.FIELD) { - // As the annotation is legal on fields only, javac itself will take care of printing an error message for this. + + Type delegateType; + Name delegateName = annotationNode.toName(annotationNode.up().getName()); + DelegateReceiver delegateReceiver; + JavacResolution reso = new JavacResolution(annotationNode.getContext()); + if (annotationNode.up().getKind() == Kind.FIELD) { + delegateReceiver = DelegateReceiver.FIELD; + delegateType = annotationNode.up().get().type; + if (delegateType == null) reso.resolveClassMember(annotationNode.up()); + delegateType = annotationNode.up().get().type; + } else if (annotationNode.up().getKind() == Kind.METHOD) { + if (!(annotationNode.up().get() instanceof JCMethodDecl)) { + annotationNode.addError("@Delegate is legal only on no-argument methods."); + return; + } + JCMethodDecl methodDecl = (JCMethodDecl) annotationNode.up().get(); + if (!methodDecl.params.isEmpty()) { + annotationNode.addError("@Delegate is legal only on no-argument methods."); + return; + } + delegateReceiver = DelegateReceiver.METHOD; + delegateType = methodDecl.restype.type; + if (delegateType == null) reso.resolveClassMember(annotationNode.up()); + delegateType = methodDecl.restype.type; + } else { + // As the annotation is legal on fields and methods only, javac itself will take care of printing an error message for this. return; } List delegateTypes = annotation.getActualExpressions("types"); List excludeTypes = annotation.getActualExpressions("excludes"); - JavacResolution reso = new JavacResolution(annotationNode.getContext()); List toDelegate = new ArrayList(); List toExclude = new ArrayList(); if (delegateTypes.isEmpty()) { - Type type = ((JCVariableDecl)annotationNode.up().get()).type; - if (type == null) reso.resolveClassMember(annotationNode.up()); - //TODO I'm fairly sure the above line (and that entire method) does effectively bupkis! - type = ((JCVariableDecl)annotationNode.up().get()).type; - if (type != null) toDelegate.add(type); + if (delegateType != null) toDelegate.add(delegateType); } else { for (Object dt : delegateTypes) { if (dt instanceof JCFieldAccess && ((JCFieldAccess)dt).name.toString().equals("class")) { @@ -167,15 +186,13 @@ public class HandleDelegate extends JavacAnnotationHandler { } } - Name delegateFieldName = annotationNode.toName(annotationNode.up().getName()); - - for (MethodSig sig : signaturesToDelegate) generateAndAdd(sig, annotationNode, delegateFieldName); + for (MethodSig sig : signaturesToDelegate) generateAndAdd(sig, annotationNode, delegateName, delegateReceiver); } - private void generateAndAdd(MethodSig sig, JavacNode annotation, Name delegateFieldName) { + private void generateAndAdd(MethodSig sig, JavacNode annotation, Name delegateName, DelegateReceiver delegateReceiver) { List toAdd = new ArrayList(); try { - toAdd.add(createDelegateMethod(sig, annotation, delegateFieldName)); + toAdd.add(createDelegateMethod(sig, annotation, delegateName, delegateReceiver)); } catch (TypeNotConvertibleException e) { annotation.addError("Can't create delegate method for " + sig.name + ": " + e.getMessage()); return; @@ -240,7 +257,7 @@ public class HandleDelegate extends JavacAnnotationHandler { } } - private JCMethodDecl createDelegateMethod(MethodSig sig, JavacNode annotation, Name delegateFieldName) throws TypeNotConvertibleException, CantMakeDelegates { + private JCMethodDecl createDelegateMethod(MethodSig sig, JavacNode annotation, Name delegateName, DelegateReceiver delegateReceiver) throws TypeNotConvertibleException, CantMakeDelegates { /* public ReturnType methodName(ParamType1 name1, ParamType2 name2, ...) throws T1, T2, ... { * (return) delegate.methodName(name1, name2); * } @@ -288,9 +305,7 @@ public class HandleDelegate extends JavacAnnotationHandler { args.append(maker.Ident(name)); } - JCExpression delegateFieldRef = maker.Select(maker.Ident(annotation.toName("this")), delegateFieldName); - - JCExpression delegateCall = maker.Apply(toList(typeArgs), maker.Select(delegateFieldRef, sig.name), toList(args)); + JCExpression delegateCall = maker.Apply(toList(typeArgs), maker.Select(delegateReceiver.get(annotation, delegateName), sig.name), toList(args)); JCStatement body = useReturn ? maker.Return(delegateCall) : maker.Exec(delegateCall); JCBlock bodyBlock = maker.Block(0, com.sun.tools.javac.util.List.of(body)); @@ -367,4 +382,22 @@ public class HandleDelegate extends JavacAnnotationHandler { binding = types.erasure(binding); return binding.toString(); } + + private enum DelegateReceiver { + METHOD { + public JCExpression get(final JavacNode node, final Name name) { + com.sun.tools.javac.util.List nilExprs = com.sun.tools.javac.util.List.nil(); + final TreeMaker maker = node.getTreeMaker(); + return maker.Apply(nilExprs, maker.Select(maker.Ident(node.toName("this")), name), nilExprs); + } + }, + FIELD { + public JCExpression get(final JavacNode node, final Name name) { + final TreeMaker maker = node.getTreeMaker(); + return maker.Select(maker.Ident(node.toName("this")), name); + } + }; + + public abstract JCExpression get(final JavacNode node, final Name name); + } } diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java b/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java index affcd4f2..d6310de1 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java +++ b/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java @@ -103,25 +103,25 @@ public class PatchDelegate { return new String(decl.name); } - private static boolean hasDelegateMarkedFields(TypeDeclaration decl) { + private static boolean hasDelegateMarkedFieldsOrMethods(TypeDeclaration decl) { if (decl.fields != null) for (FieldDeclaration field : decl.fields) { if (field.annotations == null) continue; for (Annotation ann : field.annotations) { - if (ann.type == null) continue; - TypeBinding tb = ann.type.resolveType(decl.initializerScope); - if (tb == null) continue; - if (!charArrayEquals("lombok", tb.qualifiedPackageName())) continue; - if (!charArrayEquals("Delegate", tb.qualifiedSourceName())) continue; - return true; + if (isDelegate(ann, decl)) return true; + } + } + if (decl.methods != null) for (AbstractMethodDeclaration method : decl.methods) { + if (method.annotations == null) continue; + for (Annotation ann : method.annotations) { + if (isDelegate(ann, decl)) return true; } } - return false; } public static boolean handleDelegateForType(ClassScope scope) { if (TransformEclipseAST.disableLombok) return false; - if (!hasDelegateMarkedFields(scope.referenceContext)) return false; + if (!hasDelegateMarkedFieldsOrMethods(scope.referenceContext)) return false; List stack = visited.get(); StringBuilder corrupted = null; @@ -145,16 +145,23 @@ public class PatchDelegate { stack.add(entry); try { - List methodsToDelegate = new ArrayList(); TypeDeclaration decl = scope.referenceContext; if (decl != null) { CompilationUnitDeclaration cud = scope.compilationUnitScope().referenceContext; EclipseAST eclipseAst = TransformEclipseAST.getAST(cud, true); - fillMethodBindings(cud, scope, methodsToDelegate); + List methodsToDelegate = new ArrayList(); + fillMethodBindingsForFields(cud, scope, methodsToDelegate); + if (entry.corruptedPath != null) { + eclipseAst.get(scope.referenceContext).addError("No @Delegate methods created because there's a loop: " + entry.corruptedPath); + } else { + generateDelegateMethods(eclipseAst.get(decl), methodsToDelegate, DelegateReceiver.FIELD); + } + methodsToDelegate.clear(); + fillMethodBindingsForMethods(cud, scope, methodsToDelegate); if (entry.corruptedPath != null) { eclipseAst.get(scope.referenceContext).addError("No @Delegate methods created because there's a loop: " + entry.corruptedPath); } else { - generateDelegateMethods(eclipseAst.get(decl), methodsToDelegate); + generateDelegateMethods(eclipseAst.get(decl), methodsToDelegate, DelegateReceiver.METHOD); } } } finally { @@ -181,43 +188,18 @@ public class PatchDelegate { private static Map alreadyApplied = new WeakHashMap(); private static final Object MARKER = new Object(); - private static void fillMethodBindings(CompilationUnitDeclaration cud, ClassScope scope, List methodsToDelegate) { + private static void fillMethodBindingsForFields(CompilationUnitDeclaration cud, ClassScope scope, List methodsToDelegate) { TypeDeclaration decl = scope.referenceContext; if (decl == null) return; if (decl.fields != null) for (FieldDeclaration field : decl.fields) { if (field.annotations == null) continue; for (Annotation ann : field.annotations) { - if (ann.type == null) continue; - TypeBinding tb = ann.type.resolveType(decl.initializerScope); - if (!charArrayEquals("lombok", tb.qualifiedPackageName())) continue; - if (!charArrayEquals("Delegate", tb.qualifiedSourceName())) continue; + if (!isDelegate(ann, decl)) continue; if (alreadyApplied.put(ann, MARKER) == MARKER) continue; - List rawTypes = new ArrayList(); - List excludedRawTypes = new ArrayList(); - for (MemberValuePair pair : ann.memberValuePairs()) { - if (charArrayEquals("types", pair.name)) { - if (pair.value instanceof ArrayInitializer) { - for (Expression expr : ((ArrayInitializer)pair.value).expressions) { - if (expr instanceof ClassLiteralAccess) rawTypes.add((ClassLiteralAccess) expr); - } - } - if (pair.value instanceof ClassLiteralAccess) { - rawTypes.add((ClassLiteralAccess) pair.value); - } - } - if (charArrayEquals("excludes", pair.name)) { - if (pair.value instanceof ArrayInitializer) { - for (Expression expr : ((ArrayInitializer)pair.value).expressions) { - if (expr instanceof ClassLiteralAccess) excludedRawTypes.add((ClassLiteralAccess) expr); - } - } - if (pair.value instanceof ClassLiteralAccess) { - excludedRawTypes.add((ClassLiteralAccess) pair.value); - } - } - } + List rawTypes = rawTypes(ann, "types"); + List excludedRawTypes = rawTypes(ann, "excludes"); List methodsToExclude = new ArrayList(); for (ClassLiteralAccess cla : excludedRawTypes) { @@ -251,6 +233,88 @@ public class PatchDelegate { } } + private static void fillMethodBindingsForMethods(CompilationUnitDeclaration cud, ClassScope scope, List methodsToDelegate) { + TypeDeclaration decl = scope.referenceContext; + if (decl == null) return; + + if (decl.methods != null) for (AbstractMethodDeclaration methodDecl : decl.methods) { + if (methodDecl.annotations == null) continue; + for (Annotation ann : methodDecl.annotations) { + if (!isDelegate(ann, decl)) continue; + if (alreadyApplied.put(ann, MARKER) == MARKER) continue; + if (!(methodDecl instanceof MethodDeclaration)) { + EclipseAST eclipseAst = TransformEclipseAST.getAST(cud, true); + eclipseAst.get(ann).addError("@Delegate is legal only on no-argument methods."); + continue; + } + if (methodDecl.arguments != null) { + EclipseAST eclipseAst = TransformEclipseAST.getAST(cud, true); + eclipseAst.get(ann).addError("@Delegate is legal only on no-argument methods."); + continue; + } + MethodDeclaration method = (MethodDeclaration) methodDecl; + + List rawTypes = rawTypes(ann, "types"); + List excludedRawTypes = rawTypes(ann, "excludes"); + + List methodsToExclude = new ArrayList(); + for (ClassLiteralAccess cla : excludedRawTypes) { + addAllMethodBindings(methodsToExclude, cla.type.resolveType(decl.initializerScope), new HashSet(), method.selector, ann); + } + + Set banList = new HashSet(); + for (BindingTuple excluded : methodsToExclude) banList.add(printSig(excluded.parameterized)); + + List methodsToDelegateForThisAnn = new ArrayList(); + + if (rawTypes.isEmpty()) { + addAllMethodBindings(methodsToDelegateForThisAnn, method.returnType.resolveType(decl.initializerScope), banList, method.selector, ann); + } else { + for (ClassLiteralAccess cla : rawTypes) { + addAllMethodBindings(methodsToDelegateForThisAnn, cla.type.resolveType(decl.initializerScope), banList, method.selector, ann); + } + } + + // Not doing this right now because of problems - see commented-out-method for info. + // removeExistingMethods(methodsToDelegate, decl, scope); + + String dupe = containsDuplicates(methodsToDelegateForThisAnn); + if (dupe != null) { + EclipseAST eclipseAst = TransformEclipseAST.getAST(cud, true); + eclipseAst.get(ann).addError("The method '" + dupe + "' is being delegated by more than one specified type."); + } else { + methodsToDelegate.addAll(methodsToDelegateForThisAnn); + } + } + } + } + + private static boolean isDelegate(Annotation ann, TypeDeclaration decl) { + if (ann.type == null) return false; + TypeBinding tb = ann.type.resolveType(decl.initializerScope); + if (tb == null) return false; + if (!charArrayEquals("lombok", tb.qualifiedPackageName())) return false; + if (!charArrayEquals("Delegate", tb.qualifiedSourceName())) return false; + return true; + } + + private static List rawTypes(Annotation ann, String name) { + List rawTypes = new ArrayList(); + for (MemberValuePair pair : ann.memberValuePairs()) { + if (charArrayEquals(name, pair.name)) { + if (pair.value instanceof ArrayInitializer) { + for (Expression expr : ((ArrayInitializer)pair.value).expressions) { + if (expr instanceof ClassLiteralAccess) rawTypes.add((ClassLiteralAccess) expr); + } + } + if (pair.value instanceof ClassLiteralAccess) { + rawTypes.add((ClassLiteralAccess) pair.value); + } + } + } + return rawTypes; + } + /* * We may someday finish this method. Steps to be completed: * @@ -321,11 +385,11 @@ public class PatchDelegate { // } // } - private static void generateDelegateMethods(EclipseNode typeNode, List methods) { + private static void generateDelegateMethods(EclipseNode typeNode, List methods, DelegateReceiver delegateReceiver) { CompilationUnitDeclaration top = (CompilationUnitDeclaration) typeNode.top().get(); for (BindingTuple pair : methods) { EclipseNode annNode = typeNode.getAst().get(pair.responsible); - MethodDeclaration method = createDelegateMethod(pair.fieldName, typeNode, pair, top.compilationResult, annNode); + MethodDeclaration method = createDelegateMethod(pair.fieldName, typeNode, pair, top.compilationResult, annNode, delegateReceiver); if (method != null) { SetGeneratedByVisitor visitor = new SetGeneratedByVisitor(annNode.get()); method.traverse(visitor, ((TypeDeclaration)typeNode.get()).scope); @@ -473,7 +537,7 @@ public class PatchDelegate { } } - private static MethodDeclaration createDelegateMethod(char[] name, EclipseNode typeNode, BindingTuple pair, CompilationResult compilationResult, EclipseNode annNode) { + private static MethodDeclaration createDelegateMethod(char[] name, EclipseNode typeNode, BindingTuple pair, CompilationResult compilationResult, EclipseNode annNode, DelegateReceiver delegateReceiver) { /* public ReturnType methodName(ParamType1 name1, ParamType2 name2, ...) throws T1, T2, ... { * (return) delegate.methodName(name1, name2); * } @@ -514,11 +578,7 @@ public class PatchDelegate { call.sourceStart = pS; call.sourceEnd = pE; call.nameSourcePosition = pos(source); setGeneratedBy(call, source); - FieldReference fieldRef = new FieldReference(name, pos(source)); - fieldRef.receiver = new ThisReference(pS, pE); - setGeneratedBy(fieldRef, source); - setGeneratedBy(fieldRef.receiver, source); - call.receiver = fieldRef; + call.receiver = delegateReceiver.get(source, name); call.selector = binding.selector; if (binding.typeVariables != null && binding.typeVariables.length > 0) { @@ -742,7 +802,31 @@ public class PatchDelegate { if (s.length() != c.length) return false; for (int i = 0; i < s.length(); i++) if (s.charAt(i) != c[i]) return false; return true; + } + + private enum DelegateReceiver { + METHOD { + public Expression get(final ASTNode source, char[] name) { + MessageSend call = new MessageSend(); + call.sourceStart = source.sourceStart; call.sourceEnd = source.sourceEnd; + call.nameSourcePosition = pos(source); + setGeneratedBy(call, source); + call.selector = name; + call.receiver = new ThisReference(source.sourceStart, source.sourceEnd); + setGeneratedBy(call.receiver, source); + return call; + } + }, + FIELD { + public Expression get(final ASTNode source, char[] name) { + FieldReference fieldRef = new FieldReference(name, pos(source)); + setGeneratedBy(fieldRef, source); + fieldRef.receiver = new ThisReference(source.sourceStart, source.sourceEnd); + setGeneratedBy(fieldRef.receiver, source); + return fieldRef; + } + }; - + public abstract Expression get(final ASTNode source, char[] name); } } diff --git a/test/transform/resource/after-delombok/DelegateOnMethods.java b/test/transform/resource/after-delombok/DelegateOnMethods.java new file mode 100644 index 00000000..b900f140 --- /dev/null +++ b/test/transform/resource/after-delombok/DelegateOnMethods.java @@ -0,0 +1,12 @@ +abstract class DelegateOnMethods { + public abstract Bar getBar(); + + public static interface Bar { + void bar(java.util.ArrayList list); + } + + @java.lang.SuppressWarnings("all") + public void bar(final java.util.ArrayList list) { + this.getBar().bar(list); + } +} \ No newline at end of file diff --git a/test/transform/resource/after-ecj/DelegateOnMethods.java b/test/transform/resource/after-ecj/DelegateOnMethods.java new file mode 100644 index 00000000..cb06d3c1 --- /dev/null +++ b/test/transform/resource/after-ecj/DelegateOnMethods.java @@ -0,0 +1 @@ +//ignore \ No newline at end of file diff --git a/test/transform/resource/after-eclipse/DelegateOnMethods.java b/test/transform/resource/after-eclipse/DelegateOnMethods.java new file mode 100644 index 00000000..37922e2a --- /dev/null +++ b/test/transform/resource/after-eclipse/DelegateOnMethods.java @@ -0,0 +1,13 @@ +import lombok.Delegate; +abstract class DelegateOnMethods { + public static interface Bar { + void bar(java.util.ArrayList list); + } + public @java.lang.SuppressWarnings("all") void bar(final java.util.ArrayList list) { + this.getBar().bar(list); + } + DelegateOnMethods() { + super(); + } + public abstract @Delegate Bar getBar(); +} \ No newline at end of file diff --git a/test/transform/resource/before/DelegateOnMethods.java b/test/transform/resource/before/DelegateOnMethods.java new file mode 100644 index 00000000..1606e18c --- /dev/null +++ b/test/transform/resource/before/DelegateOnMethods.java @@ -0,0 +1,11 @@ +import lombok.Delegate; + +abstract class DelegateOnMethods { + + @Delegate + public abstract Bar getBar(); + + public static interface Bar { + void bar(java.util.ArrayList list); + } +} \ No newline at end of file -- cgit From 99743e16e6741cbb858377c31acfb91ccfb027c6 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Thu, 19 Jan 2012 00:32:07 +0100 Subject: Removed unused imports from PatchFixes --- src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java | 4 ---- 1 file changed, 4 deletions(-) (limited to 'src/eclipseAgent/lombok/eclipse/agent') diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java index 86f2b9cf..62ab4b7b 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java +++ b/src/eclipseAgent/lombok/eclipse/agent/PatchFixes.java @@ -19,7 +19,6 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ - package lombok.eclipse.agent; import java.io.BufferedOutputStream; @@ -27,7 +26,6 @@ import java.io.IOException; import java.io.OutputStream; import java.lang.reflect.Field; import java.util.ArrayList; -import java.util.Iterator; import java.util.List; import java.util.Stack; @@ -42,9 +40,7 @@ import org.eclipse.jdt.core.IMethod; import org.eclipse.jdt.core.IType; import org.eclipse.jdt.core.JavaModelException; import org.eclipse.jdt.core.dom.MethodDeclaration; -import org.eclipse.jdt.core.dom.Modifier; import org.eclipse.jdt.core.dom.SimpleName; -import org.eclipse.jdt.core.dom.SingleVariableDeclaration; import org.eclipse.jdt.internal.compiler.ast.Annotation; import org.eclipse.jdt.internal.core.dom.rewrite.NodeRewriteEvent; import org.eclipse.jdt.internal.core.dom.rewrite.RewriteEvent; -- cgit From acea9efcff67c7a0bdb85175b84e75a9dad172b8 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Thu, 19 Jan 2012 00:33:18 +0100 Subject: @Delegate is no longer legal on static entities (previously no immediate errors but I don't think it would work right anyway), and prettied up the error message when you use @Delegate wrong (on static items or methods with args). Also put back something I accidentally deleted with the previous merge. --- .../lombok/eclipse/agent/PatchDelegate.java | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'src/eclipseAgent/lombok/eclipse/agent') diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java b/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java index d6310de1..e510d37b 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java +++ b/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java @@ -198,6 +198,12 @@ public class PatchDelegate { if (!isDelegate(ann, decl)) continue; if (alreadyApplied.put(ann, MARKER) == MARKER) continue; + if ((field.modifiers & ClassFileConstants.AccStatic) != 0) { + EclipseAST eclipseAst = TransformEclipseAST.getAST(cud, true); + eclipseAst.get(ann).addError(LEGALITY_OF_DELEGATE); + break; + } + List rawTypes = rawTypes(ann, "types"); List excludedRawTypes = rawTypes(ann, "excludes"); @@ -233,6 +239,8 @@ public class PatchDelegate { } } + private static final String LEGALITY_OF_DELEGATE = "@Delegate is legal only on instance fields or no-argument instance methods."; + private static void fillMethodBindingsForMethods(CompilationUnitDeclaration cud, ClassScope scope, List methodsToDelegate) { TypeDeclaration decl = scope.referenceContext; if (decl == null) return; @@ -244,13 +252,18 @@ public class PatchDelegate { if (alreadyApplied.put(ann, MARKER) == MARKER) continue; if (!(methodDecl instanceof MethodDeclaration)) { EclipseAST eclipseAst = TransformEclipseAST.getAST(cud, true); - eclipseAst.get(ann).addError("@Delegate is legal only on no-argument methods."); - continue; + eclipseAst.get(ann).addError(LEGALITY_OF_DELEGATE); + break; } if (methodDecl.arguments != null) { EclipseAST eclipseAst = TransformEclipseAST.getAST(cud, true); - eclipseAst.get(ann).addError("@Delegate is legal only on no-argument methods."); - continue; + eclipseAst.get(ann).addError(LEGALITY_OF_DELEGATE); + break; + } + if ((methodDecl.modifiers & ClassFileConstants.AccStatic) != 0) { + EclipseAST eclipseAst = TransformEclipseAST.getAST(cud, true); + eclipseAst.get(ann).addError(LEGALITY_OF_DELEGATE); + break; } MethodDeclaration method = (MethodDeclaration) methodDecl; -- cgit From 3e7bb145d3984d7472baa177ead3baf574d42e6b Mon Sep 17 00:00:00 2001 From: Roel Spilker Date: Mon, 23 Jan 2012 23:10:56 +0100 Subject: Added null-check to prevent errors in eclipse when working on incomplete files --- src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java | 1 + 1 file changed, 1 insertion(+) (limited to 'src/eclipseAgent/lombok/eclipse/agent') diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java b/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java index e510d37b..31ec52f4 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java +++ b/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java @@ -281,6 +281,7 @@ public class PatchDelegate { List methodsToDelegateForThisAnn = new ArrayList(); if (rawTypes.isEmpty()) { + if (method.returnType == null) continue; addAllMethodBindings(methodsToDelegateForThisAnn, method.returnType.resolveType(decl.initializerScope), banList, method.selector, ann); } else { for (ClassLiteralAccess cla : rawTypes) { -- cgit From 2242de2bf9886fc80858486bbb3aa171a5496885 Mon Sep 17 00:00:00 2001 From: Roel Spilker Date: Tue, 24 Jan 2012 02:52:13 +0100 Subject: Fix for issue 328: @Delegate on a field for which we also generate a getter will use the getter for delegation --- src/core/lombok/eclipse/handlers/HandleGetter.java | 23 +++++++++++-- src/core/lombok/javac/handlers/HandleGetter.java | 27 +++++++++++++++ .../lombok/eclipse/agent/PatchDelegate.java | 4 +++ src/utils/lombok/eclipse/Eclipse.java | 5 +-- .../resource/after-delombok/DelegateOnGetter.java | 35 +++++++++++++++++++ .../resource/after-ecj/DelegateOnGetter.java | 1 + .../resource/after-eclipse/DelegateOnGetter.java | 40 ++++++++++++++++++++++ .../resource/before/DelegateOnGetter.java | 18 ++++++++++ 8 files changed, 149 insertions(+), 4 deletions(-) create mode 100644 test/transform/resource/after-delombok/DelegateOnGetter.java create mode 100644 test/transform/resource/after-ecj/DelegateOnGetter.java create mode 100644 test/transform/resource/after-eclipse/DelegateOnGetter.java create mode 100644 test/transform/resource/before/DelegateOnGetter.java (limited to 'src/eclipseAgent/lombok/eclipse/agent') diff --git a/src/core/lombok/eclipse/handlers/HandleGetter.java b/src/core/lombok/eclipse/handlers/HandleGetter.java index 1d59afb4..b7d9c5ed 100644 --- a/src/core/lombok/eclipse/handlers/HandleGetter.java +++ b/src/core/lombok/eclipse/handlers/HandleGetter.java @@ -24,18 +24,23 @@ package lombok.eclipse.handlers; import static lombok.eclipse.Eclipse.*; import static lombok.eclipse.handlers.EclipseHandlerUtil.*; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import lombok.AccessLevel; +import lombok.Delegate; import lombok.Getter; +import lombok.core.AST.Kind; import lombok.core.AnnotationValues; import lombok.core.TransformationsUtil; -import lombok.core.AST.Kind; import lombok.eclipse.EclipseAnnotationHandler; import lombok.eclipse.EclipseNode; +import lombok.eclipse.agent.PatchDelegate; +import lombok.eclipse.handlers.EclipseHandlerUtil.FieldAccess; import org.eclipse.jdt.internal.compiler.ast.ASTNode; import org.eclipse.jdt.internal.compiler.ast.AllocationExpression; @@ -69,6 +74,8 @@ import org.mangosdk.spi.ProviderFor; */ @ProviderFor(EclipseAnnotationHandler.class) public class HandleGetter extends EclipseAnnotationHandler { + private static final Annotation[] EMPTY_ANNOTATIONS_ARRAY = new Annotation[0]; + public boolean generateGetterForType(EclipseNode typeNode, EclipseNode pos, AccessLevel level, boolean checkForTypeLevelGetter) { if (checkForTypeLevelGetter) { if (typeNode != null) for (EclipseNode child : typeNode.down()) { @@ -205,13 +212,25 @@ public class HandleGetter extends EclipseAnnotationHandler { } MethodDeclaration method = generateGetter((TypeDeclaration) fieldNode.up().get(), fieldNode, getterName, modifier, source, lazy); - Annotation[] copiedAnnotations = copyAnnotations(source, findAnnotations(field, TransformationsUtil.NON_NULL_PATTERN), findAnnotations(field, TransformationsUtil.NULLABLE_PATTERN)); + Annotation[] copiedAnnotations = copyAnnotations(source, findAnnotations(field, TransformationsUtil.NON_NULL_PATTERN), findAnnotations(field, TransformationsUtil.NULLABLE_PATTERN), findDelegatesAndMarkAsHandled(fieldNode)); if (copiedAnnotations.length != 0) { method.annotations = copiedAnnotations; } injectMethod(fieldNode.up(), method); } + + private static Annotation[] findDelegatesAndMarkAsHandled(EclipseNode fieldNode) { + List delegates = new ArrayList(); + for (EclipseNode child : fieldNode.down()) { + if (annotationTypeMatches(Delegate.class, child)) { + Annotation delegate = (Annotation)child.get(); + PatchDelegate.markHandled(delegate); + delegates.add(delegate); + } + } + return delegates.toArray(EMPTY_ANNOTATIONS_ARRAY); + } private MethodDeclaration generateGetter(TypeDeclaration parent, EclipseNode fieldNode, String name, int modifier, ASTNode source, boolean lazy) { diff --git a/src/core/lombok/javac/handlers/HandleGetter.java b/src/core/lombok/javac/handlers/HandleGetter.java index fe3b86a4..c9d67d7f 100644 --- a/src/core/lombok/javac/handlers/HandleGetter.java +++ b/src/core/lombok/javac/handlers/HandleGetter.java @@ -30,6 +30,7 @@ import java.util.HashMap; import java.util.Map; import lombok.AccessLevel; +import lombok.Delegate; import lombok.Getter; import lombok.core.AnnotationValues; import lombok.core.TransformationsUtil; @@ -240,16 +241,42 @@ public class HandleGetter extends JavacAnnotationHandler { List nonNulls = findAnnotations(field, TransformationsUtil.NON_NULL_PATTERN); List nullables = findAnnotations(field, TransformationsUtil.NULLABLE_PATTERN); + List delegates = findDelegatesAndRemoveFromField(field); + List annsOnMethod = nonNulls.appendList(nullables); JCMethodDecl decl = recursiveSetGeneratedBy(treeMaker.MethodDef(treeMaker.Modifiers(access, annsOnMethod), methodName, methodType, methodGenericParams, parameters, throwsClauses, methodBody, annotationMethodDefaultValue), source); if (toClearOfMarkers != null) recursiveSetGeneratedBy(toClearOfMarkers, null); + decl.mods.annotations = decl.mods.annotations.appendList(delegates); return decl; } + private static List findDelegatesAndRemoveFromField(JavacNode field) { + JCVariableDecl fieldNode = (JCVariableDecl) field.get(); + + List delegates = List.nil(); + for (JCAnnotation annotation : fieldNode.mods.annotations) { + if (typeMatches(Delegate.class, field, annotation.annotationType)) { + delegates = delegates.append(annotation); + } + } + + if (!delegates.isEmpty()) { + ListBuffer withoutDelegates = ListBuffer.lb(); + for (JCAnnotation annotation : fieldNode.mods.annotations) { + if (!delegates.contains(annotation)) { + withoutDelegates.append(annotation); + } + } + fieldNode.mods.annotations = withoutDelegates.toList(); + field.rebuild(); + } + return delegates; + } + private List createSimpleGetterBody(TreeMaker treeMaker, JavacNode field) { return List.of(treeMaker.Return(createFieldAccessor(treeMaker, field, FieldAccess.ALWAYS_FIELD))); } diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java b/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java index 31ec52f4..87335b4e 100644 --- a/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java +++ b/src/eclipseAgent/lombok/eclipse/agent/PatchDelegate.java @@ -188,6 +188,10 @@ public class PatchDelegate { private static Map alreadyApplied = new WeakHashMap(); private static final Object MARKER = new Object(); + public static void markHandled(Annotation annotation) { + alreadyApplied.put(annotation, MARKER); + } + private static void fillMethodBindingsForFields(CompilationUnitDeclaration cud, ClassScope scope, List methodsToDelegate) { TypeDeclaration decl = scope.referenceContext; if (decl == null) return; diff --git a/src/utils/lombok/eclipse/Eclipse.java b/src/utils/lombok/eclipse/Eclipse.java index fce0773b..7d21faa2 100644 --- a/src/utils/lombok/eclipse/Eclipse.java +++ b/src/utils/lombok/eclipse/Eclipse.java @@ -43,6 +43,7 @@ import org.eclipse.jdt.internal.compiler.ast.TypeReference; import org.eclipse.jdt.internal.compiler.lookup.TypeIds; public class Eclipse { + private static final Annotation[] EMPTY_ANNOTATIONS_ARRAY = new Annotation[0]; /** * Eclipse's Parser class is instrumented to not attempt to fill in the body of any method or initializer * or field initialization if this flag is set. Set it on the flag field of @@ -120,7 +121,7 @@ public class Eclipse { */ public static Annotation[] findAnnotations(FieldDeclaration field, Pattern namePattern) { List result = new ArrayList(); - if (field.annotations == null) return new Annotation[0]; + if (field.annotations == null) return EMPTY_ANNOTATIONS_ARRAY; for (Annotation annotation : field.annotations) { TypeReference typeRef = annotation.type; if (typeRef != null && typeRef.getTypeName()!= null) { @@ -131,7 +132,7 @@ public class Eclipse { } } } - return result.toArray(new Annotation[0]); + return result.toArray(EMPTY_ANNOTATIONS_ARRAY); } /** diff --git a/test/transform/resource/after-delombok/DelegateOnGetter.java b/test/transform/resource/after-delombok/DelegateOnGetter.java new file mode 100644 index 00000000..ee34018a --- /dev/null +++ b/test/transform/resource/after-delombok/DelegateOnGetter.java @@ -0,0 +1,35 @@ +class DelegateOnGetter { + private final java.util.concurrent.atomic.AtomicReference> bar = new java.util.concurrent.atomic.AtomicReference>(); + private interface Bar { + void setList(java.util.ArrayList list); + int getInt(); + } + @java.lang.SuppressWarnings("all") + public Bar getBar() { + java.util.concurrent.atomic.AtomicReference value = this.bar.get(); + if (value == null) { + synchronized (this.bar) { + value = this.bar.get(); + if (value == null) { + value = new java.util.concurrent.atomic.AtomicReference(new Bar(){ + public void setList(java.util.ArrayList list) { + } + public int getInt() { + return 42; + } + }); + this.bar.set(value); + } + } + } + return value.get(); + } + @java.lang.SuppressWarnings("all") + public void setList(final java.util.ArrayList list) { + this.getBar().setList(list); + } + @java.lang.SuppressWarnings("all") + public int getInt() { + return this.getBar().getInt(); + } +} \ No newline at end of file diff --git a/test/transform/resource/after-ecj/DelegateOnGetter.java b/test/transform/resource/after-ecj/DelegateOnGetter.java new file mode 100644 index 00000000..cb06d3c1 --- /dev/null +++ b/test/transform/resource/after-ecj/DelegateOnGetter.java @@ -0,0 +1 @@ +//ignore \ No newline at end of file diff --git a/test/transform/resource/after-eclipse/DelegateOnGetter.java b/test/transform/resource/after-eclipse/DelegateOnGetter.java new file mode 100644 index 00000000..7d5907e0 --- /dev/null +++ b/test/transform/resource/after-eclipse/DelegateOnGetter.java @@ -0,0 +1,40 @@ +import lombok.Delegate; +import lombok.Getter; +class DelegateOnGetter { + private interface Bar { + void setList(java.util.ArrayList list); + int getInt(); + } + private final @Delegate @Getter(lazy = true) java.util.concurrent.atomic.AtomicReference> bar = new java.util.concurrent.atomic.AtomicReference>(); + public @Delegate @java.lang.SuppressWarnings("all") Bar getBar() { + java.util.concurrent.atomic.AtomicReference value = this.bar.get(); + if ((value == null)) + { + synchronized (this.bar) + { + value = this.bar.get(); + if ((value == null)) + { + value = new java.util.concurrent.atomic.AtomicReference(new Bar() { + public void setList(java.util.ArrayList list) { + } + public int getInt() { + return 42; + } +}); + this.bar.set(value); + } + } + } + return value.get(); + } + public @java.lang.SuppressWarnings("all") int getInt() { + return this.getBar().getInt(); + } + public @java.lang.SuppressWarnings("all") void setList(final java.util.ArrayList list) { + this.getBar().setList(list); + } + DelegateOnGetter() { + super(); + } +} \ No newline at end of file diff --git a/test/transform/resource/before/DelegateOnGetter.java b/test/transform/resource/before/DelegateOnGetter.java new file mode 100644 index 00000000..afe09ff4 --- /dev/null +++ b/test/transform/resource/before/DelegateOnGetter.java @@ -0,0 +1,18 @@ +import lombok.Delegate; +import lombok.Getter; + +class DelegateOnGetter { + + @Delegate @Getter(lazy=true) private final Bar bar = new Bar() { + public void setList(java.util.ArrayList list) { + } + public int getInt() { + return 42; + } + }; + + private interface Bar { + void setList(java.util.ArrayList list); + int getInt(); + } +} \ No newline at end of file -- cgit