From fda9b2b8d748271706ea74bf5f1315943447c0d5 Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Wed, 15 May 2024 11:36:35 -0500 Subject: [PATCH] GROOVY-11369: STC: map entry comes before access method (pt.3) --- .../stc/StaticTypeCheckingVisitor.java | 15 +++-- .../stc/FieldsAndPropertiesSTCTest.groovy | 57 +++++++++++++++---- 2 files changed, 55 insertions(+), 17 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 917d74f8e9c..d9c64f9625a 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -1568,6 +1568,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, getterName, pexp.isImplicitThis()); @@ -1576,11 +1579,13 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re getter = findGetter(current, isserName, pexp.isImplicitThis()); getter = allowStaticAccessToMember(getter, staticOnly); } - 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) { @@ -1791,8 +1796,8 @@ private ClassNode getTypeForMapPropertyExpression(final ClassNode testClass, fin private boolean isMapProperty(final PropertyExpression pexp) { final Expression objectExpression = pexp.getObjectExpression(); - if ((isThisExpression(objectExpression) || isSuperExpression(objectExpression)) - && Arrays.asList(getTypeCheckingAnnotations()).contains(COMPILESTATIC_CLASSNODE)) { + if (isSuperExpression(objectExpression) || (isThisExpression(objectExpression) + && Arrays.asList(getTypeCheckingAnnotations()).contains(COMPILESTATIC_CLASSNODE))) { return false; } return isOrImplements(getType(objectExpression), MAP_TYPE); diff --git a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy index d75b3cf86bf..2ccad3f7dd8 100644 --- a/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy +++ b/src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy @@ -728,7 +728,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase { for (mod in ['def', 'public', 'protected', '@groovy.transform.PackageScope', 'private']) { assertScript """ class C implements Map { - @Delegate Map impl = [:].withDefault{'entry'} + @Delegate Map impl = [:].withDefault{ 'entry' } $mod getFoo() { 'getter' } void test() { @ASTTest(phase=INSTRUCTION_SELECTION, value={ @@ -768,7 +768,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 { } @@ -782,24 +782,57 @@ 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() { + def x = this.a + assert x instanceof A + //typeof(this.b) is A for STC and B for SC + @ASTTest(phase=INSTRUCTION_SELECTION, value={ + assert node.getNodeMetaData(INFERRED_TYPE).name == 'B' + }) + def z = super.b + assert z instanceof B + } + } + def map = new D() + map.put('a', new A()) + map.put('b', new A()) + map.test() + ''' } // GROOVY-5517