Skip to content

Commit

Permalink
GROOVY-11384: STC: map property for implicit-this within closure
Browse files Browse the repository at this point in the history
  • Loading branch information
eric-milles committed May 23, 2024
1 parent 6553534 commit afc9a74
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1550,8 +1550,7 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
queue.addFirst(current.getSuperClass());
Collections.addAll(queue, current.getInterfaces());
}

field = allowStaticAccessToMember(field, staticOnly);
else field = allowStaticAccessToMember(field, staticOnly);

// skip property/accessor checks for "x.@field"
if (pexp instanceof AttributeExpression) {
Expand All @@ -1572,7 +1571,7 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
property = allowStaticAccessToMember(property, staticOnly);

MethodNode getter = null;
if (!isMapProperty(pexp)) { // GROOVY-11369: map entry before getter
if (!isMapProperty(pexp, receiverType)) { // GROOVY-11369: map entry before getter
getter = findGetter(current, getterName, pexp.isImplicitThis());
getter = allowStaticAccessToMember(getter, staticOnly);
if (getter == null) {
Expand Down Expand Up @@ -1633,8 +1632,8 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
foundGetterOrSetter = (foundGetterOrSetter || getter != null || !setters.isEmpty());
}

if (readMode && !isMapProperty(pexp)) { // GROOVY-11369, GROOVY-11370, GROOVY-11372
// GROOVY-5568, GROOVY-9115, GROOVY-9123: the property may be defined by an extension
if (readMode && !isMapProperty(pexp, receiverType)) { // GROOVY-11369, GROOVY-11370, GROOVY-11372
// GROOVY-5568, GROOVY-9115, GROOVY-9123: the property may be provided by an extension method
for (ClassNode dgmReceiver : isPrimitiveType(receiverType) ? new ClassNode[]{receiverType, getWrapper(receiverType)} : new ClassNode[]{receiverType}) {
Set<MethodNode> methods = findDGMMethodsForClassNode(getSourceUnit().getClassLoader(), dgmReceiver, getterName);
for (MethodNode method : findDGMMethodsForClassNode(getSourceUnit().getClassLoader(), dgmReceiver, isserName)) {
Expand Down Expand Up @@ -1695,8 +1694,8 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
continue;
if (visitor != null) {
// TODO: type inference on maps and lists, if possible
PropertyNode node = new PropertyNode(propertyName, Opcodes.ACC_PUBLIC, propertyType, receiver.getType(), null, null, null);
node.setDeclaringClass(receiver.getType());
PropertyNode node = new PropertyNode(propertyName, Opcodes.ACC_PUBLIC, propertyType, receiverType, null, null, null);
node.setDeclaringClass(receiverType);
visitor.visitProperty(node);
}
storeType(pexp, propertyType);
Expand Down Expand Up @@ -1794,13 +1793,14 @@ private ClassNode getTypeForMapPropertyExpression(final ClassNode testClass, fin
return null;
}

private boolean isMapProperty(final PropertyExpression pexp) {
final Expression objectExpression = pexp.getObjectExpression();
private boolean isMapProperty(final PropertyExpression pexp, final ClassNode receiverType) {
Expression objectExpression = pexp.getObjectExpression();
if (isSuperExpression(objectExpression) || (isThisExpression(objectExpression)
&& (!pexp.isImplicitThis() || typeCheckingContext.getEnclosingClosure() == null)
&& Arrays.asList(getTypeCheckingAnnotations()).contains(COMPILESTATIC_CLASSNODE))) {
return false;
}
return isOrImplements(getType(objectExpression), MAP_TYPE);
return isOrImplements(receiverType, MAP_TYPE);
}

/**
Expand Down
15 changes: 13 additions & 2 deletions src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
c.@x = '1'
''',
'Cannot assign value of type java.lang.String to variable of type int'
}
}

void testInferenceFromAttributeType() {
assertScript '''
Expand Down Expand Up @@ -699,7 +699,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
'''
}

// GROOVY-11369
// GROOVY-11369, GROOVY-11384
void testMapPropertyAccess5() {
assertScript '''
def map = [:]
Expand All @@ -721,6 +721,17 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
assert !map.containsKey('metaClass')
assert map.containsKey('properties')
'''
assertScript '''
void test(Map map) {
def str = ''
str += map.empty
str += map.with{ empty }
str += map.with{ delegate.empty }
str += map.with{ {->owner.empty}() }
assert str == 'entryentryentryentry'
}
test( [:].withDefault{ 'entry' } )
'''
}

// GROOVY-11369
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ package org.codehaus.groovy.classgen.asm.sc
import groovy.test.NotYetImplemented
import groovy.transform.stc.FieldsAndPropertiesSTCTest

/**
* Unit tests for static compilation : fields and properties.
*/
final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCTest implements StaticCompilationTestSupport {

// GROOVY-5561
Expand All @@ -41,7 +44,7 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
}

// GROOVY-5579
void testUseSetterAndNotSetProperty() {
void testUseSetterNotSetProperty() {
assertScript '''
Date d = new Date()
d.time = 1
Expand Down Expand Up @@ -236,8 +239,8 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
}
assert new D().m() == 0
"""
def b = astTrees['D'][1]
assert b.contains('GETFIELD C.x')
def d = astTrees['D'][1]
assert d.contains('GETFIELD C.x')
}
}

Expand Down Expand Up @@ -280,9 +283,9 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
}
assert D.m() == 0
'''
def b = astTrees['D'][1]
assert b.contains('GETSTATIC D.x')
assert !b.contains('INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.getGroovyObjectProperty')
def d = astTrees['D'][1]
assert d.contains('GETSTATIC D.x')
assert !d.contains('INVOKESTATIC org/codehaus/groovy/runtime/ScriptBytecodeAdapter.getGroovyObjectProperty')
}

void testReadPropertyFromSuperClass() {
Expand All @@ -299,9 +302,9 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
}
assert new D().m() == 0
"""
def b = astTrees['D'][1]
assert !b.contains('GETFIELD C.x') : 'no GETFIELD in D'
assert b.contains('INVOKEVIRTUAL D.getX') : 'getX() in D'
def d = astTrees['D'][1]
assert !d.contains('GETFIELD C.x') : 'no GETFIELD in D'
assert d.contains('INVOKEVIRTUAL D.getX') : 'getX() in D'
}
}

Expand All @@ -325,8 +328,8 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
d.m()
assert d.isGetterCalled() == false
'''
def b = astTrees['D'][1]
assert b.contains('GETFIELD C.x')
def d = astTrees['D'][1]
assert d.contains('GETFIELD C.x')
}

void testUseAttributeExternal() {
Expand Down Expand Up @@ -548,7 +551,6 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
class C {
void setOut(int a) {}
}
def c = new C()
try {
} finally {
Expand All @@ -569,7 +571,6 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
void setOut(int a) {}
void setOut(String a) {}
}
def c = new C()
try {
} finally {
Expand All @@ -580,8 +581,8 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
} finally {
assert astTrees.values().any {
def code = it.toString()
code.contains('INVOKEVIRTUAL C.setOut (I)V') &&
code.contains('INVOKEVIRTUAL C.setOut (Ljava/lang/String;)V')
code.contains('INVOKEVIRTUAL C.setOut (I)V')
&& code.contains('INVOKEVIRTUAL C.setOut (Ljava/lang/String;)V')
}
}
}
Expand Down Expand Up @@ -777,7 +778,6 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
println accessedAndMutated
accessedAndMutated = 'def'
}
void test() {
cl()
assert mutated == 'abc'
Expand All @@ -804,7 +804,7 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
void testPropertyAccessOnEnumClass() {
assertScript '''
enum E { }
assert E.getModifiers() == E.modifiers
assert E.modifiers == E.getModifiers()
'''
}

Expand Down

0 comments on commit afc9a74

Please sign in to comment.