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 <[email protected]> Authored: Wed Sep 12 21:05:47 2018 +1000 Committer: Paul King <[email protected]> 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
