Skip to content

Commit 32933fa

Browse files
eric-milleseric-milles
committed
GROOVY-5450, GROOVY-9127
1 parent 12e7ac3 commit 32933fa

File tree

4 files changed

+210
-15
lines changed

4 files changed

+210
-15
lines changed

‎base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TypeCheckedTests.java

+98
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,30 @@ public void testTypeChecked17() {
416416

417417
@Test
418418
public void testTypeChecked18() {
419+
//@formatter:off
420+
String[] sources = {
421+
"Main.groovy",
422+
"class C {\n" +
423+
" boolean b\n" +
424+
"}\n" +
425+
"@groovy.transform.TypeChecked\n" +
426+
"void test() {\n" +
427+
" print(new Pogo().isFlag())\n" +
428+
"}\n" +
429+
"test()\n",
430+
431+
"Pogo.groovy",
432+
"class Pogo {\n" +
433+
" final boolean flag = true\n" +
434+
"}\n",
435+
};
436+
//@formatter:on
437+
438+
runConformTest(sources, "true");
439+
}
440+
441+
@Test
442+
public void testTypeChecked19() {
419443
//@formatter:off
420444
String[] sources = {
421445
"Main.groovy",
@@ -451,6 +475,51 @@ public void testTypeChecked18() {
451475
runConformTest(sources, "test");
452476
}
453477

478+
@Test
479+
public void testTypeChecked5450() {
480+
//@formatter:off
481+
String[] sources = {
482+
"Main.groovy",
483+
"@groovy.transform.TypeChecked\n" +
484+
"class C {\n" +
485+
" public final f\n" +
486+
" C() { f = 'yes' }\n" +
487+
" C(C that) { this.f = that.f }\n" +
488+
"}\n" +
489+
"@groovy.transform.TypeChecked\n" +
490+
"def test(C c) {\n" +
491+
" c.f = 'x'\n" +
492+
" c.@f = 'x'\n" +
493+
" c.setF('x')\n" +
494+
" c.with {f = 'x'}\n" +
495+
"}\n",
496+
};
497+
//@formatter:on
498+
499+
runNegativeTest(sources,
500+
"----------\n" +
501+
"1. ERROR in Main.groovy (at line 9)\n" +
502+
"\tc.f = 'x'\n" +
503+
"\t^^^" + (isParrotParser() ? "" : "^") + "\n" +
504+
"Groovy:[Static type checking] - Cannot set read-only property: f\n" +
505+
"----------\n" +
506+
"2. ERROR in Main.groovy (at line 10)\n" +
507+
"\tc.@f = 'x'\n" +
508+
"\t^^^^" + (isParrotParser() ? "" : "^") + "\n" +
509+
"Groovy:[Static type checking] - Cannot set read-only property: f\n" +
510+
"----------\n" +
511+
"3. ERROR in Main.groovy (at line 11)\n" +
512+
"\tc.setF('x')\n" +
513+
"\t^^^^^^^^^^^\n" +
514+
"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" +
515+
"----------\n" +
516+
"4. ERROR in Main.groovy (at line 12)\n" +
517+
"\tc.with {f = 'x'}\n" +
518+
"\t ^\n" +
519+
"Groovy:[Static type checking] - Cannot set read-only property: f\n" +
520+
"----------\n");
521+
}
522+
454523
@Test
455524
public void testTypeChecked5523() {
456525
//@formatter:off
@@ -1689,6 +1758,35 @@ public void testTypeChecked9074d() {
16891758
*/
16901759
}
16911760

1761+
@Test
1762+
public void testTypeChecked9127() {
1763+
//@formatter:off
1764+
String[] sources = {
1765+
"Main.groovy",
1766+
"class Foo {\n" +
1767+
" private String x = 'foo'\n" +
1768+
" String getX() { return x }\n" +
1769+
"}\n" +
1770+
"class Bar extends Foo {\n" +
1771+
" @groovy.transform.TypeChecked\n" +
1772+
" void writeField() {\n" +
1773+
" x = 'bar'\n" +
1774+
" }\n" +
1775+
" @Override\n" +
1776+
" String getX() { return 'baz' }\n" +
1777+
"}\n",
1778+
};
1779+
//@formatter:on
1780+
1781+
runNegativeTest(sources,
1782+
"----------\n" +
1783+
"1. ERROR in Main.groovy (at line 8)\n" +
1784+
"\tx = 'bar'\n" +
1785+
"\t^\n" +
1786+
"Groovy:[Static type checking] - Cannot set read-only property: x\n" +
1787+
"----------\n");
1788+
}
1789+
16921790
@Test
16931791
public void testTypeChecked9412() {
16941792
//@formatter:off

‎base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java

+41-3
Original file line numberDiff line numberDiff line change
@@ -1785,6 +1785,9 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
17851785
ClassNode receiverType = receiver.getType();
17861786

17871787
if (receiverType.isArray() && "length".equals(propertyName)) {
1788+
// GRECLIPSE edit -- GROOVY-5450
1789+
pexp.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, Boolean.TRUE);
1790+
// GRECLIPSE end
17881791
storeType(pexp, int_TYPE);
17891792
if (visitor != null) {
17901793
FieldNode length = new FieldNode("length", Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, int_TYPE, receiverType, null);
@@ -1842,15 +1845,19 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
18421845
*/
18431846
// skip property/accessor checks for "x.@field"
18441847
if (storeField(field, isAttributeExpression, pexp, receiverType, visitor, receiver.getData(), !readMode)) {
1848+
/* GRECLIPSE edit -- GROOVY-5450
18451849
pexp.removeNodeMetaData(StaticTypesMarker.READONLY_PROPERTY);
1850+
*/
18461851
return true;
18471852
} else if (isAttributeExpression) {
18481853
continue;
18491854
}
18501855

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

@@ -1907,12 +1914,17 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
19071914
pexp.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
19081915
}
19091916
return true;
1917+
/* GRECLIPSE edit -- GROOVY-9127
19101918
} else if (propertyNode == null) {
19111919
if (field != null && hasAccessToField(typeCheckingContext.getEnclosingClassNode(), field)) {
19121920
pexp.removeNodeMetaData(StaticTypesMarker.READONLY_PROPERTY);
19131921
} else if (getter != null) {
19141922
pexp.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, Boolean.TRUE);
19151923
}
1924+
*/
1925+
} else if (getter != null && field == null) {
1926+
pexp.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, Boolean.TRUE);
1927+
// GRECLIPSE end
19161928
}
19171929
}
19181930
foundGetterOrSetter = (foundGetterOrSetter || !setters.isEmpty() || getter != null);
@@ -2156,9 +2168,10 @@ private boolean storeField(FieldNode field, boolean returnTrueIfFieldExists, Pro
21562168
if (visitor != null) visitor.visitField(field);
21572169
checkOrMarkPrivateAccess(expressionToStoreOn, field, lhsOfAssignment);
21582170
// GRECLIPSE add
2171+
Expression objectExpression = expressionToStoreOn.getObjectExpression();
2172+
boolean accessible = hasAccessToField(objectExpression instanceof VariableExpression && ((VariableExpression) objectExpression).isSuperExpression() ? typeCheckingContext.getEnclosingClassNode() : receiver, field);
21592173
if (expressionToStoreOn instanceof AttributeExpression) {
2160-
Expression objectExpression = expressionToStoreOn.getObjectExpression();
2161-
if (!hasAccessToField(objectExpression instanceof VariableExpression && ((VariableExpression) objectExpression).isSuperExpression() ? typeCheckingContext.getEnclosingClassNode() : receiver, field)) {
2174+
if (!accessible) {
21622175
addStaticTypeError("The field " + field.getDeclaringClass().getNameWithoutPackage() + "." + field.getName() + " is not accessible", expressionToStoreOn.getProperty());
21632176
}
21642177
}
@@ -2167,6 +2180,15 @@ private boolean storeField(FieldNode field, boolean returnTrueIfFieldExists, Pro
21672180
if (delegationData != null) {
21682181
expressionToStoreOn.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
21692182
}
2183+
// GRECLIPSE add -- GROOVY-5450
2184+
if (field.isFinal()) {
2185+
MethodNode enclosing = typeCheckingContext.getEnclosingMethod();
2186+
if (enclosing == null || !enclosing.getName().endsWith("init>"))
2187+
expressionToStoreOn.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, Boolean.TRUE);
2188+
} else if (accessible) {
2189+
expressionToStoreOn.removeNodeMetaData(StaticTypesMarker.READONLY_PROPERTY);
2190+
}
2191+
// GRECLIPSE end
21702192
return true;
21712193
}
21722194

@@ -2177,6 +2199,13 @@ private boolean storeProperty(PropertyNode property, PropertyExpression expressi
21772199
if (delegationData != null) {
21782200
expressionToStoreOn.putNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER, delegationData);
21792201
}
2202+
// GRECLIPSE add -- GROOVY-5450
2203+
if (Modifier.isFinal(property.getModifiers())) {
2204+
expressionToStoreOn.putNodeMetaData(StaticTypesMarker.READONLY_PROPERTY, Boolean.TRUE);
2205+
} else {
2206+
expressionToStoreOn.removeNodeMetaData(StaticTypesMarker.READONLY_PROPERTY);
2207+
}
2208+
// GRECLIPSE end
21802209
return true;
21812210
}
21822211

@@ -2650,6 +2679,15 @@ public void visitMethodCallExpression(final MethodCallExpression mce) {
26502679
}
26512680

26522681
// GRECLIPSE add
2682+
@Override
2683+
protected void visitObjectInitializerStatements(final ClassNode node) {
2684+
// GROOVY-5450: create fake constructor node so final field analysis can allow write within non-static initializer block(s)
2685+
ConstructorNode init = new ConstructorNode(0, null, null, new BlockStatement(node.getObjectInitializerStatements(), null));
2686+
typeCheckingContext.pushEnclosingMethod(init);
2687+
super.visitObjectInitializerStatements(node);
2688+
typeCheckingContext.popEnclosingMethod();
2689+
}
2690+
26532691
@Override
26542692
public void visitExpressionStatement(ExpressionStatement statement) {
26552693
typeCheckingContext.pushTemporaryTypeInfo();
@@ -5666,7 +5704,7 @@ protected List<MethodNode> findMethod(ClassNode receiver, final String name, fin
56665704
property = curNode.getProperty(pname);
56675705
curNode = curNode.getSuperClass();
56685706
}
5669-
if (property != null) {
5707+
if (property != null && !Modifier.isFinal(property.getModifiers())) { // GRECLIPSE add
56705708
ClassNode type = property.getOriginType();
56715709
if (implementsInterfaceOrIsSubclassOf(wrapTypeIfNecessary(args[0]), wrapTypeIfNecessary(type))) {
56725710
int mods = Opcodes.ACC_PUBLIC | (property.isStatic() ? Opcodes.ACC_STATIC : 0);

‎base/org.codehaus.groovy30/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java

+44-2
Original file line numberDiff line numberDiff line change
@@ -1698,6 +1698,9 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
16981698
ClassNode receiverType = receiver.getType();
16991699

17001700
if (receiverType.isArray() && "length".equals(propertyName)) {
1701+
// GRECLIPSE edit -- GROOVY-5450
1702+
pexp.putNodeMetaData(READONLY_PROPERTY, Boolean.TRUE);
1703+
// GRECLIPSE end
17011704
storeType(pexp, int_TYPE);
17021705
if (visitor != null) {
17031706
FieldNode length = new FieldNode("length", Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, int_TYPE, receiverType, null);
@@ -1747,7 +1750,9 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
17471750
// skip property/accessor checks for "x.@field"
17481751
if (pexp instanceof AttributeExpression) {
17491752
if (field != null && storeField(field, pexp, receiverType, visitor, receiver.getData(), !readMode)) {
1753+
/* GRECLIPSE edit -- GROOVY-5450
17501754
pexp.removeNodeMetaData(READONLY_PROPERTY);
1755+
*/
17511756
return true;
17521757
}
17531758
continue;
@@ -1756,7 +1761,9 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
17561761
// skip property/accessor checks for "field", "this.field", "this.with { field }", etc. in declaring class of field
17571762
if (field != null && enclosingTypes.contains(current)) {
17581763
if (storeField(field, pexp, receiverType, visitor, receiver.getData(), !readMode)) {
1764+
/* GRECLIPSE edit -- GROOVY-5450
17591765
pexp.removeNodeMetaData(READONLY_PROPERTY);
1766+
*/
17601767
return true;
17611768
}
17621769
}
@@ -1811,12 +1818,17 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
18111818
}
18121819
pexp.removeNodeMetaData(READONLY_PROPERTY);
18131820
return true;
1821+
/* GRECLIPSE edit -- GROOVY-9127
18141822
} else if (property == null) {
18151823
if (field != null && hasAccessToField(typeCheckingContext.getEnclosingClassNode(), field)) {
18161824
pexp.removeNodeMetaData(READONLY_PROPERTY);
18171825
} else if (getter != null) {
18181826
pexp.putNodeMetaData(READONLY_PROPERTY, Boolean.TRUE);
18191827
}
1828+
*/
1829+
} else if (getter != null && field == null) {
1830+
pexp.putNodeMetaData(READONLY_PROPERTY, Boolean.TRUE);
1831+
// GRECLIPSE end
18201832
}
18211833
}
18221834
}
@@ -2025,9 +2037,12 @@ private void storeWithResolve(ClassNode type, final ClassNode receiver, final Cl
20252037
private boolean storeField(final FieldNode field, final PropertyExpression expressionToStoreOn, final ClassNode receiver, final ClassCodeVisitorSupport visitor, final String delegationData, final boolean lhsOfAssignment) {
20262038
if (visitor != null) visitor.visitField(field);
20272039
checkOrMarkPrivateAccess(expressionToStoreOn, field, lhsOfAssignment);
2040+
// GRECLIPSE add
2041+
boolean accessible = hasAccessToField(isSuperExpression(expressionToStoreOn.getObjectExpression()) ? typeCheckingContext.getEnclosingClassNode() : receiver, field);
2042+
// GRECLIPSE end
20282043

20292044
if (expressionToStoreOn instanceof AttributeExpression) { // TODO: expand to include PropertyExpression
2030-
if (!hasAccessToField(isSuperExpression(expressionToStoreOn.getObjectExpression()) ? typeCheckingContext.getEnclosingClassNode() : receiver, field)) {
2045+
if (!accessible) {
20312046
addStaticTypeError("The field " + field.getDeclaringClass().getNameWithoutPackage() + "." + field.getName() + " is not accessible", expressionToStoreOn.getProperty());
20322047
}
20332048
}
@@ -2036,6 +2051,15 @@ private boolean storeField(final FieldNode field, final PropertyExpression expre
20362051
if (delegationData != null) {
20372052
expressionToStoreOn.putNodeMetaData(IMPLICIT_RECEIVER, delegationData);
20382053
}
2054+
// GRECLIPSE add -- GROOVY-5450
2055+
if (field.isFinal()) {
2056+
MethodNode enclosing = typeCheckingContext.getEnclosingMethod();
2057+
if (enclosing == null || !enclosing.getName().endsWith("init>"))
2058+
expressionToStoreOn.putNodeMetaData(READONLY_PROPERTY, Boolean.TRUE);
2059+
} else if (accessible) {
2060+
expressionToStoreOn.removeNodeMetaData(READONLY_PROPERTY);
2061+
}
2062+
// GRECLIPSE end
20392063
return true;
20402064
}
20412065

@@ -2045,6 +2069,13 @@ private boolean storeProperty(final PropertyNode property, final PropertyExpress
20452069
if (delegationData != null) {
20462070
expressionToStoreOn.putNodeMetaData(IMPLICIT_RECEIVER, delegationData);
20472071
}
2072+
// GRECLIPSE add -- GROOVY-5450
2073+
if (Modifier.isFinal(property.getModifiers())) {
2074+
expressionToStoreOn.putNodeMetaData(READONLY_PROPERTY, Boolean.TRUE);
2075+
} else {
2076+
expressionToStoreOn.removeNodeMetaData(READONLY_PROPERTY);
2077+
}
2078+
// GRECLIPSE end
20482079
return true;
20492080
}
20502081

@@ -2384,6 +2415,17 @@ public void visitMethodCallExpression(final MethodCallExpression mce) {
23842415
typeCheckingContext.popEnclosingMethod();
23852416
}
23862417

2418+
// GRECLIPSE add
2419+
@Override
2420+
protected void visitObjectInitializerStatements(final ClassNode node) {
2421+
// GROOVY-5450: create fake constructor node so final field analysis can allow write within non-static initializer block(s)
2422+
ConstructorNode init = new ConstructorNode(0, null, null, new BlockStatement(node.getObjectInitializerStatements(), null));
2423+
typeCheckingContext.pushEnclosingMethod(init);
2424+
super.visitObjectInitializerStatements(node);
2425+
typeCheckingContext.popEnclosingMethod();
2426+
}
2427+
// GRECLIPSE end
2428+
23872429
@Override
23882430
public void visitExpressionStatement(final ExpressionStatement statement) {
23892431
typeCheckingContext.pushTemporaryTypeInfo();
@@ -5342,7 +5384,7 @@ protected List<MethodNode> findMethod(ClassNode receiver, final String name, fin
53425384
property = curNode.getProperty(pname);
53435385
curNode = curNode.getSuperClass();
53445386
}
5345-
if (property != null) {
5387+
if (property != null && !Modifier.isFinal(property.getModifiers())) { // GRECLIPSE add
53465388
ClassNode type = property.getOriginType();
53475389
if (implementsInterfaceOrIsSubclassOf(wrapTypeIfNecessary(args[0]), wrapTypeIfNecessary(type))) {
53485390
int mods = Opcodes.ACC_PUBLIC | (property.isStatic() ? Opcodes.ACC_STATIC : 0);

0 commit comments

Comments
 (0)