You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2018/09/13 06:46:09 UTC
groovy git commit: GROOVY-8703: Unexpected behavior with
@NamedVariant on constructor (closes #795)
Repository: groovy
Updated Branches:
refs/heads/master b255db420 -> d8c96ecc6
GROOVY-8703: Unexpected behavior with @NamedVariant on constructor (closes #795)
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/d8c96ecc
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/d8c96ecc
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/d8c96ecc
Branch: refs/heads/master
Commit: d8c96ecc647c5f7fc736be8752b0434477e52fd1
Parents: b255db4
Author: Paul King <pa...@asert.com.au>
Authored: Wed Sep 12 21:05:47 2018 +1000
Committer: Paul King <pa...@asert.com.au>
Committed: Thu Sep 13 16:44:45 2018 +1000
----------------------------------------------------------------------
.../groovy/groovy/transform/NamedVariant.java | 53 +++++--
.../NamedVariantASTTransformation.java | 147 +++++++++++--------
2 files changed, 123 insertions(+), 77 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/d8c96ecc/src/main/groovy/groovy/transform/NamedVariant.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/groovy/transform/NamedVariant.java b/src/main/groovy/groovy/transform/NamedVariant.java
index e36c64a..04faeb5 100644
--- a/src/main/groovy/groovy/transform/NamedVariant.java
+++ b/src/main/groovy/groovy/transform/NamedVariant.java
@@ -40,37 +40,51 @@ import java.lang.annotation.Target;
* type information. The generated method however contains no business logic
* so the chance of errors is minimal.
*
- * Any arguments identified as named arguments will be supplied as
- * part of the map. Any additional arguments are supplied in the normal
- * tuple style.
+ * Any arguments identified as named arguments will be supplied as part of the map.
+ * Any additional arguments are supplied in the normal tuple style.
*
- * Named arguments are identified in one of three ways:
+ * Named parameters are identified in one of three ways:
* <ol>
- * <li>Use one or more {@code @NamedParam} annotations to explicitly identify such arguments</li>
- * <li>Use one or more {@code @NamedDelegate} annotations to explicitly identify such arguments as
- * delegate arguments</li>
- * <li>If no arguments with {@code @NamedParam} or {@code @NamedDelegate} annotations are found the
- * first argument is assumed to be an implicit named delegate</li>
+ * <li>Use one or more {@code @NamedParam} annotations to explicitly identify such parameters</li>
+ * <li>Use one or more {@code @NamedDelegate} annotations to explicitly identify such parameters as
+ * delegate parameters</li>
+ * <li>If no parameters with {@code @NamedParam} or {@code @NamedDelegate} annotations are found then:
+ * <ul>
+ * <li>If {@code autoDelegate} is false (the default), all parameters are treated as if they were named parameters</li>
+ * <li>If {@code autoDelegate} is true, the first parameters is treated as if it was a delegate parameter</li>
+ * </ul>
+ * </li>
* </ol>
* You can also mix and match the {@code @NamedParam} and {@code @NamedDelegate} annotations.
*
* Named arguments will be supplied via the map with their property name (configurable via
* annotation attributes within {@code @NamedParam}) being the key and value being the argument value.
- * For named delegates, any properties of the delegate can become map keys. Duplicate keys across
- * delegates or named parameters are not allowed. Delegate arguments must be
- * compatible with Groovy's {@code as} cast operation from a {@code Map}.
+ * For named delegates, any properties of the delegate can become map keys.
+ * Duplicate keys across delegate properties or named parameters are not allowed.
+ * The type of delegate parameters must be compatible with Groovy's {@code as} cast operation from a {@code Map}.
*
- * Here is an example using the implicit delegate approach.
+ * Here is an example using implicit named parameters.
* <pre class="groovyTestCase">
* import groovy.transform.*
*
- * {@code @ToString(includeNames=true, includeFields=true)}
+ * {@code @NamedVariant}
+ * int makeSense(int dollars, int cents) {
+ * 100 * dollars + cents
+ * }
+ *
+ * assert makeSense(dollars: 2, cents: 50) == 250
+ * </pre>
+ * Here is an example using a delegate parameter.
+ * <pre class="groovyTestCase">
+ * import groovy.transform.*
+ *
+ * {@code @ToString(includeNames=true)}
* class Color {
* Integer r, g, b
* }
*
* {@code @NamedVariant}
- * String foo(Color shade) {
+ * String foo(@NamedDelegate Color shade) {
* shade
* }
*
@@ -103,4 +117,11 @@ public @interface NamedVariant {
* If specified, must match the optional "id" attribute in an applicable {@code VisibilityOptions} annotation.
*/
String visibilityId() default Undefined.STRING;
-}
\ No newline at end of file
+
+ /**
+ * If true, add an implicit @NamedDelegate to the first parameter if no @NamedDelegate or @NamedParam annotations are found on any parameter.
+ *
+ * @since 2.5.3
+ */
+ boolean autoDelegate() default false;
+}
http://git-wip-us.apache.org/repos/asf/groovy/blob/d8c96ecc/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
index b5e85e7..9db1cf4 100644
--- a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
@@ -89,6 +89,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
return;
}
+ boolean autoDelegate = memberHasValue(anno, "autoDelegate", true);
Parameter mapParam = param(GenericsUtils.nonGeneric(ClassHelper.MAP_TYPE), "__namedArgs");
List<Parameter> genParams = new ArrayList<Parameter>();
genParams.add(mapParam);
@@ -105,35 +106,15 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
}
}
- if (!annoFound) {
+ if (!annoFound && autoDelegate) {
// assume the first param is the delegate by default
processDelegateParam(mNode, mapParam, args, propNames, fromParams[0]);
} else {
for (Parameter fromParam : fromParams) {
- if (AnnotatedNodeUtils.hasAnnotation(fromParam, NAMED_PARAM_TYPE)) {
- AnnotationNode namedParam = fromParam.getAnnotations(NAMED_PARAM_TYPE).get(0);
- boolean required = memberHasValue(namedParam, "required", true);
- if (getMemberStringValue(namedParam, "value") == null) {
- namedParam.addMember("value", constX(fromParam.getName()));
- }
- String name = getMemberStringValue(namedParam, "value");
- if (getMemberValue(namedParam, "type") == null) {
- namedParam.addMember("type", classX(fromParam.getType()));
- }
- if (hasDuplicates(mNode, propNames, name)) return;
- // TODO check specified type is assignable from declared param type?
- // ClassNode type = getMemberClassValue(namedParam, "type");
- if (required) {
- if (fromParam.hasInitialExpression()) {
- addError("Error during " + MY_TYPE_NAME + " processing. A required parameter can't have an initial value.", mNode);
- return;
- }
- inner.addStatement(new AssertStatement(boolX(callX(varX(mapParam), "containsKey", args(constX(name)))),
- plusX(new ConstantExpression("Missing required named argument '" + name + "'. Keys found: "), callX(varX(mapParam), "keySet"))));
- }
- args.addExpression(propX(varX(mapParam), name));
- mapParam.addAnnotation(namedParam);
- fromParam.getAnnotations().remove(namedParam);
+ if (!annoFound) {
+ if (!processImplicitNamedParam(mNode, mapParam, args, propNames, fromParam)) return;
+ } else if (AnnotatedNodeUtils.hasAnnotation(fromParam, NAMED_PARAM_TYPE)) {
+ if (!processExplicitNamedParam(mNode, mapParam, inner, args, propNames, fromParam)) return;
} else if (AnnotatedNodeUtils.hasAnnotation(fromParam, NAMED_DELEGATE_TYPE)) {
if (!processDelegateParam(mNode, mapParam, args, propNames, fromParam)) return;
} else {
@@ -143,6 +124,86 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
}
}
}
+ createMapVariant(mNode, anno, mapParam, genParams, cNode, inner, args, propNames);
+ }
+
+ private boolean processImplicitNamedParam(MethodNode mNode, Parameter mapParam, ArgumentListExpression args, List<String> propNames, Parameter fromParam) {
+ boolean required = fromParam.hasInitialExpression();
+ String name = fromParam.getName();
+ if (hasDuplicates(mNode, propNames, name)) return false;
+ AnnotationNode namedParam = new AnnotationNode(NAMED_PARAM_TYPE);
+ namedParam.addMember("value", constX(name));
+ namedParam.addMember("type", classX(fromParam.getType()));
+ namedParam.addMember("required", constX(required, true));
+ mapParam.addAnnotation(namedParam);
+ args.addExpression(propX(varX(mapParam), name));
+ return true;
+ }
+
+ private boolean processExplicitNamedParam(MethodNode mNode, Parameter mapParam, BlockStatement inner, ArgumentListExpression args, List<String> propNames, Parameter fromParam) {
+ AnnotationNode namedParam = fromParam.getAnnotations(NAMED_PARAM_TYPE).get(0);
+ boolean required = memberHasValue(namedParam, "required", true);
+ if (getMemberStringValue(namedParam, "value") == null) {
+ namedParam.addMember("value", constX(fromParam.getName()));
+ }
+ String name = getMemberStringValue(namedParam, "value");
+ if (getMemberValue(namedParam, "type") == null) {
+ namedParam.addMember("type", classX(fromParam.getType()));
+ }
+ if (hasDuplicates(mNode, propNames, name)) return false;
+ // TODO check specified type is assignable from declared param type?
+ // ClassNode type = getMemberClassValue(namedParam, "type");
+ if (required) {
+ if (fromParam.hasInitialExpression()) {
+ addError("Error during " + MY_TYPE_NAME + " processing. A required parameter can't have an initial value.", mNode);
+ return false;
+ }
+ inner.addStatement(new AssertStatement(boolX(callX(varX(mapParam), "containsKey", args(constX(name)))),
+ plusX(new ConstantExpression("Missing required named argument '" + name + "'. Keys found: "), callX(varX(mapParam), "keySet"))));
+ }
+ args.addExpression(propX(varX(mapParam), name));
+ mapParam.addAnnotation(namedParam);
+ fromParam.getAnnotations().remove(namedParam);
+ return true;
+ }
+
+ private boolean processDelegateParam(MethodNode mNode, Parameter mapParam, ArgumentListExpression args, List<String> propNames, Parameter fromParam) {
+ if (isInnerClass(fromParam.getType())) {
+ if (mNode.isStatic()) {
+ addError("Error during " + MY_TYPE_NAME + " processing. Delegate type '" + fromParam.getType().getNameWithoutPackage() + "' is an inner class which is not supported.", mNode);
+ return false;
+ }
+ }
+
+ Set<String> names = new HashSet<String>();
+ List<PropertyNode> props = getAllProperties(names, fromParam.getType(), true, false, false, true, false, true);
+ for (String next : names) {
+ if (hasDuplicates(mNode, propNames, next)) return false;
+ }
+ List<MapEntryExpression> entries = new ArrayList<MapEntryExpression>();
+ for (PropertyNode pNode : props) {
+ String name = pNode.getName();
+ entries.add(new MapEntryExpression(constX(name), propX(varX(mapParam), name)));
+ AnnotationNode namedParam = new AnnotationNode(NAMED_PARAM_TYPE);
+ namedParam.addMember("value", constX(name));
+ namedParam.addMember("type", classX(pNode.getType()));
+ mapParam.addAnnotation(namedParam);
+ }
+ Expression delegateMap = new MapExpression(entries);
+ args.addExpression(castX(fromParam.getType(), delegateMap));
+ return true;
+ }
+
+ private boolean hasDuplicates(MethodNode mNode, List<String> propNames, String next) {
+ if (propNames.contains(next)) {
+ addError("Error during " + MY_TYPE_NAME + " processing. Duplicate property '" + next + "' found.", mNode);
+ return true;
+ }
+ propNames.add(next);
+ return false;
+ }
+
+ private void createMapVariant(MethodNode mNode, AnnotationNode anno, Parameter mapParam, List<Parameter> genParams, ClassNode cNode, BlockStatement inner, ArgumentListExpression args, List<String> propNames) {
Parameter namedArgKey = param(STRING_TYPE, "namedArgKey");
inner.addStatement(
new ForStatement(
@@ -185,40 +246,4 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
);
}
}
-
- private boolean processDelegateParam(MethodNode mNode, Parameter mapParam, ArgumentListExpression args, List<String> propNames, Parameter fromParam) {
- if (isInnerClass(fromParam.getType())) {
- if (mNode.isStatic()) {
- addError("Error during " + MY_TYPE_NAME + " processing. Delegate type '" + fromParam.getType().getNameWithoutPackage() + "' is an inner class which is not supported.", mNode);
- return false;
- }
- }
-
- Set<String> names = new HashSet<String>();
- List<PropertyNode> props = getAllProperties(names, fromParam.getType(), true, false, false, true, false, true);
- for (String next : names) {
- if (hasDuplicates(mNode, propNames, next)) return false;
- }
- List<MapEntryExpression> entries = new ArrayList<MapEntryExpression>();
- for (PropertyNode pNode : props) {
- String name = pNode.getName();
- entries.add(new MapEntryExpression(constX(name), propX(varX(mapParam), name)));
- AnnotationNode namedParam = new AnnotationNode(NAMED_PARAM_TYPE);
- namedParam.addMember("value", constX(name));
- namedParam.addMember("type", classX(pNode.getType()));
- mapParam.addAnnotation(namedParam);
- }
- Expression delegateMap = new MapExpression(entries);
- args.addExpression(castX(fromParam.getType(), delegateMap));
- return true;
- }
-
- private boolean hasDuplicates(MethodNode mNode, List<String> propNames, String next) {
- if (propNames.contains(next)) {
- addError("Error during " + MY_TYPE_NAME + " processing. Duplicate property '" + next + "' found.", mNode);
- return true;
- }
- propNames.add(next);
- return false;
- }
}
\ No newline at end of file