[groovy] branch GROOVY-9603 created (now 82b43d5)

[em...@apache.org]

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

emilles pushed a change to branch GROOVY-9603
in repository https://gitbox.apache.org/repos/asf/groovy.git.


      at 82b43d5  GROOVY-9603: don't apply map literal as constructor shorthand to Object

This branch includes the following new commits:

     new 82b43d5  GROOVY-9603: don't apply map literal as constructor shorthand to Object

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

[groovy] 01/01: GROOVY-9603: don't apply map literal as constructor shorthand to Object

[em...@apache.org]

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

emilles pushed a commit to branch GROOVY-9603
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 82b43d5adfa75e82f55099d42d7b52aed4f4a503
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
---
 .../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 12bf1df..a374c98 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1221,19 +1221,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 {