Skip to content

Commit e30b62e

Browse files
eric-milleseric-milles
committed
GROOVY-6277
1 parent 70b828e commit e30b62e

10 files changed

Lines changed: 1163 additions & 72 deletions

File tree

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,29 @@ public void testCompileStatic27() {
730730
runConformTest(sources, "v");
731731
}
732732

733+
@Test
734+
public void testCompileStatic28() {
735+
//@formatter:off
736+
String[] sources = {
737+
"Main.groovy",
738+
"class C {\n" +
739+
" private String getString() {''}\n" +
740+
" @groovy.transform.CompileStatic\n" +
741+
" void test() {\n" +
742+
" print new Object() {\n" +
743+
" String toString() {\n" +
744+
" string.toLowerCase()\n" +
745+
" }\n" +
746+
" }\n" +
747+
" }\n" +
748+
"}\n" +
749+
"new C().test()\n",
750+
};
751+
//@formatter:on
752+
753+
runConformTest(sources);
754+
}
755+
733756
@Test
734757
public void testCompileStatic1505() {
735758
//@formatter:off
@@ -941,6 +964,28 @@ public void testCompileStatic6276() {
941964
runConformTest(sources);
942965
}
943966

967+
@Test
968+
public void testCompileStatic6277() {
969+
//@formatter:off
970+
String[] sources = {
971+
"Main.groovy",
972+
"class One {\n" +
973+
" private getX() { 'One' }\n" +
974+
"}\n" +
975+
"class Two extends One {\n" +
976+
" public x = 'Two'\n" +
977+
"}\n" +
978+
"@groovy.transform.CompileStatic\n" +
979+
"void test() {\n" +
980+
" print(new Two().x)\n" + // Cannot call private method One#getX from class Main
981+
"}\n" +
982+
"test()\n",
983+
};
984+
//@formatter:on
985+
986+
runConformTest(sources, "Two");
987+
}
988+
944989
@Test
945990
public void testCompileStatic6610() {
946991
//@formatter:off

‎base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -463,39 +463,59 @@ public void makeCallSiteArrayInitializer() {
463463
}
464464

465465
private boolean makeGetPropertyWithGetter(final Expression receiver, final ClassNode receiverType, final String methodName, final boolean safe, final boolean implicitThis) {
466-
// does a getter exists ?
467466
String getterName = "get" + MetaClassHelper.capitalize(methodName);
468467
MethodNode getterNode = receiverType.getGetterMethod(getterName);
469-
if (getterNode==null) {
468+
if (getterNode == null) {
470469
getterName = "is" + MetaClassHelper.capitalize(methodName);
471470
getterNode = receiverType.getGetterMethod(getterName);
472471
}
473-
if (getterNode!=null && receiver instanceof ClassExpression && !CLASS_Type.equals(receiverType) && !getterNode.isStatic()) {
472+
if (getterNode != null && receiver instanceof ClassExpression && !CLASS_Type.equals(receiverType) && !getterNode.isStatic()) {
474473
return false;
475474
}
476475

477-
// GROOVY-5561: if two files are compiled in the same source unit
478-
// and that one references the other, the getters for properties have not been
476+
// GROOVY-5561: if two files are compiled in the same source unit and
477+
// one references the other, the getters for properties have not been
479478
// generated by the compiler yet (generated by the Verifier)
480479
PropertyNode propertyNode = receiverType.getProperty(methodName);
481480
if (getterNode == null && propertyNode != null) {
482-
// it is possible to use a getter
483481
String prefix = "get";
484482
if (boolean_TYPE.equals(propertyNode.getOriginType())) {
485483
prefix = "is";
486484
}
487485
getterName = prefix + MetaClassHelper.capitalize(methodName);
488486
getterNode = new MethodNode(
489487
getterName,
490-
ACC_PUBLIC,
488+
ACC_PUBLIC | (propertyNode.isStatic() ? ACC_STATIC : 0),
491489
propertyNode.getOriginType(),
492490
Parameter.EMPTY_ARRAY,
493491
ClassNode.EMPTY_ARRAY,
494492
EmptyStatement.INSTANCE);
495493
getterNode.setDeclaringClass(receiverType);
496-
if (propertyNode.isStatic()) getterNode.setModifiers(ACC_PUBLIC + ACC_STATIC);
497494
}
498-
if (getterNode!=null) {
495+
if (getterNode != null) {
496+
// GRECLIPSE add -- GROOVY-6277
497+
java.util.function.BiPredicate<MethodNode,ClassNode> accessible = (method, sender) -> {
498+
// a public method is accessible from anywhere
499+
if (method.isPublic()) return true;
500+
501+
ClassNode declaringClass = method.getDeclaringClass();
502+
503+
// any method is accessible from the declaring class
504+
if (sender.equals(declaringClass)) return true;
505+
506+
// a private method isn't accessible beyond the declaring class
507+
if (method.isPrivate()) return false;
508+
509+
// a protected method is accessible from any subclass of the declaring class
510+
if (method.isProtected() && sender.isDerivedFrom(declaringClass)) return true;
511+
512+
// a protected or package-private method is accessible from the declaring package
513+
if (java.util.Objects.equals(sender.getPackageName(), declaringClass.getPackageName())) return true;
514+
515+
return false;
516+
};
517+
if (!accessible.test(getterNode, controller.getClassNode())) return false;
518+
// GRECLIPSE end
499519
MethodCallExpression call = new MethodCallExpression(
500520
receiver,
501521
getterName,
@@ -515,15 +535,15 @@ private boolean makeGetPropertyWithGetter(final Expression receiver, final Class
515535
}
516536
}
517537

518-
// check direct interfaces (GROOVY-7149)
538+
// GROOVY-7149: check direct interfaces
519539
for (ClassNode node : receiverType.getInterfaces()) {
520540
if (makeGetPropertyWithGetter(receiver, node, methodName, safe, implicitThis)) {
521541
return true;
522542
}
523543
}
524544
// go upper level
525545
ClassNode superClass = receiverType.getSuperClass();
526-
if (superClass !=null) {
546+
if (superClass != null) {
527547
return makeGetPropertyWithGetter(receiver, superClass, methodName, safe, implicitThis);
528548
}
529549

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

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@
142142

143143
import static java.util.stream.Collectors.toMap;
144144
import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName;
145+
import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression;
145146
import static org.codehaus.groovy.ast.ClassHelper.BigDecimal_TYPE;
146147
import static org.codehaus.groovy.ast.ClassHelper.BigInteger_TYPE;
147148
import static org.codehaus.groovy.ast.ClassHelper.Boolean_TYPE;
@@ -207,6 +208,7 @@
207208
import static org.codehaus.groovy.ast.tools.WideningCategories.lowestUpperBound;
208209
import static org.codehaus.groovy.classgen.AsmClassGenerator.MINIMUM_BYTECODE_VERSION;
209210
import static org.codehaus.groovy.runtime.DefaultGroovyMethods.asBoolean;
211+
import static org.codehaus.groovy.runtime.DefaultGroovyMethods.first;
210212
import static org.codehaus.groovy.syntax.Types.ASSIGN;
211213
import static org.codehaus.groovy.syntax.Types.ASSIGNMENT_OPERATOR;
212214
import static org.codehaus.groovy.syntax.Types.COMPARE_EQUAL;
@@ -576,21 +578,17 @@ private void checkOrMarkPrivateAccess(Expression source, MethodNode mn) {
576578
}
577579

578580
private void checkSuperCallFromClosure(Expression call, MethodNode directCallTarget) {
579-
if (call instanceof MethodCallExpression && typeCheckingContext.getEnclosingClosure() != null) {
580-
Expression objectExpression = ((MethodCallExpression) call).getObjectExpression();
581-
if (objectExpression instanceof VariableExpression) {
582-
VariableExpression var = (VariableExpression) objectExpression;
583-
if (var.isSuperExpression()) {
584-
ClassNode current = typeCheckingContext.getEnclosingClassNode();
585-
LinkedList<MethodNode> list = current.getNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED);
586-
if (list == null) {
587-
list = new LinkedList<MethodNode>();
588-
current.putNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED, list);
589-
}
590-
list.add(directCallTarget);
591-
call.putNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED, current);
592-
}
581+
if (call instanceof MethodCallExpression
582+
&& typeCheckingContext.getEnclosingClosure() != null
583+
&& isSuperExpression(((MethodCallExpression) call).getObjectExpression())) {
584+
ClassNode current = typeCheckingContext.getEnclosingClassNode();
585+
LinkedList<MethodNode> list = current.getNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED);
586+
if (list == null) {
587+
list = new LinkedList<MethodNode>();
588+
current.putNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED, list);
593589
}
590+
list.add(directCallTarget);
591+
call.putNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED, current);
594592
}
595593
}
596594

@@ -1782,8 +1780,13 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
17821780
ClassNode rawType = objectExpressionType.getPlainNodeReference();
17831781
inferDiamondType((ConstructorCallExpression) objectExpression, rawType);
17841782
}
1785-
// GRECLIPSE end
1783+
/* GRECLIPSE edit -- enclosing excludes classes that skip STC
17861784
List<ClassNode> enclosingTypes = typeCheckingContext.getEnclosingClassNodes();
1785+
*/
1786+
Set<ClassNode> enclosingTypes = new LinkedHashSet<>();
1787+
enclosingTypes.add(typeCheckingContext.getEnclosingClassNode());
1788+
enclosingTypes.addAll( first(enclosingTypes).getOuterClasses());
1789+
// GRECLIPSE end
17871790
boolean staticOnlyAccess = isClassClassNodeWrappingConcreteType(objectExpressionType);
17881791
if ("this".equals(propertyName) && staticOnlyAccess) {
17891792
// Outer.this for any level of nesting
@@ -1912,7 +1915,8 @@ protected boolean existsProperty(final PropertyExpression pexp, final boolean re
19121915
boolean checkGetterOrSetter = !isThisExpression || propertyNode == null;
19131916

19141917
if (readMode && checkGetterOrSetter) {
1915-
if (getter != null) {
1918+
if (getter != null // GRECLIPSE add -- GROOVY-6277
1919+
&& hasAccessToMember(first(enclosingTypes), getter.getDeclaringClass(), getter.getModifiers())) {
19161920
ClassNode cn = inferReturnTypeGenerics(current, getter, ArgumentListExpression.EMPTY_ARGUMENTS);
19171921
storeInferredTypeForPropertyExpression(pexp, cn);
19181922
storeTargetMethod(pexp, getter);
@@ -2052,14 +2056,14 @@ private static boolean hasAccessToField(FieldNode field, ClassNode objectExpress
20522056
return false;
20532057
}
20542058
*/
2055-
private static boolean hasAccessToField(ClassNode accessor, FieldNode field) {
2056-
if (field.isPublic() || accessor.equals(field.getDeclaringClass())) {
2059+
private static boolean hasAccessToMember(final ClassNode sender, final ClassNode receiver, final int modifiers) {
2060+
if (Modifier.isPublic(modifiers) || sender.equals(receiver) || sender.getOuterClasses().contains(receiver)) {
20572061
return true;
20582062
}
2059-
if (field.isProtected()) {
2060-
return accessor.isDerivedFrom(field.getDeclaringClass());
2063+
if (Modifier.isProtected(modifiers)) {
2064+
return sender.isDerivedFrom(receiver);
20612065
} else {
2062-
return !field.isPrivate() && Objects.equals(accessor.getPackageName(), field.getDeclaringClass().getPackageName());
2066+
return !Modifier.isPrivate(modifiers) && Objects.equals(sender.getPackageName(), receiver.getPackageName());
20632067
}
20642068
}
20652069
// GRECLIPSE end
@@ -2218,8 +2222,7 @@ private boolean storeField(FieldNode field, boolean returnTrueIfFieldExists, Pro
22182222
if (visitor != null) visitor.visitField(field);
22192223
checkOrMarkPrivateAccess(expressionToStoreOn, field, lhsOfAssignment);
22202224
// GRECLIPSE add
2221-
Expression objectExpression = expressionToStoreOn.getObjectExpression();
2222-
boolean accessible = hasAccessToField(objectExpression instanceof VariableExpression && ((VariableExpression) objectExpression).isSuperExpression() ? typeCheckingContext.getEnclosingClassNode() : receiver, field);
2225+
boolean accessible = hasAccessToMember(isSuperExpression(expressionToStoreOn.getObjectExpression()) ? typeCheckingContext.getEnclosingClassNode() : receiver, field.getDeclaringClass(), field.getModifiers());
22232226
if (expressionToStoreOn instanceof AttributeExpression) {
22242227
if (!accessible) {
22252228
addStaticTypeError("The field " + field.getDeclaringClass().getNameWithoutPackage() + "." + field.getName() + " is not accessible", expressionToStoreOn.getProperty());
@@ -4277,7 +4280,7 @@ public void visitMethodCallExpression(MethodCallExpression call) {
42774280
if (!mn.isEmpty()
42784281
// GRECLIPSE add -- GROOVY-7890
42794282
&& currentReceiver.getData() == null
4280-
&& (typeCheckingContext.isInStaticContext || (receiverType.getModifiers() & Opcodes.ACC_STATIC) != 0)
4283+
&& (typeCheckingContext.isInStaticContext || Modifier.isStatic(receiverType.getModifiers()))
42814284
&& (call.isImplicitThis() || (objectExpression instanceof VariableExpression && ((VariableExpression) objectExpression).isThisExpression()))) {
42824285
// we create separate method lists just to be able to print out
42834286
// a nice error message to the user
@@ -4347,7 +4350,7 @@ public void visitMethodCallExpression(MethodCallExpression call) {
43474350
addStaticTypeError("Non static method " + owner.getName() + "#" + directMethodCallCandidate.getName() + " cannot be called from static context", call);
43484351
}
43494352
// GRECLIPSE add -- GROOVY-10341
4350-
else if (directMethodCallCandidate.isAbstract() && objectExpression instanceof VariableExpression && ((VariableExpression) objectExpression).isSuperExpression())
4353+
else if (directMethodCallCandidate.isAbstract() && isSuperExpression(objectExpression))
43514354
addStaticTypeError("Abstract method " + toMethodParametersString(directMethodCallCandidate.getName(), extractTypesFromParameters(directMethodCallCandidate.getParameters())) + " cannot be called directly", call);
43524355
// GRECLIPSE end
43534356
if (chosenReceiver == null) {
@@ -5001,9 +5004,9 @@ protected boolean checkCast(final ClassNode targetType, final Expression source)
50015004
// char c = (char) ...
50025005
} else if (sourceIsNull && isPrimitiveType(targetType) && !boolean_TYPE.equals(targetType)) {
50035006
return false;
5004-
} else if ((expressionType.getModifiers() & Opcodes.ACC_FINAL) == 0 && targetType.isInterface()) {
5007+
} else if (!Modifier.isFinal(expressionType.getModifiers()) && targetType.isInterface()) {
50055008
return true;
5006-
} else if ((targetType.getModifiers() & Opcodes.ACC_FINAL) == 0 && expressionType.isInterface()) {
5009+
} else if (!Modifier.isFinal(targetType.getModifiers()) && expressionType.isInterface()) {
50075010
return true;
50085011
} else if (!isAssignableTo(targetType, expressionType) && !implementsInterfaceOrIsSubclassOf(expressionType, targetType)) {
50095012
return false;
@@ -6003,7 +6006,7 @@ protected ClassNode getType(final ASTNode exp) {
60036006
if (variable instanceof FieldNode) {
60046007
ClassNode fieldType = variable.getOriginType();
60056008
if (isUsingGenericsOrIsArrayUsingGenerics(fieldType)) {
6006-
boolean isStatic = (variable.getModifiers() & Opcodes.ACC_STATIC) != 0;
6009+
boolean isStatic = Modifier.isStatic(variable.getModifiers());
60076010
ClassNode thisType = typeCheckingContext.getEnclosingClassNode(), declType = ((FieldNode) variable).getDeclaringClass();
60086011
Map<GenericsTypeName, GenericsType> placeholders = resolvePlaceHoldersFromDeclaration(thisType, declType, null, isStatic);
60096012

‎base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,6 @@ public void makeCallSiteArrayInitializer() {
439439
}
440440

441441
private boolean makeGetPropertyWithGetter(final Expression receiver, final ClassNode receiverType, final String propertyName, final boolean safe, final boolean implicitThis) {
442-
// does a getter exist?
443442
String getterName = "get" + capitalize(propertyName);
444443
MethodNode getterNode = receiverType.getGetterMethod(getterName);
445444
if (getterNode == null) {
@@ -450,28 +449,49 @@ private boolean makeGetPropertyWithGetter(final Expression receiver, final Class
450449
return false;
451450
}
452451

453-
// GROOVY-5561: if two files are compiled in the same source unit
454-
// and that one references the other, the getters for properties have not been
452+
// GROOVY-5561: if two files are compiled in the same source unit and
453+
// one references the other, the getters for properties have not been
455454
// generated by the compiler yet (generated by the Verifier)
456455
PropertyNode propertyNode = receiverType.getProperty(propertyName);
457456
if (getterNode == null && propertyNode != null) {
458-
// it is possible to use a getter
459457
String prefix = "get";
460458
if (boolean_TYPE.equals(propertyNode.getOriginType())) {
461459
prefix = "is";
462460
}
463461
getterName = prefix + capitalize(propertyName);
464462
getterNode = new MethodNode(
465463
getterName,
466-
ACC_PUBLIC,
464+
ACC_PUBLIC | (propertyNode.isStatic() ? ACC_STATIC : 0),
467465
propertyNode.getOriginType(),
468466
Parameter.EMPTY_ARRAY,
469467
ClassNode.EMPTY_ARRAY,
470468
EmptyStatement.INSTANCE);
471469
getterNode.setDeclaringClass(receiverType);
472-
if (propertyNode.isStatic()) getterNode.setModifiers(ACC_PUBLIC + ACC_STATIC);
473470
}
474471
if (getterNode != null) {
472+
// GRECLIPSE add -- GROOVY-6277
473+
java.util.function.BiPredicate<MethodNode,ClassNode> accessible = (method, sender) -> {
474+
// a public method is accessible from anywhere
475+
if (method.isPublic()) return true;
476+
477+
ClassNode declaringClass = method.getDeclaringClass();
478+
479+
// any method is accessible from the declaring class
480+
if (sender.equals(declaringClass)) return true;
481+
482+
// a private method isn't accessible beyond the declaring class
483+
if (method.isPrivate()) return false;
484+
485+
// a protected method is accessible from any subclass of the declaring class
486+
if (method.isProtected() && sender.isDerivedFrom(declaringClass)) return true;
487+
488+
// a protected or package-private method is accessible from the declaring package
489+
if (java.util.Objects.equals(sender.getPackageName(), declaringClass.getPackageName())) return true;
490+
491+
return false;
492+
};
493+
if (!accessible.test(getterNode, controller.getClassNode())) return false;
494+
// GRECLIPSE end
475495
MethodCallExpression call = callX(receiver, getterName);
476496
call.setImplicitThis(implicitThis);
477497
call.setMethodTarget(getterNode);
@@ -487,7 +507,7 @@ private boolean makeGetPropertyWithGetter(final Expression receiver, final Class
487507
}
488508
}
489509

490-
// check direct interfaces (GROOVY-7149)
510+
// GROOVY-7149: check direct interfaces
491511
for (ClassNode node : receiverType.getInterfaces()) {
492512
if (makeGetPropertyWithGetter(receiver, node, propertyName, safe, implicitThis)) {
493513
return true;

0 commit comments

Comments
 (0)