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 2021/09/17 21:12:25 UTC
[groovy] branch master updated: GROOVY-10238: TupleConstructor:
remove field value if handling defaults
This is an automated email from the ASF dual-hosted git repository.
emilles 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 34412a2 GROOVY-10238: TupleConstructor: remove field value if handling defaults
34412a2 is described below
commit 34412a229e4cb9b50c594aa36c9cd526174ffca8
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Sep 17 15:34:18 2021 -0500
GROOVY-10238: TupleConstructor: remove field value if handling defaults
---
.../TupleConstructorASTTransformation.java | 49 ++++--------
.../groovy/transform/stc/MethodCallsSTCTest.groovy | 2 -
.../classgen/asm/sc/StaticCompileDGMTest.groovy | 32 ++++++--
.../transform/TupleConstructorTransformTest.groovy | 93 ++++++++++++++++------
4 files changed, 108 insertions(+), 68 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
index b2cdad3..5a1e156 100644
--- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
@@ -24,6 +24,7 @@ import groovy.transform.TupleConstructor;
import groovy.transform.options.PropertyHandler;
import groovy.transform.stc.POJO;
import org.apache.groovy.ast.tools.AnnotatedNodeUtils;
+import org.apache.groovy.ast.tools.ExpressionUtils;
import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.AnnotatedNode;
import org.codehaus.groovy.ast.AnnotationNode;
@@ -49,17 +50,16 @@ 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 org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor;
import static org.apache.groovy.ast.tools.ClassNodeUtils.hasExplicitConstructor;
import static org.apache.groovy.ast.tools.ConstructorNodeUtils.checkPropNamesS;
import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility;
+import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
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;
@@ -69,17 +69,16 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.copyStatementsWithSuperAdjustment;
import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.defaultValueX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.equalsNullX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getAllProperties;
import static org.codehaus.groovy.ast.tools.GeneralUtils.ifElseS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.ifS;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.params;
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.throwS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
-import static org.codehaus.groovy.ast.ClassHelper.isObjectType;
import static org.codehaus.groovy.transform.ImmutableASTTransformation.makeImmutable;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
@@ -95,21 +94,6 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
private static final ClassNode LHMAP_TYPE = makeWithoutCaching(LinkedHashMap.class, false);
private static final ClassNode POJO_TYPE = make(POJO.class);
- private static final Map<Class<?>, Expression> primitivesInitialValues;
-
- static {
- final ConstantExpression zero = constX(0);
- final ConstantExpression zeroDecimal = constX(.0);
- primitivesInitialValues = new HashMap<Class<?>, Expression>();
- primitivesInitialValues.put(int.class, zero);
- primitivesInitialValues.put(long.class, zero);
- primitivesInitialValues.put(short.class, zero);
- primitivesInitialValues.put(byte.class, zero);
- primitivesInitialValues.put(char.class, zero);
- primitivesInitialValues.put(float.class, zeroDecimal);
- primitivesInitialValues.put(double.class, zeroDecimal);
- primitivesInitialValues.put(boolean.class, ConstantExpression.FALSE);
- }
@Override
public String getAnnotationName() {
@@ -117,6 +101,11 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
}
@Override
+ public void setCompilationUnit(CompilationUnit unit) {
+ compilationUnit = unit;
+ }
+
+ @Override
public void visit(ASTNode[] nodes, SourceUnit source) {
init(nodes, source);
AnnotatedNode parent = (AnnotatedNode) nodes[1];
@@ -294,17 +283,14 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
return param;
}
- private static Expression providedOrDefaultInitialValue(FieldNode fNode) {
- Expression initialExp = fNode.getInitialExpression() != null ? fNode.getInitialExpression() : nullX();
- final ClassNode paramType = fNode.getType();
- if (ClassHelper.isPrimitiveType(paramType) && isNull(initialExp)) {
- initialExp = primitivesInitialValues.get(paramType.getTypeClass());
+ private static Expression providedOrDefaultInitialValue(final FieldNode fNode) {
+ ClassNode fType = fNode.getType();
+ Expression init = fNode.getInitialExpression();
+ fNode.setInitialValueExpression(null); // GROOVY-10238
+ if (init == null || (ClassHelper.isPrimitiveType(fType) && ExpressionUtils.isNullConstant(init))) {
+ init = defaultValueX(fType);
}
- return initialExp;
- }
-
- private static boolean isNull(Expression exp) {
- return exp instanceof ConstantExpression && ((ConstantExpression) exp).isNullExpression();
+ return init;
}
public static void addSpecialMapConstructors(int modifiers, ClassNode cNode, String message, boolean addNoArg) {
@@ -344,9 +330,4 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
block.addStatement(checkPropNamesS(namedArgs, pojo, props));
return block;
}
-
- @Override
- public void setCompilationUnit(CompilationUnit unit) {
- this.compilationUnit = unit;
- }
}
diff --git a/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy b/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
index 8931c42..c083e82 100644
--- a/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
@@ -498,8 +498,6 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
void testMethodCallWithDefaultParams() {
assertScript '''
- import groovy.transform.*
- @TypeChecked(TypeCheckingMode.SKIP)
class Support {
Support(String name, String val, List arg=null, Set set = null, Date suffix = new Date()) {
"$name$val$suffix"
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileDGMTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileDGMTest.groovy
index 7324978..a1f378c 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileDGMTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileDGMTest.groovy
@@ -21,13 +21,34 @@ package org.codehaus.groovy.classgen.asm.sc
import groovy.transform.stc.DefaultGroovyMethodsSTCTest
/**
- * Unit tests for static compilation: DGM method calls.
+ * Unit tests for static compilation: default groovy methods.
*/
class StaticCompileDGMTest extends DefaultGroovyMethodsSTCTest implements StaticCompilationTestSupport {
+ // GROOVY-10238
+ void testMapWithDefault() {
+ assertScript '''
+ import groovy.transform.*
+
+ @CompileDynamic
+ class Outer {
+ @Canonical
+ @CompileStatic
+ static class Inner {
+ Map<String,Object> map = [:].withDefault { new Object() } // NoSuchMethodError: java.util.Map.withDefault(Lgroovy/lang/Closure;)
+ }
+ }
+
+ def obj = new Outer.Inner()
+ assert obj.toString() == 'Outer$Inner([:])'
+ assert obj.map['foo'] != null
+ assert obj.toString() != 'Outer$Inner([:])'
+ '''
+ }
+
void testThreadDotStart() {
assertScript '''
- @ASTTest(phase = INSTRUCTION_SELECTION, value = {
+ @ASTTest(phase=INSTRUCTION_SELECTION, value={
lookup('start').each {
def target = it.expression.target
assert target instanceof org.codehaus.groovy.transform.stc.ExtensionMethodNode
@@ -37,14 +58,13 @@ class StaticCompileDGMTest extends DefaultGroovyMethodsSTCTest implements Static
assert target.returnType.nameWithoutPackage == 'Thread'
}
})
- void foo() {
- start:
+ void test() {
+ start:
Thread.start {
println 'ok'
}
}
- foo()
+ test()
'''
}
}
-
diff --git a/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy b/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy
index 671bbcd..55973e1 100644
--- a/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy
@@ -22,23 +22,60 @@ import groovy.test.GroovyShellTestCase
class TupleConstructorTransformTest extends GroovyShellTestCase {
- void testOk() {
- assertScript """
- import groovy.transform.TupleConstructor
-
- @TupleConstructor
+ void testBasics() {
+ assertScript '''
+ @groovy.transform.TupleConstructor
class Person {
String firstName
String lastName
}
- def p = new Person("John", "Doe")
- assert p.firstName == "John"
- assert p.lastName == "Doe"
- """
+ def p = new Person('John', 'Doe')
+ assert p.firstName == 'John'
+ assert p.lastName == 'Doe'
+ '''
+ }
+
+ void testCopyConstructor() {
+ assertScript '''
+ @groovy.transform.TupleConstructor(force=true)
+ class Person {
+ String firstName, lastName
+ Person(Person that) {
+ this.firstName = that.firstName
+ }
+ }
+
+ def p = new Person('John', 'Doe')
+ assert p.firstName == 'John'
+ assert p.lastName == 'Doe'
+
+ p = new Person(p)
+ assert p.firstName == 'John'
+ assert p.lastName == null
+ '''
}
- void testConstructorWithPostAndFields() {
+ void testFieldsAndInitializers() {
+ assertScript '''
+ @groovy.transform.TupleConstructor(includeFields=true)
+ class Person {
+ String firstName = 'John'
+ private String lastName = 'Doe'
+ String getLastName() { lastName }
+ }
+
+ def p = new Person()
+ assert p.firstName == 'John'
+ assert p.lastName == 'Doe'
+
+ p = new Person('Jane')
+ assert p.firstName == 'Jane'
+ assert p.lastName == 'Doe'
+ '''
+ }
+
+ void testFieldsAndNamesAndPost() {
assertScript '''
import groovy.transform.*
@@ -54,7 +91,7 @@ class TupleConstructorTransformTest extends GroovyShellTestCase {
'''
}
- void testConstructorWithPreAndPost() {
+ void testSuperPropsAndPreAndPost() {
assertScript '''
import groovy.transform.*
@@ -75,8 +112,9 @@ class TupleConstructorTransformTest extends GroovyShellTestCase {
'''
}
- void testExistingEmptyConstructorTakesPrecedence_groovy7522() {
- assertScript """
+ // GROOVY-7522
+ void testExistingEmptyConstructorTakesPrecedence() {
+ assertScript '''
@groovy.transform.TupleConstructor
class Cat {
String name
@@ -86,12 +124,12 @@ class TupleConstructorTransformTest extends GroovyShellTestCase {
assert new Cat("Mr. Bigglesworth").name == null
assert Cat.declaredConstructors.size() == 1
- """
+ '''
}
void testIncludesAndExcludesTogetherResultsInError() {
def message = shouldFail {
- evaluate """
+ evaluate '''
import groovy.transform.TupleConstructor
@TupleConstructor(includes='surName', excludes='surName')
@@ -100,14 +138,14 @@ class TupleConstructorTransformTest extends GroovyShellTestCase {
}
new Person("Doe")
- """
+ '''
}
assert message.contains("Error during @TupleConstructor processing: Only one of 'includes' and 'excludes' should be supplied not both.")
}
void testIncludesWithInvalidPropertyNameResultsInError() {
def message = shouldFail {
- evaluate """
+ evaluate '''
import groovy.transform.TupleConstructor
@TupleConstructor(includes='sirName')
@@ -117,14 +155,14 @@ class TupleConstructorTransformTest extends GroovyShellTestCase {
}
def p = new Person("John", "Doe")
- """
+ '''
}
assert message.contains("Error during @TupleConstructor processing: 'includes' property 'sirName' does not exist.")
}
void testExcludesWithInvalidPropertyNameResultsInError() {
def message = shouldFail {
- evaluate """
+ evaluate '''
import groovy.transform.TupleConstructor
@TupleConstructor(excludes='sirName')
@@ -134,12 +172,13 @@ class TupleConstructorTransformTest extends GroovyShellTestCase {
}
def p = new Person("John", "Doe")
- """
+ '''
}
assert message.contains("Error during @TupleConstructor processing: 'excludes' property 'sirName' does not exist.")
}
- void testIncludesWithEmptyList_groovy7523() {
+ // GROOVY-7523
+ void testIncludesWithEmptyList() {
assertScript '''
@groovy.transform.TupleConstructor(includes=[])
class Cat {
@@ -150,7 +189,8 @@ class TupleConstructorTransformTest extends GroovyShellTestCase {
'''
}
- void testCombiningWithInheritConstructors_groovy7524() {
+ // GROOVY-7524
+ void testCombiningWithInheritConstructors() {
assertScript '''
import groovy.transform.*
@@ -176,7 +216,8 @@ class TupleConstructorTransformTest extends GroovyShellTestCase {
'''
}
- void testMultipleUsages_groovy7672() {
+ // GROOVY-7672
+ void testMultipleUsages() {
assertScript '''
import groovy.transform.*
import java.awt.Color
@@ -203,7 +244,8 @@ class TupleConstructorTransformTest extends GroovyShellTestCase {
'''
}
- void testInternalFieldsAreIncludedIfRequested_groovy6454() {
+ // GROOVY-6454
+ void testInternalFieldsAreIncludedIfRequested() {
assertScript '''
import groovy.transform.*
@@ -327,5 +369,4 @@ class TupleConstructorTransformTest extends GroovyShellTestCase {
'public Baz(Basep,Basepp,Basepubf,java.lang.Byte,Foop,Foopubf,java.lang.Short,Barp,Barpp,Barpubf,java.lang.Integer,Bazp,Bazpp,Bazpubf,java.lang.Long)'
'''
}
-
-}
\ No newline at end of file
+}