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/02/22 23:51:20 UTC
[groovy] branch master updated: GROOVY-10502: NamedVariant: improve consistency of default value treatment
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new a842616 GROOVY-10502: NamedVariant: improve consistency of default value treatment
a842616 is described below
commit a842616cbd0c2b189f223abe5036d95b67f0a8d3
Author: Paul King <pa...@asert.com.au>
AuthorDate: Mon Feb 21 23:07:54 2022 +1000
GROOVY-10502: NamedVariant: improve consistency of default value treatment
---
.../groovy/transform/NamedVariantASTTransformation.java | 17 +++++++++++------
src/spec/test/RecordSpecificationTest.groovy | 7 +++++--
.../groovy/transform/NamedVariantTransformTest.groovy | 5 +++--
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
index 9bc6d18..f3052de 100644
--- a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
@@ -57,6 +57,7 @@ import static org.codehaus.groovy.ast.ClassHelper.make;
import static org.codehaus.groovy.ast.ClassHelper.makeWithoutCaching;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.asX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
import static org.codehaus.groovy.ast.tools.GeneralUtils.boolX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callThisX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
@@ -68,14 +69,16 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.defaultValueX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.elvisX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.entryX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getAllProperties;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.ifS;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.isNullX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.list2args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.mapX;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.notNullX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
import static org.codehaus.groovy.ast.tools.GeneralUtils.plusX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
import static org.codehaus.groovy.ast.tools.GeneralUtils.ternaryX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.throwS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
@GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS)
@@ -85,6 +88,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
private static final String NAMED_VARIANT = "@" + NAMED_VARIANT_TYPE.getNameWithoutPackage();
private static final ClassNode NAMED_PARAM_TYPE = makeWithoutCaching(NamedParam.class, false);
private static final ClassNode NAMED_DELEGATE_TYPE = makeWithoutCaching(NamedDelegate.class, false);
+ private static final ClassNode ILLEGAL_ARGUMENT = makeWithoutCaching(IllegalArgumentException.class);
@Override
public void visit(final ASTNode[] nodes, final SourceUnit source) {
@@ -101,7 +105,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
boolean autoDelegate = memberHasValue(anno, "autoDelegate", true);
boolean coerce = memberHasValue(anno, "coerce", true);
- Parameter mapParam = param(GenericsUtils.nonGeneric(MAP_TYPE), "__namedArgs");
+ Parameter mapParam = param(GenericsUtils.nonGeneric(MAP_TYPE), "namedArgs");
List<Parameter> genParams = new ArrayList<>();
genParams.add(mapParam);
ClassNode cNode = mNode.getDeclaringClass();
@@ -141,7 +145,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
createMapVariant(this, mNode, anno, mapParam, genParams, cNode, inner, args, propNames);
}
- 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) {
+ 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) {
String name = fromParam.getName();
ClassNode type = fromParam.getType();
boolean required = !fromParam.hasInitialExpression();
@@ -233,6 +237,10 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
static void createMapVariant(final ErrorCollecting xform, final MethodNode mNode, final AnnotationNode anno, final Parameter mapParam, final List<Parameter> genParams, final ClassNode cNode, final BlockStatement inner, final ArgumentListExpression args, final List<String> propNames) {
Parameter namedArgKey = param(STRING_TYPE, "namedArgKey");
+ if (!(mNode instanceof ConstructorNode)) {
+ inner.getStatements().add(0, ifS(isNullX(varX(mapParam)),
+ throwS(ctorX(ILLEGAL_ARGUMENT, args(constX("Named parameter map cannot be null"))))));
+ }
inner.addStatement(
new ForStatement(
namedArgKey,
@@ -280,9 +288,6 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
defaultValue = defaultValueX(type);
}
if (defaultValue != null) {
- if (isPrimitiveType(type)) { // handle null for primitive
- value = ternaryX(notNullX(value), value, defaultValueX(type));
- }
value = ternaryX(containsKey(mapParam, name), value, defaultValue);
}
return asType(value, type, coerce);
diff --git a/src/spec/test/RecordSpecificationTest.groovy b/src/spec/test/RecordSpecificationTest.groovy
index f19fcc5..c2a2fe7 100644
--- a/src/spec/test/RecordSpecificationTest.groovy
+++ b/src/spec/test/RecordSpecificationTest.groovy
@@ -76,8 +76,11 @@ assert new ColoredPoint(5).toString() == 'ColoredPoint[x=5, y=0, color=white]'
assert new ColoredPoint(x: 5).toString() == 'ColoredPoint[x=5, y=0, color=white]'
assert new ColoredPoint(x: 0, y: 5).toString() == 'ColoredPoint[x=0, y=5, color=white]'
// end::record_point_named_args[]
-assert new ColoredPoint(x: 0, y: null).toString() == 'ColoredPoint[x=0, y=0, color=white]'
-def ex = shouldFail { new ColoredPoint(x: 0, z: 5) }
+def ex = shouldFail(ClassCastException) { new ColoredPoint(x: 0, y: null) }
+assert ex.message.contains("Cannot cast object 'null' with class 'null' to class 'int'")
+ex = shouldFail(ClassCastException) { new ColoredPoint(x: null) }
+assert ex.message.contains("Cannot cast object 'null' with class 'null' to class 'int'")
+ex = shouldFail { new ColoredPoint(x: 0, z: 5) }
assert ex.message.contains('Unrecognized namedArgKey: z')
// tag::record_point_named_args_off[]
@TupleConstructor(defaultsMode=DefaultsMode.OFF)
diff --git a/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy b/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy
index 0c882fd..7534421 100644
--- a/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy
@@ -229,6 +229,7 @@ final class NamedVariantTransformTest {
assertScript '''
import groovy.transform.*
+ import static groovy.test.GroovyAssert.shouldFail
@NamedVariant
def m(int one, int two = 42) {
@@ -238,8 +239,8 @@ final class NamedVariantTransformTest {
String result = m(one:0, two:0)
assert result == '0 0'
- result = m(one:0, two:null)
- assert result == '0 0'
+ shouldFail(MissingMethodException) { m(one:null) }
+ shouldFail(MissingMethodException) { m(one:0, two:null) }
'''
}