GROOVY-11369: STC: map entry comes before access method (pt.3)

apache · May 15, 2024 · 1099b93 · 1099b93
1 parent 364117d
commit 1099b93
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 18 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
@@ -1583,6 +1583,9 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
                     }
                 }
 
+                PropertyNode property = current.getProperty(propertyName);
+                property = allowStaticAccessToMember(property, staticOnly);
+
                 MethodNode getter = null;
                 if (!isMapProperty(pexp)) { // GROOVY-11369: map entry before getter
                     getter = findGetter(current, isserName, pexp.isImplicitThis());
@@ -1594,11 +1597,13 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
                     if (getter != null && !hasAccessToMember(typeCheckingContext.getEnclosingClassNode(), getter.getDeclaringClass(), getter.getModifiers())) {
                         getter = null; // GROOVY-11319
                     }
-                    if (readMode && getter != null && visitor != null) visitor.visitMethod(getter);
+                    if (readMode && getter != null && visitor != null) {
+                        visitor.visitMethod(getter);
+                    }
+                } else if (readMode) { // GROOVY-5001, GROOVY-5491, GROOVY-8555
+                    if (property != null) { property = null; field = null; }
                 }
 
-                PropertyNode property = current.getProperty(propertyName);
-                property = allowStaticAccessToMember(property, staticOnly);
                 // prefer explicit getter or setter over property if receiver is not 'this'
                 if (property == null || !enclosingTypes.contains(receiverType)) {
                     if (readMode) {
@@ -1622,9 +1627,9 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
                                     visitor.visitField(virtual);
                                 }
                             }
-                            SetterInfo info = new SetterInfo(current, setterName, setters);
                             BinaryExpression enclosingBinaryExpression = typeCheckingContext.getEnclosingBinaryExpression();
                             if (enclosingBinaryExpression != null) {
+                                SetterInfo info = new SetterInfo(current, setterName, setters);
                                 putSetterInfo(enclosingBinaryExpression.getLeftExpression(), info);
                             }
                             String delegationData = receiver.getData();
@@ -1752,7 +1757,7 @@ private static boolean hasAccessToMember(final ClassNode accessor, final ClassNo
         return Modifier.isProtected(modifiers) && accessor.isDerivedFrom(receiver);
     }
 
-    private MethodNode findGetter(final ClassNode current, String name, final boolean searchOuterClasses) {
+    private static MethodNode findGetter(final ClassNode current, String name, final boolean searchOuterClasses) {
         MethodNode getterMethod = current.getGetterMethod(name);
         if (getterMethod == null && searchOuterClasses && current.getOuterClass() != null) {
             return findGetter(current.getOuterClass(), name, true);
@@ -1828,7 +1833,7 @@ private ClassNode getTypeForMapPropertyExpression(final ClassNode testClass, fin
 
     private boolean isMapProperty(final PropertyExpression pexp) {
         final Expression objectExpression = pexp.getObjectExpression();
-        if ((isThisExpression(objectExpression) || isSuperExpression(objectExpression))
+        if (isThisExpression(objectExpression)
                 && Arrays.asList(getTypeCheckingAnnotations()).contains(COMPILESTATIC_CLASSNODE)) {
             return false;
         }

diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -716,7 +716,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
         for (mod in ['def', 'public', 'protected', '@groovy.transform.PackageScope', 'private']) {
             assertScript """
                 class C implements Map<String,String> {
-                    @Delegate Map<String,String> impl = [:].withDefault{'entry'}
+                    @Delegate Map<String,String> impl = [:].withDefault{ 'entry' }
                     $mod getFoo() { 'getter' }
                     void test() {
                         @ASTTest(phase=INSTRUCTION_SELECTION, value={
@@ -756,7 +756,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
         """
     }
 
-    // GROOVY-5001, GROOVY-5491, GROOVY-6144
+    // GROOVY-5001, GROOVY-5491, GROOVY-6144, GROOVY-8555
     void testMapPropertyAccess7() {
         String types = '''
             class A { }
@@ -770,24 +770,56 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
             map.put('a', new A())
             assert map.get('a') != null
             assert map.get('b') == null
-            A a = map.a
-            B b = map.b
+            def a = map.a
+            def b = map.b
+            assert a  instanceof A
+            assert b !instanceof B : 'not yet'
             a = map['a']
-          //b = map['b']
-            assert a instanceof A
-          //assert b instanceof B
+            b = map['b']
+            assert a  instanceof A
+            assert b !instanceof B : 'not yet'
         '''
         assertScript types + '''
             def test(C map) {
-                A a = map.a
-                B b = map.b
+                def a = map.a
+                def b = map.b
+                assert a  instanceof A
+                assert b !instanceof B : 'not yet'
                 a = map['a']
-              //b = map['b']
-                assert a instanceof A
-              //assert b instanceof B
+                b = map['b']
+                assert a  instanceof A
+                assert b !instanceof B : 'not yet'
             }
             test(new C().tap{ put('a', new A()) })
         '''
+        assertScript types + '''
+            C map = new C()
+            @ASTTest(phase=INSTRUCTION_SELECTION, value={
+                assert node.getNodeMetaData(INFERRED_TYPE).name == 'A'
+            })
+            def b = map.b // entry
+            assert b == null
+            map.b = null // setter
+            assert map.getB() == null
+            assert !map.containsKey('b')
+        '''
+        assertScript types + '''
+            class D extends C {
+                void test() {
+                    //this.a fails hard for SC
+                    //typeof(this.b) is A for STC and B for SC
+                    @ASTTest(phase=INSTRUCTION_SELECTION, value={
+                        assert node.getNodeMetaData(INFERRED_TYPE).name == 'A'
+                    })
+                    def b = super.b
+                    assert b instanceof A
+                }
+            }
+            def map = new D()
+            map.put('a', new A())
+            map.put('b', new A())
+            map.test()
+        '''
     }
 
     // GROOVY-5517