[groovy] branch GROOVY_2_5_X updated: GROOVY-9603: don't apply map literal as constructor shorthand to Object (closes #1288)

[pa...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

paulk pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
     new 4ffcd57  GROOVY-9603: don't apply map literal as constructor shorthand to Object (closes #1288)
4ffcd57 is described below

commit 4ffcd572c08a6d39ad94fdbc27d57fc9898d7f17
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   | 24 ++++++++---------
 .../transform/stc/ConstructorsSTCTest.groovy       | 31 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 13 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 7435780..b8af55f 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1190,20 +1190,18 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         }
     }
 
-    private void addMapAssignmentConstructorErrors(ClassNode leftRedirect, Expression leftExpression, 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);
-            }
+    private void addMapAssignmentConstructorErrors(final ClassNode leftRedirect, final Expression leftExpression, final Expression rightExpression) {
+        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(ClassNode leftExpressionType, ClassNode wrappedRHS, Expression rightExpression) {
diff --git a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
index d7b7e73..3b4cec4 100644
--- a/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ConstructorsSTCTest.groovy
@@ -116,9 +116,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
         '''
     }
 
@@ -134,6 +141,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 {