Skip to content

Commit

Permalink
GROOVY-11223: STC: create setter access error
Browse files Browse the repository at this point in the history
  • Loading branch information
eric-milles committed May 7, 2024
1 parent b505625 commit f9b4eee
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1042,8 +1042,21 @@ private boolean ensureValidSetter(final Expression expression, final Expression
}
return false;
} else {
ClassNode firstSetterType = setterType.apply(setterInfo.setters.get(0));
addAssignmentError(firstSetterType, getType(valueExpression), expression);
List<MethodNode> visibleSetters = filterMethodsByVisibility(setterInfo.setters, typeCheckingContext.getEnclosingClassNode());
if (visibleSetters.isEmpty()) {
String message; // GROOVY-11223
if (setterInfo.setters.size() == 1) {
message = String.format("Cannot access method: %2$s of class: %1$s",
prettyPrintTypeName(setterInfo.setters.get(0).getDeclaringClass()),
toMethodParametersString(setterInfo.name, extractTypesFromParameters(setterInfo.setters.get(0).getParameters())));
} else {
message = "Cannot access methods: " + prettyPrintMethodList(setterInfo.setters);
}
addStaticTypeError(message, leftExpression);
} else {
ClassNode[] tergetTypes = visibleSetters.stream().map(setterType).toArray(ClassNode[]::new);
addAssignmentError(tergetTypes.length == 1 ? tergetTypes[0] : new UnionTypeClassNode(tergetTypes), getType(valueExpression), expression);
}
return true;
}
}
Expand Down Expand Up @@ -1807,9 +1820,9 @@ private boolean storeField(final FieldNode field, final PropertyExpression expre
checkOrMarkPrivateAccess(expressionToStoreOn, field, lhsOfAssignment);
boolean accessible = hasAccessToMember(isSuperExpression(expressionToStoreOn.getObjectExpression()) ? typeCheckingContext.getEnclosingClassNode() : receiver, field.getDeclaringClass(), field.getModifiers());

if (expressionToStoreOn instanceof AttributeExpression) { // TODO: expand to include PropertyExpression
if (expressionToStoreOn instanceof AttributeExpression) {
if (!accessible) {
addStaticTypeError("The field " + field.getDeclaringClass().getNameWithoutPackage() + "." + field.getName() + " is not accessible", expressionToStoreOn.getProperty());
addStaticTypeError("Cannot access field: " + field.getName() + " of class: " + prettyPrintTypeName(field.getDeclaringClass()), expressionToStoreOn.getProperty());
}
}

Expand Down
46 changes: 39 additions & 7 deletions src/test/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
}
}
''',
'The field C.x is not accessible'
'Cannot access field: x of class: C'
}

void testShouldComplainAboutMissingAttribute() {
Expand Down Expand Up @@ -261,7 +261,7 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
}
}
''',
'The field C.x is not accessible'
'Cannot access field: x of class: C'
}

void testPropertyWithInheritance() {
Expand Down Expand Up @@ -675,13 +675,21 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
def foo = 1
}
def map = new C()
map.put('foo', 42)
assert map.foo == 42
map.put('foo', 11)
assert map.foo == 11
assert map['foo'] == 11
'''
assertScript """
def map = new ${MapType.name}()
map.put('foo', 42)
assert map.foo == 42
map.put('foo', 11)
assert map.foo == 11
assert map['foo'] == 11
map.put('bar', 22)
assert map.bar == 22
assert map['bar'] == 22
map.put('baz', 33)
assert map.baz == 33
assert map['baz'] == 33
"""
}

Expand Down Expand Up @@ -779,6 +787,26 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
'''
}

// GROOVY-11223
void testMapPropertyAccess10() {
assertScript """
def map = new ${MapType.name}()
map.foo = 11 // public setter
assert map.foo == null
assert map.getFoo() == 11
"""
shouldFailWithMessages """
def map = new ${MapType.name}()
map.bar = 22 // protected setter: Cannot assign value of type int to variable of type Object
""",
"Cannot access method: setBar(java.lang.Object) of class: ${MapType.name}"
shouldFailWithMessages """
def map = new ${MapType.name}()
map.baz = 33 // package-private setter: Cannot assign value of type int to variable of type Object
""",
"Cannot access method: setBaz(java.lang.Object) of class: ${MapType.name}"
}

void testTypeCheckerDoesNotThinkPropertyIsReadOnly() {
assertScript '''
// a base class defining a read-only property
Expand Down Expand Up @@ -1624,8 +1652,12 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
String boo = "I don't fancy fields in interfaces"
}

static class MapType extends HashMap<String,Object> {
static class MapType extends HashMap<String,Number> {
def foo = 1
protected bar = 2
@PackageScope baz = 3
protected void setBar(bar) {}
@PackageScope void setBaz(baz) {}
}

static class BaseClass {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ final class Groovy7300 extends StaticTypeCheckingTestCase implements StaticCompi
def getX() { super.@x }
}
assert false
''', 'The field A.x is not accessible'
''',
'Cannot access field: x of class: A'
}
}

0 comments on commit f9b4eee

Please sign in to comment.