You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by pa...@apache.org on 2015/05/16 06:34:34 UTC
incubator-groovy git commit: GROOVY-7422: @AnnotationCollector should
provide more control over where collected annotations are placed
Repository: incubator-groovy
Updated Branches:
refs/heads/master d9f4fa8ef -> 22d97169e
GROOVY-7422: @AnnotationCollector should provide more control over where collected annotations are placed
Project: http://git-wip-us.apache.org/repos/asf/incubator-groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-groovy/commit/22d97169
Tree: http://git-wip-us.apache.org/repos/asf/incubator-groovy/tree/22d97169
Diff: http://git-wip-us.apache.org/repos/asf/incubator-groovy/diff/22d97169
Branch: refs/heads/master
Commit: 22d97169e21bafafd96990e33a22508a81260f6c
Parents: d9f4fa8
Author: Paul King <pa...@asert.com.au>
Authored: Tue May 12 22:26:44 2015 +1000
Committer: Paul King <pa...@asert.com.au>
Committed: Sat May 16 14:32:53 2015 +1000
----------------------------------------------------------------------
.../groovy/transform/AnnotationCollector.java | 9 +
.../transform/AnnotationCollectorMode.java | 50 ++++++
.../ASTTransformationCollectorCodeVisitor.java | 171 +++++++++++++++----
.../transform/AnnotationCollectorTest.groovy | 99 +++++++++--
4 files changed, 283 insertions(+), 46 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/22d97169/src/main/groovy/transform/AnnotationCollector.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/transform/AnnotationCollector.java b/src/main/groovy/transform/AnnotationCollector.java
index 9a256bf..01a9b73 100644
--- a/src/main/groovy/transform/AnnotationCollector.java
+++ b/src/main/groovy/transform/AnnotationCollector.java
@@ -87,6 +87,15 @@ public @interface AnnotationCollector {
* Custom processors need to extend that class.
*/
String processor() default "org.codehaus.groovy.transform.AnnotationCollectorTransform";
+
+ /**
+ * When the collector annotation is replaced, whether to check for duplicates between
+ * the replacement annotations and existing explicit annotations.
+ * If you use a custom processor, it is up to that processor whether it honors or ignores
+ * this parameter. The default processor honors the parameter.
+ */
+ AnnotationCollectorMode mode() default AnnotationCollectorMode.DUPLICATE;
+
/**
* List of aliased annotations.
*/
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/22d97169/src/main/groovy/transform/AnnotationCollectorMode.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/transform/AnnotationCollectorMode.java b/src/main/groovy/transform/AnnotationCollectorMode.java
new file mode 100644
index 0000000..308d0c0
--- /dev/null
+++ b/src/main/groovy/transform/AnnotationCollectorMode.java
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package groovy.transform;
+
+public enum AnnotationCollectorMode {
+ // TODO should we support @Repeatable from Java 8?
+ /**
+ * Annotations from the annotation collection will always be inserted. After all transforms have been run, it will
+ * be an error if multiple annotations (excluding those with SOURCE retention) exist.
+ */
+ DUPLICATE,
+
+ /**
+ * Annotations from the collector will be added and any existing annotations with the same name will be removed.
+ */
+ PREFER_COLLECTOR,
+
+ /**
+ * Annotations from the collector will be ignored if any existing annotations with the same name are found.
+ */
+ PREFER_EXPLICIT,
+
+ /**
+ * Annotations from the collector will be added and any existing annotations with the same name will be removed but any new parameters found within existing annotations will be merged into the added annotation.
+ */
+ PREFER_COLLECTOR_MERGED,
+
+ /**
+ * Annotations from the collector will be ignored if any existing annotations with the same name are found but any new parameters on the collector annotation will be added to existing annotations.
+ */
+ PREFER_EXPLICIT_MERGED
+}
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/22d97169/src/main/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java b/src/main/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java
index c5da8e5..d65f81e 100644
--- a/src/main/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/ASTTransformationCollectorCodeVisitor.java
@@ -18,21 +18,23 @@
*/
package org.codehaus.groovy.transform;
+import groovy.lang.GroovyClassLoader;
+import groovy.transform.AnnotationCollector;
+import groovy.transform.AnnotationCollectorMode;
import org.codehaus.groovy.ast.AnnotatedNode;
import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.expr.ClassExpression;
import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.PropertyExpression;
import org.codehaus.groovy.control.CompilePhase;
import org.codehaus.groovy.control.SourceUnit;
import org.codehaus.groovy.control.messages.ExceptionMessage;
import org.codehaus.groovy.control.messages.SimpleMessage;
import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
import org.codehaus.groovy.syntax.SyntaxException;
-
-import groovy.lang.GroovyClassLoader;
-import groovy.transform.AnnotationCollector;
import org.codehaus.groovy.transform.trait.TraitASTTransformation;
import org.codehaus.groovy.transform.trait.Traits;
@@ -40,8 +42,12 @@ import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Map;
+import java.util.TreeMap;
/**
* This visitor walks the AST tree and collects references to Annotations that
@@ -85,13 +91,26 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
public void visitAnnotations(AnnotatedNode node) {
super.visitAnnotations(node);
- List<AnnotationNode> collected = new ArrayList<AnnotationNode>();
- for (Iterator<AnnotationNode> it = node.getAnnotations().iterator(); it.hasNext();) {
- AnnotationNode annotation = it.next();
- if (addCollectedAnnotations(collected, annotation, node)) it.remove();
+ Map<Integer, List<AnnotationNode>> existing = new TreeMap<Integer, List<AnnotationNode>>();
+ Map<Integer, List<AnnotationNode>> replacements = new LinkedHashMap<Integer, List<AnnotationNode>>();
+ Map<Integer, AnnotationCollectorMode> modes = new LinkedHashMap<Integer, AnnotationCollectorMode>();
+ int index = 0;
+ for (AnnotationNode annotation : node.getAnnotations()) {
+ findCollectedAnnotations(annotation, node, index, modes, existing, replacements);
+ index++;
}
- node.getAnnotations().addAll(collected);
-
+ for (Integer replacementIndex : replacements.keySet()) {
+ mergeCollectedAnnotations(modes.get(replacementIndex), existing, replacements.get(replacementIndex));
+ existing.put(replacementIndex, replacements.get(replacementIndex));
+ }
+ List<AnnotationNode> mergedList = new ArrayList<AnnotationNode>();
+ for (List<AnnotationNode> next : existing.values()) {
+ mergedList.addAll(next);
+ }
+
+ node.getAnnotations().clear();
+ node.getAnnotations().addAll(mergedList);
+
for (AnnotationNode annotation : node.getAnnotations()) {
Annotation transformClassAnnotation = getTransformClassAnnotation(annotation.getClassNode());
if (transformClassAnnotation == null) {
@@ -101,34 +120,105 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
addTransformsToClassNode(annotation, transformClassAnnotation);
}
}
-
+
+ private void mergeCollectedAnnotations(AnnotationCollectorMode mode, Map<Integer, List<AnnotationNode>> existing, List<AnnotationNode> replacements) {
+ switch(mode) {
+ case PREFER_COLLECTOR:
+ deleteExisting(false, existing, replacements);
+ break;
+ case PREFER_COLLECTOR_MERGED:
+ deleteExisting(true, existing, replacements);
+ break;
+ case PREFER_EXPLICIT:
+ deleteReplacement(false, existing, replacements);
+ break;
+ case PREFER_EXPLICIT_MERGED:
+ deleteReplacement(true, existing, replacements);
+ break;
+ default:
+ // nothing to do
+ }
+ }
+
+ private void deleteExisting(boolean mergeParams, Map<Integer, List<AnnotationNode>> existingMap, List<AnnotationNode> replacements) {
+ for (AnnotationNode replacement : replacements) {
+ for (Integer key : existingMap.keySet()) {
+ List<AnnotationNode> annotationNodes = new ArrayList<AnnotationNode>(existingMap.get(key));
+ Iterator<AnnotationNode> iterator = annotationNodes.iterator();
+ while (iterator.hasNext()) {
+ AnnotationNode existing = iterator.next();
+ if (replacement.getClassNode().getName().equals(existing.getClassNode().getName())) {
+ if (mergeParams) {
+ mergeParameters(replacement, existing);
+ }
+ iterator.remove();
+ }
+ }
+ existingMap.put(key, annotationNodes);
+ }
+ }
+ }
+
+ private void deleteReplacement(boolean mergeParams, Map<Integer, List<AnnotationNode>> existingMap, List<AnnotationNode> replacements) {
+ Iterator<AnnotationNode> nodeIterator = replacements.iterator();
+ while (nodeIterator.hasNext()) {
+ boolean remove = false;
+ AnnotationNode replacement = nodeIterator.next();
+ for (Integer key : existingMap.keySet()) {
+ for (AnnotationNode existing : existingMap.get(key)) {
+ if (replacement.getClassNode().getName().equals(existing.getClassNode().getName())) {
+ if (mergeParams) {
+ mergeParameters(existing, replacement);
+ }
+ remove = true;
+ }
+ }
+ }
+ if (remove) {
+ nodeIterator.remove();
+ }
+ }
+ }
+
+ private void mergeParameters(AnnotationNode to, AnnotationNode from) {
+ for (String name : from.getMembers().keySet()) {
+ if (to.getMember(name) == null) {
+ to.setMember(name, from.getMember(name));
+ }
+ }
+ }
+
private void assertStringConstant(Expression exp) {
- if (exp==null) return;
+ if (exp == null) return;
if (!(exp instanceof ConstantExpression)) {
source.getErrorCollector().addErrorAndContinue(new SyntaxErrorMessage(new SyntaxException(
- "Expected a String constant.", exp.getLineNumber(), exp.getColumnNumber()),
+ "Expected a String constant.", exp.getLineNumber(), exp.getColumnNumber()),
source));
}
ConstantExpression ce = (ConstantExpression) exp;
if (!(ce.getValue() instanceof String)) {
source.getErrorCollector().addErrorAndContinue(new SyntaxErrorMessage(new SyntaxException(
- "Expected a String constant.", exp.getLineNumber(), exp.getColumnNumber()),
+ "Expected a String constant.", exp.getLineNumber(), exp.getColumnNumber()),
source));
}
}
-
- private boolean addCollectedAnnotations(List<AnnotationNode> collected, AnnotationNode aliasNode, AnnotatedNode origin) {
+
+ private void findCollectedAnnotations(AnnotationNode aliasNode, AnnotatedNode origin, Integer index, Map<Integer, AnnotationCollectorMode> modes, Map<Integer, List<AnnotationNode>> existing, Map<Integer, List<AnnotationNode>> replacements) {
ClassNode classNode = aliasNode.getClassNode();
- boolean ret = false;
for (AnnotationNode annotation : classNode.getAnnotations()) {
if (annotation.getClassNode().getName().equals(AnnotationCollector.class.getName())) {
+ AnnotationCollectorMode mode = getMode(annotation);
+ if (mode == null) {
+ mode = AnnotationCollectorMode.DUPLICATE;
+ }
+ modes.put(index, mode);
Expression processorExp = annotation.getMember("processor");
AnnotationCollectorTransform act = null;
assertStringConstant(processorExp);
- if (processorExp!=null) {
+ if (processorExp != null) {
String className = (String) ((ConstantExpression) processorExp).getValue();
Class klass = loadTransformClass(className, aliasNode);
- if (klass!=null) {
+ if (klass != null) {
try {
act = (AnnotationCollectorTransform) klass.newInstance();
} catch (InstantiationException e) {
@@ -140,24 +230,43 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
} else {
act = new AnnotationCollectorTransform();
}
- if (act!=null) collected.addAll(act.visit(annotation, aliasNode, origin, source));
- ret = true;
+ if (act != null) {
+ replacements.put(index, act.visit(annotation, aliasNode, origin, source));
+ return;
+ }
+ }
+ }
+ if (!replacements.containsKey(index)) {
+ existing.put(index, Collections.singletonList(aliasNode));
+ }
+ }
+
+ private AnnotationCollectorMode getMode(AnnotationNode node) {
+ final Expression member = node.getMember("mode");
+ if (member != null && member instanceof PropertyExpression) {
+ PropertyExpression prop = (PropertyExpression) member;
+ Expression oe = prop.getObjectExpression();
+ if (oe instanceof ClassExpression) {
+ ClassExpression ce = (ClassExpression) oe;
+ if (ce.getType().getName().equals("groovy.transform.AnnotationCollectorMode")) {
+ return AnnotationCollectorMode.valueOf(prop.getPropertyAsString());
+ }
}
}
- return ret;
+ return null;
}
private void addTransformsToClassNode(AnnotationNode annotation, Annotation transformClassAnnotation) {
List<String> transformClassNames = getTransformClassNames(annotation, transformClassAnnotation);
- if(transformClassNames.isEmpty()) {
+ if (transformClassNames.isEmpty()) {
source.getErrorCollector().addError(new SimpleMessage("@GroovyASTTransformationClass in " +
annotation.getClassNode().getName() + " does not specify any transform class names/classes", source));
}
for (String transformClass : transformClassNames) {
- Class klass = loadTransformClass(transformClass, annotation);
- if (klass!=null) {
+ Class klass = loadTransformClass(transformClass, annotation);
+ if (klass != null) {
verifyAndAddTransform(annotation, klass);
}
}
@@ -170,7 +279,7 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
source.getErrorCollector().addErrorAndContinue(
new SimpleMessage(
"Could not find class for Transformation Processor " + transformClass
- + " declared by " + annotation.getClassNode().getName(),
+ + " declared by " + annotation.getClassNode().getName(),
source));
}
return null;
@@ -184,9 +293,9 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
private void verifyCompilePhase(AnnotationNode annotation, Class<?> klass) {
GroovyASTTransformation transformationClass = klass.getAnnotation(GroovyASTTransformation.class);
- if (transformationClass != null) {
+ if (transformationClass != null) {
CompilePhase specifiedCompilePhase = transformationClass.phase();
- if (specifiedCompilePhase.getPhaseNumber() < CompilePhase.SEMANTIC_ANALYSIS.getPhaseNumber()) {
+ if (specifiedCompilePhase.getPhaseNumber() < CompilePhase.SEMANTIC_ANALYSIS.getPhaseNumber()) {
source.getErrorCollector().addError(
new SimpleMessage(
annotation.getClassNode().getName() + " is defined to be run in compile phase " + specifiedCompilePhase + ". Local AST transformations must run in " + CompilePhase.SEMANTIC_ANALYSIS + " or later!",
@@ -195,7 +304,7 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
} else {
source.getErrorCollector().addError(
- new SimpleMessage("AST transformation implementation classes must be annotated with " + GroovyASTTransformation.class.getName() + ". " + klass.getName() + " lacks this annotation.", source));
+ new SimpleMessage("AST transformation implementation classes must be annotated with " + GroovyASTTransformation.class.getName() + ". " + klass.getName() + " lacks this annotation.", source));
}
}
@@ -207,7 +316,7 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
}
@SuppressWarnings("unchecked")
- private void addTransform(AnnotationNode annotation, Class klass) {
+ private void addTransform(AnnotationNode annotation, Class klass) {
boolean apply = !Traits.isTrait(classNode) || klass == TraitASTTransformation.class;
if (apply) {
classNode.addTransform(klass, annotation);
@@ -221,7 +330,7 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
// because compiler clients are free to choose any GroovyClassLoader for
// resolving ClassNodeS such as annotatedType, we have to compare by name,
// and cannot cast the return value to GroovyASTTransformationClass
- if (ann.annotationType().getName().equals(GroovyASTTransformationClass.class.getName())){
+ if (ann.annotationType().getName().equals(GroovyASTTransformationClass.class.getName())) {
return ann;
}
}
@@ -243,7 +352,7 @@ public class ASTTransformationCollectorCodeVisitor extends ClassCodeVisitorSuppo
result.add(klass.getName());
}
- if(names.length > 0 && classes.length > 0) {
+ if (names.length > 0 && classes.length > 0) {
source.getErrorCollector().addError(new SimpleMessage("@GroovyASTTransformationClass in " +
annotation.getClassNode().getName() +
" should specify transforms only by class names or by classes and not by both", source));
http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/22d97169/src/test/groovy/transform/AnnotationCollectorTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/transform/AnnotationCollectorTest.groovy b/src/test/groovy/transform/AnnotationCollectorTest.groovy
index 92f0221..ac22a92 100644
--- a/src/test/groovy/transform/AnnotationCollectorTest.groovy
+++ b/src/test/groovy/transform/AnnotationCollectorTest.groovy
@@ -16,21 +16,6 @@
* specific language governing permissions and limitations
* under the License.
*/
-/*
-* Copyright 2003-2012 the original author or authors.
-*
-* Licensed under the Apache License, Version 2.0 (the "License");
-* you may not use this file except in compliance with the License.
-* You may obtain a copy of the License at
-*
-* http://www.apache.org/licenses/LICENSE-2.0
-*
-* Unless required by applicable law or agreed to in writing, software
-* distributed under the License is distributed on an "AS IS" BASIS,
-* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-* See the License for the specific language governing permissions and
-* limitations under the License.
-*/
package groovy.transform
import org.codehaus.groovy.ast.*;
@@ -390,6 +375,90 @@ class AnnotationCollectorTest extends GroovyTestCase {
assert data[3][1].value == "Guillaume"
"""
}
+
+ void testAnnotationCollectorModePreferCollector() {
+ assertScript """
+ import groovy.transform.*
+
+ @ToString(includeNames=true)
+ @AnnotationCollector(mode=AnnotationCollectorMode.PREFER_COLLECTOR)
+ @interface ToStringNames {}
+
+ @ToString(excludes='prop1')
+ @ToStringNames(excludes='prop2')
+ class Dummy1 { String prop1, prop2 }
+
+ @ToString(excludes='prop1')
+ @ToStringNames
+ class Dummy2 { String prop1, prop2 }
+
+ assert new Dummy1(prop1: 'hello', prop2: 'goodbye').toString() == 'Dummy1(prop1:hello)'
+ assert new Dummy2(prop1: 'hello', prop2: 'goodbye').toString() == 'Dummy2(prop1:hello, prop2:goodbye)'
+ """
+ }
+
+ void testAnnotationCollectorModePreferCollectorMerged() {
+ assertScript """
+ import groovy.transform.*
+
+ @ToString(includeNames=true)
+ @AnnotationCollector(mode=AnnotationCollectorMode.PREFER_COLLECTOR_MERGED)
+ @interface ToStringNames {}
+
+ @ToString(excludes='prop1')
+ @ToStringNames(excludes='prop2')
+ class Dummy1 { String prop1, prop2 }
+
+ @ToString(excludes='prop1')
+ @ToStringNames
+ class Dummy2 { String prop1, prop2 }
+
+ assert new Dummy1(prop1: 'hello', prop2: 'goodbye').toString() == 'Dummy1(prop1:hello)'
+ assert new Dummy2(prop1: 'hello', prop2: 'goodbye').toString() == 'Dummy2(prop2:goodbye)'
+ """
+ }
+
+ void testAnnotationCollectorModePreferCollectorExplicit() {
+ assertScript """
+ import groovy.transform.*
+
+ @ToString(includeNames=true)
+ @AnnotationCollector(mode=AnnotationCollectorMode.PREFER_EXPLICIT)
+ @interface ToStringNames {}
+
+ @ToString(excludes='prop1')
+ @ToStringNames(excludes='prop2')
+ class Dummy1 { String prop1, prop2 }
+
+ @ToString(excludes='prop1')
+ @ToStringNames
+ class Dummy2 { String prop1, prop2 }
+
+ assert new Dummy1(prop1: 'hello', prop2: 'goodbye').toString() == 'Dummy1(goodbye)'
+ assert new Dummy2(prop1: 'hello', prop2: 'goodbye').toString() == 'Dummy2(goodbye)'
+ """
+ }
+
+ void testAnnotationCollectorModePreferCollectorExplicitMerged() {
+ assertScript """
+ import groovy.transform.*
+
+ @ToString(includeNames=true)
+ @AnnotationCollector(mode=AnnotationCollectorMode.PREFER_EXPLICIT_MERGED)
+ @interface ToStringNames {}
+
+ @ToString(excludes='prop1')
+ @ToStringNames(excludes='prop2')
+ class Dummy1 { String prop1, prop2 }
+
+ @ToString(excludes='prop1')
+ @ToStringNames
+ class Dummy2 { String prop1, prop2 }
+
+ assert new Dummy1(prop1: 'hello', prop2: 'goodbye').toString() == 'Dummy1(prop2:goodbye)'
+ assert new Dummy2(prop1: 'hello', prop2: 'goodbye').toString() == 'Dummy2(prop2:goodbye)'
+ """
+ }
}
@AnnotationCollector([ToString, EqualsAndHashCode, Immutable])