This is an automated email from the ASF dual-hosted git repository. paulk pushed a commit to branch GROOVY_4_0_X in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 568d821b3054649790abe8383480d42ab478422d Author: Paul King <[email protected]> AuthorDate: Thu Mar 31 23:38:16 2022 +1000 GROOVY-10561: @NamedVariant self referential default values are not correctly resolved --- .../transform/NamedVariantASTTransformation.java | 40 +++++++++++++++++++--- .../TupleConstructorASTTransformation.java | 5 ++- .../transform/NamedVariantTransformTest.groovy | 27 +++++++++++++++ 3 files changed, 66 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java index ad2c1403fe..0af31ef7ae 100644 --- a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java @@ -29,9 +29,13 @@ import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.MethodNode; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.PropertyNode; +import org.codehaus.groovy.ast.Variable; import org.codehaus.groovy.ast.expr.ArgumentListExpression; +import org.codehaus.groovy.ast.expr.CastExpression; +import org.codehaus.groovy.ast.expr.ConstantExpression; import org.codehaus.groovy.ast.expr.Expression; import org.codehaus.groovy.ast.expr.MethodCallExpression; +import org.codehaus.groovy.ast.expr.VariableExpression; import org.codehaus.groovy.ast.stmt.AssertStatement; import org.codehaus.groovy.ast.stmt.BlockStatement; import org.codehaus.groovy.ast.stmt.ForStatement; @@ -40,8 +44,10 @@ import org.codehaus.groovy.control.CompilePhase; import org.codehaus.groovy.control.SourceUnit; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor; @@ -121,11 +127,12 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation { if (!annoFound && autoDelegate) { // the first param is the delegate processDelegateParam(mNode, mapParam, args, propNames, fromParams[0], coerce); } else { + Map<Parameter, Expression> seen = new HashMap<>(); for (Parameter fromParam : fromParams) { if (!annoFound) { - if (!processImplicitNamedParam(this, mNode, mapParam, inner, args, propNames, fromParam, coerce)) return; + if (!processImplicitNamedParam(this, mNode, mapParam, inner, args, propNames, fromParam, coerce, seen)) return; } else if (AnnotatedNodeUtils.hasAnnotation(fromParam, NAMED_PARAM_TYPE)) { - if (!processExplicitNamedParam(mNode, mapParam, inner, args, propNames, fromParam, coerce)) return; + if (!processExplicitNamedParam(mNode, mapParam, inner, args, propNames, fromParam, coerce, seen)) return; } else if (AnnotatedNodeUtils.hasAnnotation(fromParam, NAMED_DELEGATE_TYPE)) { if (!processDelegateParam(mNode, mapParam, args, propNames, fromParam, coerce)) return; } else { @@ -140,7 +147,12 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation { createMapVariant(this, mNode, anno, mapParam, genParams, cNode, inner, args, propNames); } + // for backwards compatibility static boolean processImplicitNamedParam(final ErrorCollecting xform, final MethodNode mNode, final Parameter mapParam, final BlockStatement inner, final ArgumentListExpression args, final List<String> propNames, final Parameter fromParam, final boolean coerce) { + return processImplicitNamedParam(xform, mNode, mapParam, inner, args, propNames, fromParam, coerce, null); + } + + static boolean processImplicitNamedParam(final ErrorCollecting xform, final MethodNode mNode, final Parameter mapParam, final BlockStatement inner, final ArgumentListExpression args, final List<String> propNames, final Parameter fromParam, final boolean coerce, Map<Parameter, Expression> seen) { String name = fromParam.getName(); ClassNode type = fromParam.getType(); boolean required = !fromParam.hasInitialExpression(); @@ -156,11 +168,26 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation { inner.addStatement(new AssertStatement(boolX(containsKey(mapParam, name)), plusX(constX("Missing required named argument '" + name + "'. Keys found: "), callX(varX(mapParam), "keySet")))); } - args.addExpression(namedParamValue(mapParam, name, type, coerce, fromParam.getInitialExpression())); + Expression defValue = earlierParamIfSeen(seen, fromParam.getInitialExpression()); + Expression initExpr = namedParamValue(mapParam, name, type, coerce, defValue); + if (seen != null) { + seen.put(fromParam, initExpr); + } + args.addExpression(initExpr); return true; } - private boolean processExplicitNamedParam(final MethodNode mNode, final Parameter mapParam, final BlockStatement inner, final ArgumentListExpression args, final List<String> propNames, final Parameter fromParam, final boolean coerce) { + private static Expression earlierParamIfSeen(Map<Parameter, Expression> seen, Expression defValue) { + if (seen == null) return defValue; + // handle earlier param with or without cast + if (defValue instanceof CastExpression) { + defValue = ((CastExpression) defValue).getExpression(); + } + return defValue instanceof VariableExpression ? + seen.getOrDefault(((VariableExpression) defValue).getAccessedVariable(), defValue) : defValue; + } + + private boolean processExplicitNamedParam(final MethodNode mNode, final Parameter mapParam, final BlockStatement inner, final ArgumentListExpression args, final List<String> propNames, final Parameter fromParam, final boolean coerce, Map<Parameter, Expression> seen) { AnnotationNode namedParam = fromParam.getAnnotations(NAMED_PARAM_TYPE).get(0); String name = getMemberStringValue(namedParam, "value"); @@ -187,7 +214,10 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation { inner.addStatement(new AssertStatement(boolX(containsKey(mapParam, name)), plusX(constX("Missing required named argument '" + name + "'. Keys found: "), callX(varX(mapParam), "keySet")))); } - args.addExpression(namedParamValue(mapParam, name, type, coerce, fromParam.getInitialExpression())); + Expression defValue = earlierParamIfSeen(seen, fromParam.getInitialExpression()); + Expression initExpr = namedParamValue(mapParam, name, type, coerce, defValue); + seen.put(fromParam, initExpr); + args.addExpression(initExpr); mapParam.addAnnotation(namedParam); fromParam.getAnnotations().remove(namedParam); return true; diff --git a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java index 90b610c7fc..6cfd32ba1f 100644 --- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java @@ -54,9 +54,11 @@ import org.codehaus.groovy.control.SourceUnit; import java.util.ArrayList; import java.util.Comparator; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.Set; import static groovy.transform.DefaultsMode.AUTO; @@ -277,8 +279,9 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation genParams.add(mapParam); ArgumentListExpression args = new ArgumentListExpression(); List<String> propNames = new ArrayList<>(); + Map<Parameter, Expression> seen = new HashMap<>(); for (Parameter p : params) { - if (!processImplicitNamedParam(xform, consNode, mapParam, inner, args, propNames, p,false)) return; + if (!processImplicitNamedParam(xform, consNode, mapParam, inner, args, propNames, p, false, seen)) return; } NamedVariantASTTransformation.createMapVariant(xform, consNode, anno, mapParam, genParams, cNode, inner, args, propNames); } diff --git a/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy b/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy index a98408cf4b..88814c19aa 100644 --- a/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy +++ b/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy @@ -19,6 +19,7 @@ package org.codehaus.groovy.transform import groovy.transform.CompileStatic +import groovy.transform.NamedVariant import org.junit.Test import static groovy.test.GroovyAssert.assertScript @@ -392,4 +393,30 @@ final class NamedVariantTransformTest { assert mapper.settings.firstDataRow == 1 ''' } + + @NamedVariant // GROOVY-10561 + String fileInSourceSet(String language = 'java', String extension = language) { + return "$language -> .$extension" + } + + @NamedVariant // GROOVY-10561 + String foo(String a = 'a', String b = a, String c = (String) a) { + return "$a $b $c" + } + + @Test // GROOVY-10561 + void testReferenceToEarlierParam() { + assert fileInSourceSet() == 'java -> .java' + assert fileInSourceSet('groovy') == 'groovy -> .groovy' + assert fileInSourceSet(language: 'kotlin', extension: 'kt') == 'kotlin -> .kt' + assert fileInSourceSet(language: 'groovy') == 'groovy -> .groovy' + } + + @Test // GROOVY-10561 + void testEarlierParamInExpression() { + assert foo() == 'a a a' + assert foo('c') == 'c c c' + assert foo('c', 'd') == 'c d c' + assert foo('c', 'd', 'e') == 'c d e' + } }
