Skip to content

Commit

Permalink
GROOVY-6277
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Jan 22, 2022
1 parent 70b828e commit e30b62e
Show file tree
Hide file tree
Showing 10 changed files with 1,163 additions and 72 deletions.
45 changes: 45 additions & 0 deletions ...re.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/StaticCompilationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,29 @@ public void testCompileStatic27() {
runConformTest(sources, "v");
}

@Test
public void testCompileStatic28() {
//@formatter:off
String[] sources = {
"Main.groovy",
"class C {\n" +
" private String getString() {''}\n" +
" @groovy.transform.CompileStatic\n" +
" void test() {\n" +
" print new Object() {\n" +
" String toString() {\n" +
" string.toLowerCase()\n" +
" }\n" +
" }\n" +
" }\n" +
"}\n" +
"new C().test()\n",
};
//@formatter:on

runConformTest(sources);
}

@Test
public void testCompileStatic1505() {
//@formatter:off
Expand Down Expand Up @@ -941,6 +964,28 @@ public void testCompileStatic6276() {
runConformTest(sources);
}

@Test
public void testCompileStatic6277() {
//@formatter:off
String[] sources = {
"Main.groovy",
"class One {\n" +
" private getX() { 'One' }\n" +
"}\n" +
"class Two extends One {\n" +
" public x = 'Two'\n" +
"}\n" +
"@groovy.transform.CompileStatic\n" +
"void test() {\n" +
" print(new Two().x)\n" + // Cannot call private method One#getX from class Main
"}\n" +
"test()\n",
};
//@formatter:on

runConformTest(sources, "Two");
}

@Test
public void testCompileStatic6610() {
//@formatter:off
Expand Down
42 changes: 31 additions & 11 deletions ....codehaus.groovy25/src/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,39 +463,59 @@ public void makeCallSiteArrayInitializer() {
}

private boolean makeGetPropertyWithGetter(final Expression receiver, final ClassNode receiverType, final String methodName, final boolean safe, final boolean implicitThis) {
// does a getter exists ?
String getterName = "get" + MetaClassHelper.capitalize(methodName);
MethodNode getterNode = receiverType.getGetterMethod(getterName);
if (getterNode==null) {
if (getterNode == null) {
getterName = "is" + MetaClassHelper.capitalize(methodName);
getterNode = receiverType.getGetterMethod(getterName);
}
if (getterNode!=null && receiver instanceof ClassExpression && !CLASS_Type.equals(receiverType) && !getterNode.isStatic()) {
if (getterNode != null && receiver instanceof ClassExpression && !CLASS_Type.equals(receiverType) && !getterNode.isStatic()) {
return false;
}

// GROOVY-5561: if two files are compiled in the same source unit
// and that one references the other, the getters for properties have not been
// GROOVY-5561: if two files are compiled in the same source unit and
// one references the other, the getters for properties have not been
// generated by the compiler yet (generated by the Verifier)
PropertyNode propertyNode = receiverType.getProperty(methodName);
if (getterNode == null && propertyNode != null) {
// it is possible to use a getter
String prefix = "get";
if (boolean_TYPE.equals(propertyNode.getOriginType())) {
prefix = "is";
}
getterName = prefix + MetaClassHelper.capitalize(methodName);
getterNode = new MethodNode(
getterName,
ACC_PUBLIC,
ACC_PUBLIC | (propertyNode.isStatic() ? ACC_STATIC : 0),
propertyNode.getOriginType(),
Parameter.EMPTY_ARRAY,
ClassNode.EMPTY_ARRAY,
EmptyStatement.INSTANCE);
getterNode.setDeclaringClass(receiverType);
if (propertyNode.isStatic()) getterNode.setModifiers(ACC_PUBLIC + ACC_STATIC);
}
if (getterNode!=null) {
if (getterNode != null) {
// GRECLIPSE add -- GROOVY-6277
java.util.function.BiPredicate<MethodNode,ClassNode> accessible = (method, sender) -> {
// a public method is accessible from anywhere
if (method.isPublic()) return true;

ClassNode declaringClass = method.getDeclaringClass();

// any method is accessible from the declaring class
if (sender.equals(declaringClass)) return true;

// a private method isn't accessible beyond the declaring class
if (method.isPrivate()) return false;

// a protected method is accessible from any subclass of the declaring class
if (method.isProtected() && sender.isDerivedFrom(declaringClass)) return true;

// a protected or package-private method is accessible from the declaring package
if (java.util.Objects.equals(sender.getPackageName(), declaringClass.getPackageName())) return true;

return false;
};
if (!accessible.test(getterNode, controller.getClassNode())) return false;
// GRECLIPSE end
MethodCallExpression call = new MethodCallExpression(
receiver,
getterName,
Expand All @@ -515,15 +535,15 @@ private boolean makeGetPropertyWithGetter(final Expression receiver, final Class
}
}

// check direct interfaces (GROOVY-7149)
// GROOVY-7149: check direct interfaces
for (ClassNode node : receiverType.getInterfaces()) {
if (makeGetPropertyWithGetter(receiver, node, methodName, safe, implicitThis)) {
return true;
}
}
// go upper level
ClassNode superClass = receiverType.getSuperClass();
if (superClass !=null) {
if (superClass != null) {
return makeGetPropertyWithGetter(receiver, superClass, methodName, safe, implicitThis);
}

Expand Down
59 changes: 31 additions & 28 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 @@ -142,6 +142,7 @@

import static java.util.stream.Collectors.toMap;
import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName;
import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression;
import static org.codehaus.groovy.ast.ClassHelper.BigDecimal_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.BigInteger_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.Boolean_TYPE;
Expand Down Expand Up @@ -207,6 +208,7 @@
import static org.codehaus.groovy.ast.tools.WideningCategories.lowestUpperBound;
import static org.codehaus.groovy.classgen.AsmClassGenerator.MINIMUM_BYTECODE_VERSION;
import static org.codehaus.groovy.runtime.DefaultGroovyMethods.asBoolean;
import static org.codehaus.groovy.runtime.DefaultGroovyMethods.first;
import static org.codehaus.groovy.syntax.Types.ASSIGN;
import static org.codehaus.groovy.syntax.Types.ASSIGNMENT_OPERATOR;
import static org.codehaus.groovy.syntax.Types.COMPARE_EQUAL;
Expand Down Expand Up @@ -576,21 +578,17 @@ private void checkOrMarkPrivateAccess(Expression source, MethodNode mn) {
}

private void checkSuperCallFromClosure(Expression call, MethodNode directCallTarget) {
if (call instanceof MethodCallExpression && typeCheckingContext.getEnclosingClosure() != null) {
Expression objectExpression = ((MethodCallExpression) call).getObjectExpression();
if (objectExpression instanceof VariableExpression) {
VariableExpression var = (VariableExpression) objectExpression;
if (var.isSuperExpression()) {
ClassNode current = typeCheckingContext.getEnclosingClassNode();
LinkedList<MethodNode> list = current.getNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED);
if (list == null) {
list = new LinkedList<MethodNode>();
current.putNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED, list);
}
list.add(directCallTarget);
call.putNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED, current);
}
if (call instanceof MethodCallExpression
&& typeCheckingContext.getEnclosingClosure() != null
&& isSuperExpression(((MethodCallExpression) call).getObjectExpression())) {
ClassNode current = typeCheckingContext.getEnclosingClassNode();
LinkedList<MethodNode> list = current.getNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED);
if (list == null) {
list = new LinkedList<MethodNode>();
current.putNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED, list);
}
list.add(directCallTarget);
call.putNodeMetaData(StaticTypesMarker.SUPER_MOP_METHOD_REQUIRED, current);
}
}

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

if (readMode && checkGetterOrSetter) {
if (getter != null) {
if (getter != null // GRECLIPSE add -- GROOVY-6277
&& hasAccessToMember(first(enclosingTypes), getter.getDeclaringClass(), getter.getModifiers())) {
ClassNode cn = inferReturnTypeGenerics(current, getter, ArgumentListExpression.EMPTY_ARGUMENTS);
storeInferredTypeForPropertyExpression(pexp, cn);
storeTargetMethod(pexp, getter);
Expand Down Expand Up @@ -2052,14 +2056,14 @@ private static boolean hasAccessToField(FieldNode field, ClassNode objectExpress
return false;
}
*/
private static boolean hasAccessToField(ClassNode accessor, FieldNode field) {
if (field.isPublic() || accessor.equals(field.getDeclaringClass())) {
private static boolean hasAccessToMember(final ClassNode sender, final ClassNode receiver, final int modifiers) {
if (Modifier.isPublic(modifiers) || sender.equals(receiver) || sender.getOuterClasses().contains(receiver)) {
return true;
}
if (field.isProtected()) {
return accessor.isDerivedFrom(field.getDeclaringClass());
if (Modifier.isProtected(modifiers)) {
return sender.isDerivedFrom(receiver);
} else {
return !field.isPrivate() && Objects.equals(accessor.getPackageName(), field.getDeclaringClass().getPackageName());
return !Modifier.isPrivate(modifiers) && Objects.equals(sender.getPackageName(), receiver.getPackageName());
}
}
// GRECLIPSE end
Expand Down Expand Up @@ -2218,8 +2222,7 @@ 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);
boolean accessible = hasAccessToMember(isSuperExpression(expressionToStoreOn.getObjectExpression()) ? typeCheckingContext.getEnclosingClassNode() : receiver, field.getDeclaringClass(), field.getModifiers());
if (expressionToStoreOn instanceof AttributeExpression) {
if (!accessible) {
addStaticTypeError("The field " + field.getDeclaringClass().getNameWithoutPackage() + "." + field.getName() + " is not accessible", expressionToStoreOn.getProperty());
Expand Down Expand Up @@ -4277,7 +4280,7 @@ public void visitMethodCallExpression(MethodCallExpression call) {
if (!mn.isEmpty()
// GRECLIPSE add -- GROOVY-7890
&& currentReceiver.getData() == null
&& (typeCheckingContext.isInStaticContext || (receiverType.getModifiers() & Opcodes.ACC_STATIC) != 0)
&& (typeCheckingContext.isInStaticContext || Modifier.isStatic(receiverType.getModifiers()))
&& (call.isImplicitThis() || (objectExpression instanceof VariableExpression && ((VariableExpression) objectExpression).isThisExpression()))) {
// we create separate method lists just to be able to print out
// a nice error message to the user
Expand Down Expand Up @@ -4347,7 +4350,7 @@ public void visitMethodCallExpression(MethodCallExpression call) {
addStaticTypeError("Non static method " + owner.getName() + "#" + directMethodCallCandidate.getName() + " cannot be called from static context", call);
}
// GRECLIPSE add -- GROOVY-10341
else if (directMethodCallCandidate.isAbstract() && objectExpression instanceof VariableExpression && ((VariableExpression) objectExpression).isSuperExpression())
else if (directMethodCallCandidate.isAbstract() && isSuperExpression(objectExpression))
addStaticTypeError("Abstract method " + toMethodParametersString(directMethodCallCandidate.getName(), extractTypesFromParameters(directMethodCallCandidate.getParameters())) + " cannot be called directly", call);
// GRECLIPSE end
if (chosenReceiver == null) {
Expand Down Expand Up @@ -5001,9 +5004,9 @@ protected boolean checkCast(final ClassNode targetType, final Expression source)
// char c = (char) ...
} else if (sourceIsNull && isPrimitiveType(targetType) && !boolean_TYPE.equals(targetType)) {
return false;
} else if ((expressionType.getModifiers() & Opcodes.ACC_FINAL) == 0 && targetType.isInterface()) {
} else if (!Modifier.isFinal(expressionType.getModifiers()) && targetType.isInterface()) {
return true;
} else if ((targetType.getModifiers() & Opcodes.ACC_FINAL) == 0 && expressionType.isInterface()) {
} else if (!Modifier.isFinal(targetType.getModifiers()) && expressionType.isInterface()) {
return true;
} else if (!isAssignableTo(targetType, expressionType) && !implementsInterfaceOrIsSubclassOf(expressionType, targetType)) {
return false;
Expand Down Expand Up @@ -6003,7 +6006,7 @@ protected ClassNode getType(final ASTNode exp) {
if (variable instanceof FieldNode) {
ClassNode fieldType = variable.getOriginType();
if (isUsingGenericsOrIsArrayUsingGenerics(fieldType)) {
boolean isStatic = (variable.getModifiers() & Opcodes.ACC_STATIC) != 0;
boolean isStatic = Modifier.isStatic(variable.getModifiers());
ClassNode thisType = typeCheckingContext.getEnclosingClassNode(), declType = ((FieldNode) variable).getDeclaringClass();
Map<GenericsTypeName, GenericsType> placeholders = resolvePlaceHoldersFromDeclaration(thisType, declType, null, isStatic);

Expand Down
34 changes: 27 additions & 7 deletions ....codehaus.groovy30/src/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,6 @@ public void makeCallSiteArrayInitializer() {
}

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

// GROOVY-5561: if two files are compiled in the same source unit
// and that one references the other, the getters for properties have not been
// GROOVY-5561: if two files are compiled in the same source unit and
// one references the other, the getters for properties have not been
// generated by the compiler yet (generated by the Verifier)
PropertyNode propertyNode = receiverType.getProperty(propertyName);
if (getterNode == null && propertyNode != null) {
// it is possible to use a getter
String prefix = "get";
if (boolean_TYPE.equals(propertyNode.getOriginType())) {
prefix = "is";
}
getterName = prefix + capitalize(propertyName);
getterNode = new MethodNode(
getterName,
ACC_PUBLIC,
ACC_PUBLIC | (propertyNode.isStatic() ? ACC_STATIC : 0),
propertyNode.getOriginType(),
Parameter.EMPTY_ARRAY,
ClassNode.EMPTY_ARRAY,
EmptyStatement.INSTANCE);
getterNode.setDeclaringClass(receiverType);
if (propertyNode.isStatic()) getterNode.setModifiers(ACC_PUBLIC + ACC_STATIC);
}
if (getterNode != null) {
// GRECLIPSE add -- GROOVY-6277
java.util.function.BiPredicate<MethodNode,ClassNode> accessible = (method, sender) -> {
// a public method is accessible from anywhere
if (method.isPublic()) return true;

ClassNode declaringClass = method.getDeclaringClass();

// any method is accessible from the declaring class
if (sender.equals(declaringClass)) return true;

// a private method isn't accessible beyond the declaring class
if (method.isPrivate()) return false;

// a protected method is accessible from any subclass of the declaring class
if (method.isProtected() && sender.isDerivedFrom(declaringClass)) return true;

// a protected or package-private method is accessible from the declaring package
if (java.util.Objects.equals(sender.getPackageName(), declaringClass.getPackageName())) return true;

return false;
};
if (!accessible.test(getterNode, controller.getClassNode())) return false;
// GRECLIPSE end
MethodCallExpression call = callX(receiver, getterName);
call.setImplicitThis(implicitThis);
call.setMethodTarget(getterNode);
Expand All @@ -487,7 +507,7 @@ private boolean makeGetPropertyWithGetter(final Expression receiver, final Class
}
}

// check direct interfaces (GROOVY-7149)
// GROOVY-7149: check direct interfaces
for (ClassNode node : receiverType.getInterfaces()) {
if (makeGetPropertyWithGetter(receiver, node, propertyName, safe, implicitThis)) {
return true;
Expand Down
Loading

0 comments on commit e30b62e

Please sign in to comment.