Skip to content

Commit

Permalink
GROOVY-5450, GROOVY-9127
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Oct 11, 2021
1 parent 12e7ac3 commit 32933fa
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 15 deletions.
98 changes: 98 additions & 0 deletions ...ovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TypeCheckedTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,30 @@ public void testTypeChecked17() {

@Test
public void testTypeChecked18() {
//@formatter:off
String[] sources = {
"Main.groovy",
"class C {\n" +
" boolean b\n" +
"}\n" +
"@groovy.transform.TypeChecked\n" +
"void test() {\n" +
" print(new Pogo().isFlag())\n" +
"}\n" +
"test()\n",

"Pogo.groovy",
"class Pogo {\n" +
" final boolean flag = true\n" +
"}\n",
};
//@formatter:on

runConformTest(sources, "true");
}

@Test
public void testTypeChecked19() {
//@formatter:off
String[] sources = {
"Main.groovy",
Expand Down Expand Up @@ -451,6 +475,51 @@ public void testTypeChecked18() {
runConformTest(sources, "test");
}

@Test
public void testTypeChecked5450() {
//@formatter:off
String[] sources = {
"Main.groovy",
"@groovy.transform.TypeChecked\n" +
"class C {\n" +
" public final f\n" +
" C() { f = 'yes' }\n" +
" C(C that) { this.f = that.f }\n" +
"}\n" +
"@groovy.transform.TypeChecked\n" +
"def test(C c) {\n" +
" c.f = 'x'\n" +
" c.@f = 'x'\n" +
" c.setF('x')\n" +
" c.with {f = 'x'}\n" +
"}\n",
};
//@formatter:on

runNegativeTest(sources,
"----------\n" +
"1. ERROR in Main.groovy (at line 9)\n" +
"\tc.f = 'x'\n" +
"\t^^^" + (isParrotParser() ? "" : "^") + "\n" +
"Groovy:[Static type checking] - Cannot set read-only property: f\n" +
"----------\n" +
"2. ERROR in Main.groovy (at line 10)\n" +
"\tc.@f = 'x'\n" +
"\t^^^^" + (isParrotParser() ? "" : "^") + "\n" +
"Groovy:[Static type checking] - Cannot set read-only property: f\n" +
"----------\n" +
"3. ERROR in Main.groovy (at line 11)\n" +
"\tc.setF('x')\n" +
"\t^^^^^^^^^^^\n" +
"Groovy:[Static type checking] - Cannot find matching method C#setF(java.lang.String). Please check if the declared type is correct and if the method exists.\n" +
"----------\n" +
"4. ERROR in Main.groovy (at line 12)\n" +
"\tc.with {f = 'x'}\n" +
"\t ^\n" +
"Groovy:[Static type checking] - Cannot set read-only property: f\n" +
"----------\n");
}

@Test
public void testTypeChecked5523() {
//@formatter:off
Expand Down Expand Up @@ -1689,6 +1758,35 @@ public void testTypeChecked9074d() {
*/
}

@Test
public void testTypeChecked9127() {
//@formatter:off
String[] sources = {
"Main.groovy",
"class Foo {\n" +
" private String x = 'foo'\n" +
" String getX() { return x }\n" +
"}\n" +
"class Bar extends Foo {\n" +
" @groovy.transform.TypeChecked\n" +
" void writeField() {\n" +
" x = 'bar'\n" +
" }\n" +
" @Override\n" +
" String getX() { return 'baz' }\n" +
"}\n",
};
//@formatter:on

runNegativeTest(sources,
"----------\n" +
"1. ERROR in Main.groovy (at line 8)\n" +
"\tx = 'bar'\n" +
"\t^\n" +
"Groovy:[Static type checking] - Cannot set read-only property: x\n" +
"----------\n");
}

@Test
public void testTypeChecked9412() {
//@formatter:off
Expand Down
44 changes: 41 additions & 3 deletions ...rg.codehaus.groovy25/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1785,6 +1785,9 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
ClassNode receiverType = receiver.getType();

if (receiverType.isArray() && "length".equals(propertyName)) {
// GRECLIPSE edit -- GROOVY-5450
pexp.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, Boolean.TRUE);
// GRECLIPSE end
storeType(pexp, int_TYPE);
if (visitor != null) {
FieldNode length = new FieldNode("length", Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, int_TYPE, receiverType, null);
Expand Down Expand Up @@ -1842,15 +1845,19 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
*/
// skip property/accessor checks for "x.@field"
if (storeField(field, isAttributeExpression, pexp, receiverType, visitor, receiver.getData(), !readMode)) {
/* GRECLIPSE edit -- GROOVY-5450
pexp.removeNodeMetaData(StaticTypesMarker.READONLY_PROPERTY);
*/
return true;
} else if (isAttributeExpression) {
continue;
}

// skip property/accessor checks for "field", "this.field", "this.with { field }", etc. in declaring class of field
if (storeField(field, enclosingTypes.contains(current), pexp, receiverType, visitor, receiver.getData(), !readMode)) {
/* GRECLIPSE edit -- GROOVY-5450
pexp.removeNodeMetaData(StaticTypesMarker.READONLY_PROPERTY);
*/
return true;
}

Expand Down Expand Up @@ -1907,12 +1914,17 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
pexp.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
}
return true;
/* GRECLIPSE edit -- GROOVY-9127
} else if (propertyNode == null) {
if (field != null && hasAccessToField(typeCheckingContext.getEnclosingClassNode(), field)) {
pexp.removeNodeMetaData(StaticTypesMarker.READONLY_PROPERTY);
} else if (getter != null) {
pexp.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, Boolean.TRUE);
}
*/
} else if (getter != null && field == null) {
pexp.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, Boolean.TRUE);
// GRECLIPSE end
}
}
foundGetterOrSetter = (foundGetterOrSetter || !setters.isEmpty() || getter != null);
Expand Down Expand Up @@ -2156,9 +2168,10 @@ private boolean storeField(FieldNode field, boolean returnTrueIfFieldExists, Pro
if (visitor != null) visitor.visitField(field);
checkOrMarkPrivateAccess(expressionToStoreOn, field, lhsOfAssignment);
// GRECLIPSE add
Expression objectExpression = expressionToStoreOn.getObjectExpression();
boolean accessible = hasAccessToField(objectExpression instanceof VariableExpression && ((VariableExpression) objectExpression).isSuperExpression() ? typeCheckingContext.getEnclosingClassNode() : receiver, field);
if (expressionToStoreOn instanceof AttributeExpression) {
Expression objectExpression = expressionToStoreOn.getObjectExpression();
if (!hasAccessToField(objectExpression instanceof VariableExpression && ((VariableExpression) objectExpression).isSuperExpression() ? typeCheckingContext.getEnclosingClassNode() : receiver, field)) {
if (!accessible) {
addStaticTypeError("The field " + field.getDeclaringClass().getNameWithoutPackage() + "." + field.getName() + " is not accessible", expressionToStoreOn.getProperty());
}
}
Expand All @@ -2167,6 +2180,15 @@ private boolean storeField(FieldNode field, boolean returnTrueIfFieldExists, Pro
if (delegationData != null) {
expressionToStoreOn.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
}
// GRECLIPSE add -- GROOVY-5450
if (field.isFinal()) {
MethodNode enclosing = typeCheckingContext.getEnclosingMethod();
if (enclosing == null || !enclosing.getName().endsWith("init>"))
expressionToStoreOn.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, Boolean.TRUE);
} else if (accessible) {
expressionToStoreOn.removeNodeMetaData(StaticTypesMarker.READONLY_PROPERTY);
}
// GRECLIPSE end
return true;
}

Expand All @@ -2177,6 +2199,13 @@ private boolean storeProperty(PropertyNode property, PropertyExpression expressi
if (delegationData != null) {
expressionToStoreOn.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
}
// GRECLIPSE add -- GROOVY-5450
if (Modifier.isFinal(property.getModifiers())) {
expressionToStoreOn.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, Boolean.TRUE);
} else {
expressionToStoreOn.removeNodeMetaData(StaticTypesMarker.READONLY_PROPERTY);
}
// GRECLIPSE end
return true;
}

Expand Down Expand Up @@ -2650,6 +2679,15 @@ public void visitMethodCallExpression(final MethodCallExpression mce) {
}

// GRECLIPSE add
@Override
protected void visitObjectInitializerStatements(final ClassNode node) {
// GROOVY-5450: create fake constructor node so final field analysis can allow write within non-static initializer block(s)
ConstructorNode init = new ConstructorNode(0, null, null, new BlockStatement(node.getObjectInitializerStatements(), null));
typeCheckingContext.pushEnclosingMethod(init);
super.visitObjectInitializerStatements(node);
typeCheckingContext.popEnclosingMethod();
}

@Override
public void visitExpressionStatement(ExpressionStatement statement) {
typeCheckingContext.pushTemporaryTypeInfo();
Expand Down Expand Up @@ -5666,7 +5704,7 @@ protected List<MethodNode> findMethod(ClassNode receiver, final String name, fin
property = curNode.getProperty(pname);
curNode = curNode.getSuperClass();
}
if (property != null) {
if (property != null && !Modifier.isFinal(property.getModifiers())) { // GRECLIPSE add
ClassNode type = property.getOriginType();
if (implementsInterfaceOrIsSubclassOf(wrapTypeIfNecessary(args[0]), wrapTypeIfNecessary(type))) {
int mods = Opcodes.ACC_PUBLIC | (property.isStatic() ? Opcodes.ACC_STATIC : 0);
Expand Down
46 changes: 44 additions & 2 deletions ...rg.codehaus.groovy30/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1698,6 +1698,9 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
ClassNode receiverType = receiver.getType();

if (receiverType.isArray() && "length".equals(propertyName)) {
// GRECLIPSE edit -- GROOVY-5450
pexp.putNodeMetaData(READONLY_PROPERTY, Boolean.TRUE);
// GRECLIPSE end
storeType(pexp, int_TYPE);
if (visitor != null) {
FieldNode length = new FieldNode("length", Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, int_TYPE, receiverType, null);
Expand Down Expand Up @@ -1747,7 +1750,9 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
// skip property/accessor checks for "x.@field"
if (pexp instanceof AttributeExpression) {
if (field != null && storeField(field, pexp, receiverType, visitor, receiver.getData(), !readMode)) {
/* GRECLIPSE edit -- GROOVY-5450
pexp.removeNodeMetaData(READONLY_PROPERTY);
*/
return true;
}
continue;
Expand All @@ -1756,7 +1761,9 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
// skip property/accessor checks for "field", "this.field", "this.with { field }", etc. in declaring class of field
if (field != null && enclosingTypes.contains(current)) {
if (storeField(field, pexp, receiverType, visitor, receiver.getData(), !readMode)) {
/* GRECLIPSE edit -- GROOVY-5450
pexp.removeNodeMetaData(READONLY_PROPERTY);
*/
return true;
}
}
Expand Down Expand Up @@ -1811,12 +1818,17 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
}
pexp.removeNodeMetaData(READONLY_PROPERTY);
return true;
/* GRECLIPSE edit -- GROOVY-9127
} else if (property == null) {
if (field != null && hasAccessToField(typeCheckingContext.getEnclosingClassNode(), field)) {
pexp.removeNodeMetaData(READONLY_PROPERTY);
} else if (getter != null) {
pexp.putNodeMetaData(READONLY_PROPERTY, Boolean.TRUE);
}
*/
} else if (getter != null && field == null) {
pexp.putNodeMetaData(READONLY_PROPERTY, Boolean.TRUE);
// GRECLIPSE end
}
}
}
Expand Down Expand Up @@ -2025,9 +2037,12 @@ private void storeWithResolve(ClassNode type, final ClassNode receiver, final Cl
private boolean storeField(final FieldNode field, final PropertyExpression expressionToStoreOn, final ClassNode receiver, final ClassCodeVisitorSupport visitor, final String delegationData, final boolean lhsOfAssignment) {
if (visitor != null) visitor.visitField(field);
checkOrMarkPrivateAccess(expressionToStoreOn, field, lhsOfAssignment);
// GRECLIPSE add
boolean accessible = hasAccessToField(isSuperExpression(expressionToStoreOn.getObjectExpression()) ? typeCheckingContext.getEnclosingClassNode() : receiver, field);
// GRECLIPSE end

if (expressionToStoreOn instanceof AttributeExpression) { // TODO: expand to include PropertyExpression
if (!hasAccessToField(isSuperExpression(expressionToStoreOn.getObjectExpression()) ? typeCheckingContext.getEnclosingClassNode() : receiver, field)) {
if (!accessible) {
addStaticTypeError("The field " + field.getDeclaringClass().getNameWithoutPackage() + "." + field.getName() + " is not accessible", expressionToStoreOn.getProperty());
}
}
Expand All @@ -2036,6 +2051,15 @@ private boolean storeField(final FieldNode field, final PropertyExpression expre
if (delegationData != null) {
expressionToStoreOn.putNodeMetaData(IMPLICIT_RECEIVER, delegationData);
}
// GRECLIPSE add -- GROOVY-5450
if (field.isFinal()) {
MethodNode enclosing = typeCheckingContext.getEnclosingMethod();
if (enclosing == null || !enclosing.getName().endsWith("init>"))
expressionToStoreOn.putNodeMetaData(READONLY_PROPERTY, Boolean.TRUE);
} else if (accessible) {
expressionToStoreOn.removeNodeMetaData(READONLY_PROPERTY);
}
// GRECLIPSE end
return true;
}

Expand All @@ -2045,6 +2069,13 @@ private boolean storeProperty(final PropertyNode property, final PropertyExpress
if (delegationData != null) {
expressionToStoreOn.putNodeMetaData(IMPLICIT_RECEIVER, delegationData);
}
// GRECLIPSE add -- GROOVY-5450
if (Modifier.isFinal(property.getModifiers())) {
expressionToStoreOn.putNodeMetaData(READONLY_PROPERTY, Boolean.TRUE);
} else {
expressionToStoreOn.removeNodeMetaData(READONLY_PROPERTY);
}
// GRECLIPSE end
return true;
}

Expand Down Expand Up @@ -2384,6 +2415,17 @@ public void visitMethodCallExpression(final MethodCallExpression mce) {
typeCheckingContext.popEnclosingMethod();
}

// GRECLIPSE add
@Override
protected void visitObjectInitializerStatements(final ClassNode node) {
// GROOVY-5450: create fake constructor node so final field analysis can allow write within non-static initializer block(s)
ConstructorNode init = new ConstructorNode(0, null, null, new BlockStatement(node.getObjectInitializerStatements(), null));
typeCheckingContext.pushEnclosingMethod(init);
super.visitObjectInitializerStatements(node);
typeCheckingContext.popEnclosingMethod();
}
// GRECLIPSE end

@Override
public void visitExpressionStatement(final ExpressionStatement statement) {
typeCheckingContext.pushTemporaryTypeInfo();
Expand Down Expand Up @@ -5342,7 +5384,7 @@ protected List<MethodNode> findMethod(ClassNode receiver, final String name, fin
property = curNode.getProperty(pname);
curNode = curNode.getSuperClass();
}
if (property != null) {
if (property != null && !Modifier.isFinal(property.getModifiers())) { // GRECLIPSE add
ClassNode type = property.getOriginType();
if (implementsInterfaceOrIsSubclassOf(wrapTypeIfNecessary(args[0]), wrapTypeIfNecessary(type))) {
int mods = Opcodes.ACC_PUBLIC | (property.isStatic() ? Opcodes.ACC_STATIC : 0);
Expand Down
Loading

0 comments on commit 32933fa

Please sign in to comment.