You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2019/11/29 00:00:54 UTC
[groovy] 01/01: GROOVY-9183: use property initial value as default
for map.getOrDefault
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-9183
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit a2632cdace7dbccd0e8af1a3a9487d279f067e0b
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Nov 28 18:00:29 2019 -0600
GROOVY-9183: use property initial value as default for map.getOrDefault
if no initial value expression was supplied:
- primitive (or wrapper) types: zero
- reference types: null
---
.../transform/NamedVariantASTTransformation.java | 71 +++++++++--------
.../transform/NamedVariantTransformTest.groovy | 92 +++++++++++++++++++---
2 files changed, 121 insertions(+), 42 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
index dcd444a..ae9ea5c 100644
--- a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
@@ -24,17 +24,14 @@ import groovy.transform.NamedVariant;
import org.apache.groovy.ast.tools.AnnotatedNodeUtils;
import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.AnnotationNode;
-import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
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.expr.ArgumentListExpression;
-import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.MapEntryExpression;
-import org.codehaus.groovy.ast.expr.MapExpression;
import org.codehaus.groovy.ast.stmt.AssertStatement;
import org.codehaus.groovy.ast.stmt.BlockStatement;
import org.codehaus.groovy.ast.stmt.ForStatement;
@@ -45,13 +42,17 @@ import org.codehaus.groovy.control.SourceUnit;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
+import java.util.Optional;
import java.util.Set;
import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor;
import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
import static org.apache.groovy.ast.tools.ClassNodeUtils.isInnerClass;
import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility;
+import static org.codehaus.groovy.ast.ClassHelper.MAP_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.STRING_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.getWrapper;
+import static org.codehaus.groovy.ast.ClassHelper.isNumberType;
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;
@@ -62,8 +63,11 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX;
+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.list2args;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.mapX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
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;
@@ -72,33 +76,33 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
@GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS)
public class NamedVariantASTTransformation extends AbstractASTTransformation {
- private static final Class MY_CLASS = NamedVariant.class;
- private static final ClassNode MY_TYPE = make(MY_CLASS);
- private static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
+
+ private static final ClassNode NAMED_VARIANT_TYPE = make(NamedVariant.class);
+ 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);
@Override
- public void visit(ASTNode[] nodes, SourceUnit source) {
+ public void visit(final ASTNode[] nodes, final SourceUnit source) {
init(nodes, source);
MethodNode mNode = (MethodNode) nodes[1];
AnnotationNode anno = (AnnotationNode) nodes[0];
- if (!MY_TYPE.equals(anno.getClassNode())) return;
+ if (!NAMED_VARIANT_TYPE.equals(anno.getClassNode())) return;
Parameter[] fromParams = mNode.getParameters();
if (fromParams.length == 0) {
- addError("Error during " + MY_TYPE_NAME + " processing. No-args method not supported.", mNode);
+ addError("Error during " + NAMED_VARIANT + " processing. No-args method not supported.", mNode);
return;
}
boolean autoDelegate = memberHasValue(anno, "autoDelegate", true);
- Parameter mapParam = param(GenericsUtils.nonGeneric(ClassHelper.MAP_TYPE), "__namedArgs");
- List<Parameter> genParams = new ArrayList<Parameter>();
+ Parameter mapParam = param(GenericsUtils.nonGeneric(MAP_TYPE), "__namedArgs");
+ List<Parameter> genParams = new ArrayList<>();
genParams.add(mapParam);
ClassNode cNode = mNode.getDeclaringClass();
- final BlockStatement inner = new BlockStatement();
+ BlockStatement inner = new BlockStatement();
ArgumentListExpression args = new ArgumentListExpression();
- final List<String> propNames = new ArrayList<String>();
+ List<String> propNames = new ArrayList<>();
// first pass, just check for absence of annotations of interest
boolean annoFound = false;
@@ -129,7 +133,7 @@ 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) {
+ private boolean processImplicitNamedParam(final MethodNode mNode, final Parameter mapParam, final ArgumentListExpression args, final List<String> propNames, final Parameter fromParam) {
boolean required = fromParam.hasInitialExpression();
String name = fromParam.getName();
if (hasDuplicates(mNode, propNames, name)) return false;
@@ -142,7 +146,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
return true;
}
- private boolean processExplicitNamedParam(MethodNode mNode, Parameter mapParam, BlockStatement inner, ArgumentListExpression args, List<String> propNames, Parameter fromParam) {
+ private boolean processExplicitNamedParam(final MethodNode mNode, final Parameter mapParam, final BlockStatement inner, final ArgumentListExpression args, final List<String> propNames, final Parameter fromParam) {
AnnotationNode namedParam = fromParam.getAnnotations(NAMED_PARAM_TYPE).get(0);
boolean required = memberHasValue(namedParam, "required", true);
if (getMemberStringValue(namedParam, "value") == null) {
@@ -153,15 +157,15 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
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");
+ // 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);
+ addError("Error during " + NAMED_VARIANT + " 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"))));
+ plusX(constX("Missing required named argument '" + name + "'. Keys found: "), callX(varX(mapParam), "keySet"))));
}
args.addExpression(propX(varX(mapParam), name));
mapParam.addAnnotation(namedParam);
@@ -169,62 +173,65 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
return true;
}
- private boolean processDelegateParam(MethodNode mNode, Parameter mapParam, ArgumentListExpression args, List<String> propNames, Parameter fromParam) {
+ private boolean processDelegateParam(final MethodNode mNode, final Parameter mapParam, final ArgumentListExpression args, final List<String> propNames, final 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);
+ addError("Error during " + NAMED_VARIANT + " processing. Delegate type '" + fromParam.getType().getNameWithoutPackage() + "' is an inner class which is not supported.", mNode);
return false;
}
}
- Set<String> names = new HashSet<String>();
+ Set<String> names = new HashSet<>();
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>();
+ List<MapEntryExpression> entries = new ArrayList<>();
for (PropertyNode pNode : props) {
String name = pNode.getName();
- entries.add(new MapEntryExpression(constX(name), propX(varX(mapParam), name)));
+ // create entry [name: __namedArgs.getOrDefault('name', initialValue)]
+ Expression defaultValue = Optional.ofNullable(pNode.getInitialExpression()).orElseGet(() ->
+ isNumberType(pNode.getType()) ? castX(getWrapper(pNode.getType()), constX(0)) : nullX());
+ entries.add(entryX(constX(name), callX(varX(mapParam), "getOrDefault", args(constX(name), defaultValue))));
+ // create annotation @NamedParam(value='name', type=DelegateType)
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);
+ Expression delegateMap = mapX(entries);
args.addExpression(castX(fromParam.getType(), delegateMap));
return true;
}
- private boolean hasDuplicates(MethodNode mNode, List<String> propNames, String next) {
+ private boolean hasDuplicates(final MethodNode mNode, final List<String> propNames, final String next) {
if (propNames.contains(next)) {
- addError("Error during " + MY_TYPE_NAME + " processing. Duplicate property '" + next + "' found.", mNode);
+ addError("Error during " + NAMED_VARIANT + " 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) {
+ private void createMapVariant(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");
inner.addStatement(
new ForStatement(
namedArgKey,
callX(varX(mapParam), "keySet"),
- new AssertStatement(boolX(callX(list2args(propNames), "contains", varX(namedArgKey))),
- plusX(new ConstantExpression("Unrecognized namedArgKey: "), varX(namedArgKey)))
+ new AssertStatement(boolX(callX(list2args(propNames), "contains", varX(namedArgKey))), plusX(constX("Unrecognized namedArgKey: "), varX(namedArgKey)))
));
Parameter[] genParamsArray = genParams.toArray(Parameter.EMPTY_ARRAY);
// TODO account for default params giving multiple signatures
if (cNode.hasMethod(mNode.getName(), genParamsArray)) {
- addError("Error during " + MY_TYPE_NAME + " processing. Class " + cNode.getNameWithoutPackage() +
+ addError("Error during " + NAMED_VARIANT + " processing. Class " + cNode.getNameWithoutPackage() +
" already has a named-arg " + (mNode instanceof ConstructorNode ? "constructor" : "method") +
" of type " + genParams, mNode);
return;
}
- final BlockStatement body = new BlockStatement();
+ BlockStatement body = new BlockStatement();
int modifiers = getVisibility(anno, mNode, mNode.getClass(), mNode.getModifiers());
if (mNode instanceof ConstructorNode) {
body.addStatement(stmt(ctorX(ClassNode.THIS, args)));
diff --git a/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy b/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy
index 3455a77..198f951 100644
--- a/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/NamedVariantTransformTest.groovy
@@ -18,13 +18,18 @@
*/
package org.codehaus.groovy.transform
-import groovy.test.GroovyShellTestCase
+import groovy.transform.CompileStatic
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
/**
* Tests for the {@code @NamedVariant} transformation.
*/
-class NamedVariantTransformTest extends GroovyShellTestCase {
+@CompileStatic
+final class NamedVariantTransformTest {
+ @Test
void testNamedParam() {
assertScript '''
import groovy.transform.*
@@ -43,7 +48,7 @@ class NamedVariantTransformTest extends GroovyShellTestCase {
@NamedVariant
String foo(a, @NamedParam String b2, @NamedDelegate Color shade, int c, @NamedParam(required=true) d, @NamedDelegate Animal pet) {
- "$a $b2 $c $d ${pet.type?.toUpperCase()}:$pet.name $shade"
+ "$a $b2 $c $d ${pet.type?.toUpperCase()}:$pet.name $shade"
}
def result = foo(b2: 'b param', g: 12, b: 42, r: 12, 'foo', 42, d:true, type: 'Dog', name: 'Rover')
@@ -51,6 +56,7 @@ class NamedVariantTransformTest extends GroovyShellTestCase {
'''
}
+ @Test
void testNamedParamWithRename() {
assertScript '''
import groovy.transform.*
@@ -69,6 +75,7 @@ class NamedVariantTransformTest extends GroovyShellTestCase {
'''
}
+ @Test
void testNamedParamConstructor() {
assertScript """
import groovy.transform.*
@@ -77,9 +84,9 @@ class NamedVariantTransformTest extends GroovyShellTestCase {
class Color {
@NamedVariant
Color(@NamedParam int r, @NamedParam int g, @NamedParam int b) {
- this.r = r
- this.g = g
- this.b = b
+ this.r = r
+ this.g = g
+ this.b = b
}
private int r, g, b
}
@@ -88,6 +95,7 @@ class NamedVariantTransformTest extends GroovyShellTestCase {
"""
}
+ @Test
void testNamedParamConstructorVisibility() {
assertScript """
import groovy.transform.*
@@ -99,9 +107,9 @@ class NamedVariantTransformTest extends GroovyShellTestCase {
@VisibilityOptions(PUBLIC)
@NamedVariant
private Color(@NamedParam int r, @NamedParam int g, @NamedParam int b) {
- this.r = r
- this.g = g
- this.b = b
+ this.r = r
+ this.g = g
+ this.b = b
}
}
@@ -111,6 +119,7 @@ class NamedVariantTransformTest extends GroovyShellTestCase {
"""
}
+ @Test
void testNamedParamInnerClass() {
assertScript '''
import groovy.transform.*
@@ -145,17 +154,80 @@ class NamedVariantTransformTest extends GroovyShellTestCase {
'''
}
+ @Test
void testGeneratedMethodsSkipped() {
assertScript '''
import groovy.transform.*
import static org.codehaus.groovy.transform.NamedVariantTransformTest.*
@NamedVariant
- def baz(@NamedDelegate Storm st, @NamedDelegate Switch sw) { st.front + sw.back }
+ def baz(@NamedDelegate Storm storm_, @NamedDelegate Switch switch_) { storm_.front + switch_.back }
assert baz(front: 'Hello', back: 'World') == 'HelloWorld'
'''
}
+ @Test // GROOVY-9183
+ void testNamedDelegateWithPrimitiveValues() {
+ assertScript '''
+ import groovy.transform.*
+
+ class Color {
+ int r, g, b
+ }
+
+ @NamedVariant
+ Color makeColor(@NamedDelegate Color color) {
+ color
+ }
+
+ def color = makeColor(r: 128, g: 128)
+ assert color.r == 128
+ assert color.g == 128
+ assert color.b == 0
+ '''
+ }
+
+ @Test // GROOVY-9183
+ void testNamedDelegateWithPropertyDefaults() {
+ assertScript '''
+ import groovy.transform.*
+
+ class RowMapper {
+ final Settings settings
+
+ @NamedVariant
+ RowMapper(Settings settings) {
+ this.settings = settings
+ }
+
+ @NamedVariant
+ static RowMapper parse(@NamedDelegate Settings settings, Reader reader) {
+ // settings missing the initializer values from properties not passed as named arguments
+ new RowMapper(settings).parseImpl(reader)
+ }
+
+ private RowMapper parseImpl(Reader source) {
+ // do work here
+ return this
+ }
+
+ @Immutable
+ static class Settings {
+ String separator = ','
+ Boolean headers = true
+ Integer headersRow = 0
+ Integer firstDataRow = 1
+ }
+ }
+
+ def mapper = RowMapper.parse(separator: '\t', new StringReader(''))
+
+ assert mapper.settings.headers == true
+ assert mapper.settings.headersRow == 0
+ assert mapper.settings.firstDataRow == 1
+ '''
+ }
+
static class Storm { String front }
static class Switch { String back }
}