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 2020/06/23 02:53:15 UTC
[groovy] branch GROOVY_3_0_X updated: GROOVY-9603: don't apply map
literal as constructor shorthand to Object (closes #1288)
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
new e956399 GROOVY-9603: don't apply map literal as constructor shorthand to Object (closes #1288)
e956399 is described below
commit e9563991ff27a16d456f28cee229a26b4901cb43
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Jun 22 10:40:01 2020 -0500
GROOVY-9603: don't apply map literal as constructor shorthand to Object (closes #1288)
---
.../transform/stc/StaticTypeCheckingVisitor.java | 22 +++++++--------
.../transform/stc/ConstructorsSTCTest.groovy | 31 ++++++++++++++++++++++
2 files changed, 41 insertions(+), 12 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 723965c..021bef5 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1220,19 +1220,17 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
private void addMapAssignmentConstructorErrors(final ClassNode leftRedirect, final Expression leftExpression, final Expression rightExpression) {
- // if left type is not a list but right type is a map, then we're in the case of a groovy
- // constructor type : A a = [x:2, y:3]
- // In this case, more checks can be performed
- if (!implementsInterfaceOrIsSubclassOf(leftRedirect, MAP_TYPE) && rightExpression instanceof MapExpression) {
- if (!(leftExpression instanceof VariableExpression) || !((VariableExpression) leftExpression).isDynamicTyped()) {
- ArgumentListExpression argList = args(rightExpression);
- ClassNode[] argTypes = getArgumentTypes(argList);
- checkGroovyStyleConstructor(leftRedirect, argTypes, rightExpression);
- // perform additional type checking on arguments
- MapExpression mapExpression = (MapExpression) rightExpression;
- checkGroovyConstructorMap(leftExpression, leftRedirect, mapExpression);
- }
+ if (!(rightExpression instanceof MapExpression) || (leftExpression instanceof VariableExpression && ((VariableExpression) leftExpression).isDynamicTyped())
+ || leftRedirect.equals(OBJECT_TYPE) || implementsInterfaceOrIsSubclassOf(leftRedirect, MAP_TYPE)) {
+ return;
}
+
+ // groovy constructor shorthand: A a = [x:2, y:3]
+ ClassNode[] argTypes = getArgumentTypes(args(rightExpression));
+ checkGroovyStyleConstructor(leftRedirect, argTypes, rightExpression);
+ // perform additional type checking on arguments
+ MapExpression mapExpression = (MapExpression) rightExpression;
+ checkGroovyConstructorMap(leftExpression, leftRedirect, mapExpression);
}
private void checkTypeGenerics(final ClassNode leftExpressionType, final ClassNode wrappedRHS, final Expression rightExpression) {
diff --git a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
index e2bc7e4..628fa7a 100644
--- a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
@@ -124,9 +124,16 @@ class ConstructorsSTCTest extends StaticTypeCheckingTestCase {
void testConstructMap() {
assertScript '''
def a = [:]
+ assert a instanceof Map
+
Map b = [:]
+ assert b instanceof Map
+
Object c = [:]
+ assert c instanceof Map
+
HashMap d = [:]
+ assert d instanceof HashMap
'''
}
@@ -142,6 +149,30 @@ class ConstructorsSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-9603
+ void testDoNotConstructFromValuedMap() {
+ assertScript '''
+ void test(Map<String, Object> map) {
+ // assign to local variable to establish standard behavior
+ def foobar = [foo: 'bar']
+ map.proper = foobar
+ assert map.proper['foo'] == 'bar'
+
+ // put map literal into "map" parameter in various forms:
+
+ map.put('proper', [key: 'abc'])
+ assert map.proper['key'] == 'abc'
+
+ map['proper'] = [key: 'def']
+ assert map.proper['key'] == 'def'
+
+ map.proper = [key: 'ghi']
+ assert map.proper['key'] == 'ghi'
+ }
+ test([:])
+ '''
+ }
+
void testConstructFromCoercedMap() {
assertScript '''
class A {