diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java index dee145e84bb..fa7c4d82630 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java @@ -31,6 +31,7 @@ import org.codehaus.groovy.ast.expr.MethodCallExpression; import org.codehaus.groovy.ast.expr.MethodReferenceExpression; import org.codehaus.groovy.ast.tools.GeneralUtils; +import org.codehaus.groovy.classgen.AsmClassGenerator; import org.codehaus.groovy.classgen.asm.BytecodeHelper; import org.codehaus.groovy.classgen.asm.MethodReferenceExpressionWriter; import org.codehaus.groovy.classgen.asm.WriterController; @@ -111,12 +112,21 @@ public void writeMethodReferenceExpression(final MethodReferenceExpression metho } else { // TODO: move the findMethodRefMethod and checking to StaticTypeCheckingVisitor methodRefMethod = findMethodRefMethod(methodRefName, parametersWithExactType, typeOrTargetRef, typeOrTargetRefType); + if (methodReferenceExpression.getNodeMetaData(StaticTypesMarker.PV_METHODS_ACCESS) != null) { // GROOVY-11301, GROOVY-11365: access bridge indicated + Map bridgeMethods = typeOrTargetRefType.redirect().getNodeMetaData(StaticCompilationMetadataKeys.PRIVATE_BRIDGE_METHODS); + if (bridgeMethods != null) methodRefMethod = bridgeMethods.getOrDefault(methodRefMethod, methodRefMethod); // bridge may not have been generated + } } validate(methodReferenceExpression, typeOrTargetRefType, methodRefName, methodRefMethod, parametersWithExactType, resolveClassNodeGenerics(extractPlaceholders(functionalType), null, abstractMethod.getReturnType())); - if (isExtensionMethod(methodRefMethod)) { + if (isBridgeMethod(methodRefMethod)) { + targetIsArgument = true; // GROOVY-11301, GROOVY-11365 + if (isClassExpression) { // method expects an instance argument + methodRefMethod = addSyntheticMethodForDGSM(methodRefMethod); + } + } else if (isExtensionMethod(methodRefMethod)) { ExtensionMethodNode extensionMethodNode = (ExtensionMethodNode) methodRefMethod; methodRefMethod = extensionMethodNode.getExtensionMethodNode(); boolean isStatic = extensionMethodNode.isStaticExtension(); @@ -208,6 +218,8 @@ private void validate(final MethodReferenceExpression methodReference, final Cla addFatalError(error, methodReference); } else if (methodNode.isVoidMethod() && !ClassHelper.isPrimitiveVoid(samReturnType)) { addFatalError("Invalid return type: void is not convertible to " + samReturnType.getText(), methodReference); + } else if (!AsmClassGenerator.isMemberDirectlyAccessible(methodNode.getModifiers(), methodNode.getDeclaringClass(), controller.getClassNode())) { + addFatalError("Cannot access method: " + methodName + " of class: " + methodNode.getDeclaringClass().getText(), methodReference); // GROOVY-11365 } else if (samParameters.length > 0 && isTypeReferringInstanceMethod(methodReference.getExpression(), methodNode) && !isAssignableTo(samParameters[0].getType(), targetType)) { throw new RuntimeParserException("Invalid receiver type: " + samParameters[0].getType().getText() + " is not compatible with " + targetType.getText(), methodReference.getExpression()); } @@ -422,6 +434,11 @@ private void addFatalError(final String msg, final ASTNode node) { //-------------------------------------------------------------------------- + private static boolean isBridgeMethod(final MethodNode mn) { + int staticSynthetic = Opcodes.ACC_STATIC | Opcodes.ACC_SYNTHETIC; + return ((mn.getModifiers() & staticSynthetic) == staticSynthetic) && mn.getName().startsWith("access$"); + } + private static boolean isConstructorReference(final String name) { return "new".equals(name); } diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index 3a6d96d9f31..f2a9c0032c7 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -591,14 +591,13 @@ private static void addPrivateFieldOrMethodAccess(final Expression source, final } /** - * Checks for private field access from inner or outer class. + * Checks for private field access from closure or nestmate. */ private void checkOrMarkPrivateAccess(final Expression source, final FieldNode fn, final boolean lhsOfAssignment) { if (fn != null && fn.isPrivate() && !fn.isSynthetic()) { ClassNode declaringClass = fn.getDeclaringClass(); ClassNode enclosingClass = typeCheckingContext.getEnclosingClassNode(); - if (declaringClass == enclosingClass && typeCheckingContext.getEnclosingClosure() == null) return; - if (declaringClass == enclosingClass || getOutermost(declaringClass) == getOutermost(enclosingClass)) { + if (declaringClass == enclosingClass ? typeCheckingContext.getEnclosingClosure() != null : getOutermost(declaringClass) == getOutermost(enclosingClass)) { StaticTypesMarker accessKind = lhsOfAssignment ? PV_FIELDS_MUTATION : PV_FIELDS_ACCESS; addPrivateFieldOrMethodAccess(source, declaringClass, accessKind, fn); } @@ -606,29 +605,26 @@ private void checkOrMarkPrivateAccess(final Expression source, final FieldNode f } /** - * Checks for private method call from inner or outer class. + * Checks for private or protected method access from closure or nestmate. */ private void checkOrMarkPrivateAccess(final Expression source, final MethodNode mn) { ClassNode declaringClass = mn.getDeclaringClass(); - ClassNode enclosingClassNode = typeCheckingContext.getEnclosingClassNode(); - if (declaringClass != enclosingClassNode || typeCheckingContext.getEnclosingClosure() != null) { - int mods = mn.getModifiers(); - boolean sameModule = declaringClass.getModule() == enclosingClassNode.getModule(); - String packageName = declaringClass.getPackageName(); - if (packageName == null) { - packageName = ""; - } - if (Modifier.isPrivate(mods) && sameModule) { + ClassNode enclosingClass = typeCheckingContext.getEnclosingClassNode(); + if (declaringClass != enclosingClass || typeCheckingContext.getEnclosingClosure() != null) { + if (mn.isPrivate() + && declaringClass.getModule() == enclosingClass.getModule()) { addPrivateFieldOrMethodAccess(source, declaringClass, PV_METHODS_ACCESS, mn); - } else if (Modifier.isProtected(mods) && !packageName.equals(enclosingClassNode.getPackageName()) - && !implementsInterfaceOrIsSubclassOf(enclosingClassNode, declaringClass)) { - ClassNode cn = enclosingClassNode; - while ((cn = cn.getOuterClass()) != null) { + } else if (mn.isProtected() + && !inSamePackage(enclosingClass, declaringClass) + && (!implementsInterfaceOrIsSubclassOf(enclosingClass, declaringClass) + || typeCheckingContext.getEnclosingClosure() != null)) { + ClassNode cn = enclosingClass; + do { if (implementsInterfaceOrIsSubclassOf(cn, declaringClass)) { addPrivateFieldOrMethodAccess(source, cn, PV_METHODS_ACCESS, mn); break; } - } + } while ((cn = cn.getOuterClass()) != null); } } } @@ -2651,7 +2647,8 @@ && isStringType(getType(nameExpr))) { ClassNode ownerType = receiverType; candidates.stream() - .map(candidate -> { + .peek(candidate -> checkOrMarkPrivateAccess(expression, candidate)) // GROOVY-11365 + .map (candidate -> { ClassNode returnType = candidate.getReturnType(); if (!candidate.isStatic() && GenericsUtils.hasUnresolvedGenerics(returnType)) { Map spec = new HashMap<>(); // GROOVY-11364 diff --git a/src/test/groovy/transform/stc/MethodReferenceTest.groovy b/src/test/groovy/transform/stc/MethodReferenceTest.groovy index cc2c8f61c6f..8c1bdc7b641 100644 --- a/src/test/groovy/transform/stc/MethodReferenceTest.groovy +++ b/src/test/groovy/transform/stc/MethodReferenceTest.groovy @@ -1526,22 +1526,44 @@ final class MethodReferenceTest { @Test // GROOVY-11301 void testInnerClassPrivateMethodReference() { - def script = ''' + assertScript shell, ''' + @CompileStatic class C { static class D { private static String m() { 'D' } } - @CompileStatic static main(args) { Supplier str = D::m assert str.get() == 'D' } } ''' - if (Runtime.version().feature() < 15) { - shouldFail(shell, IllegalAccessError, script) - } else { - assertScript(shell, script) - } + } + + @Test // GROOVY-11365 + void testInnerClassProtectedMethodReference() { + assertScript shell, '''package p + abstract class A { + protected E op(E e) { result = e } + protected E result + } + + true + ''' + assertScript shell, ''' + @CompileStatic + class C extends p.A { + void test() { + def runnable = { -> + Consumer consumer = this::op + consumer.accept(42) // IllegalAccessError + } + runnable.run() + assert result == Integer.valueOf(42) + } + } + + new C().test() + ''' } }