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 2022/04/14 03:17:58 UTC
[groovy] 02/09: GROOVY-10561: @NamedVariant self referential default values are not correctly resolved
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 <pa...@asert.com.au>
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'
+ }
}